[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

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

[Stephen J. Fuhry]

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.

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

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

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

[Stephen J. Fuhry]

Thanks for the explanation Jouni. I missed the has_psr guard in
intel_alpm_enable_lobf() -- LOBF can't be active while PSR2 is running,
so the double-read consequence is benign.

Dropping this patch.

✗ LGCI.VerificationFailed: failure for drm/i915/display: guard intel_alpm_get_error() against non-ALPM sinks

[Patchwork]

== Series Details ==

Series: drm/i915/display: guard intel_alpm_get_error() against non-ALPM sinks
URL   : https://patchwork.freedesktop.org/series/168433/
State : failure

== Summary ==

Series author address 'fuhrysteve@gmail.com' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://intel-gfx-ci-public.igk.intel.com:8080/job/xe_pw_trigger/1186757/ (on master)

✗ LGCI.VerificationFailed: failure for drm/i915/display: guard intel_alpm_get_error() against non-ALPM sinks (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/display: guard intel_alpm_get_error() against non-ALPM sinks (rev2)
URL   : https://patchwork.freedesktop.org/series/168435/
State : failure

== Summary ==

Series author address 'fuhrysteve@gmail.com' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://gfx-ci.igk.intel.com:8080/job/CI_PW_kernel/179293/ (on built-in)