[Intel-gfx] [PATCH] drm/i915: Resume DP MST before doing any kind of modesetting

[Lyude Paul]

Unfortunately MST is a wild beast, and doesn't work at all like other
connectors. This being said, putting it above intel_display_resume() was the
first thing I tried but that didn't work. Originally I had thought putting it
this high up was required because I had assumed drm_mode_config_reset() was
doing modesetting as well but doing some investigation, it doesn't look like
that call actually does anything. It looks like the real culprit here is
intel_runtime_pm_enable_interrupts(), so long as the call to
intel_dp_mst_resume() is above that everything works. So now I'm a little unsure
why this is working like it is, although I can definitely see the patch fixes
the issue. I'm going to edit the patch to reflect this, and see if I can get any
more insight as to why this fixes it.


On Wed, 2016-02-24 at 08:03 +0530, Thulasimani, Sivakumar wrote:
> 
> On 2/24/2016 3:41 AM, Lyude wrote:
> > As it turns out, resuming DP MST is racey since we don't make sure MST
> > is ready before we start modesetting, it just usually happens to be
> > ready in time. This isn't the case on all systems, particularly a
> > ThinkPad T560 with displays connected through the dock. On these
> > systems, resuming the laptop while connected to the dock usually results
> > in blank monitors. Making sure MST is ready before doing any kind of
> > modesetting fixes this issue.
> basic question since i haven't worked on MST much. MST should work like any
> other digital panel on resume. i.e detect followed by modeset. in the 
> modified
> commit mentioned below is it failing to detect the panel or failing at 
> the modeset ?
> if we are depending on the intel_display_resume, how about moving the
> intel_dp_mst_resume just above intel_display_resume?
> 
> 
> Generic question to others in mail list on i915_drm_resume
> we are doing a modeset and then doing the detect/hpd init.
> shouldn't this be the other way round ? almost all displays
> will pass a modeset even if display is not connected so we
> are spending time on modeset even for displays that were
> removed during the suspend state. if this is to simply
> drm_state being saved and restored,
> > We originally changed the resume order in
> > 
> > 	commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw state")
> > 
> > to fix a ton of WARN_ON's after resume, but this doesn't seem to be an
> > issue now anyhow.
> > 
> > CC: stable at vger.kernel.org
> > Signed-off-by: Lyude <cpaul at redhat.com>
> > ---
> >   drivers/gpu/drm/i915/i915_drv.c | 10 ++++++++--
> >   1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index f357058..4dcf3dd 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -733,6 +733,14 @@ static int i915_drm_resume(struct drm_device *dev)
> >   	intel_opregion_setup(dev);
> >   
> >   	intel_init_pch_refclk(dev);
> > +
> > +	/*
> > +	 * We need to make sure that we resume MST before doing anything
> > +	 * display related, otherwise we risk trying to bring up a display
> > on
> > +	 * MST before the hub is actually ready
> > +	 */
> > +	intel_dp_mst_resume(dev);
> > +
> This does not look proper. if the CD clock is turned off during suspend
> our dpcd read itself might fail till we enable CD Clock.
> 
> regards,
> Sivakumar
> >   	drm_mode_config_reset(dev);
> >   
> >   	/*
> > @@ -765,8 +773,6 @@ static int i915_drm_resume(struct drm_device *dev)
> >   	intel_display_resume(dev);
> >   	drm_modeset_unlock_all(dev);
> >   
> > -	intel_dp_mst_resume(dev);
> > -
> >   	/*
> >   	 * ... but also need to make sure that hotplug processing
> >   	 * doesn't cause havoc. Like in the driver load code we don't
>