drm/i915: Move the policy for placement of the GGTT vma into the caller

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 <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180220134208.24988-2-chris@chris-wilson.co.uk

1 files changed, 35 insertions, 10 deletions

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8cdf4dd2b334..75baa5dab877 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2068,13 +2068,16 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
 }
 
 struct i915_vma *
-intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
+intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
+			   unsigned int rotation,
+			   unsigned long *out_flags)
 {
 	struct drm_device *dev = fb->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	struct i915_ggtt_view view;
 	struct i915_vma *vma;
+	unsigned int pinctl;
 	u32 alignment;
 
 	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -2102,7 +2105,20 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
 
 	atomic_inc(&dev_priv->gpu_error.pending_fb_pin);
 
-	vma = i915_gem_object_pin_to_display_plane(obj, alignment, &view);
+	pinctl = 0;
+
+	/* Valleyview is definitely limited to scanning out the first
+	 * 512MiB. Lets presume this behaviour was inherited from the
+	 * g4x display engine and that all earlier gen are similarly
+	 * limited. Testing suggests that it is a little more
+	 * complicated than this. For example, Cherryview appears quite
+	 * happy to scanout from anywhere within its global aperture.
+	 */
+	if (HAS_GMCH_DISPLAY(dev_priv))
+		pinctl |= PIN_MAPPABLE;
+
+	vma = i915_gem_object_pin_to_display_plane(obj,
+						   alignment, &view, pinctl);
 	if (IS_ERR(vma))
 		goto err;
 
@@ -2123,7 +2139,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
 		 * something and try to run the system in a "less than optimal"
 		 * mode that matches the user configuration.
 		 */
-		i915_vma_pin_fence(vma);
+		if (i915_vma_pin_fence(vma) == 0 && vma->fence)
+			*out_flags |= PLANE_HAS_FENCE;
 	}
 
 	i915_vma_get(vma);
@@ -2134,11 +2151,12 @@ err:
 	return vma;
 }
 
-void intel_unpin_fb_vma(struct i915_vma *vma)
+void intel_unpin_fb_vma(struct i915_vma *vma, unsigned long flags)
 {
 	lockdep_assert_held(&vma->vm->i915->drm.struct_mutex);
 
-	i915_vma_unpin_fence(vma);
+	if (flags & PLANE_HAS_FENCE)
+		i915_vma_unpin_fence(vma);
 	i915_gem_object_unpin_from_display_plane(vma);
 	i915_vma_put(vma);
 }
@@ -2808,7 +2826,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 valid_fb:
 	mutex_lock(&dev->struct_mutex);
 	intel_state->vma =
-		intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
+		intel_pin_and_fence_fb_obj(fb,
+					   primary->state->rotation,
+					   &intel_state->flags);
 	mutex_unlock(&dev->struct_mutex);
 	if (IS_ERR(intel_state->vma)) {
 		DRM_ERROR("failed to pin boot fb on pipe %d: %li\n",
@@ -12700,7 +12720,9 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	} else {
 		struct i915_vma *vma;
 
-		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
+		vma = intel_pin_and_fence_fb_obj(fb,
+						 new_state->rotation,
+						 &to_intel_plane_state(new_state)->flags);
 		if (!IS_ERR(vma))
 			to_intel_plane_state(new_state)->vma = vma;
 		else
@@ -12755,7 +12777,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 	vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma);
 	if (vma) {
 		mutex_lock(&plane->dev->struct_mutex);
-		intel_unpin_fb_vma(vma);
+		intel_unpin_fb_vma(vma, to_intel_plane_state(old_state)->flags);
 		mutex_unlock(&plane->dev->struct_mutex);
 	}
 }
@@ -13111,7 +13133,9 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 			goto out_unlock;
 		}
 	} else {
-		vma = intel_pin_and_fence_fb_obj(fb, new_plane_state->rotation);
+		vma = intel_pin_and_fence_fb_obj(fb,
+						 new_plane_state->rotation,
+						 &to_intel_plane_state(new_plane_state)->flags);
 		if (IS_ERR(vma)) {
 			DRM_DEBUG_KMS("failed to pin object\n");
 
@@ -13142,7 +13166,8 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 
 	old_vma = fetch_and_zero(&to_intel_plane_state(old_plane_state)->vma);
 	if (old_vma)
-		intel_unpin_fb_vma(old_vma);
+		intel_unpin_fb_vma(old_vma,
+				   to_intel_plane_state(old_plane_state)->flags);
 
 out_unlock:
 	mutex_unlock(&dev_priv->drm.struct_mutex);