> -----Original Message----- > From: [email protected] <[email protected]> > Sent: Wednesday, September 9, 2026 5:02 PM > To: Garg, Nemesa <[email protected]> > Cc: [email protected]; [email protected] > Subject: Re: [PATCH 2/2] drm/i915/psr: Clear stale sel fetch enable bits on > sel > fetch disable > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [High] The patch introduces `clear_psr2_sel_fetch` to clear stale hardware > bits when selective fetch is disabled, but immediately returns early, skipping > the addition of planes to the atomic state. This causes the fix to fail during > non-modeset transitions. > -- > > commit 6d95f584528dbe05e1cd976db461a45c72fb5319 > Author: Nemesa Garg <[email protected]> > drm/i915/psr: Clear stale sel fetch enable bits on sel fetch disable > > This commit attempts to clear stale selective fetch enable bits by tracking > the > transition in clear_psr2_sel_fetch and having the plane/cursor arm paths > write 0 to the registers. It updates intel_psr2_sel_fetch_update() to record > when selective fetch is turned off. > > Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8739 > > [ ... ] > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index f490beb66629e..872e253db1786 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > [ ... ] > > @@ -2901,6 +2903,19 @@ int intel_psr2_sel_fetch_update(struct > intel_atomic_state *state, > > bool full_update = false, su_area_changed; > > int i, ret; > > > > + /* > > + * Selective fetch is not always usable, for instance it is dropped > > + * while pipe CRC is active. The planes keep their selective fetch > > + * enable bit set in hardware over that, and a plane disabled while > > + * selective fetch is off never gets the bit cleared. Once selective > > + * fetch comes back the hardware would resume fetching for a plane > that > > + * is no longer enabled and keep its DDB range reserved, so have the > > + * plane update drop the bit for every plane of the pipe as selective > > + * fetch is turned off. > > + */ > > + crtc_state->clear_psr2_sel_fetch = old_crtc_state- > >enable_psr2_sel_fetch && > > + !crtc_state->enable_psr2_sel_fetch; > > + > > if (!crtc_state->enable_psr2_sel_fetch) > > return 0; > > [Severity: High] > Will this early return skip adding unmodified planes to the atomic state > during non-modeset transitions? > > During a non-modeset atomic commit (like a fastset triggered by color > management or CDCLK updates) that disables PSR2 selective fetch, the core > drm_atomic_helper_check_modeset() function does not automatically add > all planes to the state. > > When intel_psr2_sel_fetch_update() detects this transition, it sets > clear_psr2_sel_fetch to true, but then immediately returns 0 here because > enable_psr2_sel_fetch is false. > > Because the function returns before reaching the > drm_atomic_add_affected_planes() call further down, unmodified planes are > not added to the atomic state. Could this cause their plane arm callbacks to > be skipped entirely, leaving the hardware bits active and potentially causing > the display corruption this patch was intended to fix? > enable_psr2_sel_fetch is recomputed from intel_modeset_pipe_config(), which intel_atomic_check_config() calls just for crtcs that need modeset. So old_crtc_state->enable_psr2_sel_fetch && !enable_psr2_sel_fetch implies the crtc needed a modeset when drm_atomic_helper_check_modeset() ran, and that has already called drm_atomic_add_affected_planes() for it. The fastset downgrade in intel_crtc_check_fastset() happens after that, so the planes stay in the state.
Colour management does not recompute the PSR config, and the CDCLK path goes through intel_modeset_pipe(), which calls intel_plane_add_affected() itself. Thanks and Regards, Nemesa > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20260909110332.3528029- > [email protected]?part=2
