[PATCH 0/2] Fix stale selective fetch enable bit

[PATCH 0/2] Fix stale selective fetch enable bit

[Nemesa Garg]

 Selective fetch is dropped while pipe CRC is active, but the per-plane
SEL_FETCH_PLANE_CTL()/SEL_FETCH_CUR_CTL() enable bits stay set in
hardware. A plane disabled during that window never gets its bit cleared,
since the disable path is guarded by enable_psr2_sel_fetch. When selective
fetch returns, hardware keeps fetching for a plane that is gone and holds
its DDB range.

Commit 7f1172a2ac0d ("drm/i915/display: Clear SEL_FETCH_PLANE_CTL on
plane disable") fixed that by swapping the guard for
HAS_PSR2_SEL_FETCH(), but that caused unclaimed register access on
pipes driving HDMI.

Patch 1 reverts it. Patch 2 clears the bits at the point 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.

Nemesa Garg (2):
  Revert "drm/i915/display: Clear SEL_FETCH_PLANE_CTL on plane disable"
  drm/i915/psr: Clear stale sel fetch enable bits on sel fetch disable

 drivers/gpu/drm/i915/display/intel_cursor.c   | 20 ++++++++---------
 .../drm/i915/display/intel_display_types.h    |  6 +++++
 drivers/gpu/drm/i915/display/intel_psr.c      | 19 ++++++++++++++++
 .../drm/i915/display/skl_universal_plane.c    | 22 +++++++------------
 4 files changed, 42 insertions(+), 25 deletions(-)

-- 
2.25.1

[PATCH 1/2] Revert "drm/i915/display: Clear SEL_FETCH_PLANE_CTL on plane disable"

[Nemesa Garg]

This reverts commit 7f1172a2ac0d7e50850785e2e65789c8aac8411a.

This commit replaced the crtc_state->enable_psr2_sel_fetch guard in
icl_plane_disable_sel_fetch_arm() and i9xx_cursor_disable_sel_fetch_arm()
with HAS_PSR2_SEL_FETCH(). This is a display version check and
says nothing about the pipe, so every plane and cursor disable on a
display 12+ platform started writing SEL_FETCH_PLANE_CTL() /
SEL_FETCH_CUR_CTL(), including on pipes that do not implement them.
It shows up as an unclaimed register access on pipes driving HDMI where
selective fetch was never enabled.

The stale selective fetch enable bit that commit addressed is handled
in the next patch.

Fixes: 7f1172a2ac0d ("drm/i915/display: Clear SEL_FETCH_PLANE_CTL on plane disable")
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16876
Cc: stable@vger.kernel.org
Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c       | 15 +++++----------
 .../gpu/drm/i915/display/skl_universal_plane.c    | 15 +++++----------
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index cce041e1da51..90173040d825 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -531,18 +531,13 @@ static int i9xx_check_cursor(struct intel_crtc_state *crtc_state,
 }
 
 static void i9xx_cursor_disable_sel_fetch_arm(struct intel_dsb *dsb,
-					      struct intel_plane *plane)
+					      struct intel_plane *plane,
+					      const struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(plane);
 	enum pipe pipe = plane->pipe;
 
-	/*
-	 * Clear this whenever the hardware has selective fetch, not just when
-	 * the current state uses it. The cursor may have been enabled with
-	 * selective fetch earlier and had its enable bit orphaned when the
-	 * feature was switched off.
-	 */
-	if (!HAS_PSR2_SEL_FETCH(display))
+	if (!crtc_state->enable_psr2_sel_fetch)
 		return;
 
 	intel_de_write_dsb(display, dsb, SEL_FETCH_CUR_CTL(pipe), 0);
@@ -592,7 +587,7 @@ static void i9xx_cursor_update_sel_fetch_arm(struct intel_dsb *dsb,
 		if (crtc_state->enable_psr2_su_region_et)
 			wa_16021440873(dsb, plane, crtc_state, plane_state);
 		else
-			i9xx_cursor_disable_sel_fetch_arm(dsb, plane);
+			i9xx_cursor_disable_sel_fetch_arm(dsb, plane, crtc_state);
 	}
 }
 
