Re: [PATCH 3/4] drm/i915/alpm: Replace hardcoded LFPS cycle with proper calculation

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

On Thu, 2025-08-28 at 07:35 +0000, Manna, Animesh wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf
> > Of Jouni
> > Högander
> > Sent: Wednesday, August 13, 2025 12:36 PM
> > To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
> > Cc: Hogander, Jouni <jouni.hogander@intel.com>
> > Subject: [PATCH 3/4] drm/i915/alpm: Replace hardcoded LFPS cycle
> > with
> > proper calculation
> > 
> > Currently LFPS is hadcoded for different port clocks. Replace this
> > with proper
> > calculation.
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_alpm.c | 90 ++++++++++---------
> > ----
> >  1 file changed, 38 insertions(+), 52 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index ae556a885c2a..b2123305f128 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -58,58 +58,43 @@ static int get_silence_period_symbols(const
> > struct
> > intel_crtc_state *crtc_state)
> >  		1000 / 1000;
> >  }
> > 
> > -/*
> > - * See Bspec: 71632 for the table
> > - *
> > - * Half cycle duration:
> > - *
> > - * Link rates 1.62 - 4.32 and tLFPS_Cycle = 70 ns
> > - * FLOOR( (Link Rate * tLFPS_Cycle) / (2 * 10) )
> > - *
> > - * Link rates 5.4 - 8.1
> > - * PORT_ALPM_LFPS_CTL[ LFPS Cycle Count ] = 10
> > - * LFPS Period chosen is the mid-point of the min:max values from
> > the table
> > - * FLOOR( LFPS Period in Symbol clocks /
> > - * (2 * PORT_ALPM_LFPS_CTL[ LFPS Cycle Count ]) )
> > - */
> > -static bool _lnl_get_lfps_half_cycle(int link_rate, int
> > *lfps_half_cycle)
> > +static int get_lfps_cycle_min_max_time(const struct
> > intel_crtc_state
> > *crtc_state,
> > +				       int *min, int *max)
> >  {
> > -	switch (link_rate) {
> > -	case 162000:
> > -		*lfps_half_cycle = 5;
> > -		break;
> > -	case 216000:
> > -		*lfps_half_cycle = 7;
> > -		break;
> > -	case 243000:
> > -		*lfps_half_cycle = 8;
> > -		break;
> > -	case 270000:
> > -		*lfps_half_cycle = 9;
> > -		break;
> > -	case 324000:
> > -		*lfps_half_cycle = 11;
> > -		break;
> > -	case 432000:
> > -		*lfps_half_cycle = 15;
> > -		break;
> > -	case 540000:
> > -		*lfps_half_cycle = 12;
> > -		break;
> > -	case 648000:
> > -		*lfps_half_cycle = 15;
> > -		break;
> > -	case 675000:
> > -		*lfps_half_cycle = 15;
> > -		break;
> > -	case 810000:
> > -		*lfps_half_cycle = 19;
> > -		break;
> > -	default:
> > -		*lfps_half_cycle = -1;
> > -		return false;
> > +	if (crtc_state->port_clock < 540000) {
> > +		*min = 65 * LFPS_CYCLE_COUNT;
> > +		*max = 75 * LFPS_CYCLE_COUNT;
> > +	} else if (crtc_state->port_clock <= 810000) {
> > +		*min = 140;
> > +		*max = 800;
> > +	} else {
> > +		*min = *max = -1;
> > +		return -1;
> >  	}
> > -	return true;
> > +
> > +	return 0;
> > +}
> > +
> > +static int get_lfps_cycle_time(const struct intel_crtc_state
> > +*crtc_state) {
> > +	int tlfps_cycle_min, tlfps_cycle_max, ret;
> > +
> > +	ret = get_lfps_cycle_min_max_time(crtc_state,
> > &tlfps_cycle_min,
> > +					  &tlfps_cycle_max);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return tlfps_cycle_min +  (tlfps_cycle_max -
> > tlfps_cycle_min) / 2; }
> > +
> > +static int get_lfps_half_cycle_clocks(const struct
> > intel_crtc_state
> > +*crtc_state) {
> > +	int lfps_cycle_time = get_lfps_cycle_time(crtc_state);
> > +
> > +	if (lfps_cycle_time < 0)
> > +		return -1;
> > +
> > +	return lfps_cycle_time * crtc_state->port_clock / 1000 /
> > 1000 / 20;
> 
> What 20 means above? if it is derived from LFPS cycle count then good
> to use LFPS_CYCLE_COUNT macro.

You are right. It's 2 * LFPS cycle count. I will fix this.

BR,

Jouni Högander

> 
> Regards,
> Animesh
> 
> >  }
> > 
> >  /*
> > @@ -161,8 +146,9 @@ _lnl_compute_aux_less_alpm_params(struct
> > intel_dp *intel_dp,
> >  	aux_less_wake_lines =
> > intel_usecs_to_scanlines(&crtc_state-
> > > hw.adjusted_mode,
> >  						      
> > aux_less_wake_time);
> >  	silence_period = get_silence_period_symbols(crtc_state);
> > -	if (!_lnl_get_lfps_half_cycle(crtc_state->port_clock,
> > -				      &lfps_half_cycle))
> > +
> > +	lfps_half_cycle = get_lfps_half_cycle_clocks(crtc_state);
> > +	if (lfps_half_cycle < 0)
> >  		return false;
> > 
> >  	if (aux_less_wake_lines > ALPM_CTL_AUX_LESS_WAKE_TIME_MASK
> > > > 
> > --
> > 2.43.0
>