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

[Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>]

Yeah, this is true. I'm going to send a v2 with 50ms timeout. I forgot that.

I sign up Paulo for this review! :)

On Wed, Sep 17, 2014 at 8:50 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:

On Tue, Sep 16, 2014 at 07:19:08PM -0400, Rodrigo Vivi wrote:
> 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.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
>  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 2f0eee5..658a911 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1885,6 +1885,17 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
>       WARN_ON(dev_priv->psr.active);
>       lockdep_assert_held(&dev_priv->psr.lock);
>
> +     /* We have to make sure PSR is ready for re-enable
> +      * otherwise it keeps disabled until next full enable/disable cycle.
> +      * PSR might take up to 24 ms to get fully disabled
> +      * and be ready for re-enable.
> +      */
> +     if (wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev)) &

24ms = frametime for roughly 40Hz. Looks awfully like something Bspec
authors just pulled out of thin air.

Generally our timeout for waiting for one vblank is 50ms (20Hz, which is a
lot less than any sane panel does even with DRRS).

Overall series looks sane, please sign someone up for detailed review.

Thanks, Daniel

> +                   EDP_PSR_STATUS_STATE_MASK) == 0, 24)) {
> +             DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");
> +             return;
> +     }
> +
>       /* Enable/Re-enable PSR on the host */
>       intel_edp_psr_enable_source(intel_dp);
>
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

--

Rodrigo Vivi

Blog: http://blog.vivi.eng.br

 

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