[Intel-gfx] [PATCH 4/6] drm/i915/skl: Always wait for pipes to update after a flush

[Lyude]

As we've learned, all watermark updates on Skylake have to be strictly
atomic or things fail. While the bspec doesn't mandate that we need to
wait for pipes to finish after the third iteration of flushes, not doing
so gives us the opportunity to break this atomicity later. This example
assumes that we're lucky enough not to be interrupted by the scheduler
at any point during this:

 - Start with pipe A and pipe B enabled
 - Enable pipe C
 - Flush pipe A in pass 1, wait until update finishes
 - Flush pipe B in pass 3, continue without waiting for next vblank
 - Start another wm update
 - We enter the next vblank for pipe B before we finish writing all the
   vm values
 - *Underrun*

As such, we always need to wait for each pipe we flush to update so as
to never break this atomicity.

Fixes: 0e8fb7ba7ca5 ("drm/i915/skl: Flush the WM configuration")
Signed-off-by: Lyude <cpaul at redhat.com>
Cc: stable at vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Daniel Vetter <daniel.vetter at intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
Cc: Hans de Goede <hdegoede at redhat.com> <cpaul at redhat.com>
Cc: Matt Roper <matthew.d.roper at intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 788db86..2e31df4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3859,8 +3859,11 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
 	/*
 	 * Third pass: flush the pipes that got more space allocated.
 	 *
-	 * We don't need to actively wait for the update here, next vblank
-	 * will just get more DDB space with the correct WM values.
+	 * While the hardware doesn't require to wait for the next vblank here,
+	 * continuing before the pipe finishes updating could result in us
+	 * trying to update the wm values again before the pipe finishes
+	 * updating, which results in the hardware using intermediate wm values
+	 * and subsequently underrunning pipes.
 	 */
 	for_each_intel_crtc(dev, crtc) {
 		if (!crtc->active)
@@ -3876,6 +3879,16 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
 			continue;
 
 		skl_wm_flush_pipe(dev_priv, pipe, 3);
+
+		/*
+		 * The only time we can get away with not waiting for an update
+		 * is when we just enabled the pipe, e.g. when it doesn't have
+		 * vblanks enabled anyway.
+		 */
+		if (drm_crtc_vblank_get(&crtc->base) == 0) {
+			intel_wait_for_vblank(dev, pipe);
+			drm_crtc_vblank_put(&crtc->base);
+		}
 	}
 }
 
-- 
2.7.4