[PATCH] drm/i915/cdclk: Don't trust boot readout for per-pipe cdclk/voltage tracking

[PATCH] drm/i915/cdclk: Don't trust boot readout for per-pipe cdclk/voltage tracking

[Eduardo Diaz]

On Panther Lake (xe3lpd) laptops, cold boot reliably corrupts the
internal eDP panel: pipe A gets a "Selective fetch area calculation
failed in pipe A" warning immediately followed by a CPU pipe A FIFO
underrun, and the panel stays corrupted for the rest of the session.
A subsequent suspend/resume cycle (or any other full re-modeset)
"fixes" it, which pointed at cdclk/voltage-level tracking rather than
a genuine hardware race.

intel_modeset_readout_hw_state() runs once at driver probe (and again
on resume) to figure out what firmware/GOP left the display in. For
each already-active pipe it calls intel_cdclk_update_hw_state(),
which seeds cdclk_state->min_cdclk[]/min_voltage_level[] directly
from the freshly read-out crtc_state. That treats "firmware left this
pipe active with mode X" as proof that this driver's own cdclk,
voltage-level and DBUF setup for mode X is already established in
hardware. It isn't -- only firmware's own, entirely separate code
path has ever touched those registers.

The OS driver's first real modeset for an inherited pipe typically
targets the same native panel mode, so the freshly computed
crtc_state->min_cdclk/min_voltage_level trivially match this
readout-seeded baseline. intel_cdclk_update_crtc_min_cdclk() and
intel_cdclk_update_crtc_min_voltage_level() then conclude nothing
changed and skip the recalculation, on the one commit where it
actually matters: taking a pipe from firmware ownership to being
correctly configured by this driver.

