[Intel-gfx] [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[Ville Syrjälä]

On Tue, Nov 29, 2016 at 10:41:45PM +0200, Imre Deak wrote:
> On Tue, 2016-11-29 at 22:27 +0200, Ville Syrjälä wrote:
> > On Tue, Nov 29, 2016 at 10:14:23PM +0200, Imre Deak wrote:
> > > On Tue, 2016-11-29 at 21:55 +0200, Ville Syrjälä wrote:
> > > > On Tue, Nov 29, 2016 at 09:40:29PM +0200, Imre Deak wrote:
> > > > > For LSPCON initialization during system resume we need AUX
> > > > > functionality, but we call the corresponding encoder reset hook
> > > > > with all
> > > > > interrupts disabled. Without interrupts we'll do a poll-wait
> > > > > for AUX
> > > > > transfer completions, which adds a significant delay if the
> > > > > transfers
> > > > > timeout/need to be retried for some reason.
> > > > > 
> > > > > Fix this by enabling interrupts before calling the reset hooks.
> > > > > Note
> > > > > that while this will enable AUX interrupts it will keep HPD
> > > > > interrupts
> > > > > disabled, in a similar way to the init time output setup code.
> > > > > 
> > > > > This issue existed since LSPCON support was added.
> > > > > 
> > > > > v2:
> > > > > - Rebased on drm-tip.
> > > > > 
> > > > > Cc: Shashank Sharma <shashank.sharma at intel.com>
> > > > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_drv.c | 5 ++++-
> > > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > > > b/drivers/gpu/drm/i915/i915_drv.c
> > > > > index 8dac298..2cea2ef 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > > @@ -1582,18 +1582,21 @@ static int i915_drm_resume(struct
> > > > > drm_device *dev)
> > > > >  	intel_opregion_setup(dev_priv);
> > > > >  
> > > > >  	intel_init_pch_refclk(dev_priv);
> > > > > -	drm_mode_config_reset(dev);
> > > > >  
> > > > >  	/*
> > > > >  	 * Interrupts have to be enabled before any batches
> > > > > are run. If not the
> > > > >  	 * GPU will hang. i915_gem_init_hw() will initiate
> > > > > batches to
> > > > >  	 * update/restore the context.
> > > > >  	 *
> > > > > +	 * drm_mode_config_reset() needs AUX interrupts.
> > > > > +	 *
> > > > >  	 * Modeset enabling in intel_modeset_init_hw() also
> > > > > needs working
> > > > >  	 * interrupts.
> > > > >  	 */
> > > > >  	intel_runtime_pm_enable_interrupts(dev_priv);
> > > > >  
> > > > > +	drm_mode_config_reset(dev);
> > > > > +
> > > > 
> > > > Didn't look like anything should seriously explode.
> > > > 
> > > > Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > 
> > > > There is a slight concern on g4x/vlv/chv that an AUX interrupts
> > > > would
> > > > trigger the hpd irq handler, which doesn't realize it's supposed
> > > > to
> > > > ignore the actual hpd bits in PORT_HOTPLUG_STAT. So any aux
> > > > before we
> > > > enable hpd processing for real could do something bad. So I guess
> > > > we
> > > > should add some kind of software tracking for that stuff like we
> > > > have
> > > > for PIPESTAT.
> > > 
> > > Didn't think about that, but BSpec tells me those are masked by the
> > > HPD
> > > IRQ enable bits in PORT_HOTPLUG_EN and those we enable only later.
> > > Otherwise this would be also a problem during output setup time.
> > 
> > Hmm. Are they really masked? I though it's just an IER effectively.
> 
> I only tried for real on BXT/SKL where I had to enable the interrupts
> (in PCH_PORT_HOTPLUG) for HPD sensing. The CHV BSpec suggests the same
> for the live state bits, but yes it's not clear if the long/short
> detect bits are completely masked by the enable flags or they are just
> not propagated if not enabled. Will give it a try tomorrow.

Hmm. Yeah, we did in fact chat about this. Already forgot. Spec seems to
suggest you are correct. But checking on actual hw is always a good
idea.

> 
> > 
> > > 
> > > --Imre
> > > 
> > > > 
> > > > >  	mutex_lock(&dev->struct_mutex);
> > > > >  	if (i915_gem_init_hw(dev)) {
> > > > >  		DRM_ERROR("failed to re-initialize GPU,
> > > > > declaring wedged!\n");
> > > > > -- 
> > > > > 2.5.0
> > > > > 
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx at lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > 
> > 

-- 
Ville Syrjälä
Intel OTC