[Intel-gfx] [PATCH 1/2] drm/i915: Fix locking for intel_enable_pipe_a()
Daniel Vetter
daniel at ffwll.ch
Mon Aug 11 13:29:56 CEST 2014
On Mon, Aug 11, 2014 at 01:15:35PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> intel_enable_pipe_a() gets called with all the modeset locks already
> held (by drm_modeset_lock_all()), so trying to grab the same
> locks using another drm_modeset_acquire_ctx is going to fail miserably.
>
> Move most of the drm_modeset_acquire_ctx handling (init/drop/fini)
> out from intel_{get,release}_load_detect_pipe() into the callers
> (intel_{crt,tv}_detect()). Only the actual locking and backoff
> handling is left in intel_get_load_detect_pipe(). And in
> intel_enable_pipe_a() we just share the mode_config.acquire_ctx from
> drm_modeset_lock_all() which is already holding all the relevant locks.
>
> It's perfectly legal to lock the same ww_mutex multiple times using the
> same ww_acquire_ctx. drm_modeset_lock() will convert the returned
> -EALREADY into 0, so the caller doesn't need to do antyhing special.
>
> Fixes a hang on resume on my 830.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: stable at vger.kernel.org (for 3.16)
> ---
> drivers/gpu/drm/i915/intel_crt.c | 7 ++++++-
> drivers/gpu/drm/i915/intel_display.c | 21 ++++-----------------
> drivers/gpu/drm/i915/intel_drv.h | 3 +--
> drivers/gpu/drm/i915/intel_tv.c | 7 ++++++-
> 4 files changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 2efaf8e..e8abfce 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -699,16 +699,21 @@ intel_crt_detect(struct drm_connector *connector, bool force)
> goto out;
> }
>
> + drm_modeset_acquire_init(&ctx, 0);
> +
> /* for pre-945g platforms use load detect */
> if (intel_get_load_detect_pipe(connector, NULL, &tmp, &ctx)) {
> if (intel_crt_detect_ddc(connector))
> status = connector_status_connected;
> else
> status = intel_crt_load_detect(crt);
> - intel_release_load_detect_pipe(connector, &tmp, &ctx);
> + intel_release_load_detect_pipe(connector, &tmp);
> } else
> status = connector_status_unknown;
>
> + drm_modeset_drop_locks(&ctx);
> + drm_modeset_acquire_fini(&ctx);
> +
> out:
> intel_display_power_put(dev_priv, power_domain);
> return status;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 51f48d9..7953b46 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8440,8 +8440,6 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
> connector->base.id, connector->name,
> encoder->base.id, encoder->name);
>
> - drm_modeset_acquire_init(ctx, 0);
> -
> retry:
> ret = drm_modeset_lock(&config->connection_mutex, ctx);
> if (ret)
> @@ -8552,15 +8550,11 @@ fail_unlock:
> goto retry;
> }
>
> - drm_modeset_drop_locks(ctx);
> - drm_modeset_acquire_fini(ctx);
> -
> return false;
> }
>
> void intel_release_load_detect_pipe(struct drm_connector *connector,
> - struct intel_load_detect_pipe *old,
> - struct drm_modeset_acquire_ctx *ctx)
> + struct intel_load_detect_pipe *old)
> {
> struct intel_encoder *intel_encoder =
> intel_attached_encoder(connector);
> @@ -8584,17 +8578,12 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> drm_framebuffer_unreference(old->release_fb);
> }
>
> - goto unlock;
> return;
> }
>
> /* Switch crtc and encoder back off if necessary */
> if (old->dpms_mode != DRM_MODE_DPMS_ON)
> connector->funcs->dpms(connector, old->dpms_mode);
> -
> -unlock:
> - drm_modeset_drop_locks(ctx);
> - drm_modeset_acquire_fini(ctx);
> }
>
> static int i9xx_pll_refclk(struct drm_device *dev,
> @@ -12652,7 +12641,7 @@ static void intel_enable_pipe_a(struct drm_device *dev)
> struct intel_connector *connector;
> struct drm_connector *crt = NULL;
> struct intel_load_detect_pipe load_detect_temp;
> - struct drm_modeset_acquire_ctx ctx;
> + struct drm_modeset_acquire_ctx *ctx = dev->mode_config.acquire_ctx;
>
> /* We can't just switch on the pipe A, we need to set things up with a
> * proper mode and output configuration. As a gross hack, enable pipe A
> @@ -12669,10 +12658,8 @@ static void intel_enable_pipe_a(struct drm_device *dev)
> if (!crt)
> return;
>
> - if (intel_get_load_detect_pipe(crt, NULL, &load_detect_temp, &ctx))
> - intel_release_load_detect_pipe(crt, &load_detect_temp, &ctx);
> -
> -
> + if (intel_get_load_detect_pipe(crt, NULL, &load_detect_temp, ctx))
> + intel_release_load_detect_pipe(crt, &load_detect_temp);
> }
>
> static bool
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1b3d1d7..0dd23f1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -834,8 +834,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
> struct intel_load_detect_pipe *old,
> struct drm_modeset_acquire_ctx *ctx);
> void intel_release_load_detect_pipe(struct drm_connector *connector,
> - struct intel_load_detect_pipe *old,
> - struct drm_modeset_acquire_ctx *ctx);
> + struct intel_load_detect_pipe *old);
> int intel_pin_and_fence_fb_obj(struct drm_device *dev,
> struct drm_i915_gem_object *obj,
> struct intel_engine_cs *pipelined);
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index e211eef..32186a6 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1323,11 +1323,16 @@ intel_tv_detect(struct drm_connector *connector, bool force)
> struct intel_load_detect_pipe tmp;
> struct drm_modeset_acquire_ctx ctx;
>
> + drm_modeset_acquire_init(&ctx, 0);
> +
> if (intel_get_load_detect_pipe(connector, &mode, &tmp, &ctx)) {
> type = intel_tv_detect_type(intel_tv, connector);
> - intel_release_load_detect_pipe(connector, &tmp, &ctx);
> + intel_release_load_detect_pipe(connector, &tmp);
> } else
> return connector_status_unknown;
> +
> + drm_modeset_drop_locks(&ctx);
> + drm_modeset_acquire_fini(&ctx);
> } else
> return connector->status;
>
> --
> 1.8.5.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list