Re: [PATCH v7 10/20] 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] drm/i915/alpm: uninitialized variables in get_lfps_cycle_time()
--

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

drm/i915/alpm: Half LFPS cycle calculation

This patch updates the LFPS cycle period minimum and maximum calculation for
DP2.1 ALPM by splitting it into HAS_LT_PHY() and non-LT-PHY helpers that check
whether the output type is eDP.

> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 2bf2ca9564391..b6de938990ddb 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -81,24 +81,49 @@ static int get_silence_period_symbols(const struct intel_crtc_state *crtc_state)
>  		1000 / 1000;
>  }
>  
> -static void get_lfps_cycle_min_max_time(const struct intel_crtc_state *crtc_state,
> -					int *min, int *max)
> +static void lnl_get_lfps_period_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 {
> +	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;
> +		}
> +	}
> +}
> +
> +static void lt_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)) {
>  		*min = 140;
>  		*max = 800;
> +	} else {
> +		*min = 320;
> +		*max = 1600;
>  	}
>  }
>  
> +static void get_lfps_period_min_max_time(const struct intel_crtc_state *crtc_state,
> +					int *min, int *max)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +
> +	if (HAS_LT_PHY(display))
> +		lt_get_lfps_period_min_max_time(crtc_state, min, max);
> +	else
> +		lnl_get_lfps_period_min_max_time(crtc_state, min, max);
> +}
> +
>  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 tlfps_cycle_min and tlfps_cycle_max be used uninitialized in
get_lfps_cycle_time()?

In get_lfps_cycle_time(), tlfps_cycle_min and tlfps_cycle_max are declared
without initialization and passed to get_lfps_period_min_max_time(). When
HAS_LT_PHY(display) is false, get_lfps_period_min_max_time() calls
lnl_get_lfps_period_min_max_time(), which only assigns *min and *max when
intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP) is true.

For non-eDP outputs on !HAS_LT_PHY(display) platforms, *min and *max are
never written before get_lfps_cycle_time() computes:

	return tlfps_cycle_min +  (tlfps_cycle_max - tlfps_cycle_min) / 2;

In particular, get_establishment_period() calls get_lfps_cycle_time() when
lttpr_count > 0, which only occurs in the !intel_crtc_has_type(crtc_state,
INTEL_OUTPUT_EDP) branch:

drivers/gpu/drm/i915/display/intel_alpm.c:get_establishment_period() {
	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
		num_ml_phy_lock = 7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1;
	} else {
		num_ml_phy_lock = 7;

		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);
		...
	}
}

Would this leave tlfps_cycle_min and tlfps_cycle_max uninitialized at
runtime for non-eDP DP outputs and trigger -Wmaybe-uninitialized or
-Wsometimes-uninitialized warnings during the build?

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