Re: [PATCH v3 04/21] drm/i915/psr: Rename intel_psr_enabled

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

On Fri, 2024-02-02 at 07:34 +0000, Manna, Animesh wrote:
> 
> 
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander@intel.com>
> > Sent: Friday, January 19, 2024 3:40 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Manna, Animesh <animesh.manna@intel.com>; Hogander, Jouni
> > <jouni.hogander@intel.com>
> > Subject: [PATCH v3 04/21] drm/i915/psr: Rename intel_psr_enabled
> > 
> > Intel_psr_enabled is now misleading name as we are using main link
> > on with
> > panel replay. I.e. link retraining is not controlled by hardware.
> > Rename
> > intel_psr_enabled as intel_psr_hw_controls_link_retrain.
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c  | 2 +-
> > drivers/gpu/drm/i915/display/intel_psr.c | 2 +-
> > drivers/gpu/drm/i915/display/intel_psr.h | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index ab415f41924d..e7cda3162ea2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4951,7 +4951,7 @@ intel_dp_needs_link_retrain(struct intel_dp
> > *intel_dp)
> >          * Also when exiting PSR, HW will retrain the link anyways
> > fixing
> >          * any link status error.
> >          */
> > -       if (intel_psr_enabled(intel_dp))
> > +       if (intel_psr_hw_controls_link_retrain(intel_dp))
> >                 return false;
> > 
> >         if (drm_dp_dpcd_read_phy_link_status(&intel_dp->aux,
> > DP_PHY_DPRX, diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index d11f8ea6e0a9..7b3290f4e0b4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -3011,7 +3011,7 @@ void intel_psr_short_pulse(struct intel_dp
> > *intel_dp)
> >         mutex_unlock(&psr->lock);
> >  }
> > 
> > -bool intel_psr_enabled(struct intel_dp *intel_dp)
> > +bool intel_psr_hw_controls_link_retrain(struct intel_dp *intel_dp)
> 
> Based on CAN_PSR() check the function will return early and only get
> executed for psr. No sure still do we need to rename it?

Ok. For me it was just surprice what it does and why this function
exists, thus renaming. Much more descriptive. Also we will soon have
main link off with Panel Replay as well then this is not about having
PSR or Panel Replay enabled, but HW controlling link retraining.

I'm fine with dropping the patch if you have strong opinion on this.

BR,

Jouni Högander

> 
> Regards,
> Animesh 
> >  {
> >         bool ret;
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > b/drivers/gpu/drm/i915/display/intel_psr.h
> > index cde781df84d5..f7c5cc07864f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > @@ -45,7 +45,7 @@ void intel_psr_get_config(struct intel_encoder
> > *encoder,  void intel_psr_irq_handler(struct intel_dp *intel_dp,
> > u32 psr_iir);
> > void intel_psr_short_pulse(struct intel_dp *intel_dp);  void
> > intel_psr_wait_for_idle_locked(const struct intel_crtc_state
> > *new_crtc_state); -bool intel_psr_enabled(struct intel_dp
> > *intel_dp);
> > +bool intel_psr_hw_controls_link_retrain(struct intel_dp
> > *intel_dp);
> >  int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> >                                 struct intel_crtc *crtc);
> >  void intel_psr2_program_trans_man_trk_ctl(const struct
> > intel_crtc_state
> > *crtc_state);
> > --
> > 2.34.1
>