[PATCH] drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW

[ville.syrjala@linux.intel.com]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

In order to make cursor updates actually safe wrt. watermark programming
we have to clear the legacy_cursor_update flag in the atomic state. That
will cause the regular atomic update path to do the necessary vblank
wait after the plane update if needed, otherwise the vblank wait would
be skipped and we'd feed the optimal watermarks to the hardware before
the plane update has actually happened.

To make the slow vs. fast path determination in
intel_legacy_cursor_update() a little simpler we can ignore the actual
visibility of the plane (which can only get computed once we've already
chosen out path) and instead we simply check whether the fb is being
set or cleared by the user. This means a fully clipped but logically
visible cursor will be considered visible as far as watermark
programming is concerned. We can do that for the cursor since it's a
fixed size plane and the clipped size doesn't play a role in the
watermark computation.

This should fix underruns that can occur when the cursor gets
enable/disabled or the size gets changed. Hopefully it's good enough
that only pure cursor movement and flips go through unthrottled.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Reported-by: Uwe Kleine-König <uwe@kleine-koenig.org>
Fixes: f79f26921ee1 ("drm/i915: Add a cursor hack to allow converting legacy page flip to atomic, v3.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_pm.c      | 20 ++++++++++++--------
 2 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b05d9c85384b..356ac04093e8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13031,6 +13031,17 @@ static int intel_atomic_commit(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	int ret = 0;
 
+	/*
+	 * The intel_legacy_cursor_update() fast path takes care
+	 * of avoiding the vblank waits for simple cursor
+	 * movement and flips. For cursor on/off and size changes,
+	 * we want to perform the vblank waits so that watermark
+	 * updates happen during the correct frames. Gen9+ have
+	 * double buffered watermarks and so shouldn't need this.
+	 */
+	if (INTEL_GEN(dev_priv) < 9)
+		state->legacy_cursor_update = false;
+
 	ret = drm_atomic_helper_setup_commit(state, nonblock);
 	if (ret)
 		return ret;
@@ -13455,8 +13466,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	    old_plane_state->src_h != src_h ||
 	    old_plane_state->crtc_w != crtc_w ||
 	    old_plane_state->crtc_h != crtc_h ||
-	    !old_plane_state->visible ||
-	    old_plane_state->fb->modifier != fb->modifier)
+	    !old_plane_state->fb != !fb)
 		goto slow;
 
 	new_plane_state = intel_plane_duplicate_state(plane);
@@ -13479,10 +13489,6 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	if (ret)
 		goto out_free;
 
-	/* Visibility changed, must take slowpath. */
-	if (!new_plane_state->visible)
-		goto slow_free;
-
 	ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex);
 	if (ret)
 		goto out_free;
@@ -13522,9 +13528,12 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	new_plane_state->fb = old_fb;
 	to_intel_plane_state(new_plane_state)->vma = old_vma;
 
-	intel_plane->update_plane(plane,
-				  to_intel_crtc_state(crtc->state),
-				  to_intel_plane_state(plane->state));
+	if (plane->state->visible)
+		intel_plane->update_plane(plane,
+					  to_intel_crtc_state(crtc->state),
+					  to_intel_plane_state(plane->state));
+	else
+		intel_plane->disable_plane(plane, crtc);
 
 	intel_cleanup_plane_fb(plane, new_plane_state);
 
@@ -13534,8 +13543,6 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 	intel_plane_destroy_state(plane, new_plane_state);
 	return ret;
 
-slow_free:
-	intel_plane_destroy_state(plane, new_plane_state);
 slow:
 	return drm_atomic_helper_update_plane(plane, crtc, fb,
 					      crtc_x, crtc_y, crtc_w, crtc_h,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fe243c65de1a..4de8c40acc7e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1831,20 +1831,24 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
 				   const struct intel_plane_state *pstate,
 				   uint32_t mem_value)
 {
+	int cpp;
+
 	/*
-	 * We treat the cursor plane as always-on for the purposes of watermark
-	 * calculation.  Until we have two-stage watermark programming merged,
-	 * this is necessary to avoid flickering.
+	 * Treat cursor with fb as always visible since cursor updates
+	 * can happen faster than the vrefresh rate, and the current
+	 * watermark code doesn't handle that correctly. Cursor updates
+	 * which set/clear the fb or change the cursor size are going
+	 * to get throttled by intel_legacy_cursor_update() to work
+	 * around this problem with the watermark code.
 	 */
-	int cpp = 4;
-	int width = pstate->base.visible ? pstate->base.crtc_w : 64;
-
-	if (!cstate->base.active)
+	if (!cstate->base.active || !pstate->base.fb)
 		return 0;
 
+	cpp = pstate->base.fb->format->cpp[0];
+
 	return ilk_wm_method2(cstate->pixel_rate,
 			      cstate->base.adjusted_mode.crtc_htotal,
-			      width, cpp, mem_value);
+			      pstate->base.crtc_w, cpp, mem_value);
 }
 
 /* Only for WM_LP. */
-- 
2.10.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx