Re: [PATCH v5 08/15] drm/i915/alpm: Half LFPS cycle calculation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `get_lfps_half_cycle_clocks()` incorrectly uses the DP port clock (link rate) instead of the symbol clock, which inflates the cycle count and causes it to exceed the 5-bit register mask for DP 2.1 UHBR rates, completely breaking ALPM.
--

--- Patch [8]: [PATCH v5 08/15] drm/i915/alpm: Half LFPS cycle calculation ---
commit 3b44d7e9f6ce14c0e3f4bb3319784906c402099b
Author: Animesh Manna <animesh.manna@intel.com>

drm/i915/alpm: Half LFPS cycle calculation

Add support for half LFPS cycle calculation for DP2.1 ALPM as dependent
parameters got changed.

> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 078e04bb69d29..98aa418a6ff77 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -80,12 +80,18 @@ static int get_silence_period_symbols(const struct intel_crtc_state *crtc_state)

[Severity: High]
This isn't a bug introduced by this patch, but since this patch series
enables DP 2.1 ALPM, it exposes an issue in get_lfps_half_cycle_clocks()
(which is located nearby in this file).

Does get_lfps_half_cycle_clocks() incorrectly use the DP port clock
(link rate) instead of the symbol clock?

For legacy 8b/10b encoding, the link rate in 10 kbps is numerically equal
to the symbol clock in kHz. However, for 128b/132b (UHBR) rates introduced
in DP 2.1, the link rate is 3.2x the symbol clock. This inflates the symbol
count calculation by a factor of 3.2.

When this inflated value is checked against the 5-bit mask
(PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK) in
_lnl_compute_aux_less_alpm_params(), it unconditionally fails because it
exceeds the maximum value of 31. This causes ALPM configuration to abort for
any DP 2.1 UHBR display.

Other ALPM functions like get_silence_period_symbols() correctly use
intel_dp_link_symbol_clock(crtc_state->port_clock). Should
get_lfps_half_cycle_clocks() also be updated to use the symbol clock?

>  static void get_lfps_cycle_min_max_time(const struct intel_crtc_state *crtc_state,
>  					int *min, int *max)
>  {
> -	if (crtc_state->port_clock < 540000) {
> -		*min = 65 * LFPS_CYCLE_COUNT;
> -		*max = 75 * LFPS_CYCLE_COUNT;
> -	} else {
> -		*min = 140;
> -		*max = 800;
> +	struct intel_display *display = to_intel_display(crtc_state);
> +
> +	*min = 320;
> +	*max = 1600;
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
> +		if (crtc_state->port_clock < 540000 && DISPLAY_VER(display) < 35) {
> +			*min = 65 * LFPS_CYCLE_COUNT;
> +			*max = 75 * LFPS_CYCLE_COUNT;
> +		} else {
> +			*min = 140;
> +			*max = 800;
> +		}
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825054536.3455704-1-animesh.manna@intel.com?part=8