Re: [PATCH v6 07/18] drm/i915/alpm: Modify AUX_LESS_WAKE_TIME bitfield for xe3lpd

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/i915/alpm: ALPM AUX-less wake time incorrectly restricted to legacy 6-bit maximum on xe3lpd
--

commit fefc7fdf839fcf4154d50e19eac0f82242cca5ad
Author: Animesh Manna <animesh.manna@intel.com>
Subject: drm/i915/alpm: Modify AUX_LESS_WAKE_TIME bitfield for xe3lpd

This commit widens the ALPM_CTL_AUX_LESS_WAKE_TIME bitfield from 6 bits
to 8 bits for xe3lpd (display version 35) and later. It introduces new
register macros and updates lnl_alpm_configure() to program the wake time
using the appropriate macro based on the display version.

> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 924fa8dd4638f..dc025058a6266 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -610,8 +610,12 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
>  	if (intel_alpm_is_alpm_aux_less(intel_dp, crtc_state)) {
>  		alpm_ctl = ALPM_CTL_ALPM_ENABLE |
>  			ALPM_CTL_ALPM_AUX_LESS_ENABLE |
> -			ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS |
> -			ALPM_CTL_AUX_LESS_WAKE_TIME(crtc_state->alpm_state.aux_less_wake_lines);
> +			ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS;
> +
> +		if (DISPLAY_VER(display) < 35)
> +			alpm_ctl |= ALPM_CTL_AUX_LESS_WAKE_TIME(crtc_state->alpm_state.aux_less_wake_lines);
> +		else
> +			alpm_ctl |= ALPM_CTL_AUX_LESS_WAKE_TIME_XE3LPD(crtc_state->alpm_state.aux_less_wake_lines);

[Severity: Medium]
While this successfully programs the wider 8-bit wake time on xe3lpd hardware,
does _lnl_compute_aux_less_alpm_params() in the same file also need to be
updated to use this new mask?

Looking at _lnl_compute_aux_less_alpm_params(), the bounds check is still
hardcoded to the 6-bit legacy mask ALPM_CTL_AUX_LESS_WAKE_TIME_MASK on all
platforms:

	if (aux_less_wake_lines > ALPM_CTL_AUX_LESS_WAKE_TIME_MASK ||
	    silence_period > PORT_ALPM_CTL_SILENCE_PERIOD_MASK ||
	    lfps_half_cycle > PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK)
		return false;

Will this restrict aux_less_wake_lines to a maximum of 63, causing validation
to fail and aborting ALPM if the computed wake time is larger on xe3lpd?

Additionally, the safest params fallback is also restricted to the legacy mask:

	if (display->params.psr_safest_params)
		aux_less_wake_lines = ALPM_CTL_AUX_LESS_WAKE_TIME_MASK;

Will this cap the safest params to 63 instead of 255 on xe3lpd?

>  
>  		if (intel_dp->as_sdp_supported)
>  			intel_alpm_configure_pr_as_sdp(intel_dp, crtc_state);

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