[Intel-gfx] [PATCH] drm/i915: Avoid double mutex lock applying pipe A quirk during sanitize_crtc()

Chris Wilson chris at chris-wilson.co.uk
Mon Jun 9 08:47:10 CEST 2014


Thomas found that his machine would deadlock reloading the i915.ko
module after resume. He identified that this was caused by the
reacquisition of the connection mutex inside intel_enable_pipe_a()
during the CRTC sanitization routine. This will only affect machines
that quirk PIPE A, i.e. the original 830m chipsets.

This patch move the locking into a wrapper function so that
intel_enable_pipe_a() can bypass the locking knowing that it already
holds the correct locks.

Reported-by: Thomas Richter <richter at rus.uni-stuttgart.de>
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Thomas Richter <richter at rus.uni-stuttgart.de>
Cc: Daniel Vetter <daniel at ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   54 ++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c1f79a1..26d3424 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8387,9 +8387,10 @@ mode_fits_in_fbdev(struct drm_device *dev,
 #endif
 }
 
-bool intel_get_load_detect_pipe(struct drm_connector *connector,
-				struct drm_display_mode *mode,
-				struct intel_load_detect_pipe *old)
+static bool
+__intel_get_load_detect_pipe(struct drm_connector *connector,
+			     struct drm_display_mode *mode,
+			     struct intel_load_detect_pipe *old)
 {
 	struct intel_crtc *intel_crtc;
 	struct intel_encoder *intel_encoder =
@@ -8405,7 +8406,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 		      connector->base.id, connector->name,
 		      encoder->base.id, encoder->name);
 
-	mutex_lock(&dev->mode_config.connection_mutex);
+	lockdep_assert_held(&dev->mode_config.connection_mutex);
 
 	/*
 	 * Algorithm gets a little messy:
@@ -8449,7 +8450,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	 */
 	if (!crtc) {
 		DRM_DEBUG_KMS("no pipe available for load-detect\n");
-		goto fail_unlock_connector;
+		return false;
 	}
 
 	mutex_lock(&crtc->mutex);
@@ -8503,14 +8504,13 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	else
 		intel_crtc->new_config = NULL;
 	mutex_unlock(&crtc->mutex);
-fail_unlock_connector:
-	mutex_unlock(&dev->mode_config.connection_mutex);
 
 	return false;
 }
 
-void intel_release_load_detect_pipe(struct drm_connector *connector,
-				    struct intel_load_detect_pipe *old)
+static void
+__intel_release_load_detect_pipe(struct drm_connector *connector,
+				 struct intel_load_detect_pipe *old)
 {
 	struct intel_encoder *intel_encoder =
 		intel_attached_encoder(connector);
@@ -8518,6 +8518,8 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 	struct drm_crtc *crtc = encoder->crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
+	lockdep_assert_held(&connector->dev->mode_config.connection_mutex);
+
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
 		      connector->base.id, connector->name,
 		      encoder->base.id, encoder->name);
@@ -8533,17 +8535,32 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 			drm_framebuffer_unregister_private(old->release_fb);
 			drm_framebuffer_unreference(old->release_fb);
 		}
+	} else {
+		/* Switch crtc and encoder back off if necessary */
+		if (old->dpms_mode != DRM_MODE_DPMS_ON)
+			connector->funcs->dpms(connector, old->dpms_mode);
+	}
+	mutex_unlock(&crtc->mutex);
+}
 
-		mutex_unlock(&crtc->mutex);
+bool intel_get_load_detect_pipe(struct drm_connector *connector,
+				struct drm_display_mode *mode,
+				struct intel_load_detect_pipe *old)
+{
+	mutex_lock(&connector->dev->mode_config.connection_mutex);
+	if (!__intel_get_load_detect_pipe(connector, mode, old)) {
 		mutex_unlock(&connector->dev->mode_config.connection_mutex);
-		return;
+		return false;
 	}
 
-	/* Switch crtc and encoder back off if necessary */
-	if (old->dpms_mode != DRM_MODE_DPMS_ON)
-		connector->funcs->dpms(connector, old->dpms_mode);
+	/* lock will be released by intel_release_load_detect_pipe() */
+	return true;
+}
 
-	mutex_unlock(&crtc->mutex);
+void intel_release_load_detect_pipe(struct drm_connector *connector,
+				    struct intel_load_detect_pipe *old)
+{
+	__intel_release_load_detect_pipe(connector, old);
 	mutex_unlock(&connector->dev->mode_config.connection_mutex);
 }
 
@@ -12354,10 +12371,9 @@ static void intel_enable_pipe_a(struct drm_device *dev)
 	if (!crt)
 		return;
 
-	if (intel_get_load_detect_pipe(crt, NULL, &load_detect_temp))
-		intel_release_load_detect_pipe(crt, &load_detect_temp);
-
-
+	lockdep_assert_held(&dev->mode_config.connection_mutex);
+	if (__intel_get_load_detect_pipe(crt, NULL, &load_detect_temp))
+		__intel_release_load_detect_pipe(crt, &load_detect_temp);
 }
 
 static bool
-- 
1.7.9.5




More information about the Intel-gfx mailing list