> -----Original Message----- > From: Hogander, Jouni <[email protected]> > Sent: Wednesday, September 9, 2026 1:54 PM > To: [email protected]; Garg, Nemesa <[email protected]>; > [email protected] > Subject: Re: [PATCH 2/2] drm/i915/psr: Clear stale sel fetch enable bits on > sel > fetch disable > > On Wed, 2026-09-09 at 11:55 +0530, Nemesa Garg wrote: > > Selective fetch is dropped while pipe CRC is active, and the planes > > keep their SEL_FETCH_PLANE_CTL / SEL_FETCH_CUR_CTL enable bit set in > > hardware over that. A plane disabled while selective fetch is off > > never gets the bit cleared, as the disable path is guarded by > > enable_psr2_sel_fetch. > > Once selective fetch comes back the hardware resumes fetching for a > > plane that is no longer enabled and keeps its DDB range reserved. > > > > Clear the bits as selective fetch is turned off instead. Atomic check > > has both the old and the new crtc state, so record the transition > > there and let the plane and cursor arm paths write the registers to 0 > > for that commit. > > > > Fixes: b1f5279b5981 ("drm/i915/psr: Move plane sel fetch configuration > > into plane source files") > > Closes: > > https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8739 > > Assisted-by: Copilot:Claude-Opus-5 > > Signed-off-by: Nemesa Garg <[email protected]> > > --- > > drivers/gpu/drm/i915/display/intel_cursor.c | 7 +++++-- > > .../drm/i915/display/intel_display_types.h | 6 ++++++ > > drivers/gpu/drm/i915/display/intel_psr.c | 19 > > +++++++++++++++++++ > > .../drm/i915/display/skl_universal_plane.c | 9 ++++----- > > 4 files changed, 34 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c > > b/drivers/gpu/drm/i915/display/intel_cursor.c > > index 90173040d825..4afd275de86e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cursor.c > > +++ b/drivers/gpu/drm/i915/display/intel_cursor.c > > @@ -537,7 +537,8 @@ static void > > i9xx_cursor_disable_sel_fetch_arm(struct intel_dsb *dsb, > > struct intel_display *display = to_intel_display(plane); > > enum pipe pipe = plane->pipe; > > > > - if (!crtc_state->enable_psr2_sel_fetch) > > + if (!crtc_state->enable_psr2_sel_fetch && > > + !crtc_state->clear_psr2_sel_fetch) > > return; > > > > intel_de_write_dsb(display, dsb, SEL_FETCH_CUR_CTL(pipe), 0); @@ > > -570,8 +571,10 @@ static void i9xx_cursor_update_sel_fetch_arm(struct > > intel_dsb *dsb, > > struct intel_display *display = to_intel_display(plane); > > enum pipe pipe = plane->pipe; > > > > - if (!crtc_state->enable_psr2_sel_fetch) > > + if (!crtc_state->enable_psr2_sel_fetch) { > > + i9xx_cursor_disable_sel_fetch_arm(dsb, plane, > > crtc_state); > > return; > > + } > > > > if (drm_rect_height(&plane_state->psr2_sel_fetch_area) > 0) { > > if (crtc_state->enable_psr2_su_region_et) { diff --git > > a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index 9016be52c7ea..eae05a83db80 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1190,6 +1190,12 @@ struct intel_crtc_state { > > bool has_sel_update; > > bool enable_psr2_sel_fetch; > > bool enable_psr2_su_region_et; > > + /* > > + * Commit time directive rather than a piece of pipe state: > > drop the > > + * stale selective fetch enable bits as selective fetch is > > turned off. > > + * Not compared by the state checker. > > + */ > > + bool clear_psr2_sel_fetch; > > bool req_psr2_sdp_prior_scanline; > > bool has_panel_replay; > > bool link_off_after_as_sdp_when_pr_active; > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index f490beb66629..79ce50831492 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -2889,6 +2889,8 @@ int intel_psr2_sel_fetch_update(struct > > intel_atomic_state *state, > > struct intel_crtc *crtc) > > { > > struct intel_display *display = to_intel_display(state); > > + const struct intel_crtc_state *old_crtc_state = > > + intel_atomic_get_old_crtc_state(state, crtc); > > struct intel_crtc_state *crtc_state = > > intel_atomic_get_new_crtc_state(state, crtc); > > struct intel_plane_state *new_plane_state, *old_plane_state; > > struct intel_plane *plane; > > @@ -2901,6 +2903,23 @@ 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. The pipe is known to implement these > > registers > > + * because selective fetch was enabled on it. Turning > > selective fetch > > + * off is a modeset, so all the planes of the pipe are > > already in the > > + * state and get programmed by this commit. > > + */ > > + crtc_state->clear_psr2_sel_fetch = old_crtc_state->hw.active > > && > > + old_crtc_state->enable_psr2_sel_fetch && > > + !crtc_state->enable_psr2_sel_fetch; > > Do you really need to have check for old_crtc_state->hw.active as well? > Is it possible that old_crtc_state->hw.active == false && old_crtc_state- > >enable_psr2_sel_fetch == true? > Will drop this check as with pipe disabled sel_fetch cant be true.
Thanks and Regards, Nemesa > BR, > Jouni Högander > > > + > > if (!crtc_state->enable_psr2_sel_fetch) > > return 0; > > > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > index 07a683293352..eb5ed981b40f 100644 > > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > @@ -885,7 +885,8 @@ static void > > icl_plane_disable_sel_fetch_arm(struct intel_dsb *dsb, > > struct intel_display *display = to_intel_display(plane); > > enum pipe pipe = plane->pipe; > > > > - if (!crtc_state->enable_psr2_sel_fetch) > > + if (!crtc_state->enable_psr2_sel_fetch && > > + !crtc_state->clear_psr2_sel_fetch) > > return; > > > > intel_de_write_dsb(display, dsb, SEL_FETCH_PLANE_CTL(pipe, > > plane->id), 0); > > @@ -1634,10 +1635,8 @@ static void > > icl_plane_update_sel_fetch_arm(struct intel_dsb *dsb, > > struct intel_display *display = to_intel_display(plane); > > enum pipe pipe = plane->pipe; > > > > - if (!crtc_state->enable_psr2_sel_fetch) > > - return; > > - > > - if (drm_rect_height(&plane_state->psr2_sel_fetch_area) > 0) > > + if (crtc_state->enable_psr2_sel_fetch && > > + drm_rect_height(&plane_state->psr2_sel_fetch_area) > 0) > > > intel_de_write_dsb(display, dsb, > > SEL_FETCH_PLANE_CTL(pipe, plane->id), > > SEL_FETCH_PLANE_CTL_ENABLE); > > else
