On Mon, Aug 11, 2014 at 01:15:35PM +0300, [email protected] wrote: > From: Ville Syrjälä <[email protected]> > > 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ä <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]> Cc: [email protected] (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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
