From a5509abda48ecfc133fac6268e83fc1a93dba039 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Fri, 17 Feb 2017 17:01:59 +0200 Subject: [PATCH] drm/i915: Fix legacy cursor vs. watermarks for ILK-BDW MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Cc: Daniel Vetter Cc: Uwe Kleine-König Reported-by: Uwe Kleine-König Fixes: f79f26921ee1 ("drm/i915: Add a cursor hack to allow converting legacy page flip to atomic, v3.") Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/20170217150159.11683-1-ville.syrjala@linux.intel.com Reviewed-by: Maarten Lankhorst Tested-by: Rafael Ristovski --- drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++----------- drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++--------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9d8c08c4b105..9908bba5b3d3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12955,6 +12955,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; @@ -13389,8 +13400,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); @@ -13413,10 +13423,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; @@ -13456,9 +13462,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); @@ -13468,8 +13477,6 @@ out_free: 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 c6938350a6c4..b00c95e4a81f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1841,20 +1841,24 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate, const struct intel_plane_state *pstate, uint32_t mem_value) { - /* - * 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. - */ - int cpp = 4; - int width = pstate->base.visible ? pstate->base.crtc_w : 64; + int cpp; - if (!cstate->base.active) + /* + * 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. + */ + 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. */