From 5935485f8eee356f6bb7b2b1cfb43d69e5c03662 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 20 Feb 2018 13:42:06 +0000 Subject: drm/i915: Move the policy for placement of the GGTT vma into the caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we make the unilateral decision inside i915_gem_object_pin_to_display() where the VMA should resided (inside the fence and mappable region or above?). This is not our decision to make as it impacts on how the display engine can use the resulting scanout object, and it would rather instruct us where to place the VMA so that it can enable the features it wants. As such, make the pin flags an argument to i915_gem_object_pin_to_display() and control them from intel_pin_and_fence_fb_obj() Whilst taking control of the mapping for ourselves, start tracking how we use it to avoid trying to free a fence we never claimed: <3>[ 227.151869] GEM_BUG_ON(vma->fence->pin_count <= 0) <4>[ 227.152064] ------------[ cut here ]------------ <2>[ 227.152068] kernel BUG at drivers/gpu/drm/i915/i915_vma.h:391! <4>[ 227.152084] invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI <0>[ 227.152092] Dumping ftrace buffer: <0>[ 227.152099] (ftrace buffer empty) <4>[ 227.152102] Modules linked in: i915 snd_hda_codec_analog snd_hda_codec_generic coretemp snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm lpc_ich e1000e mei_me mei prime_numbers <4>[ 227.152131] CPU: 1 PID: 1587 Comm: kworker/u16:49 Tainted: G U 4.16.0-rc1-gbab67b2f6177-kasan_7+ #1 <4>[ 227.152134] Hardware name: Dell Inc. OptiPlex 755 /0PU052, BIOS A08 02/19/2008 <4>[ 227.152236] Workqueue: events_unbound intel_atomic_commit_work [i915] <4>[ 227.152292] RIP: 0010:intel_unpin_fb_vma+0x23a/0x2a0 [i915] <4>[ 227.152295] RSP: 0018:ffff88005aad7b68 EFLAGS: 00010286 <4>[ 227.152300] RAX: 0000000000000026 RBX: ffff88005c359580 RCX: 0000000000000000 <4>[ 227.152304] RDX: 0000000000000026 RSI: ffffffff8707d840 RDI: ffffed000b55af63 <4>[ 227.152307] RBP: ffff880056817e58 R08: 0000000000000001 R09: 0000000000000000 <4>[ 227.152311] R10: ffff88005aad7b88 R11: 0000000000000000 R12: ffff8800568184d0 <4>[ 227.152314] R13: ffff880065b5ab08 R14: 0000000000000000 R15: dffffc0000000000 <4>[ 227.152318] FS: 0000000000000000(0000) GS:ffff88006ac40000(0000) knlGS:0000000000000000 <4>[ 227.152322] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 <4>[ 227.152325] CR2: 00007f5fb25550a8 CR3: 0000000068c78000 CR4: 00000000000006e0 <4>[ 227.152328] Call Trace: <4>[ 227.152385] intel_cleanup_plane_fb+0x6b/0xd0 [i915] <4>[ 227.152395] drm_atomic_helper_cleanup_planes+0x166/0x280 <4>[ 227.152452] intel_atomic_commit_tail+0x159d/0x3380 [i915] <4>[ 227.152463] ? process_one_work+0x66e/0x1460 <4>[ 227.152516] ? skl_update_crtcs+0x9c0/0x9c0 [i915] <4>[ 227.152523] ? lock_acquire+0x13d/0x390 <4>[ 227.152527] ? lock_acquire+0x13d/0x390 <4>[ 227.152534] process_one_work+0x71a/0x1460 <4>[ 227.152540] ? __schedule+0x815/0x1e20 <4>[ 227.152547] ? pwq_dec_nr_in_flight+0x2b0/0x2b0 <4>[ 227.152553] ? _raw_spin_lock_irq+0xa/0x40 <4>[ 227.152559] worker_thread+0xdf/0xf60 <4>[ 227.152569] ? process_one_work+0x1460/0x1460 <4>[ 227.152573] kthread+0x2cf/0x3c0 <4>[ 227.152578] ? _kthread_create_on_node+0xa0/0xa0 <4>[ 227.152583] ret_from_fork+0x3a/0x50 <4>[ 227.152591] Code: c6 00 11 86 c0 48 c7 c7 e0 bd 85 c0 e8 60 e7 a9 c4 0f ff e9 1f fe ff ff 48 c7 c6 40 10 86 c0 48 c7 c7 e0 ca 85 c0 e8 2b 95 bd c4 <0f> 0b 48 89 ef e8 4c 44 e8 c4 e9 ef fd ff ff e8 42 44 e8 c4 e9 <1>[ 227.152720] RIP: intel_unpin_fb_vma+0x23a/0x2a0 [i915] RSP: ffff88005aad7b68 v2: i915_vma_pin_fence() is a no-op if a fence isn't required, so check vma->fence as well. Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Ville Syrjälä Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180220134208.24988-2-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_fbdev.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_fbdev.c') diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index da48af11eb6b..3d8ce3a62743 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -177,6 +177,7 @@ static int intelfb_create(struct drm_fb_helper *helper, struct fb_info *info; struct drm_framebuffer *fb; struct i915_vma *vma; + unsigned long flags = 0; bool prealloc = false; void __iomem *vaddr; int ret; @@ -211,7 +212,9 @@ static int intelfb_create(struct drm_fb_helper *helper, * This also validates that any existing fb inherited from the * BIOS is suitable for own access. */ - vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base, DRM_MODE_ROTATE_0); + vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base, + DRM_MODE_ROTATE_0, + &flags); if (IS_ERR(vma)) { ret = PTR_ERR(vma); goto out_unlock; @@ -268,6 +271,7 @@ static int intelfb_create(struct drm_fb_helper *helper, DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08x\n", fb->width, fb->height, i915_ggtt_offset(vma)); ifbdev->vma = vma; + ifbdev->vma_flags = flags; intel_runtime_pm_put(dev_priv); mutex_unlock(&dev->struct_mutex); @@ -275,7 +279,7 @@ static int intelfb_create(struct drm_fb_helper *helper, return 0; out_unpin: - intel_unpin_fb_vma(vma); + intel_unpin_fb_vma(vma, flags); out_unlock: intel_runtime_pm_put(dev_priv); mutex_unlock(&dev->struct_mutex); @@ -513,7 +517,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) if (ifbdev->vma) { mutex_lock(&ifbdev->helper.dev->struct_mutex); - intel_unpin_fb_vma(ifbdev->vma); + intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags); mutex_unlock(&ifbdev->helper.dev->struct_mutex); } -- cgit v1.2.3 From e3c017f15f7ee4c088697d41ee4260986c42a885 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 20 Feb 2018 13:42:07 +0000 Subject: drm/i915/fbdev: Use the PLANE_HAS_FENCE flags from the time of pinning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the information about the fence state from the time of pinning to determine if the fbdev writes are going through a fence. This avoids any confusion in cases where the fence may appear or disappear unconnected to the use by fbdev. Suggested-by: Ville Syrjälä Signed-off-by: Chris Wilson Cc: Ville Syrjälä Reviewed-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20180220134208.24988-3-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_fbdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/intel_fbdev.c') diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 3d8ce3a62743..055f409f8b75 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -48,7 +48,8 @@ static void intel_fbdev_invalidate(struct intel_fbdev *ifbdev) { struct drm_i915_gem_object *obj = ifbdev->fb->obj; - unsigned int origin = ifbdev->vma->fence ? ORIGIN_GTT : ORIGIN_CPU; + unsigned int origin = + ifbdev->vma_flags & PLANE_HAS_FENCE ? ORIGIN_GTT : ORIGIN_CPU; intel_fb_obj_invalidate(obj, origin); } -- cgit v1.2.3