drm/i915: fixup the plane->pipe fixup code

Commit Message

Daniel Vetter Sept. 8, 2012, 4:14 p.m. UTC

We need to check whether the _other plane is on our pipe, not whether
our plane is on the other pipe. Otherwise if not both pipes/planes are
active, we won't properly clean up the mess and set up our desired
plane->pipe mapping.

v2: Fixup the logic, I've totally fumbled it. Noticed by Chris Wilson.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51265
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49838
Signed-off-by: Daniel Vetter <[e-mail]>
---
 drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

Comments

Chris Wilson Sept. 8, 2012, 7 p.m. UTC | #1

On Sat,  8 Sep 2012 18:14:52 +0200, Daniel Vetter <[e-mail]> wrote:
> We need to check whether the _other plane is on our pipe, not whether
> our plane is on the other pipe. Otherwise if not both pipes/planes are
> active, we won't properly clean up the mess and set up our desired
> plane->pipe mapping.
> 
> v2: Fixup the logic, I've totally fumbled it. Noticed by Chris Wilson.

Frankly I'd be happier if we did both as that way we should be
idiotproof, and more importantly obviously safe.
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fd9c275..46f4598 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7994,11 +7994,27 @@  static void intel_enable_pipe_a(struct drm_device *dev)
 
 }
 
+static bool
+intel_check_plane_mapping(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	u32 reg, val;
+
+	reg = DSPCNTR(!crtc->plane);
+	val = I915_READ(reg);
+
+	if ((val & DISPLAY_PLANE_ENABLE) &&
+	    (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
+		return false;
+
+	return true;
+}
+
 static void intel_sanitize_crtc(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 reg, val;
+	u32 reg;
 
 	/* Clear any frame start delays used for debugging left by the BIOS */
 	reg = PIPECONF(crtc->pipe);
@@ -8006,17 +8022,10 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc)
 
 	/* We need to sanitize the plane -> pipe mapping first because this will
 	 * disable the crtc (and hence change the state) if it is wrong. */
-	if (!HAS_PCH_SPLIT(dev)) {
+	if (!HAS_PCH_SPLIT(dev) && !intel_check_plane_mapping(crtc)) {
 		struct intel_connector *connector;
 		bool plane;
 
-		reg = DSPCNTR(crtc->plane);
-		val = I915_READ(reg);
-
-		if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
-		    (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
-			goto ok;
-
 		DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
 			      crtc->base.base.id);
 
@@ -8040,7 +8049,6 @@  static void intel_sanitize_crtc(struct intel_crtc *crtc)
 		WARN_ON(crtc->active);
 		crtc->base.enabled = false;
 	}
-ok:
 
 	if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
 	    crtc->pipe == PIPE_A && !crtc->active) {