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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

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);
>