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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 20 Jul 2016 17:00:00 -0400
From:	Lyude <cpaul@...hat.com>
To:	intel-gfx@...ts.freedesktop.org
Cc:	Lyude <cpaul@...hat.com>, stable@...r.kernel.org,
	Ville Syrjälä 
	<ville.syrjala@...ux.intel.com>,
	Daniel Vetter <daniel.vetter@...el.com>,
	Radhakrishna Sripada <radhakrishna.sripada@...el.com>,
	Matt Roper <matthew.d.roper@...el.com>,
	Jani Nikula <jani.nikula@...ux.intel.com>,
	David Airlie <airlied@...ux.ie>,
	dri-devel@...ts.freedesktop.org (open list:INTEL DRM DRIVERS (excluding
	Poulsbo, Moorestow...), linux-kernel@...r.kernel.org (open list))
Subject: [PATCH 4/6] drm/i915/skl: Always wait for pipes to update after a flush

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@...hat.com>
Cc: stable@...r.kernel.org
Cc: Ville Syrjälä <ville.syrjala@...ux.intel.com>
Cc: Daniel Vetter <daniel.vetter@...el.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@...el.com>
Cc: Hans de Goede <hdegoede@...hat.com> <cpaul@...hat.com>
Cc: Matt Roper <matthew.d.roper@...el.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