[Intel-gfx] [PATCH] drm/i915: Make sure PSR is ready for been re-enabled.

[Paulo Zanoni]

2014-09-24 19:16 GMT-03:00 Rodrigo Vivi <rodrigo.vivi at intel.com>:
> Let's make sure PSR is propperly disabled before to re-enabled it.
>
> According to Spec, after disabled PSR CTL, the Idle state might occur
> up to 24ms, that is one full frame time (1/refresh rate),
> plus SRD exit training time (max of 6ms),
> plus SRD aux channel handshake (max of 1.5ms).
>
> So if something went wrong PSR will be disabled until next full
> enable/disable setup.
>
> v2: The 24ms above takes in account 16ms for refresh rate on 60Hz mode. However
> on low frequency modes this can take longer. So let's use 50ms for safeness.
>
> v3: Move wait out of psr.lock critical area.
>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

Again, since we already waited for 100ms while queueing
intel_edp_psr_work, an additional 50ms is basically useless. I'd
really like this function to just have an I915_READ instead of yet
another wait, so any necessary wait-time-tuning would be part of the
schedule_delayed_work(dev_priv->psr.work) call.

That said, the current patch is already an improvement, so:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

But I'd prefer the solution I proposed :)

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a119b9b..b8699b0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2208,6 +2208,17 @@ static void intel_edp_psr_work(struct work_struct *work)
>                 container_of(work, typeof(*dev_priv), psr.work.work);
>         struct intel_dp *intel_dp = dev_priv->psr.enabled;
>
> +       /* We have to make sure PSR is ready for re-enable
> +        * otherwise it keeps disabled until next full enable/disable cycle.
> +        * PSR might take some time to get fully disabled
> +        * and be ready for re-enable.
> +        */
> +       if (wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev_priv->dev)) &
> +                     EDP_PSR_STATUS_STATE_MASK) == 0, 50)) {
> +               DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
> +               return;
> +       }
> +
>         mutex_lock(&dev_priv->psr.lock);
>         intel_dp = dev_priv->psr.enabled;
>
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni