Re: [PATCH 5/8] drm/i915/alpm: Auxless wake time calculation for Xe3p

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 23 Oct 2025, Animesh Manna <animesh.manna@intel.com> wrote:
> Add support for auxless waketime calculation for DP2.1 ALPM
> as dependent parameter got changed.
>
> 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 | 78 +++++++++++++++++++----
>  1 file changed, 67 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 0f6b15bca3be..ee5b1e3d79d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -96,6 +96,66 @@ static int get_lfps_half_cycle_clocks(const struct intel_crtc_state *crtc_state)
>  		1000 / (2 * LFPS_CYCLE_COUNT);
>  }
>  
> +static int get_tphy2_p2_to_p0(struct intel_dp *intel_dp)
> +{
> +	struct intel_display *display = to_intel_display(intel_dp);
> +
> +	return DISPLAY_VER(display) >= 35 ? (40 * 1000) : (12 * 1000);
> +}
> +
> +static int get_establishment_period(struct intel_dp *intel_dp,
> +				    const struct intel_crtc_state *crtc_state)
> +{
> +	int port_clock = crtc_state->port_clock;
> +	int t1 = 50 * 1000;
> +	int tps4 = (port_clock >= 1000000) ? (396 * 32) : (252 * 10);

intel_dp_is_uhbr()

> +	int tml_phy_lock = 1000 * 1000 * tps4 / port_clock / 10;

That would overflow 32 bits.

> +	int lttpr_count = 0;
> +	int establishment_period;
> +	int tcds;
> +
> +	if (!intel_dp_is_edp(intel_dp)) {

Please turn the branches around to not have the !.

> +		lttpr_count = drm_dp_lttpr_count(intel_dp->lttpr_common_caps);
> +		tcds = 7 * tml_phy_lock;
> +	} else {
> +		tcds = (7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1) * tml_phy_lock;
> +	}
> +
> +	if (lttpr_count) {
> +		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 = (lttpr_count - 1) * 7 * tml_phy_lock;
> +
> +		/* tdrl is same as tcds*/
> +		establishment_period = tlw + tlfps_period + tdcs + tacds + tds + tcds;
> +	} else {
> +		/* TODO: Add a check for data realign by DPCD 0x116[3] */
> +
> +		establishment_period = (SILENCE_PERIOD_TIME + t1 + tcds);
> +	}
> +
> +	return establishment_period;
> +}
> +
> +static int get_switch_to_active(int port_clock)
> +{
> +	int switch_to_active;
> +
> +	if (port_clock >= 1000000) {

Again, there are uhbr helpers.

> +		int symbol_clock = port_clock / intel_dp_link_symbol_size(port_clock);
> +
> +		switch_to_active = 32 * DIV_ROUND_UP((396 + 3 + 64), symbol_clock);
> +	} else {
> +		switch_to_active = 0;
> +	}
> +
> +	return switch_to_active;
> +}
> +
>  /*
>   * AUX-Less Wake Time = CEILING( ((PHY P2 to P0) + tLFPS_Period, Max+
>   * tSilence, Max+ tPHY Establishment + tCDS) / tline)
> @@ -115,19 +175,15 @@ 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(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 = 12 * 1000;
> -	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 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;
> +	int tphy2_p2_to_p0 = get_tphy2_p2_to_p0(intel_dp);
> +	int establishment_period = get_establishment_period(intel_dp, crtc_state);
> +	int switch_to_active = get_switch_to_active(crtc_state->port_clock);
>  
>  	return DIV_ROUND_UP(tphy2_p2_to_p0 + get_lfps_cycle_time(crtc_state) +
> -			    SILENCE_PERIOD_TIME + t1 + tcds, 1000);
> +			    establishment_period + switch_to_active, 1000);

Looks like you should *first* refactor this to separate functions,
before adding functionality.

>  }
>  
>  static int
> @@ -139,7 +195,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);

-- 
Jani Nikula, Intel