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

From: Daniel Vetter <daniel@ffwll.ch>
To: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH 4/4] drm/i915: Make sure PSR is ready for been re-enabled.
Date: Wed, 17 Sep 2014 17:50:04 +0200	[thread overview]
Message-ID: <20140917155004.GI31703@phenom.ffwll.local> (raw)
In-Reply-To: <1410909548-4945-4-git-send-email-rodrigo.vivi@intel.com>

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@intel.com>
> ---
>  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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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