[PATCH] drm/i915/psr: clear the Panel Replay error status register

[PATCH] drm/i915/psr: clear the Panel Replay error status register

[Jake Steinman]

psr_get_status_and_error_status() selects the DPCD offset to read the error
status from based on whether Panel Replay is enabled:

	offset = intel_dp->psr.panel_replay_enabled ?
		 DP_PANEL_REPLAY_ERROR_STATUS : DP_PSR_ERROR_STATUS;

but intel_psr_short_pulse() acknowledges it unconditionally to the PSR
register:

	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, error_status);

Under Panel Replay the error is therefore read from DP_PANEL_REPLAY_ERROR_STATUS
(0x2020) and the acknowledgement written to DP_PSR_ERROR_STATUS (0x2006).
DP_PANEL_REPLAY_ERROR_STATUS is never written anywhere in the tree; it appears
only in the read above and in its own #define.

The sink's Panel Replay error latch can consequently never be cleared. Once it
latches, every subsequent short pulse re-reads the same errors, so PSR is
disabled with sink_not_reliable set permanently, and until a short pulse
arrives the driver keeps Panel Replay enabled while the sink is reporting
errors it cannot see.

Observed on a Dell XPS 16 DA16260 (Panther Lake, Arc B390, display version
30.00) with the eDP Panel Replay quirk from commit cb8d155b0806 removed
locally so the feature could be exercised. The sink reports a persistently
latched error:

	Sink PANEL-REPLAY status: 0x2 [active, display from RFB]
	Sink PANEL-REPLAY error status: 0x1:
		PANEL-REPLAY Link CRC error

which survives across reads indefinitely, while dmesg stays silent and Panel
Replay Selective Update remains enabled.

Use the same conditional offset when clearing.

Signed-off-by: Jake Steinman <j@metarealtyinc.ca>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -3885,7 +3885,10 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
 			"PSR_ERROR_STATUS unhandled errors %x\n",
 			error_status & ~errors);
 	/* clear status register */
-	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, error_status);
+	drm_dp_dpcd_writeb(&intel_dp->aux,
+			   psr->panel_replay_enabled ?
+			   DP_PANEL_REPLAY_ERROR_STATUS : DP_PSR_ERROR_STATUS,
+			   error_status);
 
 	if (!psr->panel_replay_enabled) {
 		psr_alpm_check(intel_dp);

Re: [PATCH] drm/i915/psr: clear the Panel Replay error status register

[sashiko-bot]

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

New issues:
- [High] The patch fails to clear the Panel Replay error status because it evaluates a state variable (`psr->panel_replay_enabled`) that has already been cleared upon error detection.

Pre-existing issues:
- [High] If `drm_dp_dpcd_readb` reads 0 bytes, the driver falsely assumes success and uses uninitialized stack variables.
--

commit 1da2d4e9ad55e4f8b758998afbe0f6630bae226f
Author: Jake Steinman <j@metarealtyinc.ca>
drm/i915/psr: clear the Panel Replay error status register

This commit aims to fix an issue where the Panel Replay error status register
is never cleared on the sink. It updates intel_psr_short_pulse() to use
the same conditional offset for clearing the error as is used for reading it.

> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 52836496040ff..1d0372c5f895f 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -3891,7 +3891,10 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
>  			"PSR_ERROR_STATUS unhandled errors %x\n",
>  			error_status & ~errors);

[Severity: High]
This is a pre-existing issue, but if drm_dp_dpcd_readb() reads 0 bytes, it
returns 0.

In psr_get_status_and_error_status(), the check if (ret != 1) return ret; will
then return 0.

Does this cause intel_psr_short_pulse() to wrongly treat the read as a success
and proceed to use uninitialized stack variables status and error_status?

>  	/* clear status register */
> -	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, error_status);
> +	drm_dp_dpcd_writeb(&intel_dp->aux,
> +			   psr->panel_replay_enabled ?
> +			   DP_PANEL_REPLAY_ERROR_STATUS : DP_PSR_ERROR_STATUS,
> +			   error_status);

[Severity: High]
Does this code still write to the wrong register during an error condition?

If an error is detected, the earlier check in intel_psr_short_pulse() calls
intel_psr_disable_locked():

intel_psr_short_pulse()
  if ((!psr->panel_replay_enabled && status == DP_PSR_SINK_INTERNAL_ERROR) ||
      (error_status & errors)) {
      intel_psr_disable_locked(intel_dp);
      ...
  }

