[PATCH] drm/i915/display: guard intel_alpm_get_error() against non-ALPM sinks

[PATCH] drm/i915/display: guard intel_alpm_get_error() against non-ALPM sinks

[Stephen J. Fuhry]

intel_alpm_get_error() unconditionally reads DP_RECEIVER_ALPM_STATUS via
AUX from any DP connection that receives a short-pulse interrupt.  However,
alpm_dpcd is only populated by intel_edp_init_dpcd(), so it is zero for
all external (non-eDP) DP and DP-MST connections.  On such connections the
AUX read fails, producing a spurious

  [drm] *ERROR* Error reading ALPM status

and setting sink_alpm_error = true, which is then visible in the ALPM
debug output even on hardware that never enabled ALPM.

intel_alpm_disable() already has the same guard (DISPLAY_VER check plus
!intel_dp->alpm_dpcd); extend the same logic to intel_alpm_get_error() so
it returns early without attempting the AUX transaction when ALPM is not
supported by the sink.

Reproducer: ThinkPad T14 Gen1 (Alder Lake-P / i915 DISPLAY_VER 12) docked
via Lenovo ThinkPad USB-C Dock Gen2 with external DP-MST monitors.  On
every DPMS wake the error fires once on the dock's upstream DP port.

Fixes: 2063174c22da ("drm/i915/lobf: Check for sink error and disable LOBF")
Signed-off-by: Stephen J. Fuhry <fuhrysteve@gmail.com>
---
 drivers/gpu/drm/i915/display/intel_alpm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
index a7350ce8e716..4bb0c00741c4 100644
--- a/drivers/gpu/drm/i915/display/intel_alpm.c
+++ b/drivers/gpu/drm/i915/display/intel_alpm.c
@@ -611,6 +611,9 @@ bool intel_alpm_get_error(struct intel_dp *intel_dp)
 	u8 val;
 	int r;

