[Intel-gfx] [PATCH v2 6/6] drm/i915: Check PSR errors instead of retrain while PSR is enabled

[Souza, Jose]

On Fri, 2018-10-19 at 14:02 -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2018-10-10 at 17:41 -0700, José Roberto de Souza wrote:
> > When a PSR error happens sink also update the link status values
> > while source do not retrain link as required in the PSR exit
> > sequence.
> > So in the short pulse handling it was returning earlier and doing a
> > full detection and attempting to retrain but it fails because for
> > most sinks the main link is disabled while PSR is active, before
> > even
> > check PSR errors.
> > 
> > Just call intel_psr_short_pulse() before
> > intel_dp_needs_link_retrain() is also not the right fix as
> > intel_dp_needs_link_retrain() would return true and trigger a full
> > detection and trying to retrain link while PSR HW is also doing
> > that.
> > 
> > Check for PSR active is also not safe as it could be inactive due a
> > frontbuffer invalidate and still doing the PSR exit sequence.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  |  7 +++++++
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  drivers/gpu/drm/i915/intel_psr.c | 15 +++++++++++++++
> >  3 files changed, 23 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 1826d491efd7..2a21a9e29eb2 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4396,6 +4396,13 @@ intel_dp_needs_link_retrain(struct intel_dp
> > *intel_dp)
> >  	if (!intel_dp->link_trained)
> >  		return false;
> >  
> > +	/*
> > +	 * While PSR is enabled, HW will control main-link and retrain
> > when
> > +	 * exiting PSR
> > +	 */
> > +	if (intel_psr_enabled(intel_dp))
> 
> You need a check for !link_standby. The link status is expected to be
> good if we are not turning the link fully off.

Even with link on while PSR is active, sink could lost sync with main
link.

> 
> -DK
> 
> > +		return false;
> > +
> >  	if (!intel_dp_get_link_status(intel_dp, link_status))
> >  		return false;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 3dea7a1bda7f..147a116cfc71 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1958,6 +1958,7 @@ void intel_psr_irq_handler(struct
> > drm_i915_private *dev_priv, u32 psr_iir);
> >  void intel_psr_short_pulse(struct intel_dp *intel_dp);
> >  int intel_psr_wait_for_idle(const struct intel_crtc_state
> > *new_crtc_state,
> >  			    u32 *out_value);
> > +bool intel_psr_enabled(struct intel_dp *intel_dp);
> >  
> >  /* intel_runtime_pm.c */
> >  int intel_power_domains_init(struct drm_i915_private *);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index ad09130cb4ad..e4429f2f3034 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -1138,3 +1138,18 @@ void intel_psr_short_pulse(struct intel_dp
> > *intel_dp)
> >  exit:
> >  	mutex_unlock(&psr->lock);
> >  }
> > +
> > +bool intel_psr_enabled(struct intel_dp *intel_dp)
> > +{
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +	bool ret;
> > +
> > +	if (!CAN_PSR(dev_priv) || !intel_dp_is_edp(intel_dp))
> > +		return false;
> > +
> > +	mutex_lock(&dev_priv->psr.lock);
> > +	ret = (dev_priv->psr.dp == intel_dp && dev_priv->psr.enabled);
> > +	mutex_unlock(&dev_priv->psr.lock);
> > +
> > +	return ret;
> > +}