<div dir="ltr">Yeah, this is true. I'm going to send a v2 with 50ms timeout. I forgot that.<div><br></div><div>I sign up Paulo for this review! :)</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 17, 2014 at 8:50 AM, Daniel Vetter <span dir="ltr"><<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Sep 16, 2014 at 07:19:08PM -0400, Rodrigo Vivi wrote:<br>
> Let's make sure PSR is propperly disabled before to re-enabled it.<br>
><br>
> According to Spec, after disabled PSR CTL, the Idle state might occur<br>
> up to 24ms, that is one full frame time (1/refresh rate),<br>
> plus SRD exit training time (max of 6ms),<br>
> plus SRD aux channel handshake (max of 1.5ms).<br>
><br>
> So if something went wrong PSR will be disabled until next full<br>
> enable/disable setup.<br>
><br>
> Signed-off-by: Rodrigo Vivi <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>><br>
> ---<br>
>  drivers/gpu/drm/i915/intel_dp.c | 11 +++++++++++<br>
>  1 file changed, 11 insertions(+)<br>
><br>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c<br>
> index 2f0eee5..658a911 100644<br>
> --- a/drivers/gpu/drm/i915/intel_dp.c<br>
> +++ b/drivers/gpu/drm/i915/intel_dp.c<br>
> @@ -1885,6 +1885,17 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)<br>
>       WARN_ON(dev_priv->psr.active);<br>
>       lockdep_assert_held(&dev_priv->psr.lock);<br>
><br>
> +     /* We have to make sure PSR is ready for re-enable<br>
> +      * otherwise it keeps disabled until next full enable/disable cycle.<br>
> +      * PSR might take up to 24 ms to get fully disabled<br>
> +      * and be ready for re-enable.<br>
> +      */<br>
> +     if (wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev)) &<br>
<br>
</div></div>24ms = frametime for roughly 40Hz. Looks awfully like something Bspec<br>
authors just pulled out of thin air.<br>
<br>
Generally our timeout for waiting for one vblank is 50ms (20Hz, which is a<br>
lot less than any sane panel does even with DRRS).<br>
<br>
Overall series looks sane, please sign someone up for detailed review.<br>
<br>
Thanks, Daniel<br>
<span class=""><br>
> +                   EDP_PSR_STATUS_STATE_MASK) == 0, 24)) {<br>
> +             DRM_ERROR("Timed out waiting for PSR Idle for re-enable\n");<br>
> +             return;<br>
> +     }<br>
> +<br>
>       /* Enable/Re-enable PSR on the host */<br>
>       intel_edp_psr_enable_source(intel_dp);<br>
><br>
> --<br>
> 1.9.3<br>
><br>
</span>> _______________________________________________<br>
> Intel-gfx mailing list<br>
> <a href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a><br>
> <a href="http://lists.freedesktop.org/mailman/listinfo/intel-gfx" target="_blank">http://lists.freedesktop.org/mailman/listinfo/intel-gfx</a><br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Daniel Vetter<br>
Software Engineer, Intel Corporation<br>
<a href="tel:%2B41%20%280%29%2079%20365%2057%2048" value="+41793655748">+41 (0) 79 365 57 48</a> - <a href="http://blog.ffwll.ch" target="_blank">http://blog.ffwll.ch</a><br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Rodrigo Vivi</div><div>Blog: <a href="http://blog.vivi.eng.br" target="_blank">http://blog.vivi.eng.br</a></div><div> </div>
</div>