Re: [PATCH v6 09/18] 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:
- [High] display/alpm: uninitialized stack variables used for ALPM hardware programming
--

commit b718ea1f4bfa5d8848c313251d36c92815933b23
Author: Animesh Manna <animesh.manna@intel.com>

drm/i915/alpm: Half LFPS cycle calculation

This commit adds 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 cb24bba801a19..14ecb27095b29 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
[ ... ]
> +static void lnl_get_lfps_period_min_max_time(const struct intel_crtc_state *crtc_state,
> +					     int *min, int *max)
> +{
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
> +		if (crtc_state->port_clock < 540000) {
> +			*min = 65 * LFPS_CYCLE_COUNT;
> +			*max = 75 * LFPS_CYCLE_COUNT;
> +		} else {
> +			*min = 140;
> +			*max = 800;
> +		}
> +	}
> +}

[Severity: High]
Does this function leave min and max completely uninitialized for
non-eDP configurations?

For instance, when connecting an external DisplayPort sink on LunarLake
with ALPM capabilities, intel_crtc_has_type(..., INTEL_OUTPUT_EDP) would
be false, and the function returns without setting these parameters.

[ ... ]
>  static int get_lfps_cycle_time(const struct intel_crtc_state *crtc_state)
>  {
>  	int tlfps_cycle_min, tlfps_cycle_max;
>  
> -	get_lfps_cycle_min_max_time(crtc_state, &tlfps_cycle_min,
> -				    &tlfps_cycle_max);
> +	get_lfps_period_min_max_time(crtc_state, &tlfps_cycle_min,
> +				     &tlfps_cycle_max);
>  
>  	return tlfps_cycle_min +  (tlfps_cycle_max - tlfps_cycle_min) / 2;
>  }

[Severity: High]
Can these stack variables be consumed while still uninitialized?

Because lnl_get_lfps_period_min_max_time() lacks a non-eDP fallback, it
appears tlfps_cycle_min and tlfps_cycle_max could retain their uninitialized
stack values here when called for a standard DisplayPort setup.

These garbage values would then be used in the arithmetic calculation and
subsequently programmed into hardware ALPM registers. Could this potentially
cause display link corruption, link training failures, or hardware timeouts?

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