Re: [PATCH v6 14/18] drm/i915/alpm: Replace is_edp() with alpm_is_possible()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/i915/alpm: invalid DPCD capability checks in intel_alpm_is_possible()
- [High] drm/i915/alpm: ALPM_CTL left enabled on hot-unplug due to dynamic capability checks
--

commit f95070b21da14e9f299aa7eaa97682597723ecc9
Author: Animesh Manna <animesh.manna@intel.com>
Subject: drm/i915/alpm: Replace is_edp() with alpm_is_possible()

This commit introduces a separate intel_alpm_is_possible() function to
check for ALPM capability from DPCD information, replacing earlier checks
that relied solely on the panel being eDP.

> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 06c1a59aa2c51..29a93a252fe9b 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c

[ ... ]

> @@ -69,6 +69,13 @@ void intel_alpm_init(struct intel_dp *intel_dp)
>  	mutex_init(&intel_dp->alpm.lock);
>  }
>  
> +bool intel_alpm_is_possible(struct intel_dp *intel_dp)
> +{
> +	return (intel_dp->alpm_dpcd ||
> +		intel_dp->lttpr_common_caps[DP_LTTPR_ALPM_CAPABILITIES -
> +					    DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV]);
> +}

[Severity: High]
Does this logic correctly enforce DP requirements for ALPM?

It looks like this uses a logical OR instead of requiring both the sink and
LTTPR to support ALPM. Additionally, this seems to access the raw capability
byte without masking the specific ALPM capability bit, which could mean
reserved bits will be evaluated as true. 

Can this break standard DP link training invariants when connecting a display
with LTTPRs that support ALPM while the sink does not, or when a sink has
reserved bits set but no ALPM support?

[ ... ]

> @@ -832,7 +839,7 @@ void intel_alpm_lobf_disable(const struct intel_crtc_state *new_crtc_state)
>  
>  		intel_dp = enc_to_intel_dp(encoder);
>  
> -		if (!intel_dp_is_edp(intel_dp))
> +		if (!intel_alpm_is_possible(intel_dp))
>  			continue;
>  
>  		mutex_lock(&intel_dp->alpm.lock);

[Severity: High]
Could this dynamic capability check cause ALPM_CTL to be left enabled during
a hot-unplug event?

During a hot-unplug event, the cached DPCD capabilities like alpm_dpcd can
be cleared before the teardown hooks execute. Because this teardown function
checks these dynamic capabilities via intel_alpm_is_possible() instead of
the old hardware state, it might return early and fail to clear the ALPM_CTL
hardware register.

Would this leave the transcoder with ALPM enabled when it is supposed to be
disabled, potentially causing state mismatches on subsequent modesets?

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