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

[Imre Deak <imre.deak@intel.com>]

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@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@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@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.

--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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx