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

[Manna, Animesh]

> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander at intel.com>
> Sent: Friday, February 2, 2024 1:17 PM
> To: Manna, Animesh <animesh.manna at intel.com>; intel-
> gfx at lists.freedesktop.org
> Subject: Re: [PATCH v3 04/21] drm/i915/psr: Rename intel_psr_enabled
> 
> On Fri, 2024-02-02 at 07:34 +0000, Manna, Animesh wrote:
> >
> >
> > > -----Original Message-----
> > > From: Hogander, Jouni <jouni.hogander at intel.com>
> > > Sent: Friday, January 19, 2024 3:40 PM
> > > To: intel-gfx at lists.freedesktop.org
> > > Cc: Manna, Animesh <animesh.manna at intel.com>; Hogander, Jouni
> > > <jouni.hogander at 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 at 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.

Do not see any value addition, though no strong objection.

Regards,
Animesh
> 
> 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
> >