+	if (!intel_dp->alpm_dpcd)
+		return false;
+
 	r = drm_dp_dpcd_readb(aux, DP_RECEIVER_ALPM_STATUS, &val);
 	if (r != 1) {
 		drm_err(display->drm, "Error reading ALPM status\n");
--
2.39.5

Re: [PATCH] drm/i915/display: guard intel_alpm_get_error() against non-ALPM sinks

[Hogander, Jouni]

On Thu, 2026-06-11 at 08:54 -0400, Stephen J. Fuhry wrote:
> intel_alpm_get_error() unconditionally reads DP_RECEIVER_ALPM_STATUS
> via
> AUX from any DP connection that receives a short-pulse interrupt. 
> However,
> alpm_dpcd is only populated by intel_edp_init_dpcd(), so it is zero
> for
> all external (non-eDP) DP and DP-MST connections.  On such
> connections the
> AUX read fails, producing a spurious
> 
>   [drm] *ERROR* Error reading ALPM status
> 
> and setting sink_alpm_error = true, which is then visible in the ALPM
> debug output even on hardware that never enabled ALPM.
> 
> intel_alpm_disable() already has the same guard (DISPLAY_VER check
> plus
> !intel_dp->alpm_dpcd); extend the same logic to
> intel_alpm_get_error() so
> it returns early without attempting the AUX transaction when ALPM is
> not
> supported by the sink.
> 
> Reproducer: ThinkPad T14 Gen1 (Alder Lake-P / i915 DISPLAY_VER 12)
> docked
> via Lenovo ThinkPad USB-C Dock Gen2 with external DP-MST monitors. 
> On
> every DPMS wake the error fires once on the dock's upstream DP port.
> 
> Fixes: 2063174c22da ("drm/i915/lobf: Check for sink error and disable
> LOBF")
> Signed-off-by: Stephen J. Fuhry <fuhrysteve@gmail.com>

As intel_alpm.c is not supporting non-eDP ATM for now I think your fix
proposal is ok:

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_alpm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> b/drivers/gpu/drm/i915/display/intel_alpm.c
> index a7350ce8e716..4bb0c00741c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -611,6 +611,9 @@ bool intel_alpm_get_error(struct intel_dp
> *intel_dp)
>  	u8 val;
>  	int r;
> 
> +	if (!intel_dp->alpm_dpcd)
> +		return false;
> +
>  	r = drm_dp_dpcd_readb(aux, DP_RECEIVER_ALPM_STATUS, &val);
>  	if (r != 1) {
>  		drm_err(display->drm, "Error reading ALPM
> status\n");
> --
> 2.39.5

Re: [PATCH] drm/i915/display: guard intel_alpm_get_error() against non-ALPM sinks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] intel_alpm_get_error() clears the hardware error state on read, causing a secondary caller within the same short pulse interrupt to miss the error.
--

commit 7e6bdc8ef5d560933201e9367240c8dcc3b248fe
Author: Stephen J. Fuhry <fuhrysteve@gmail.com>

drm/i915/display: guard intel_alpm_get_error() against non-ALPM sinks

intel_alpm_get_error() unconditionally reads DP_RECEIVER_ALPM_STATUS via
AUX from any DP connection that receives a short-pulse interrupt. However,
alpm_dpcd is only populated by intel_edp_init_dpcd(), so it is zero for
all external (non-eDP) DP and DP-MST connections. This patch adds a check
to return early when ALPM is not supported.

> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index c6963ea420cc9..07c1a96ddc6e2 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -629,6 +629,9 @@ bool intel_alpm_get_error(struct intel_dp *intel_dp)
>  	u8 val;
>  	int r;
>  
> +	if (!intel_dp->alpm_dpcd)
> +		return false;
> +
>  	r = drm_dp_dpcd_readb(aux, DP_RECEIVER_ALPM_STATUS, &val);
>  	if (r != 1) {
>  		drm_err(display->drm, "Error reading ALPM status\n");
>  		return true;
>  	}
>  
>  	if (val & DP_ALPM_LOCK_TIMEOUT_ERROR) {
>  		drm_dbg_kms(display->drm, "ALPM lock timeout error\n");
>  
>  		/* Clearing error */
>  		drm_dp_dpcd_writeb(aux, DP_RECEIVER_ALPM_STATUS, val);

[Severity: High]
This is a pre-existing issue, but does clearing the hardware error state here
cause subsequent checks to miss the error?

During an HPD short pulse, intel_dp_short_pulse() first calls
intel_psr_short_pulse(), which calls psr_alpm_check(). This invokes
intel_alpm_get_error() which reads and clears the DP_ALPM_LOCK_TIMEOUT_ERROR
bit:

drivers/gpu/drm/i915/display/intel_psr.c:psr_alpm_check() {
    ...
    if (intel_alpm_get_error(intel_dp)) {
        intel_psr_disable_locked(intel_dp);
        psr->sink_not_reliable = true;
    }
    ...
}

Later in the same sequence, intel_dp_short_pulse() directly calls
intel_alpm_get_error() a second time:

drivers/gpu/drm/i915/display/intel_dp.c:intel_dp_short_pulse() {
    ...
    intel_psr_short_pulse(intel_dp);

    if (intel_alpm_get_error(intel_dp)) {
        intel_alpm_disable(intel_dp);
        intel_dp->alpm.sink_alpm_error = true;
    }
    ...
}

Because the DPCD error bit was already cleared by the first call, the second
call will return false.

While psr_alpm_check() disables PSR via intel_psr_disable_locked(), that
function only disables ALPM/LOBF for Panel Replay, and psr_alpm_check() only
runs when Panel Replay is NOT enabled.

For standard PSR-enabled eDP panels, does the second check failing mean we
skip calling intel_alpm_disable(intel_dp), leaving LOBF improperly enabled
despite the ALPM timeout?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611125408.676626-1-fuhrysteve@gmail.com?part=1

Re: [PATCH] drm/i915/display: fix ALPM error handling in psr_alpm_check()

[Hogander, Jouni]

On Thu, 2026-06-11 at 10:49 -0400, Stephen J. Fuhry wrote:
> intel_dp_short_pulse() calls intel_psr_short_pulse() before the
> direct
> intel_alpm_get_error() check.  For PSR2-enabled panels,
> intel_psr_short_pulse()
> calls psr_alpm_check(), which reads and clears
> DP_ALPM_LOCK_TIMEOUT_ERROR via
> intel_alpm_get_error().  The subsequent direct call in
> intel_dp_short_pulse()
> then reads zero and skips calling intel_alpm_disable() and setting
> sink_alpm_error, leaving LOBF potentially enabled despite the ALPM
> error.
> 
> Fix this by having psr_alpm_check() also call intel_alpm_disable()
> and set
> sink_alpm_error when it handles an ALPM error.  This mirrors what the
> direct
> call in intel_dp_short_pulse() would have done, ensuring correct
> cleanup
> regardless of which code path consumes the error bit first.
> 
> intel_alpm_disable() is already called under psr->lock from
> intel_psr_disable_locked() for the Panel Replay path, so the lock
> ordering
> is established and safe.

This one we don't need. LOBF can't be enabled while PSR2 is enabled. If
ALPM error gets triggered when PSR2 is enabled -> PSR2 is disabled.
After that LOBF might be used. If there is a problem with LOBF then
ALPM error is supposed to get triggered again and then LOBF is
disabled.

BR,
Jouni Högander

> 
> Suggested-by: sashiko AI review <sashiko-bot@kernel.org>
> Fixes: 2063174c22da ("drm/i915/lobf: Check for sink error and disable
> LOBF")
> Signed-off-by: Stephen J. Fuhry <fuhrysteve@gmail.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 9382ad1e01d8..0701c8b94aa2 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -3869,6 +3869,8 @@ static void psr_alpm_check(struct intel_dp
> *intel_dp)
>  	if (intel_alpm_get_error(intel_dp)) {
>  		intel_psr_disable_locked(intel_dp);
>  		psr->sink_not_reliable = true;
> +		intel_alpm_disable(intel_dp);
> +		intel_dp->alpm.sink_alpm_error = true;
>  	}
>  }
> --
> 2.39.5