Re: [PATCH v6 06/18] drm/i915/alpm: Auxless wake time calculation for Xe3p

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

On Tue, 2026-09-15 at 23:34 +0530, Animesh Manna wrote:
> Add support for auxless waketime calculation for DP2.1 ALPM
> as dependent parameter got changed.
> 
> v1: Initial version.
> v2: Use intel_dp_is_uhbr(). [Jani]
> v3:
> - Separate out switch to active latency calculation.
> - Use macro definition for ML_PHY_LOCK length. [Suraj]
> v4:
> - Fix 32-bit signed integer overflow. [sashiko]
> - Fix negative return of drm_dp_lttpr_count(). [sashiko]
> - Introduce get_ml_phy_lock_len() and get_dp_link_symbol_size().
> [Jouni]

My comment was referring existing intel_dp.c:intel_dp_link_symbol_size

> - Align variable naming with bpec formula of establishment_period.
> [Jouni]
> 
> Bspec: 71477
> 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 | 69 +++++++++++++++++++--
> --
>  1 file changed, 58 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> b/drivers/gpu/drm/i915/display/intel_alpm.c
> index d5823174576a..924fa8dd4638 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -102,21 +102,64 @@ static int get_lfps_half_cycle_clocks(const
> struct intel_crtc_state *crtc_state)
>  		1000 / (2 * LFPS_CYCLE_COUNT);
>  }
>  
> +#define ML_PHY_LOCK_LEN		252
> +#define ML_PHY_LOCK_LEN_UHBR	396
> +
>  static int get_tphy2_p2_to_p0(const struct intel_crtc_state
> *crtc_state)
>  {
> -	return 12 * 1000;
> +	struct intel_display *display =
> to_intel_display(crtc_state);
> +
> +	return DISPLAY_VER(display) >= 35 ? (20 * 1000) : (12 *
> 1000);

This should actually use HAS_LT_PHY().

> +}
> +
> +static int get_ml_phy_lock_len(const struct intel_crtc_state
> *crtc_state)
> +{
> +	return intel_dp_is_uhbr(crtc_state) ? ML_PHY_LOCK_LEN_UHBR :
> ML_PHY_LOCK_LEN;
> +}
> +
> +static int get_dp_link_symbol_size(const struct intel_crtc_state
> *crtc_state)
> +{
> +	return intel_dp_is_uhbr(crtc_state) ? 32 : 10;
>  }

This is duplicate of intel_dp_link_symbol_size.

>  
> -static int get_establishment_period(const struct intel_crtc_state
> *crtc_state)
> +static int get_establishment_period(struct intel_dp *intel_dp,
> +				    const struct intel_crtc_state
> *crtc_state)
>  {
>  	int t1 = 50 * 1000;
> -	int tps4 = 252;
> -	/* port_clock is link rate in 10kbit/s units */
> -	int tml_phy_lock = 1000 * 1000 * tps4 / crtc_state-
> >port_clock;
> +	int tps4 = get_ml_phy_lock_len(crtc_state) *
> get_dp_link_symbol_size(crtc_state);
> +	/*
> +	 * port_clock is link rate in 10kbit/s units, so multiplying
> by
> +	 * (1000 * 1000)/10 = 100000
> +	 */
> +	int tml_phy_lock = 100000 * tps4 / crtc_state->port_clock;

tml_phy_lock:

For 8b/10b: 
252 Symbol clocks * ( 10 / (Link Rate in Mbps) )

For 128b/132b:
396 Super Symbol clocks * ( 32/ (Link Rate in Mbps) )

In comment you should have:
* (10000 * 10000)/intel_dp_link_symbol_size()

> +	int lttpr_count = 0;
>  	int tcds, establishment_period;
>  
> -	tcds = (7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1) *
> tml_phy_lock;
> -	establishment_period = (SILENCE_PERIOD_TIME + t1 + tcds);
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
> +		tcds = (7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1) *
> tml_phy_lock;
> +	} else {
> +		tcds = 7 * tml_phy_lock;
> +		lttpr_count = drm_dp_lttpr_count(intel_dp-
> >lttpr_common_caps);
> +	}
> +
> +	if (lttpr_count > 0) {
> +		/* tlw and tcs values are taken from DP2.1 spec */
> +		int tlw = 13000;
> +		int tcs = 10000;
> +		int tlfps_period = get_lfps_cycle_time(crtc_state);
> +		int tdcs = (SILENCE_PERIOD_TIME + t1 + tcs +
> +			    (lttpr_count - 1) * (tlw +
> tlfps_period));
> +		int tacds = 70000;
> +		int tds = 7 * tml_phy_lock;
> +
> +		/* tdrl is same as tcds*/
> +		establishment_period = tlw + tlfps_period + tdcs +
> tacds +
> +				       (lttpr_count - 1) * tds +
> tcds;
> +	} else {
> +		/* TODO: Add a check for data realign by DPCD
> 0x116[3] */
> +
> +		establishment_period = (SILENCE_PERIOD_TIME + t1 +
> tcds);
> +	}
>  
>  	return establishment_period;
>  }
> @@ -138,12 +181,16 @@ static int get_establishment_period(const
> struct intel_crtc_state *crtc_state)
>   * within the CDS period complete within the CDS period regardless
> of
>   * entry into the period
>   * tML_PHY_LOCK = TPS4 Length * ( 10 / (Link Rate in MHz) )

Here also switch 10 as Rate Div.

BR,
Jouni Högander

> - * TPS4 Length = 252 Symbols
> + * ML_PHY_LOCK Length (8b/10b): 252
> + * ML_PHY_LOCK Length (128b/132b): 396
> + * TPS4 Length = ML_PHY_LOCK * Rate Div
> + * Rate Div = 32 for 128b/132b and 10 for 8b/10b.
>   */
> -static int _lnl_compute_aux_less_wake_time(const struct
> intel_crtc_state *crtc_state)
> +static int _lnl_compute_aux_less_wake_time(struct intel_dp
> *intel_dp,
> +					   const struct
> intel_crtc_state *crtc_state)
>  {
>  	int tphy2_p2_to_p0 = get_tphy2_p2_to_p0(crtc_state);
> -	int establishment_period =
> get_establishment_period(crtc_state);
> +	int establishment_period =
> get_establishment_period(intel_dp, crtc_state);
>  
>  	return DIV_ROUND_UP(tphy2_p2_to_p0 +
> get_lfps_cycle_time(crtc_state) +
>  			    establishment_period, 1000);
> @@ -158,7 +205,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);
> +		_lnl_compute_aux_less_wake_time(intel_dp,
> 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);