Re: [PATCH] drm/i915/psr: Fix drm_WARN_ON in intel_psr_disable

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Fri, 2025-02-14 at 04:27 +0000, Kandpal, Suraj wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf
> > Of Jouni
> > Högander
> > Sent: Thursday, February 13, 2025 4:46 PM
> > To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> > Cc: Hogander, Jouni <jouni.hogander@intel.com>
> > Subject: [PATCH] drm/i915/psr: Fix drm_WARN_ON in intel_psr_disable
> > 
> > Currently intel_psr_disable is dumping out warning if PSR is not
> > supported. On
> > monitor supporting only Panel Replay we are seeing this warning.
> > Fix this by
> > checking Panel Replay support as well.
> 
> LGTM,
> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

Thank you very much Suraj.

> 
> Note: Should we be looking into separating the code for panel replay
> and psr with functions being
> Shared between the two files ? Will make going through the code much
> easier. Just wondering if that
> Makes sense in the future >

I'm not yet buying this idea as they share so much. I have been
thinking adding helpers for the purpose related to problem fixed in
this patch:

has_psr() // psr1, psr2, panel replay
has_psr1()
has_psr2()
has_panel_replay()

that would ease readability. What do you think?

BR,

Jouni Högander

> 
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 861e50ceef85..c77eb1ba3db3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2186,7 +2186,8 @@ void intel_psr_disable(struct intel_dp
> > *intel_dp,
> >  	if (!old_crtc_state->has_psr)
> >  		return;
> > 
> > -	if (drm_WARN_ON(display->drm, !CAN_PSR(intel_dp)))
> > +	if (drm_WARN_ON(display->drm, !CAN_PSR(intel_dp) &&
> > +			!CAN_PANEL_REPLAY(intel_dp))
> )
> >  		return;
> > 
> >  	mutex_lock(&intel_dp->psr.lock);
> > --
> > 2.43.0
>