Re: [PATCH v5 02/15] drm/i915/alpm: Move alpm sink capabality readout in separate function

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The refactoring silently drops error propagation when reading the ALPM DPCD capability, altering eDP initialization failure behavior despite the 'Non functional change' claim.
- [Medium] The uninitialized stack variable `dpcd` can be assigned to `intel_dp->alpm_dpcd` if `drm_dp_dpcd_readb()` returns 0.
--

--- Patch [2]: [PATCH v5 02/15] drm/i915/alpm: Move alpm sink capabality readout in separate function ---
commit 7a8fa9f136849fe0967523fc134415ba1067f31f
Author: Animesh Manna <animesh.manna@intel.com>

drm/i915/alpm: Move alpm sink capabality readout in separate function

This commit refactors the ALPM sink capability readout logic into a new
standalone function, intel_alpm_init_dpcd().

> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index f1383764b7028..542900e29a014 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
[ ... ]
> @@ -43,6 +43,16 @@ bool intel_alpm_is_alpm_aux_less(struct intel_dp *intel_dp,
>  		(crtc_state->has_lobf && intel_alpm_aux_less_wake_supported(intel_dp));
>  }
>  
> +void intel_alpm_init_dpcd(struct intel_dp *intel_dp)
> +{
> +	u8 dpcd;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP, &dpcd) < 0)
> +		return;
> +
> +	intel_dp->alpm_dpcd = dpcd;

[Severity: Medium]
If drm_dp_dpcd_readb() performs a short read and returns 0, the error check
will evaluate to false. Does this result in assigning uninitialized stack 
memory from dpcd to intel_dp->alpm_dpcd?

The previous API used here (drm_dp_dpcd_read_byte) explicitly checked for
short reads and returned a negative error, which avoided this scenario.

> +}
> +
[ ... ]
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 113d767e62e9c..bf626a1e189b1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
[ ... ]
> @@ -4810,11 +4809,7 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp, struct intel_connector *connector
>  	 */
>  	intel_dp_init_source_oui(intel_dp);
>  
> -	/* Read the ALPM DPCD caps */
> -	ret = drm_dp_dpcd_read_byte(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> -				    &intel_dp->alpm_dpcd);
> -	if (ret < 0)
> -		return false;
> +	intel_alpm_init_dpcd(intel_dp);

[Severity: Medium]
The commit message states this is a non-functional change, but the previous
code returned false and aborted eDP initialization if the AUX read failed.

Since the new intel_alpm_init_dpcd() function returns void, does this 
inadvertently swallow the error and change the initialization failure behavior
for eDP?

>  
>  	/*
>  	 * This has to be called after intel_dp->edp_dpcd is filled, PSR checks

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