> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Jouni
> Högander
> Sent: Friday, March 14, 2025 12:20 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx
> Cc: Hogander, Jouni <jouni.hogander@xxxxxxxxx>
> Subject: [PATCH] drm/i915/psr: Add PSR pause/resume reference count
>
> We have now seen this:
>
> <4> [2120.434153] i915 0000:00:02.0: [drm] drm_WARN_ON(psr->paused)
> <4> [2120.434196] WARNING: CPU: 3 PID: 4430 at
> drivers/gpu/drm/i915/display/intel_psr.c:2227
> intel_psr_pause+0x16e/0x180 [i915]
>
> Comment for drm_WARN_ON(display->drm, psr->paused) in
> intel_psr_pause says:
>
> "If we ever hit this, we will need to add refcount to pause/resume"
>
> This patch is implementing PSR pause/resume refcount.
>
> Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> ---
> .../drm/i915/display/intel_display_types.h | 2 +-
> drivers/gpu/drm/i915/display/intel_psr.c | 26 +++++++++----------
> 2 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 99a6fd2900b9c..65c808bba1c6c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1620,7 +1620,7 @@ struct intel_psr {
> bool sink_support;
> bool source_support;
> bool enabled;
> - bool paused;
> + int pause_counter;
> enum pipe pipe;
> enum transcoder transcoder;
> bool active;
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 4e938bad808cc..4d4ecf7555b66 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2024,7 +2024,7 @@ static void intel_psr_enable_locked(struct intel_dp
> *intel_dp,
>
> intel_psr_enable_source(intel_dp, crtc_state);
> intel_dp->psr.enabled = true;
> - intel_dp->psr.paused = false;
> + intel_dp->psr.pause_counter = 0;
>
> /*
> * Link_ok is sticky and set here on PSR enable. We can assume link
> @@ -2210,7 +2210,6 @@ void intel_psr_disable(struct intel_dp *intel_dp,
> */
> void intel_psr_pause(struct intel_dp *intel_dp) {
> - struct intel_display *display = to_intel_display(intel_dp);
> struct intel_psr *psr = &intel_dp->psr;
>
> if (!CAN_PSR(intel_dp) && !CAN_PANEL_REPLAY(intel_dp)) @@ -
> 2223,12 +2222,10 @@ void intel_psr_pause(struct intel_dp *intel_dp)
> return;
> }
>
> - /* If we ever hit this, we will need to add refcount to pause/resume
> */
> - drm_WARN_ON(display->drm, psr->paused);
> -
> - intel_psr_exit(intel_dp);
> - intel_psr_wait_exit_locked(intel_dp);
> - psr->paused = true;
> + if (intel_dp->psr.pause_counter++ == 0) {
> + intel_psr_exit(intel_dp);
> + intel_psr_wait_exit_locked(intel_dp);
> + }
>
> mutex_unlock(&psr->lock);
>
> @@ -2251,13 +2248,14 @@ void intel_psr_resume(struct intel_dp *intel_dp)
>
> mutex_lock(&psr->lock);
>
> - if (!psr->paused)
> - goto unlock;
> + if (!psr->enabled) {
Any reason not to check intel_dp->psr.pause_counter here, maybe we can check for !psr->enabled && psr->pause_counter > 0.
Other changes are LGTM.
Regards,
Animesh
> + mutex_unlock(&psr->lock);
> + return;
> + }
>
> - psr->paused = false;
> - intel_psr_activate(intel_dp);
> + if (--intel_dp->psr.pause_counter == 0)
> + intel_psr_activate(intel_dp);
>
> -unlock:
> mutex_unlock(&psr->lock);
> }
>
> @@ -3322,7 +3320,7 @@ void intel_psr_flush(struct intel_display *display,
> * we have to ensure that the PSR is not activated until
> * intel_psr_resume() is called.
> */
> - if (intel_dp->psr.paused)
> + if (intel_dp->psr.pause_counter)
> goto unlock;
>
> if (origin == ORIGIN_FLIP ||
> --
> 2.43.0
