Re: [PATCH 2/2] drm/i915/psr: Clear stale sel fetch enable bits on sel fetch disable

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Wed, 2026-09-09 at 16:33 +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.
> 
> v2: Drop the old_crtc_state->hw.active check. [Jouni]

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
> 
> 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 <nemesa.garg@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cursor.c       |  7 +++++--
>  .../gpu/drm/i915/display/intel_display_types.h    |  2 ++
>  drivers/gpu/drm/i915/display/intel_psr.c          | 15
> +++++++++++++++
>  .../gpu/drm/i915/display/skl_universal_plane.c    |  9 ++++-----
>  4 files changed, 26 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..ec99ff10391c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1190,6 +1190,8 @@ struct intel_crtc_state {
>  	bool has_sel_update;
>  	bool enable_psr2_sel_fetch;
>  	bool enable_psr2_su_region_et;
> +	/* Drop the stale selective fetch enable bits as selective
> fetch is turned off */
> +	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..872e253db178 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,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;
>  
> 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