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 <[email protected]>
Reviewed-by: Jouni Högander <[email protected]> > --- > 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);
