<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> </head> <body> <div class="PlainText"> </div> <div class="PlainText" dir="auto"> > -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Friday, February 14, 2025 1:01 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; > intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH] drm/i915/psr: Fix drm_WARN_ON in intel_psr_disable > > 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? That sounds better specially keeping in mind that psr checks happen all over the code and this Should reduce some confusion when it comes to checking psr versions without having to really have a Deeper look as to how those variables work </div> <div class="PlainText" dir="auto"> </div> <div class="PlainText" dir="auto">Regards,</div> <div class="PlainText" dir="auto">Suraj Kandpal</div> <div class="PlainText" dir="auto"> > > 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 > > </div> </body> </html>