Re: [PATCH v6 10/18] drm/i915/alpm: Modify LFPS cycle count for DP ALPM

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

On Thu, 2026-10-01 at 15:10 +0300, Hogander, Jouni wrote:
> On Thu, 2026-10-01 at 09:22 +0300, Hogander, Jouni wrote:
> > On Tue, 2026-09-15 at 23:34 +0530, Animesh Manna wrote:
> > > For DP minimum LFPS count is set to minimum 16.
> > > 
> > > Cc: Jouni Högander <jouni.hogander@intel.com>
> > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_alpm.c | 16 +++++++++++++---
> > >  1 file changed, 13 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > > index 14ecb27095b2..22ed341763fa 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > > @@ -24,7 +24,8 @@
> > >  				(SILENCE_PERIOD_MAX_TIME -
> > > 	\
> > >  				 SILENCE_PERIOD_MIN_TIME) / 2)
> > >  
> > > -#define LFPS_CYCLE_COUNT 10
> > > +#define LFPS_CYCLE_COUNT	10
> > > +#define LFPS_CYCLE_COUNT_DP	16
> > >  
> > >  bool intel_alpm_aux_wake_supported(struct intel_dp *intel_dp)
> > >  {
> > > @@ -121,10 +122,19 @@ static int get_lfps_cycle_time(const struct
> > > intel_crtc_state *crtc_state)
> > >  	return tlfps_cycle_min +  (tlfps_cycle_max -
> > > tlfps_cycle_min) / 2;
> > >  }
> > >  
> > > +static int get_lfps_cycle_count(const struct intel_crtc_state
> > > *crtc_state)
> > > +{
> > > +	/* External DP uses a minimum of 16 LFPS cycles. */
> > 
> > I think this comment is useless. Anyways:
> > 
> > Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
> > 
> > > +	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> > > +		return LFPS_CYCLE_COUNT_DP;
> > > +
> > > +	return LFPS_CYCLE_COUNT;
> > > +}
> > > +
> > >  static int get_lfps_half_cycle_clocks(const struct
> > > intel_crtc_state
> > > *crtc_state)
> > >  {
> > >  	return get_lfps_cycle_time(crtc_state) * crtc_state-
> > > > port_clock / 1000 /
> 
> Just realized that this has to be
> intel_dp_link_symbol_clock(crtc_state>port_clock). For eDP it have
> been
> working as crtc_state->port_clock ==
> intel_dp_link_symbol_clock(crtc_state>port_clock) always.

Maybe also good to rename this as get_lfps_half_cycle_symbols.

BR,
Jouni Högander

> 
> BR,
> Jouni Högander
> 
> > > -		1000 / (2 * LFPS_CYCLE_COUNT);
> > > +		1000 / (2 * get_lfps_cycle_count(crtc_state));
> > >  }
> > >  
> > >  #define ML_PHY_LOCK_LEN		252
> > > @@ -779,7 +789,7 @@ void intel_alpm_port_configure(struct
> > > intel_dp
> > > *intel_dp,
> > >  			PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
> > >  			PORT_ALPM_CTL_SILENCE_PERIOD(
> > >  				crtc_state-
> > > > alpm_state.silence_period_sym_clocks);
> > > -		lfps_ctl_val =
> > > PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT(LFPS_CYCLE_COUNT) |
> > > +		lfps_ctl_val =
> > > PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT(get_lfps_cycle_count(crtc_sta
> > > te
> > > ))
> > > > 
> > >  			PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURAT
> > > IO
> > > N(
> > >  				crtc_state-
> > > > alpm_state.lfps_half_cycle_num_of_syms) |
> > >  			PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE
> > > _D
> > > UR
> > > ATION(
> > 
>