Re: [Intel-gfx] [PATCH] drm/i915/psr: Add continuous full frame bit together with single

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Tue, Nov 29, 2022 at 09:51:00AM +0200, Jouni Högander wrote:
> Currently we are observing occasionally display flickering or complete
> freeze. This is narrowed down to be caused by single full frame update
> (SFF).
> 
> SFF bit after it's written gets cleared by HW in subsequent vblank
> i.e. when the update is sent to the panel. SFF bit is required to be
> written together with partial frame update (PFU) bit. After the SFF
> bit gets cleared by the HW psr2 man trk ctl register still contains
> PFU bit. If there is subsequent update for any reason we will end up
> having selective update/fetch configuration where start line is 0 and
> end line is 0. Also selective fetch configuration for the planes is
> not properly performed. This seems to be causing problems with some
> panels.
> 
> Using CFF without SFF doesn't work either because it may happen that
> psr2 man track ctl register is overwritten by next update before
> vblank triggers sending the update. This is causing problems to
> psr_invalidate/flush. Using CFF and SFF together solves the problems
> as SFF is cleared only by HW in subsequent vblank.
> 
> Fix the flickering/freeze issue by adding continuous full frame with
> single full frame update and switch to partial frame update only when
> selective update area is properly calculated and configured.
> 
> This is also workaround for HSD 14014971508
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> 
> Reported-by: Lee Shawn C <shawn.c.lee@intel.com>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>

Looks ok to me. Wonder still if that is the way PSR2 was supposed to work
or are we simply trying to fix/tackle some hw issues?

Anyways, if it helps and doesn't break anything else,

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 5b678916e6db..88388201684e 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1510,7 +1510,8 @@ static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
>  			       PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
>  			       man_trk_ctl_enable_bit_get(dev_priv) |
>  			       man_trk_ctl_partial_frame_bit_get(dev_priv) |
> -			       man_trk_ctl_single_full_frame_bit_get(dev_priv));
> +			       man_trk_ctl_single_full_frame_bit_get(dev_priv) |
> +			       man_trk_ctl_continuos_full_frame(dev_priv));
>  
>  	/*
>  	 * Display WA #0884: skl+
> @@ -1624,11 +1625,8 @@ static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
>  	val |= man_trk_ctl_partial_frame_bit_get(dev_priv);
>  
>  	if (full_update) {
> -		/*
> -		 * Not applying Wa_14014971508:adlp as we do not support the
> -		 * feature that requires this workaround.
> -		 */
>  		val |= man_trk_ctl_single_full_frame_bit_get(dev_priv);
> +		val |= man_trk_ctl_continuos_full_frame(dev_priv);
>  		goto exit;
>  	}
>  
> @@ -2307,12 +2305,15 @@ static void _psr_flush_handle(struct intel_dp *intel_dp)
>  			/* can we turn CFF off? */
>  			if (intel_dp->psr.busy_frontbuffer_bits == 0) {
>  				u32 val = man_trk_ctl_enable_bit_get(dev_priv) |
> -					  man_trk_ctl_partial_frame_bit_get(dev_priv) |
> -					  man_trk_ctl_single_full_frame_bit_get(dev_priv);
> +					man_trk_ctl_partial_frame_bit_get(dev_priv) |
> +					man_trk_ctl_single_full_frame_bit_get(dev_priv) |
> +					man_trk_ctl_continuos_full_frame(dev_priv);
>  
>  				/*
> -				 * turn continuous full frame off and do a single
> -				 * full frame
> +				 * turn continuous full frame off and do a single full frame. Still
> +				 * keep cff bit enabled as we don't have proper SU configuration in
> +				 * case update is sent for any reason after sff bit gets cleared by
> +				 * the HW on next vblank.
>  				 */
>  				intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
>  					       val);
> -- 
> 2.34.1
>