Hello Jose,
See my comment below.
On Tue, 2022-04-12 at 13:55 -0700, José Roberto de Souza wrote:
> After commit 805f04d42a6b ("drm/i915/display/psr: Use continuos full
> frame to handle frontbuffer invalidations") was merged we started to
> get some drm_WARN_ON(&dev_priv->drm, !(tmp &
> PSR2_MAN_TRK_CTL_ENABLE))
> in tests that are executed in pipe B.
>
> This is probably due psr2_sel_fetch_cff_enabled being left set during
> PSR disable in the pipe A, so the PSR2_MAN_TRK_CTL write in
> intel_psr2_program_trans_man_trk_ctl() is skipped in pipe B and then
> we get the warning when actually enabling PSR after planes
> programing.
> We don't get such warnings when running tests in pipe A because
> PSR2_MAN_TRK_CTL is only cleared when enabling PSR2 with hardware
> tracking.
It sounds a bit scary pipe A would have such impact on pipe B...
drm_WARN_ON(&dev_priv->drm, !(tmp & PSR2_MAN_TRK_CTL_ENABLE))
is wrong for ADLP. Please keep in mind such bit doesn't exist in ADLP.
This WARN is actually checking SFF bit on ADLP which is reset by HW
after sending the update frame. We were just lucky (or unlucky
depending how you see it) not seeing this earlier. Proper fix would be
to remove this warning for ADLP?
>
> Was not able to reproduce this issue but cleaning the PSR state
> disable will not harm anything at all.
>
> Fixes: 805f04d42a6b ("drm/i915/display/psr: Use continuos full frame
> to handle frontbuffer invalidations")
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5634
> Cc: Jouni Högander <jouni.hogander@xxxxxxxxx>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 8ec7c161284be..06db407e2749f 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1353,6 +1353,9 @@ static void intel_psr_disable_locked(struct
> intel_dp *intel_dp)
> drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_RECEIVER_ALPM_CONFIG, 0);
>
> intel_dp->psr.enabled = false;
> + intel_dp->psr.psr2_enabled = false;
> + intel_dp->psr.psr2_sel_fetch_enabled = false;
> + intel_dp->psr.psr2_sel_fetch_cff_enabled = false;
> }
>
> /**
BR,
Jouni Högander
