On Thu, Jun 01, 2017 at 07:05:41PM +0300, Ville Syrjälä wrote: > On Thu, Jun 01, 2017 at 05:48:16PM +0300, Jani Nikula wrote: > > On Thu, 01 Jun 2017, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > If intel_crtc_disable_noatomic() were to ever get called during resume > > > e'd end up deadlocking since resume has its own acqcuire_ctx but > > > intel_crtc_disable_noatomic() still tries to use the > > > mode_config.acquire_ctx. Pass down the correct acquire ctx from the top. > > > > Same here, have we seen this, or "just" theoretical? > > Happens on my 830 after adding the pipes powerwell. Before that the > machine resumed with both pipes off and the pipe A quirk left > crtc->active as false so intel_crtc_disable_noatomic() didn't get > called. > > Hmm. Actually, now that I think about it more, I believe this should > happen for S4 even without the new powerwell since both pipes should > have been enabled by the BIOS when resuming from S4. I'll have to > run a proper to test to verify that. Indeed. In fact it deadlocks already during the thaw phase between creating the hibernation image and writing to it disk. > > > > > BR, > > Jani. > > > > > > > > Cc: stable@xxxxxxxxxxxxxxx > > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > > Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 9 +++++---- > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index 2f76a63efe8c..4e3c64ed4131 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -5839,7 +5839,8 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state, > > > intel_update_watermarks(intel_crtc); > > > } > > > > > > -static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) > > > +static void intel_crtc_disable_noatomic(struct drm_crtc *crtc, > > > + struct drm_modeset_acquire_ctx *ctx) > > > { > > > struct intel_encoder *encoder; > > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > > > @@ -5869,7 +5870,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) > > > return; > > > } > > > > > > - state->acquire_ctx = crtc->dev->mode_config.acquire_ctx; > > > + state->acquire_ctx = ctx; > > > > > > /* Everything's already locked, -EDEADLK can't happen. */ > > > crtc_state = intel_atomic_get_crtc_state(state, intel_crtc); > > > @@ -15257,7 +15258,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, > > > plane = crtc->plane; > > > crtc->base.primary->state->visible = true; > > > crtc->plane = !plane; > > > - intel_crtc_disable_noatomic(&crtc->base); > > > + intel_crtc_disable_noatomic(&crtc->base, ctx); > > > crtc->plane = plane; > > > } > > > > > > @@ -15273,7 +15274,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, > > > /* Adjust the state of the output pipe according to whether we > > > * have active connectors/encoders. */ > > > if (crtc->active && !intel_crtc_has_encoders(crtc)) > > > - intel_crtc_disable_noatomic(&crtc->base); > > > + intel_crtc_disable_noatomic(&crtc->base, ctx); > > > > > > if (crtc->active || HAS_GMCH_DISPLAY(dev_priv)) { > > > /* > > > > -- > > Jani Nikula, Intel Open Source Technology Center > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC