I found issue with this patch:
There is a time window between psr_invalidate/flush and next vblank
where atomic commit with selective update will cause full frame update
being lost due to overwriting the configuration.
I will send a new version.
On Fri, 2022-11-25 at 15:43 +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 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.
>
> Fix this by using continuous full frame update instead 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@xxxxxxxxxxxxxxx>
> Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Cc: Mika Kahola <mika.kahola@xxxxxxxxx>
>
> Reported-by: Lee Shawn C <shawn.c.lee@xxxxxxxxx>
> Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 21 ++-------------------
> 1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 5b678916e6db..41b0718eb3a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1480,13 +1480,6 @@ static u32 man_trk_ctl_enable_bit_get(struct
> drm_i915_private *dev_priv)
> PSR2_MAN_TRK_CTL_ENABLE;
> }
>
> -static u32 man_trk_ctl_single_full_frame_bit_get(struct
> drm_i915_private *dev_priv)
> -{
> - return IS_ALDERLAKE_P(dev_priv) || DISPLAY_VER(dev_priv) >=
> 14 ?
> - ADLP_PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME :
> - PSR2_MAN_TRK_CTL_SF_SINGLE_FULL_FRAME;
> -}
> -
> static u32 man_trk_ctl_partial_frame_bit_get(struct drm_i915_private
> *dev_priv)
> {
> return IS_ALDERLAKE_P(dev_priv) || DISPLAY_VER(dev_priv) >=
> 14 ?
> @@ -1510,7 +1503,7 @@ 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_continuos_full_frame(dev_priv));
>
> /*
> * Display WA #0884: skl+
> @@ -1624,11 +1617,7 @@ 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;
> }
>
> @@ -2306,16 +2295,10 @@ static void _psr_flush_handle(struct intel_dp
> *intel_dp)
> if (intel_dp->psr.psr2_sel_fetch_cff_enabled) {
> /* 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);
> -
> /*
> * turn continuous full frame off and
> do a single
> * full frame
> */
> - intel_de_write(dev_priv,
> PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
> - val);
> intel_de_write(dev_priv,
> CURSURFLIVE(intel_dp->psr.pipe), 0);
> intel_dp-
> >psr.psr2_sel_fetch_cff_enabled = false;
> }
