[Intel-gfx] [PATCH v6 8/9] drm/i915: Force PSR1 exit when getting pipe CRC

[Souza, Jose]

On Thu, 2019-03-07 at 16:00 -0800, José Roberto de Souza wrote:
> If PSR1 is active when pipe CRC is enabled the CRC calculations will
> be inhibit by the transition to low power states that PSR1 brings.
> So lets force a PSR1 exit and as soon as pipe CRC is enabled it will
> block PSR1 activation and avoid CRC timeouts when running IGT tests.
> 
> There is a little window between the call to force exit PSR and the
> write to pipe CRC registers that needs to happen within the minimum
> of 6 idles frames otherwise PSR1 will be active again causing the CRC
> timeouts but anyways this will at least reduce the occurrence of CRC
> timeouts.
> 
> This can possibily fix issues present right now but I did not found
> any open, I mostly got this issue from previous CI runs of this
> series, bellow some exambles:
> 
> * igt at kms_color@pipe-b-ctm-0-75:
> - shard-apl:          PASS -> FAIL +9
> 
> * igt at kms_cursor_legacy@flip-vs-cursor-busy-crc-legacy:
> - shard-apl:          PASS -> DMESG-FAIL +17
> 
> * igt at kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-pgflip-blt:
> - shard-kbl:          PASS -> DMESG-FAIL +12
> 
> * igt at kms_pipe_crc_basic@read-crc-pipe-c:
> - shard-kbl:          PASS -> FAIL +7
> 
> v6: s/PSR/PSR1 (Dhinakaran)

I forgot to add DK's rv-b

https://patchwork.freedesktop.org/patch/290564/?series=57628&rev=1#comment_547027

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 36 ++++++++++++++++++++--------
> ----
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 9847f6b0cd9a..053dbba6abde 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -452,6 +452,7 @@ static void hsw_activate_psr1(struct intel_dp
> *intel_dp)
>  	 * frames, we'll go with 9 frames for now
>  	 */
>  	idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency
> + 1);
> +
>  	val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>  
>  	val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT;
> @@ -853,6 +854,20 @@ void intel_psr_disable(struct intel_dp
> *intel_dp,
>  	cancel_work_sync(&dev_priv->psr.work);
>  }
>  
> +static void psr_force_hw_tracking_exit(struct drm_i915_private
> *dev_priv)
> +{
> +	/*
> +	 * Display WA #0884: all
> +	 * This documented WA for bxt can be safely applied
> +	 * broadly so we can force HW tracking to exit PSR
> +	 * instead of disabling and re-enabling.
> +	 * Workaround tells us to write 0 to CUR_SURFLIVE_A,
> +	 * but it makes more sense write to the current active
> +	 * pipe.
> +	 */
> +	I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0);
> +}
> +
>  /**
>   * intel_psr_update - Update PSR state
>   * @intel_dp: Intel DP
> @@ -877,8 +892,13 @@ void intel_psr_update(struct intel_dp *intel_dp,
>  	enable = crtc_state->has_psr && psr_global_enabled(psr->debug);
>  	psr2_enable = intel_psr2_enabled(dev_priv, crtc_state);
>  
> -	if (enable == psr->enabled && psr2_enable == psr->psr2_enabled)
> +	if (enable == psr->enabled && psr2_enable == psr->psr2_enabled) 
> {
> +		/* Force a PSR exit when enabling CRC to avoid CRC
> timeouts */
> +		if (crtc_state->crc_enabled && psr->enabled)
> +			psr_force_hw_tracking_exit(dev_priv);
> +
>  		goto unlock;
> +	}
>  
>  	if (psr->enabled)
>  		intel_psr_disable_locked(intel_dp);
> @@ -1148,18 +1168,8 @@ void intel_psr_flush(struct drm_i915_private
> *dev_priv,
>  	dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits;
>  
>  	/* By definition flush = invalidate + flush */
> -	if (frontbuffer_bits) {
> -		/*
> -		 * Display WA #0884: all
> -		 * This documented WA for bxt can be safely applied
> -		 * broadly so we can force HW tracking to exit PSR
> -		 * instead of disabling and re-enabling.
> -		 * Workaround tells us to write 0 to CUR_SURFLIVE_A,
> -		 * but it makes more sense write to the current active
> -		 * pipe.
> -		 */
> -		I915_WRITE(CURSURFLIVE(dev_priv->psr.pipe), 0);
> -	}
> +	if (frontbuffer_bits)
> +		psr_force_hw_tracking_exit(dev_priv);
>  
>  	if (!dev_priv->psr.active && !dev_priv-
> >psr.busy_frontbuffer_bits)
>  		schedule_work(&dev_priv->psr.work);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20190308/67e8de00/attachment.sig>