On Fri, Nov 09, 2018 at 12:20:11PM -0800, José Roberto de Souza wrote:
> When a PSR error happens sink sets the PSR errors register and also
> set the link to a error status.
> So in the short pulse handling it was returning earlier and doing a
> full detection and attempting to retrain but it fails as PSR HW is
> in change of the main-link.
>
> Just call intel_psr_short_pulse() before
> intel_dp_needs_link_retrain() is not the right fix as
> intel_dp_needs_link_retrain() would return true and trigger a full
> detection while PSR HW is still in change of main-link.
>
> Check for PSR active is also not safe as it could be inactive due a
> frontbuffer invalidate and still doing the PSR exit sequence.
>
> v3: added comment in intel_dp_needs_link_retrain()
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
I wonder if we shouldn't disable psr and then move with
link training on situations like this instead of dropping some
link trainings... Not sure if I trust PSR more to take care of
this case. But well, maybe this case is only happening on
recoverable PSR errors where hpd pulses end up happening so
this should be ok.
Let's try this approach, but please consider the option
to disable psr instead if we face some issues around here...
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 11 +++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_psr.c | 15 +++++++++++++++
> 3 files changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 577c166f6483..158c6f25d2e2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4383,6 +4383,17 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
> if (!intel_dp->link_trained)
> return false;
>
> + /*
> + * While PSR source HW is enabled, it will control main-link sending
> + * frames, enabling and disabling it so trying to do a retrain will fail
> + * as the link would or not be on or it could mix training patterns
> + * and frame data at the same time causing retrain to fail.
> + * Also when exiting PSR, HW will retrain the link anyways fixing
> + * any link status error.
> + */
> + if (intel_psr_enabled(intel_dp))
> + 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 cc7fab2b61f4..12f96297299e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2046,6 +2046,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_quirks.c */
> void intel_init_quirks(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 48df16a02fac..f940305b72e4 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1107,3 +1107,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;
> +}
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
