On Mon, Jun 09, 2014 at 07:47:10AM +0100, Chris Wilson wrote:
> 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@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Thomas Richter <richter@xxxxxxxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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);
> + }
This part looks like an unrelated change?
-Daniel
> + 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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