Fix this in two parts:

 - Invalidate the per-pipe min_cdclk[]/min_voltage_level[] tracking
   right after boot-time readout, so the first real atomic commit is
   guaranteed to see a difference.

 - Stop early-returning in intel_cdclk_update_crtc_min_cdclk() and
   intel_cdclk_update_crtc_min_voltage_level() based on the crtc_state
   comparison alone. That comparison is unreliable for exactly the
   same reason (it's derived from the same readout), and skips the
   real, tracked-state check below it.

Bisected on real hardware (Lenovo Yoga 9i 14IPH11, Panther Lake) down
to a narrow window between v6.19.10 (clean on every cold boot) and
v7.1.10 (broken on every cold boot); confirmed via live kernel
tracing that the skip path fires unconditionally on this platform
from the very first post-boot atomic commit onward. This fix
eliminates the FIFO underrun across many consecutive cold boots on
the same hardware, with no regression observed across suspend/resume.

The investigation and this fix were developed with the assistance of
Claude (Anthropic), driven and verified end to end on the affected
hardware by the Signed-off-by below.

Signed-off-by: Eduardo Diaz <iamedu@gmail.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 45 ++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_cdclk.h    |  1 +
 .../drm/i915/display/intel_modeset_setup.c    |  1 +
 3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index a53d887271..209a2373d1 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2981,11 +2981,14 @@ static int intel_cdclk_update_crtc_min_cdclk(struct intel_atomic_state *state,
 	bool allow_cdclk_decrease = intel_any_crtc_needs_modeset(state);
 	int ret;
 
-	if (new_min_cdclk == old_min_cdclk)
-		return 0;
-
-	if (!allow_cdclk_decrease && new_min_cdclk < old_min_cdclk)
-		return 0;
+	/*
+	 * old_min_cdclk comes from the previous crtc_state, which after
+	 * boot-time readout reflects whatever firmware/GOP left running,
+	 * not what this driver has programmed. For an inherited pipe it
+	 * equals new_min_cdclk by construction (same mode, same formula),
+	 * so an early return here would skip the recalculation that
+	 * matters. Always continue on to the cdclk_state check below.
+	 */
 
 	cdclk_state = intel_atomic_get_cdclk_state(state);
 	if (IS_ERR(cdclk_state))
@@ -3026,12 +3029,10 @@ static int intel_cdclk_update_crtc_min_voltage_level(struct intel_atomic_state *
 	bool allow_voltage_level_decrease = intel_any_crtc_needs_modeset(state);
 	int ret;
 
-	if (new_min_voltage_level == old_min_voltage_level)
-		return 0;
-
-	if (!allow_voltage_level_decrease &&
-	    new_min_voltage_level < old_min_voltage_level)
-		return 0;
+	/*
+	 * old_min_voltage_level is unreliable for the same reason; see
+	 * intel_cdclk_update_crtc_min_cdclk().
+	 */
 
 	cdclk_state = intel_atomic_get_cdclk_state(state);
 	if (IS_ERR(cdclk_state))
@@ -3705,6 +3706,28 @@ void intel_cdclk_update_hw_state(struct intel_display *display)
 	cdclk_state->dbuf_bw_min_cdclk = intel_dbuf_bw_min_cdclk(display, dbuf_bw_state);
 }
 
+/*
+ * intel_cdclk_update_hw_state() seeds min_cdclk[]/min_voltage_level[]
+ * from readout's crtc_state, i.e. from whatever firmware/GOP left
+ * running, not from anything this driver has programmed. A pipe's
+ * first real modeset usually targets the same native mode, so the
+ * freshly computed value matches this seeded baseline and
+ * intel_cdclk_update_crtc_min_cdclk()/_min_voltage_level() conclude
+ * nothing changed, skipping the recalculation that matters. Call
+ * this after readout so the first real commit sees a difference.
+ */
+void intel_cdclk_invalidate_min_tracking(struct intel_display *display)
+{
+	struct intel_cdclk_state *cdclk_state =
+		to_intel_cdclk_state(display->cdclk.obj.state);
+	enum pipe pipe;
+
+	for_each_pipe(display, pipe) {
+		cdclk_state->min_cdclk[pipe] = 0;
+		cdclk_state->min_voltage_level[pipe] = 0;
+	}
+}
+
 void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc)
 {
 	struct intel_display *display = to_intel_display(crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index a60cbf745e..1517d3605a 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -46,6 +46,7 @@ int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joi
 struct intel_cdclk_state *
 intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
 void intel_cdclk_update_hw_state(struct intel_display *display);
+void intel_cdclk_invalidate_min_tracking(struct intel_display *display);
 void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc);
 int intel_cdclk_update_dbuf_bw_min_cdclk(struct intel_atomic_state *state,
 					 int old_min_cdclk, int new_min_cdclk,
diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index 6aed881737..c7be6e63e7 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -880,6 +880,7 @@ static void intel_modeset_readout_hw_state(struct intel_display *display)
 	intel_bw_update_hw_state(display);
 	intel_dbuf_bw_update_hw_state(display);
 	intel_cdclk_update_hw_state(display);
+	intel_cdclk_invalidate_min_tracking(display);
 
 	intel_pmdemand_init_pmdemand_params(display, pmdemand_state);
 }
-- 
2.55.0

Re: [PATCH] drm/i915/cdclk: Don't trust boot readout for per-pipe cdclk/voltage tracking

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Zeroing `min_cdclk` and `min_voltage_level` during boot readout corrupts the global state tracking for inherited pipes, leading to FIFO underruns on partial commits.
- [High] The boot-time invalidation of cdclk/voltage tracking is fragile and gets silently undone during hardware sanitization, breaking the fix for the remaining active pipes.
- [Medium] Moving the early return in the CDCLK update paths forces unnecessary global state duplication and memory allocation on every page flip.
- [Low] The `old_min_cdclk` and `old_min_voltage_level` parameters are now ignored and immediately overwritten.
--

commit edef4e8a0b14b779ca2831158248f1bc907d372c
Author: Eduardo Diaz <iamedu@gmail.com>

drm/i915/cdclk: Don't trust boot readout for per-pipe cdclk/voltage tracking

This commit addresses a bug where cold boot corrupts the internal eDP panel
due to cdclk/voltage tracking assuming firmware-programmed state as its own.
It invalidates the tracking state during readout and forces the cdclk and
voltage update functions to skip early returns.

> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index a53d887271778..209a2373d17aa 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2981,11 +2981,14 @@ static int intel_cdclk_update_crtc_min_cdclk(struct intel_atomic_state *state,
>  	bool allow_cdclk_decrease = intel_any_crtc_needs_modeset(state);
>  	int ret;
>  
> -	if (new_min_cdclk == old_min_cdclk)
> -		return 0;
> -
> -	if (!allow_cdclk_decrease && new_min_cdclk < old_min_cdclk)
> -		return 0;
> +	/*
> +	 * old_min_cdclk comes from the previous crtc_state, which after
> +	 * boot-time readout reflects whatever firmware/GOP left running,
> +	 * not what this driver has programmed. For an inherited pipe it
> +	 * equals new_min_cdclk by construction (same mode, same formula),
> +	 * so an early return here would skip the recalculation that
> +	 * matters. Always continue on to the cdclk_state check below.
> +	 */
>  
>  	cdclk_state = intel_atomic_get_cdclk_state(state);
>  	if (IS_ERR(cdclk_state))

[Severity: Medium]
Moving this early return causes the state duplication and memory allocations
in intel_atomic_get_cdclk_state() to occur unconditionally on every commit,
including every page flip. Does this introduce a performance regression or
latency spike in this hot path?

[Severity: Low]
Additionally, since the old_min_cdclk value passed into the function is now
overwritten right after intel_atomic_get_cdclk_state() returns:

    old_min_cdclk = cdclk_state->min_cdclk[crtc->pipe];

Does this make the old_min_cdclk parameter passed by the caller dead code
that is never actually read?

[ ... ]

> @@ -3026,12 +3029,10 @@ static int intel_cdclk_update_crtc_min_voltage_level(struct intel_atomic_state *
>  	bool allow_voltage_level_decrease = intel_any_crtc_needs_modeset(state);
>  	int ret;
>  
> -	if (new_min_voltage_level == old_min_voltage_level)
> -		return 0;
> -
> -	if (!allow_voltage_level_decrease &&
> -	    new_min_voltage_level < old_min_voltage_level)
> -		return 0;
> +	/*
> +	 * old_min_voltage_level is unreliable for the same reason; see
> +	 * intel_cdclk_update_crtc_min_cdclk().
> +	 */
>  
>  	cdclk_state = intel_atomic_get_cdclk_state(state);
>  	if (IS_ERR(cdclk_state))

[Severity: Low]
Similarly to old_min_cdclk, is the old_min_voltage_level parameter now ignored
and overwritten before it is ever used?

[ ... ]

> @@ -3705,6 +3706,28 @@ void intel_cdclk_update_hw_state(struct intel_display *display)
>  	cdclk_state->dbuf_bw_min_cdclk = intel_dbuf_bw_min_cdclk(display, dbuf_bw_state);
>  }
>  
> +/*
> + * intel_cdclk_update_hw_state() seeds min_cdclk[]/min_voltage_level[]
> + * from readout's crtc_state, i.e. from whatever firmware/GOP left
> + * running, not from anything this driver has programmed. A pipe's
> + * first real modeset usually targets the same native mode, so the
> + * freshly computed value matches this seeded baseline and
> + * intel_cdclk_update_crtc_min_cdclk()/_min_voltage_level() conclude
> + * nothing changed, skipping the recalculation that matters. Call
> + * this after readout so the first real commit sees a difference.
> + */
> +void intel_cdclk_invalidate_min_tracking(struct intel_display *display)
> +{
> +	struct intel_cdclk_state *cdclk_state =
> +		to_intel_cdclk_state(display->cdclk.obj.state);
> +	enum pipe pipe;
> +
> +	for_each_pipe(display, pipe) {
> +		cdclk_state->min_cdclk[pipe] = 0;
> +		cdclk_state->min_voltage_level[pipe] = 0;
> +	}
> +}

[Severity: High]
Does zeroing min_cdclk for all pipes corrupt the state for inherited pipes
that are not included in the first atomic commit?

If the OS performs a partial commit (e.g., a cursor update on a single pipe)
on a multi-display setup, intel_cdclk_update_crtc_min_cdclk() will only
update the tracking for the pipe in the commit.

Because intel_compute_min_cdclk() takes the maximum of all pipes' requirements,
will the untouched active pipes with 0 tracked requirement be starved of their
necessary clock speed, leading to a FIFO underrun?

[Severity: High]
Can this boot-time invalidation be silently undone during hardware sanitization?

If a CRTC requires sanitization during boot, intel_crtc_disable_noatomic()
calls intel_cdclk_crtc_disable_noatomic(), which in turn calls
intel_cdclk_update_hw_state().

intel_cdclk_update_hw_state() loops over all pipes and does:

    cdclk_state->min_cdclk[pipe] = crtc_state->min_cdclk;

Since crtc_state->min_cdclk still contains the readout value, does this
re-seed the tracking values for the remaining active pipes, circumventing
the fix entirely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830161703.11570-1-iamedu@gmail.com?part=1

Re: [PATCH] drm/i915/cdclk: Don't trust boot readout for per-pipe cdclk/voltage tracking

[Jani Nikula]

On Sun, 30 Aug 2026, Eduardo Diaz <iamedu@gmail.com> wrote:
> On Panther Lake (xe3lpd) laptops, cold boot reliably corrupts the
> internal eDP panel: pipe A gets a "Selective fetch area calculation
> failed in pipe A" warning immediately followed by a CPU pipe A FIFO
> underrun, and the panel stays corrupted for the rest of the session.
> A subsequent suspend/resume cycle (or any other full re-modeset)
> "fixes" it, which pointed at cdclk/voltage-level tracking rather than
> a genuine hardware race.
>
> intel_modeset_readout_hw_state() runs once at driver probe (and again
> on resume) to figure out what firmware/GOP left the display in. For
> each already-active pipe it calls intel_cdclk_update_hw_state(),
> which seeds cdclk_state->min_cdclk[]/min_voltage_level[] directly
> from the freshly read-out crtc_state. That treats "firmware left this
> pipe active with mode X" as proof that this driver's own cdclk,
> voltage-level and DBUF setup for mode X is already established in
> hardware. It isn't -- only firmware's own, entirely separate code
> path has ever touched those registers.
>
> The OS driver's first real modeset for an inherited pipe typically
> targets the same native panel mode, so the freshly computed
> crtc_state->min_cdclk/min_voltage_level trivially match this
> readout-seeded baseline. intel_cdclk_update_crtc_min_cdclk() and
> intel_cdclk_update_crtc_min_voltage_level() then conclude nothing
> changed and skip the recalculation, on the one commit where it
> actually matters: taking a pipe from firmware ownership to being
> correctly configured by this driver.
>
> Fix this in two parts:
>
>  - Invalidate the per-pipe min_cdclk[]/min_voltage_level[] tracking
>    right after boot-time readout, so the first real atomic commit is
>    guaranteed to see a difference.
>
>  - Stop early-returning in intel_cdclk_update_crtc_min_cdclk() and
>    intel_cdclk_update_crtc_min_voltage_level() based on the crtc_state
>    comparison alone. That comparison is unreliable for exactly the
>    same reason (it's derived from the same readout), and skips the
>    real, tracked-state check below it.
>
> Bisected on real hardware (Lenovo Yoga 9i 14IPH11, Panther Lake) down
> to a narrow window between v6.19.10 (clean on every cold boot) and
> v7.1.10 (broken on every cold boot); confirmed via live kernel
> tracing that the skip path fires unconditionally on this platform
> from the very first post-boot atomic commit onward. This fix
> eliminates the FIFO underrun across many consecutive cold boots on
> the same hardware, with no regression observed across suspend/resume.

Do you have a regressing commit? Nearly 600 commits were merged to i915
display alone between 6.19 and 7.1 so it's not really a narrow window.

Have you filed a bug as described at [1]? With debugs, logs, and
everything.

Have you tried with 1786d2688781 ("drm/i915/cdclk: Avoid spurious cdclk
sanitization on PTL+")?


BR,
Jani.


[1] https://drm.pages.freedesktop.org/intel-docs/how-to-file-i915-bugs.html


> The investigation and this fix were developed with the assistance of
> Claude (Anthropic), driven and verified end to end on the affected
> hardware by the Signed-off-by below.
>
> Signed-off-by: Eduardo Diaz <iamedu@gmail.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 45 ++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_cdclk.h    |  1 +
>  .../drm/i915/display/intel_modeset_setup.c    |  1 +
>  3 files changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index a53d887271..209a2373d1 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2981,11 +2981,14 @@ static int intel_cdclk_update_crtc_min_cdclk(struct intel_atomic_state *state,
>  	bool allow_cdclk_decrease = intel_any_crtc_needs_modeset(state);
>  	int ret;
>  
> -	if (new_min_cdclk == old_min_cdclk)
> -		return 0;
> -
> -	if (!allow_cdclk_decrease && new_min_cdclk < old_min_cdclk)
> -		return 0;
> +	/*
> +	 * old_min_cdclk comes from the previous crtc_state, which after
> +	 * boot-time readout reflects whatever firmware/GOP left running,
> +	 * not what this driver has programmed. For an inherited pipe it
> +	 * equals new_min_cdclk by construction (same mode, same formula),
> +	 * so an early return here would skip the recalculation that
> +	 * matters. Always continue on to the cdclk_state check below.
> +	 */
>  
>  	cdclk_state = intel_atomic_get_cdclk_state(state);
>  	if (IS_ERR(cdclk_state))
> @@ -3026,12 +3029,10 @@ static int intel_cdclk_update_crtc_min_voltage_level(struct intel_atomic_state *
>  	bool allow_voltage_level_decrease = intel_any_crtc_needs_modeset(state);
>  	int ret;
>  
> -	if (new_min_voltage_level == old_min_voltage_level)
> -		return 0;
> -
> -	if (!allow_voltage_level_decrease &&
> -	    new_min_voltage_level < old_min_voltage_level)
> -		return 0;
> +	/*
> +	 * old_min_voltage_level is unreliable for the same reason; see
> +	 * intel_cdclk_update_crtc_min_cdclk().
> +	 */
>  
>  	cdclk_state = intel_atomic_get_cdclk_state(state);
>  	if (IS_ERR(cdclk_state))
> @@ -3705,6 +3706,28 @@ void intel_cdclk_update_hw_state(struct intel_display *display)
>  	cdclk_state->dbuf_bw_min_cdclk = intel_dbuf_bw_min_cdclk(display, dbuf_bw_state);
>  }
>  
> +/*
> + * intel_cdclk_update_hw_state() seeds min_cdclk[]/min_voltage_level[]
> + * from readout's crtc_state, i.e. from whatever firmware/GOP left
> + * running, not from anything this driver has programmed. A pipe's
> + * first real modeset usually targets the same native mode, so the
> + * freshly computed value matches this seeded baseline and
> + * intel_cdclk_update_crtc_min_cdclk()/_min_voltage_level() conclude
> + * nothing changed, skipping the recalculation that matters. Call
> + * this after readout so the first real commit sees a difference.
> + */
> +void intel_cdclk_invalidate_min_tracking(struct intel_display *display)
> +{
> +	struct intel_cdclk_state *cdclk_state =
> +		to_intel_cdclk_state(display->cdclk.obj.state);
> +	enum pipe pipe;
> +
> +	for_each_pipe(display, pipe) {
> +		cdclk_state->min_cdclk[pipe] = 0;
> +		cdclk_state->min_voltage_level[pipe] = 0;
> +	}
> +}
> +
>  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc)
>  {
>  	struct intel_display *display = to_intel_display(crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index a60cbf745e..1517d3605a 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -46,6 +46,7 @@ int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joi
>  struct intel_cdclk_state *
>  intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
>  void intel_cdclk_update_hw_state(struct intel_display *display);
> +void intel_cdclk_invalidate_min_tracking(struct intel_display *display);
>  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc);
>  int intel_cdclk_update_dbuf_bw_min_cdclk(struct intel_atomic_state *state,
>  					 int old_min_cdclk, int new_min_cdclk,
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index 6aed881737..c7be6e63e7 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -880,6 +880,7 @@ static void intel_modeset_readout_hw_state(struct intel_display *display)
>  	intel_bw_update_hw_state(display);
>  	intel_dbuf_bw_update_hw_state(display);
>  	intel_cdclk_update_hw_state(display);
> +	intel_cdclk_invalidate_min_tracking(display);
>  
>  	intel_pmdemand_init_pmdemand_params(display, pmdemand_state);
>  }

-- 
Jani Nikula, Intel

Re: [PATCH] drm/i915/cdclk: Don't trust boot readout for per-pipe cdclk/voltage tracking

[Eduardo Díaz]

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

I filed a report here:
https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9074 debug logs
weren't enabled though.

I don't believe I've tried 1786d2688781 ("drm/i915/cdclk: Avoid spurious
cdclk
sanitization on PTL+"), will do so later today.

On Mon, Aug 31, 2026 at 12:39 AM Jani Nikula <jani.nikula@linux.intel.com>
wrote:

> On Sun, 30 Aug 2026, Eduardo Diaz <iamedu@gmail.com> wrote:
> > On Panther Lake (xe3lpd) laptops, cold boot reliably corrupts the
> > internal eDP panel: pipe A gets a "Selective fetch area calculation
> > failed in pipe A" warning immediately followed by a CPU pipe A FIFO
> > underrun, and the panel stays corrupted for the rest of the session.
> > A subsequent suspend/resume cycle (or any other full re-modeset)
> > "fixes" it, which pointed at cdclk/voltage-level tracking rather than
> > a genuine hardware race.
> >
> > intel_modeset_readout_hw_state() runs once at driver probe (and again
> > on resume) to figure out what firmware/GOP left the display in. For
> > each already-active pipe it calls intel_cdclk_update_hw_state(),
> > which seeds cdclk_state->min_cdclk[]/min_voltage_level[] directly
> > from the freshly read-out crtc_state. That treats "firmware left this
> > pipe active with mode X" as proof that this driver's own cdclk,
> > voltage-level and DBUF setup for mode X is already established in
> > hardware. It isn't -- only firmware's own, entirely separate code
> > path has ever touched those registers.
> >
> > The OS driver's first real modeset for an inherited pipe typically
> > targets the same native panel mode, so the freshly computed
> > crtc_state->min_cdclk/min_voltage_level trivially match this
> > readout-seeded baseline. intel_cdclk_update_crtc_min_cdclk() and
> > intel_cdclk_update_crtc_min_voltage_level() then conclude nothing
> > changed and skip the recalculation, on the one commit where it
> > actually matters: taking a pipe from firmware ownership to being
> > correctly configured by this driver.
> >
> > Fix this in two parts:
> >
> >  - Invalidate the per-pipe min_cdclk[]/min_voltage_level[] tracking
> >    right after boot-time readout, so the first real atomic commit is
> >    guaranteed to see a difference.
> >
> >  - Stop early-returning in intel_cdclk_update_crtc_min_cdclk() and
> >    intel_cdclk_update_crtc_min_voltage_level() based on the crtc_state
> >    comparison alone. That comparison is unreliable for exactly the
> >    same reason (it's derived from the same readout), and skips the
> >    real, tracked-state check below it.
> >
> > Bisected on real hardware (Lenovo Yoga 9i 14IPH11, Panther Lake) down
> > to a narrow window between v6.19.10 (clean on every cold boot) and
> > v7.1.10 (broken on every cold boot); confirmed via live kernel
> > tracing that the skip path fires unconditionally on this platform
> > from the very first post-boot atomic commit onward. This fix
> > eliminates the FIFO underrun across many consecutive cold boots on
> > the same hardware, with no regression observed across suspend/resume.
>
> Do you have a regressing commit? Nearly 600 commits were merged to i915
> display alone between 6.19 and 7.1 so it's not really a narrow window.
>
> Have you filed a bug as described at [1]? With debugs, logs, and
> everything.
>
> Have you tried with 1786d2688781 ("drm/i915/cdclk: Avoid spurious cdclk
> sanitization on PTL+")?
>
>
> BR,
> Jani.
>
>
> [1]
> https://drm.pages.freedesktop.org/intel-docs/how-to-file-i915-bugs.html
>
>
> > The investigation and this fix were developed with the assistance of
> > Claude (Anthropic), driven and verified end to end on the affected
> > hardware by the Signed-off-by below.
> >
> > Signed-off-by: Eduardo Diaz <iamedu@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c    | 45 ++++++++++++++-----
> >  drivers/gpu/drm/i915/display/intel_cdclk.h    |  1 +
> >  .../drm/i915/display/intel_modeset_setup.c    |  1 +
> >  3 files changed, 36 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index a53d887271..209a2373d1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -2981,11 +2981,14 @@ static int
> intel_cdclk_update_crtc_min_cdclk(struct intel_atomic_state *state,
> >       bool allow_cdclk_decrease = intel_any_crtc_needs_modeset(state);
> >       int ret;
> >
> > -     if (new_min_cdclk == old_min_cdclk)
> > -             return 0;
> > -
> > -     if (!allow_cdclk_decrease && new_min_cdclk < old_min_cdclk)
> > -             return 0;
> > +     /*
> > +      * old_min_cdclk comes from the previous crtc_state, which after
> > +      * boot-time readout reflects whatever firmware/GOP left running,
> > +      * not what this driver has programmed. For an inherited pipe it
> > +      * equals new_min_cdclk by construction (same mode, same formula),
> > +      * so an early return here would skip the recalculation that
> > +      * matters. Always continue on to the cdclk_state check below.
> > +      */
> >
> >       cdclk_state = intel_atomic_get_cdclk_state(state);
> >       if (IS_ERR(cdclk_state))
> > @@ -3026,12 +3029,10 @@ static int
> intel_cdclk_update_crtc_min_voltage_level(struct intel_atomic_state *
> >       bool allow_voltage_level_decrease =
> intel_any_crtc_needs_modeset(state);
> >       int ret;
> >
> > -     if (new_min_voltage_level == old_min_voltage_level)
> > -             return 0;
> > -
> > -     if (!allow_voltage_level_decrease &&
> > -         new_min_voltage_level < old_min_voltage_level)
> > -             return 0;
> > +     /*
> > +      * old_min_voltage_level is unreliable for the same reason; see
> > +      * intel_cdclk_update_crtc_min_cdclk().
> > +      */
> >
> >       cdclk_state = intel_atomic_get_cdclk_state(state);
> >       if (IS_ERR(cdclk_state))
> > @@ -3705,6 +3706,28 @@ void intel_cdclk_update_hw_state(struct
> intel_display *display)
> >       cdclk_state->dbuf_bw_min_cdclk = intel_dbuf_bw_min_cdclk(display,
> dbuf_bw_state);
> >  }
> >
> > +/*
> > + * intel_cdclk_update_hw_state() seeds min_cdclk[]/min_voltage_level[]
> > + * from readout's crtc_state, i.e. from whatever firmware/GOP left
> > + * running, not from anything this driver has programmed. A pipe's
> > + * first real modeset usually targets the same native mode, so the
> > + * freshly computed value matches this seeded baseline and
> > + * intel_cdclk_update_crtc_min_cdclk()/_min_voltage_level() conclude
> > + * nothing changed, skipping the recalculation that matters. Call
> > + * this after readout so the first real commit sees a difference.
> > + */
> > +void intel_cdclk_invalidate_min_tracking(struct intel_display *display)
> > +{
> > +     struct intel_cdclk_state *cdclk_state =
> > +             to_intel_cdclk_state(display->cdclk.obj.state);
> > +     enum pipe pipe;
> > +
> > +     for_each_pipe(display, pipe) {
> > +             cdclk_state->min_cdclk[pipe] = 0;
> > +             cdclk_state->min_voltage_level[pipe] = 0;
> > +     }
> > +}
> > +
> >  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc)
> >  {
> >       struct intel_display *display = to_intel_display(crtc);
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
> b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > index a60cbf745e..1517d3605a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > @@ -46,6 +46,7 @@ int intel_cdclk_state_set_joined_mbus(struct
> intel_atomic_state *state, bool joi
> >  struct intel_cdclk_state *
> >  intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
> >  void intel_cdclk_update_hw_state(struct intel_display *display);
> > +void intel_cdclk_invalidate_min_tracking(struct intel_display *display);
> >  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc);
> >  int intel_cdclk_update_dbuf_bw_min_cdclk(struct intel_atomic_state
> *state,
> >                                        int old_min_cdclk, int
> new_min_cdclk,
> > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > index 6aed881737..c7be6e63e7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > @@ -880,6 +880,7 @@ static void intel_modeset_readout_hw_state(struct
> intel_display *display)
> >       intel_bw_update_hw_state(display);
> >       intel_dbuf_bw_update_hw_state(display);
> >       intel_cdclk_update_hw_state(display);
> > +     intel_cdclk_invalidate_min_tracking(display);
> >
> >       intel_pmdemand_init_pmdemand_params(display, pmdemand_state);
> >  }
>
> --
> Jani Nikula, Intel
>

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

Re: [PATCH] drm/i915/cdclk: Don't trust boot readout for per-pipe cdclk/voltage tracking

[Eduardo Díaz]

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

OK, confirmed. That patch fixes the issue

Thanks!



On Mon, Aug 31, 2026 at 7:52 AM Eduardo Díaz <iamedu@gmail.com> wrote:

> I filed a report here:
> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9074 debug logs
> weren't enabled though.
>
> I don't believe I've tried 1786d2688781 ("drm/i915/cdclk: Avoid spurious
> cdclk
> sanitization on PTL+"), will do so later today.
>
> On Mon, Aug 31, 2026 at 12:39 AM Jani Nikula <jani.nikula@linux.intel.com>
> wrote:
>
>> On Sun, 30 Aug 2026, Eduardo Diaz <iamedu@gmail.com> wrote:
>> > On Panther Lake (xe3lpd) laptops, cold boot reliably corrupts the
>> > internal eDP panel: pipe A gets a "Selective fetch area calculation
>> > failed in pipe A" warning immediately followed by a CPU pipe A FIFO
>> > underrun, and the panel stays corrupted for the rest of the session.
>> > A subsequent suspend/resume cycle (or any other full re-modeset)
>> > "fixes" it, which pointed at cdclk/voltage-level tracking rather than
>> > a genuine hardware race.
>> >
>> > intel_modeset_readout_hw_state() runs once at driver probe (and again
>> > on resume) to figure out what firmware/GOP left the display in. For
>> > each already-active pipe it calls intel_cdclk_update_hw_state(),
>> > which seeds cdclk_state->min_cdclk[]/min_voltage_level[] directly
>> > from the freshly read-out crtc_state. That treats "firmware left this
>> > pipe active with mode X" as proof that this driver's own cdclk,
>> > voltage-level and DBUF setup for mode X is already established in
>> > hardware. It isn't -- only firmware's own, entirely separate code
>> > path has ever touched those registers.
>> >
>> > The OS driver's first real modeset for an inherited pipe typically
>> > targets the same native panel mode, so the freshly computed
>> > crtc_state->min_cdclk/min_voltage_level trivially match this
>> > readout-seeded baseline. intel_cdclk_update_crtc_min_cdclk() and
>> > intel_cdclk_update_crtc_min_voltage_level() then conclude nothing
>> > changed and skip the recalculation, on the one commit where it
>> > actually matters: taking a pipe from firmware ownership to being
>> > correctly configured by this driver.
>> >
>> > Fix this in two parts:
>> >
>> >  - Invalidate the per-pipe min_cdclk[]/min_voltage_level[] tracking
>> >    right after boot-time readout, so the first real atomic commit is
>> >    guaranteed to see a difference.
>> >
>> >  - Stop early-returning in intel_cdclk_update_crtc_min_cdclk() and
>> >    intel_cdclk_update_crtc_min_voltage_level() based on the crtc_state
>> >    comparison alone. That comparison is unreliable for exactly the
>> >    same reason (it's derived from the same readout), and skips the
>> >    real, tracked-state check below it.
>> >
>> > Bisected on real hardware (Lenovo Yoga 9i 14IPH11, Panther Lake) down
>> > to a narrow window between v6.19.10 (clean on every cold boot) and
>> > v7.1.10 (broken on every cold boot); confirmed via live kernel
>> > tracing that the skip path fires unconditionally on this platform
>> > from the very first post-boot atomic commit onward. This fix
>> > eliminates the FIFO underrun across many consecutive cold boots on
>> > the same hardware, with no regression observed across suspend/resume.
>>
>> Do you have a regressing commit? Nearly 600 commits were merged to i915
>> display alone between 6.19 and 7.1 so it's not really a narrow window.
>>
>> Have you filed a bug as described at [1]? With debugs, logs, and
>> everything.
>>
>> Have you tried with 1786d2688781 ("drm/i915/cdclk: Avoid spurious cdclk
>> sanitization on PTL+")?
>>
>>
>> BR,
>> Jani.
>>
>>
>> [1]
>> https://drm.pages.freedesktop.org/intel-docs/how-to-file-i915-bugs.html
>>
>>
>> > The investigation and this fix were developed with the assistance of
>> > Claude (Anthropic), driven and verified end to end on the affected
>> > hardware by the Signed-off-by below.
>> >
>> > Signed-off-by: Eduardo Diaz <iamedu@gmail.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_cdclk.c    | 45 ++++++++++++++-----
>> >  drivers/gpu/drm/i915/display/intel_cdclk.h    |  1 +
>> >  .../drm/i915/display/intel_modeset_setup.c    |  1 +
>> >  3 files changed, 36 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > index a53d887271..209a2373d1 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> > @@ -2981,11 +2981,14 @@ static int
>> intel_cdclk_update_crtc_min_cdclk(struct intel_atomic_state *state,
>> >       bool allow_cdclk_decrease = intel_any_crtc_needs_modeset(state);
>> >       int ret;
>> >
>> > -     if (new_min_cdclk == old_min_cdclk)
>> > -             return 0;
>> > -
>> > -     if (!allow_cdclk_decrease && new_min_cdclk < old_min_cdclk)
>> > -             return 0;
>> > +     /*
>> > +      * old_min_cdclk comes from the previous crtc_state, which after
>> > +      * boot-time readout reflects whatever firmware/GOP left running,
>> > +      * not what this driver has programmed. For an inherited pipe it
>> > +      * equals new_min_cdclk by construction (same mode, same formula),
>> > +      * so an early return here would skip the recalculation that
>> > +      * matters. Always continue on to the cdclk_state check below.
>> > +      */
>> >
>> >       cdclk_state = intel_atomic_get_cdclk_state(state);
>> >       if (IS_ERR(cdclk_state))
>> > @@ -3026,12 +3029,10 @@ static int
>> intel_cdclk_update_crtc_min_voltage_level(struct intel_atomic_state *
>> >       bool allow_voltage_level_decrease =
>> intel_any_crtc_needs_modeset(state);
>> >       int ret;
>> >
>> > -     if (new_min_voltage_level == old_min_voltage_level)
>> > -             return 0;
>> > -
>> > -     if (!allow_voltage_level_decrease &&
>> > -         new_min_voltage_level < old_min_voltage_level)
>> > -             return 0;
>> > +     /*
>> > +      * old_min_voltage_level is unreliable for the same reason; see
>> > +      * intel_cdclk_update_crtc_min_cdclk().
>> > +      */
>> >
>> >       cdclk_state = intel_atomic_get_cdclk_state(state);
>> >       if (IS_ERR(cdclk_state))
>> > @@ -3705,6 +3706,28 @@ void intel_cdclk_update_hw_state(struct
>> intel_display *display)
>> >       cdclk_state->dbuf_bw_min_cdclk = intel_dbuf_bw_min_cdclk(display,
>> dbuf_bw_state);
>> >  }
>> >
>> > +/*
>> > + * intel_cdclk_update_hw_state() seeds min_cdclk[]/min_voltage_level[]
>> > + * from readout's crtc_state, i.e. from whatever firmware/GOP left
>> > + * running, not from anything this driver has programmed. A pipe's
>> > + * first real modeset usually targets the same native mode, so the
>> > + * freshly computed value matches this seeded baseline and
>> > + * intel_cdclk_update_crtc_min_cdclk()/_min_voltage_level() conclude
>> > + * nothing changed, skipping the recalculation that matters. Call
>> > + * this after readout so the first real commit sees a difference.
>> > + */
>> > +void intel_cdclk_invalidate_min_tracking(struct intel_display *display)
>> > +{
>> > +     struct intel_cdclk_state *cdclk_state =
>> > +             to_intel_cdclk_state(display->cdclk.obj.state);
>> > +     enum pipe pipe;
>> > +
>> > +     for_each_pipe(display, pipe) {
>> > +             cdclk_state->min_cdclk[pipe] = 0;
>> > +             cdclk_state->min_voltage_level[pipe] = 0;
>> > +     }
>> > +}
>> > +
>> >  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc)
>> >  {
>> >       struct intel_display *display = to_intel_display(crtc);
>> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> > index a60cbf745e..1517d3605a 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> > @@ -46,6 +46,7 @@ int intel_cdclk_state_set_joined_mbus(struct
>> intel_atomic_state *state, bool joi
>> >  struct intel_cdclk_state *
>> >  intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
>> >  void intel_cdclk_update_hw_state(struct intel_display *display);
>> > +void intel_cdclk_invalidate_min_tracking(struct intel_display
>> *display);
>> >  void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc);
>> >  int intel_cdclk_update_dbuf_bw_min_cdclk(struct intel_atomic_state
>> *state,
>> >                                        int old_min_cdclk, int
>> new_min_cdclk,
>> > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> > index 6aed881737..c7be6e63e7 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> > @@ -880,6 +880,7 @@ static void intel_modeset_readout_hw_state(struct
>> intel_display *display)
>> >       intel_bw_update_hw_state(display);
>> >       intel_dbuf_bw_update_hw_state(display);
>> >       intel_cdclk_update_hw_state(display);
>> > +     intel_cdclk_invalidate_min_tracking(display);
>> >
>> >       intel_pmdemand_init_pmdemand_params(display, pmdemand_state);
>> >  }
>>
>> --
>> Jani Nikula, Intel
>>
>

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