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

Chris Wilson
2014-06-09T06:47:38Z

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@rus.uni-stuttgart.de>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Thomas Richter <richter@rus.uni-stuttgart.de>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org