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?
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