intel_psr_disable_locked() unconditionally clears the flag:
  intel_dp->psr.panel_replay_enabled = false;

When we later attempt to clear the status register, psr->panel_replay_enabled
will evaluate to false, causing the error status to be incorrectly written to
DP_PSR_ERROR_STATUS instead of DP_PANEL_REPLAY_ERROR_STATUS.

>  
>  	if (!psr->panel_replay_enabled) {
>  		psr_alpm_check(intel_dp);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901203633.92628-1-j@metarealtyinc.ca?part=1

[PATCH v2] drm/i915/psr: clear the Panel Replay error status register

[Jake Steinman]

psr_get_status_and_error_status() selects the DPCD offset to read the error
status from based on whether Panel Replay is enabled:

	offset = intel_dp->psr.panel_replay_enabled ?
		 DP_PANEL_REPLAY_ERROR_STATUS : DP_PSR_ERROR_STATUS;

but intel_psr_short_pulse() acknowledges it unconditionally to the PSR
register. Under Panel Replay the error is therefore read from
DP_PANEL_REPLAY_ERROR_STATUS (0x2020) and the acknowledgement written to
DP_PSR_ERROR_STATUS (0x2006). DP_PANEL_REPLAY_ERROR_STATUS is never written
anywhere in the tree; it appears only in the read above and in its own #define.

The sink's Panel Replay error latch can consequently never be cleared. Once it
latches, every subsequent short pulse re-reads the same errors, so PSR is
disabled with sink_not_reliable set permanently, and until a short pulse
arrives the driver keeps Panel Replay enabled while the sink is reporting
errors it cannot see.

Observed on a Dell XPS 16 DA16260 (Panther Lake, Arc B390, display version
30.00) with the eDP Panel Replay quirk from commit cb8d155b0806 removed
locally so the feature could be exercised. The sink reports a persistently
latched error:

	Sink PANEL-REPLAY status: 0x2 [active, display from RFB]
	Sink PANEL-REPLAY error status: 0x1:
		PANEL-REPLAY Link CRC error

which survives across reads indefinitely, while dmesg stays silent and Panel
Replay Selective Update remains enabled.

Use the same conditional offset when clearing. psr->panel_replay_enabled
cannot be used at that point because intel_psr_disable_locked() clears it
earlier in the same function whenever an error was detected, which is exactly
the case that needs the Panel Replay offset, so save it beforehand.

v2: use a copy of panel_replay_enabled taken before
    intel_psr_disable_locked() clears it. In v1 the condition was evaluated
    after the disable, so it selected DP_PSR_ERROR_STATUS in the error path
    and the patch was a no-op there. Caught by Sashiko AI review.

Signed-off-by: Jake Steinman <j@metarealtyinc.ca>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -3840,6 +3840,7 @@
 	struct intel_display *display = to_intel_display(intel_dp);
 	struct intel_psr *psr = &intel_dp->psr;
 	u8 status, error_status;
+	bool panel_replay_enabled;
 	const u8 errors = DP_PSR_RFB_STORAGE_ERROR |
 			  DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
 			  DP_PSR_LINK_CRC_ERROR;
@@ -3860,6 +3861,12 @@
 		goto exit;
 	}
 
+	/*
+	 * Save this before intel_psr_disable_locked() clears it; the error
+	 * status is acknowledged to a different DPCD address depending on it.
+	 */
+	panel_replay_enabled = psr->panel_replay_enabled;
+
 	if ((!psr->panel_replay_enabled && status == DP_PSR_SINK_INTERNAL_ERROR) ||
 	    (error_status & errors)) {
 		intel_psr_disable_locked(intel_dp);
@@ -3885,7 +3892,10 @@
 			"PSR_ERROR_STATUS unhandled errors %x\n",
 			error_status & ~errors);
 	/* clear status register */
