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

[Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>]

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@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@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.

-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;
> +}

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx