Re: [PATCH 4/4] drm/i915/alpm: Use actual lfps cycle and silence periods in wake time

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

On Thu, 2025-08-28 at 07:54 +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 4/4] drm/i915/alpm: Use actual lfps cycle and
> > silence
> > periods in wake time
> > 
> > Currently we are using maximum lfps cycle and silence period times
> > when
> > calculating AUXLess wake time. Use actual values instead.
> 
> Could not locate mid of min-max is the actual value or not. Good to
> know the reference if I missed it.
> As per bspec software can use any values between minimum and maximum,
> so mid of min-max may be good.

AUXLess wake time formula in Bspec is using maximum values as lfps
cycle and silence period times. I'll guess that is just because those
will work for the whole range. I do not see any reason why couldn't we
use values we have actually configured as these parameters.

For LFPS cycle and silence period times Bspec is proposing using mid of
min-max and this is what we are currently doing.

BR,

Jouni Högander
>  
> > 
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> 
> The code changes LGTM,
> Reviewed-by: Animesh Manna <animesh.manna@intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_alpm.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index b2123305f128..4e8aa167b97b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -116,21 +116,19 @@ static int get_lfps_half_cycle_clocks(const
> > struct
> > intel_crtc_state *crtc_state)
> >   * tML_PHY_LOCK = TPS4 Length * ( 10 / (Link Rate in MHz) )
> >   * TPS4 Length = 252 Symbols
> >   */
> > -static int _lnl_compute_aux_less_wake_time(int port_clock)
> > +static int _lnl_compute_aux_less_wake_time(const struct
> > +intel_crtc_state *crtc_state)
> >  {
> >  	int tphy2_p2_to_p0 = 12 * 1000;
> > -	int tlfps_period_max = 800;
> > -	int tsilence_max = 180;
> >  	int t1 = 50 * 1000;
> >  	int tps4 = 252;
> >  	/* port_clock is link rate in 10kbit/s units */
> > -	int tml_phy_lock = 1000 * 1000 * tps4 / port_clock;
> > +	int tml_phy_lock = 1000 * 1000 * tps4 / crtc_state-
> > >port_clock;
> >  	int num_ml_phy_lock = 7 + DIV_ROUND_UP(6500, tml_phy_lock)
> > + 1;
> >  	int t2 = num_ml_phy_lock * tml_phy_lock;
> >  	int tcds = 1 * t2;
> > 
> > -	return DIV_ROUND_UP(tphy2_p2_to_p0 + tlfps_period_max +
> > tsilence_max +
> > -			    t1 + tcds, 1000);
> > +	return DIV_ROUND_UP(tphy2_p2_to_p0 +
> > get_lfps_cycle_time(crtc_state) +
> > +			    SILENCE_PERIOD_TIME + t1 + tcds,
> > 1000);
> >  }
> > 
> >  static int
> > @@ -142,7 +140,7 @@ _lnl_compute_aux_less_alpm_params(struct
> > intel_dp *intel_dp,
> >  		lfps_half_cycle;
> > 
> >  	aux_less_wake_time =
> > -		_lnl_compute_aux_less_wake_time(crtc_state-
> > >port_clock);
> > +		_lnl_compute_aux_less_wake_time(crtc_state);
> >  	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);
> > --
> > 2.43.0
>