-	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, error_status);
+	drm_dp_dpcd_writeb(&intel_dp->aux,
+			   panel_replay_enabled ?
+			   DP_PANEL_REPLAY_ERROR_STATUS : DP_PSR_ERROR_STATUS,
+			   error_status);
 
 	if (!psr->panel_replay_enabled) {
 		psr_alpm_check(intel_dp);

Re: [PATCH v2] drm/i915/psr: clear the Panel Replay error status register

[Hogander, Jouni]

On Tue, 2026-09-01 at 16:59 -0400, Jake Steinman wrote:
> psr_get_status_and_error_status() selects the DPCD offset to read the
> error
> status from based on whether Panel Replay is enabled:
> 
> 	offset = intel_dp->psr.panel_replay_enabled ?
> 		 DP_PANEL_REPLAY_ERROR_STATUS : DP_PSR_ERROR_STATUS;
> 
> but intel_psr_short_pulse() acknowledges it unconditionally to the
> PSR
> register. Under Panel Replay the error is therefore read from
> DP_PANEL_REPLAY_ERROR_STATUS (0x2020) and the acknowledgement written
> to
> DP_PSR_ERROR_STATUS (0x2006). DP_PANEL_REPLAY_ERROR_STATUS is never
> written
> anywhere in the tree; it appears only in the read above and in its
> own #define.
> 
> The sink's Panel Replay error latch can consequently never be
> cleared. Once it
> latches, every subsequent short pulse re-reads the same errors, so
> PSR is
> disabled with sink_not_reliable set permanently, and until a short
> pulse
> arrives the driver keeps Panel Replay enabled while the sink is
> reporting
> errors it cannot see.
> 
> Observed on a Dell XPS 16 DA16260 (Panther Lake, Arc B390, display
> version
> 30.00) with the eDP Panel Replay quirk from commit cb8d155b0806
> removed
> locally so the feature could be exercised. The sink reports a
> persistently
> latched error:
> 
> 	Sink PANEL-REPLAY status: 0x2 [active, display from RFB]
> 	Sink PANEL-REPLAY error status: 0x1:
> 		PANEL-REPLAY Link CRC error
> 
> which survives across reads indefinitely, while dmesg stays silent
> and Panel
> Replay Selective Update remains enabled.
> 
> Use the same conditional offset when clearing. psr-
> >panel_replay_enabled
> cannot be used at that point because intel_psr_disable_locked()
> clears it
> earlier in the same function whenever an error was detected, which is
> exactly
> the case that needs the Panel Replay offset, so save it beforehand.
> 
> v2: use a copy of panel_replay_enabled taken before
>     intel_psr_disable_locked() clears it. In v1 the condition was
> evaluated
>     after the disable, so it selected DP_PSR_ERROR_STATUS in the
> error path
>     and the patch was a no-op there. Caught by Sashiko AI review.
> 
> Signed-off-by: Jake Steinman <j@metarealtyinc.ca>

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

> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -3840,6 +3840,7 @@
>  	struct intel_display *display = to_intel_display(intel_dp);
>  	struct intel_psr *psr = &intel_dp->psr;
>  	u8 status, error_status;
> +	bool panel_replay_enabled;
>  	const u8 errors = DP_PSR_RFB_STORAGE_ERROR |
>  			  DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
>  			  DP_PSR_LINK_CRC_ERROR;
> @@ -3860,6 +3861,12 @@
>  		goto exit;
>  	}
>  
> +	/*
> +	 * Save this before intel_psr_disable_locked() clears it;
> the error
> +	 * status is acknowledged to a different DPCD address
> depending on it.
> +	 */
> +	panel_replay_enabled = psr->panel_replay_enabled;
> +
>  	if ((!psr->panel_replay_enabled && status ==
> DP_PSR_SINK_INTERNAL_ERROR) ||
>  	    (error_status & errors)) {
>  		intel_psr_disable_locked(intel_dp);
> @@ -3885,7 +3892,10 @@
>  			"PSR_ERROR_STATUS unhandled errors %x\n",
>  			error_status & ~errors);
>  	/* clear status register */
> -	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS,
> error_status);
> +	drm_dp_dpcd_writeb(&intel_dp->aux,
> +			   panel_replay_enabled ?
> +			   DP_PANEL_REPLAY_ERROR_STATUS :
> DP_PSR_ERROR_STATUS,
> +			   error_status);
>  
>  	if (!psr->panel_replay_enabled) {
>  		psr_alpm_check(intel_dp);

Re: [PATCH v2] drm/i915/psr: clear the Panel Replay error status register

[Rodrigo Vivi]

On Wed, Sep 02, 2026 at 05:05:32AM +0000, Hogander, Jouni wrote:
> On Tue, 2026-09-01 at 16:59 -0400, Jake Steinman wrote:
> > psr_get_status_and_error_status() selects the DPCD offset to read the
> > error
> > status from based on whether Panel Replay is enabled:
> > 
> > 	offset = intel_dp->psr.panel_replay_enabled ?
> > 		 DP_PANEL_REPLAY_ERROR_STATUS : DP_PSR_ERROR_STATUS;
> > 
> > but intel_psr_short_pulse() acknowledges it unconditionally to the
> > PSR
> > register. Under Panel Replay the error is therefore read from
> > DP_PANEL_REPLAY_ERROR_STATUS (0x2020) and the acknowledgement written
> > to
> > DP_PSR_ERROR_STATUS (0x2006). DP_PANEL_REPLAY_ERROR_STATUS is never
> > written
> > anywhere in the tree; it appears only in the read above and in its
> > own #define.
> > 
> > The sink's Panel Replay error latch can consequently never be
> > cleared. Once it
> > latches, every subsequent short pulse re-reads the same errors, so
> > PSR is
> > disabled with sink_not_reliable set permanently, and until a short
> > pulse
> > arrives the driver keeps Panel Replay enabled while the sink is
> > reporting
> > errors it cannot see.
> > 
> > Observed on a Dell XPS 16 DA16260 (Panther Lake, Arc B390, display
> > version
> > 30.00) with the eDP Panel Replay quirk from commit cb8d155b0806
> > removed
> > locally so the feature could be exercised. The sink reports a
> > persistently
> > latched error:
> > 
> > 	Sink PANEL-REPLAY status: 0x2 [active, display from RFB]
> > 	Sink PANEL-REPLAY error status: 0x1:
> > 		PANEL-REPLAY Link CRC error
> > 
> > which survives across reads indefinitely, while dmesg stays silent
> > and Panel
> > Replay Selective Update remains enabled.
> > 
> > Use the same conditional offset when clearing. psr-
> > >panel_replay_enabled
> > cannot be used at that point because intel_psr_disable_locked()
> > clears it
> > earlier in the same function whenever an error was detected, which is
> > exactly
> > the case that needs the Panel Replay offset, so save it beforehand.
> > 
> > v2: use a copy of panel_replay_enabled taken before
> >     intel_psr_disable_locked() clears it. In v1 the condition was
> > evaluated
> >     after the disable, so it selected DP_PSR_ERROR_STATUS in the
> > error path
> >     and the patch was a no-op there. Caught by Sashiko AI review.
> > 
> > Signed-off-by: Jake Steinman <j@metarealtyinc.ca>
> 
> Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

I made small adjustments in the commit message to make checkpatch happy
then I pushed to drm-intel-next.

Thanks for the patch and review.

> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -3840,6 +3840,7 @@
> >  	struct intel_display *display = to_intel_display(intel_dp);
> >  	struct intel_psr *psr = &intel_dp->psr;
> >  	u8 status, error_status;
> > +	bool panel_replay_enabled;
> >  	const u8 errors = DP_PSR_RFB_STORAGE_ERROR |
> >  			  DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> >  			  DP_PSR_LINK_CRC_ERROR;
> > @@ -3860,6 +3861,12 @@
> >  		goto exit;
> >  	}
> >  
> > +	/*
> > +	 * Save this before intel_psr_disable_locked() clears it;
> > the error
> > +	 * status is acknowledged to a different DPCD address
> > depending on it.
> > +	 */
> > +	panel_replay_enabled = psr->panel_replay_enabled;
> > +
> >  	if ((!psr->panel_replay_enabled && status ==
> > DP_PSR_SINK_INTERNAL_ERROR) ||
> >  	    (error_status & errors)) {
> >  		intel_psr_disable_locked(intel_dp);
> > @@ -3885,7 +3892,10 @@
> >  			"PSR_ERROR_STATUS unhandled errors %x\n",
> >  			error_status & ~errors);
> >  	/* clear status register */
> > -	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS,
> > error_status);
> > +	drm_dp_dpcd_writeb(&intel_dp->aux,
> > +			   panel_replay_enabled ?
> > +			   DP_PANEL_REPLAY_ERROR_STATUS :
> > DP_PSR_ERROR_STATUS,
> > +			   error_status);
> >  
> >  	if (!psr->panel_replay_enabled) {
> >  		psr_alpm_check(intel_dp);
>