@@ -701,7 +696,7 @@ static void i9xx_cursor_update_arm(struct intel_dsb *dsb,
 	if (plane_state)
 		i9xx_cursor_update_sel_fetch_arm(dsb, plane, crtc_state, plane_state);
 	else
-		i9xx_cursor_disable_sel_fetch_arm(dsb, plane);
+		i9xx_cursor_disable_sel_fetch_arm(dsb, plane, crtc_state);
 
 	if (plane->cursor.base != base ||
 	    plane->cursor.size != fbc_ctl ||
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 5cda1ab90e40..07a683293352 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -879,18 +879,13 @@ skl_plane_disable_arm(struct intel_dsb *dsb,
 }
 
 static void icl_plane_disable_sel_fetch_arm(struct intel_dsb *dsb,
-					    struct intel_plane *plane)
+					    struct intel_plane *plane,
+					    const struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(plane);
 	enum pipe pipe = plane->pipe;
 
-	/*
-	 * Clear this whenever the hardware has selective fetch, not just when
-	 * the current state uses it. The plane may have been enabled with
-	 * selective fetch earlier and had its enable bit orphaned when the
-	 * feature was switched off.
-	 */
-	if (!HAS_PSR2_SEL_FETCH(display))
+	if (!crtc_state->enable_psr2_sel_fetch)
 		return;
 
 	intel_de_write_dsb(display, dsb, SEL_FETCH_PLANE_CTL(pipe, plane->id), 0);
@@ -926,7 +921,7 @@ icl_plane_disable_arm(struct intel_dsb *dsb,
 
 	skl_write_plane_wm(dsb, plane, crtc_state);
 
-	icl_plane_disable_sel_fetch_arm(dsb, plane);
+	icl_plane_disable_sel_fetch_arm(dsb, plane, crtc_state);
 
 	if (plane_has_normalizer(plane))
 		intel_de_write_dsb(display, dsb,
@@ -1646,7 +1641,7 @@ static void icl_plane_update_sel_fetch_arm(struct intel_dsb *dsb,
 		intel_de_write_dsb(display, dsb, SEL_FETCH_PLANE_CTL(pipe, plane->id),
 				   SEL_FETCH_PLANE_CTL_ENABLE);
 	else
-		icl_plane_disable_sel_fetch_arm(dsb, plane);
+		icl_plane_disable_sel_fetch_arm(dsb, plane, crtc_state);
 }
 
 static void
-- 
2.25.1

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

[Nemesa Garg]

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 <nemesa.garg@intel.com>
---
 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;
+
 	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
-- 
2.25.1

✓ i915.CI.BAT: success for Fix stale selective fetch enable bit

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 5639 bytes --]

== Series Details ==

Series: Fix stale selective fetch enable bit
URL   : https://patchwork.freedesktop.org/series/173663/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_19108 -> Patchwork_173663v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/index.html

Participating hosts (38 -> 38)
------------------------------

  Additional (1): bat-adls-6 
  Missing    (1): bat-dg2-13 

Known issues
------------

  Here are the changes found in Patchwork_173663v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_lmem_swapping@parallel-random-engines:
    - bat-adls-6:         NOTRUN -> [SKIP][1] ([i915#4613]) +3 other tests skip
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@gem_tiled_pread_basic@basic:
    - bat-adls-6:         NOTRUN -> [SKIP][2] ([i915#15656])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@gem_tiled_pread_basic@basic.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-cfl-8109u:       [PASS][3] -> [DMESG-WARN][4] ([i915#13735]) +34 other tests dmesg-warn
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19108/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html

  * igt@intel_hwmon@hwmon-read:
    - bat-adls-6:         NOTRUN -> [SKIP][5] ([i915#7707]) +1 other test skip
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@intel_hwmon@hwmon-read.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-adls-6:         NOTRUN -> [SKIP][6] ([i915#4103]) +1 other test skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-adls-6:         NOTRUN -> [SKIP][7] ([i915#16361])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-adls-6:         NOTRUN -> [SKIP][8]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-adls-6:         NOTRUN -> [SKIP][9] ([i915#5354])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@kms_pm_backlight@basic-brightness.html

  * igt@kms_psr@psr-primary-mmap-gtt:
    - bat-adls-6:         NOTRUN -> [SKIP][10] ([i915#1072] / [i915#9732]) +3 other tests skip
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@kms_psr@psr-primary-mmap-gtt.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-adls-6:         NOTRUN -> [SKIP][11] ([i915#3555])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-read:
    - bat-adls-6:         NOTRUN -> [SKIP][12] ([i915#3291]) +2 other tests skip
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-adls-6/igt@prime_vgem@basic-fence-read.html

  
#### Possible fixes ####

  * igt@i915_selftest@live:
    - bat-arlh-2:         [INCOMPLETE][13] ([i915#16547]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19108/bat-arlh-2/igt@i915_selftest@live.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-arlh-2/igt@i915_selftest@live.html

  * igt@i915_selftest@live@gt_heartbeat:
    - bat-arlh-2:         [INCOMPLETE][15] ([i915#17007]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19108/bat-arlh-2/igt@i915_selftest@live@gt_heartbeat.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/bat-arlh-2/igt@i915_selftest@live@gt_heartbeat.html

  
  [i915#1072]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1072
  [i915#13735]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13735
  [i915#15656]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15656
  [i915#16361]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/16361
  [i915#16547]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/16547
  [i915#17007]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/17007
  [i915#3291]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3291
  [i915#3555]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3555
  [i915#4103]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4103
  [i915#4613]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4613
  [i915#5354]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5354
  [i915#7707]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/7707
  [i915#9732]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9732


Build changes
-------------

  * Linux: CI_DRM_19108 -> Patchwork_173663v1

  CI-20190529: 20190529
  CI_DRM_19108: dd822035c9dcd89e47cbfa75da6f3119617fbada @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9086: a89061ecccede222b8da2f310f3bf71ed908c365 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_173663v1: dd822035c9dcd89e47cbfa75da6f3119617fbada @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_173663v1/index.html

[-- Attachment #2: Type: text/html, Size: 6565 bytes --]

Re: [PATCH 1/2] Revert "drm/i915/display: Clear SEL_FETCH_PLANE_CTL on plane disable"

[Hogander, Jouni]

On Wed, 2026-09-09 at 11:55 +0530, Nemesa Garg wrote:
> This reverts commit 7f1172a2ac0d7e50850785e2e65789c8aac8411a.
> 
> This commit replaced the crtc_state->enable_psr2_sel_fetch guard in
> icl_plane_disable_sel_fetch_arm() and
> i9xx_cursor_disable_sel_fetch_arm()
> with HAS_PSR2_SEL_FETCH(). This is a display version check and
> says nothing about the pipe, so every plane and cursor disable on a
> display 12+ platform started writing SEL_FETCH_PLANE_CTL() /
> SEL_FETCH_CUR_CTL(), including on pipes that do not implement them.
> It shows up as an unclaimed register access on pipes driving HDMI
> where
> selective fetch was never enabled.
> 
> The stale selective fetch enable bit that commit addressed is handled
> in the next patch.
> 
> Fixes: 7f1172a2ac0d ("drm/i915/display: Clear SEL_FETCH_PLANE_CTL on
> plane disable")
> Closes:
> https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16876
> Cc: stable@vger.kernel.org
> Signed-off-by: Nemesa Garg <nemesa.garg@intel.com>

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_cursor.c       | 15 +++++--------
> --
>  .../gpu/drm/i915/display/skl_universal_plane.c    | 15 +++++--------
> --
>  2 files changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c
> b/drivers/gpu/drm/i915/display/intel_cursor.c
> index cce041e1da51..90173040d825 100644
> --- a/drivers/gpu/drm/i915/display/intel_cursor.c
> +++ b/drivers/gpu/drm/i915/display/intel_cursor.c
> @@ -531,18 +531,13 @@ static int i9xx_check_cursor(struct
> intel_crtc_state *crtc_state,
>  }
>  
>  static void i9xx_cursor_disable_sel_fetch_arm(struct intel_dsb *dsb,
> -					      struct intel_plane
> *plane)
> +					      struct intel_plane
> *plane,
> +					      const struct
> intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(plane);
>  	enum pipe pipe = plane->pipe;
>  
> -	/*
> -	 * Clear this whenever the hardware has selective fetch, not
> just when
> -	 * the current state uses it. The cursor may have been
> enabled with
> -	 * selective fetch earlier and had its enable bit orphaned
> when the
> -	 * feature was switched off.
> -	 */
> -	if (!HAS_PSR2_SEL_FETCH(display))
> +	if (!crtc_state->enable_psr2_sel_fetch)
>  		return;
>  
>  	intel_de_write_dsb(display, dsb, SEL_FETCH_CUR_CTL(pipe),
> 0);
> @@ -592,7 +587,7 @@ static void
> i9xx_cursor_update_sel_fetch_arm(struct intel_dsb *dsb,
>  		if (crtc_state->enable_psr2_su_region_et)
>  			wa_16021440873(dsb, plane, crtc_state,
> plane_state);
>  		else
> -			i9xx_cursor_disable_sel_fetch_arm(dsb,
> plane);
> +			i9xx_cursor_disable_sel_fetch_arm(dsb,
> plane, crtc_state);
>  	}
>  }
>  
> @@ -701,7 +696,7 @@ static void i9xx_cursor_update_arm(struct
> intel_dsb *dsb,
>  	if (plane_state)
>  		i9xx_cursor_update_sel_fetch_arm(dsb, plane,
> crtc_state, plane_state);
>  	else
> -		i9xx_cursor_disable_sel_fetch_arm(dsb, plane);
> +		i9xx_cursor_disable_sel_fetch_arm(dsb, plane,
> crtc_state);
>  
>  	if (plane->cursor.base != base ||
>  	    plane->cursor.size != fbc_ctl ||
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 5cda1ab90e40..07a683293352 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -879,18 +879,13 @@ skl_plane_disable_arm(struct intel_dsb *dsb,
>  }
>  
>  static void icl_plane_disable_sel_fetch_arm(struct intel_dsb *dsb,
> -					    struct intel_plane
> *plane)
> +					    struct intel_plane
> *plane,
> +					    const struct
> intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(plane);
>  	enum pipe pipe = plane->pipe;
>  
> -	/*
> -	 * Clear this whenever the hardware has selective fetch, not
> just when
> -	 * the current state uses it. The plane may have been
> enabled with
> -	 * selective fetch earlier and had its enable bit orphaned
> when the
> -	 * feature was switched off.
> -	 */
> -	if (!HAS_PSR2_SEL_FETCH(display))
> +	if (!crtc_state->enable_psr2_sel_fetch)
>  		return;
>  
>  	intel_de_write_dsb(display, dsb, SEL_FETCH_PLANE_CTL(pipe,
> plane->id), 0);
> @@ -926,7 +921,7 @@ icl_plane_disable_arm(struct intel_dsb *dsb,
>  
>  	skl_write_plane_wm(dsb, plane, crtc_state);
>  
> -	icl_plane_disable_sel_fetch_arm(dsb, plane);
> +	icl_plane_disable_sel_fetch_arm(dsb, plane, crtc_state);
>  
>  	if (plane_has_normalizer(plane))
>  		intel_de_write_dsb(display, dsb,
> @@ -1646,7 +1641,7 @@ static void
> icl_plane_update_sel_fetch_arm(struct intel_dsb *dsb,
>  		intel_de_write_dsb(display, dsb,
> SEL_FETCH_PLANE_CTL(pipe, plane->id),
>  				   SEL_FETCH_PLANE_CTL_ENABLE);
>  	else
> -		icl_plane_disable_sel_fetch_arm(dsb, plane);
> +		icl_plane_disable_sel_fetch_arm(dsb, plane,
> crtc_state);
>  }
>  
>  static void

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

[Hogander, Jouni]

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 <nemesa.garg@intel.com>
> ---
>  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

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

[Garg, Nemesa]

> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Wednesday, September 9, 2026 1:54 PM
> To: intel-xe@lists.freedesktop.org; Garg, Nemesa <nemesa.garg@intel.com>;
> intel-gfx@lists.freedesktop.org
> 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 <nemesa.garg@intel.com>
> > ---
> >  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

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

[Nemesa Garg]

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]

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

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

[sashiko-bot]

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 <nemesa.garg@intel.com>
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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909110332.3528029-1-nemesa.garg@intel.com?part=2

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

[Garg, Nemesa]

> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Wednesday, September 9, 2026 5:02 PM
> To: Garg, Nemesa <nemesa.garg@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> 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 <nemesa.garg@intel.com>
> 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-
> 1-nemesa.garg@intel.com?part=2

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

[Hogander, Jouni]

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