Re: [PATCH v5 12/15] 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] Flawed logical OR (||) for LTTPR ALPM capabilities and incomplete feature integration.
- [Medium] DPCD capability registers are evaluated without applying capability bitmasks.
--

--- Patch [12]: [PATCH v5 12/15] drm/i915/alpm: Replace is_edp() with alpm_is_possible() ---
commit 7013cc56cbeba3729d838fd8bd915ecee9e1de24
Author: Animesh Manna <animesh.manna@intel.com>

drm/i915/alpm: Replace is_edp() with alpm_is_possible()

Add a separate alpm_is_possible() which will check for both edp and dp.

Cc: Jouni Högander <jouni.hogander@intel.com>

> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index d23f3bdf13399..f3f6f40f96244 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c

[ ... ]

> @@ -72,6 +72,16 @@ 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)
> +{
> +	struct intel_display *display = to_intel_display(intel_dp);
> +
> +	return (DISPLAY_VER(display) >= 12 && intel_dp->alpm_dpcd) ||
> +		(DISPLAY_VER(display) >= 35 &&
> +		intel_dp->lttpr_common_caps[DP_LTTPR_ALPM_CAPABILITIES -
> +					    DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV]);
> +}

[Severity: High]
Should this logical OR (||) be an AND (&&) for LTTPR capabilities? By using
OR, if the LTTPR supports ALPM but the sink does not (or vice versa), this
function will return true.

Could this lead to mismatched configurations? For example, if the LTTPR
supports ALPM but the sink doesn't, intel_alpm_enable_sink() will write
DP_ALPM_MODE_AUX_LESS to the sink's DP_RECEIVER_ALPM_CONFIG register even
though the sink lacks support.

Additionally, downstream functions like intel_alpm_lobf_compute_config()
exclusively check the sink's capability. Does using a logical OR here
effectively render the LTTPR check dead code in those paths?

[Severity: Medium]
Is it safe to evaluate the raw DPCD capability bytes (intel_dp->alpm_dpcd
and lttpr_common_caps) directly as booleans?

Since DPCD 0x02e is a bitfield containing multiple distinct bits (like
DP_ALPM_PM_STATE_2A_SUPPORT), a non-zero value doesn't necessarily mean
basic ALPM capability is supported.

Should this use existing helpers like intel_alpm_aux_wake_supported() or
explicitly apply the DP_ALPM_CAP bitmasks to prevent falsely detecting ALPM
support on reserved bits?

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