[PATCH] drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

[PATCH] drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

[Gabríel Arthúr Pétursson]

After sanitizing cdclk programming by pre-os, the cdclk frequency is set
to the lowest supported setting. After which, modesetting needs to
recalculate the appropriate frequency.

When upgrading the kernel, we encountered an issue where we were left
with a blank screen at boot on a number of monitors and the following
message in dmesg:

	i915 0000:00:02.0: [drm] *ERROR* CPU pipe A FIFO underrun

Which bisected to these two commits, depending on the exact monitor used
during the bisect:

	ba91b9eecb47 ("drm/i915/cdclk: Decouple cdclk from state->modeset")
	74c31271a1d9 ("drm/i915: Avoid triggering unwanted cdclk changes due to dbuf bandwidth changes")

Although both commits look correct, before they hid the need to explicitly
trigger CDCLK recalculation after sanitization.

Signed-off-by: Gabríel Arthúr Pétursson <gabriel.petursson@jbtmarel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 10 ++++++++--
 drivers/gpu/drm/i915/display/intel_cdclk.h |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index a47736613f6e..5eba50fde396 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1267,6 +1267,8 @@ static void skl_sanitize_cdclk(struct intel_display *display)
 	display->cdclk.hw.cdclk = 0;
 	/* force full PLL disable + enable */
 	display->cdclk.hw.vco = ~0;
+	/* modesetting may require another cdclk programming */
+	display->cdclk.hw.sanitized = true;
 }
 
 static void skl_cdclk_init_hw(struct intel_display *display)
@@ -2365,9 +2367,10 @@ static void bxt_sanitize_cdclk(struct intel_display *display)
 
 	/* force cdclk programming */
 	display->cdclk.hw.cdclk = 0;
-
 	/* force full PLL disable + enable */
 	display->cdclk.hw.vco = ~0;
+	/* modesetting may require another cdclk programming */
+	display->cdclk.hw.sanitized = true;
 }
 
 static void bxt_cdclk_init_hw(struct intel_display *display)
@@ -3598,9 +3601,10 @@ static int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 
 int intel_cdclk_atomic_check(struct intel_atomic_state *state)
 {
+	struct intel_display *display = to_intel_display(state);
 	const struct intel_cdclk_state *old_cdclk_state;
 	struct intel_cdclk_state *new_cdclk_state;
-	bool need_cdclk_calc = false;
+	bool need_cdclk_calc = display->cdclk.hw.sanitized;
 	int ret;
 
 	ret = intel_cdclk_modeset_checks(state, &need_cdclk_calc);
@@ -3631,6 +3635,8 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state)
 		ret = intel_modeset_calc_cdclk(state);
 		if (ret)
 			return ret;
+
+		display->cdclk.hw.sanitized = false;
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 1ff7d078b42c..bae2b7ea2da9 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -18,6 +18,7 @@ struct intel_display;
 struct intel_cdclk_config {
 	unsigned int cdclk, vco, ref, bypass;
 	u8 voltage_level;
+	bool sanitized;
 	/* This field is only valid for Xe2LPD and above. */
 	bool joined_mbus;
 };
-- 
2.54.0

Re: [PATCH] drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Modifying global hardware state (`display->cdclk.hw.sanitized`) within the stateless atomic check phase violates DRM atomic semantics and breaks `TEST_ONLY` commits.
--

commit 8f701efc4d5758e65427e1e613e7e204861dc6ef
Author: Gabríel Arthúr Pétursson <gabriel.petursson@jbtmarel.com>

drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

This commit explicitly triggers CDCLK recalculation after sanitization during
modesetting. It addresses an issue where monitors display a blank screen at
boot accompanied by CPU pipe A FIFO underrun errors.

> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 189ae2d3cfc9e..38b384d825d10 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c

[ ... ]

> @@ -3596,9 +3599,10 @@ static int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
>  
>  int intel_cdclk_atomic_check(struct intel_atomic_state *state)
>  {
> +	struct intel_display *display = to_intel_display(state);
>  	const struct intel_cdclk_state *old_cdclk_state;
>  	struct intel_cdclk_state *new_cdclk_state;
> -	bool need_cdclk_calc = false;
> +	bool need_cdclk_calc = display->cdclk.hw.sanitized;

[Severity: High]
Does reading this global hardware state here lack protection against concurrent
atomic checks?

>  	int ret;
>  
>  	ret = intel_cdclk_modeset_checks(state, &need_cdclk_calc);
> @@ -3629,6 +3633,8 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state)
>  		ret = intel_modeset_calc_cdclk(state);
>  		if (ret)
>  			return ret;
> +
> +		display->cdclk.hw.sanitized = false;

[Severity: High]
Does modifying global hardware state within the stateless atomic check phase
violate DRM atomic semantics and break TEST_ONLY commits?

If a userspace Wayland compositor performs a TEST_ONLY atomic commit before a
real commit, it will trigger intel_cdclk_atomic_check() and clear this
sanitized flag. The subsequent real commit will then see sanitized == false
and fail to recalculate CDCLK, potentially resulting in the exact CPU pipe
FIFO underrun and blank screen issues this patch intended to fix.

>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/AM9PR06MB78601E2636164181A4689CC281EF2@AM9PR06MB7860.eurprd06.prod.outlook.com?part=1

Re: [PATCH] drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

[Ville Syrjälä]

On Mon, Jun 22, 2026 at 02:52:29PM +0000, Gabríel Arthúr Pétursson wrote:
> After sanitizing cdclk programming by pre-os, the cdclk frequency is set
> to the lowest supported setting. After which, modesetting needs to
> recalculate the appropriate frequency.
> 
> When upgrading the kernel, we encountered an issue where we were left
> with a blank screen at boot on a number of monitors and the following
> message in dmesg:
> 
> 	i915 0000:00:02.0: [drm] *ERROR* CPU pipe A FIFO underrun
> 
> Which bisected to these two commits, depending on the exact monitor used
> during the bisect:
> 
> 	ba91b9eecb47 ("drm/i915/cdclk: Decouple cdclk from state->modeset")
> 	74c31271a1d9 ("drm/i915: Avoid triggering unwanted cdclk changes due to dbuf bandwidth changes")
> 
> Although both commits look correct, before they hid the need to explicitly
> trigger CDCLK recalculation after sanitization.

This smells like the same thing I already fixed with
commit 3f9de66f8acb ("drm/i915/cdclk: Fix up CDCLK_FREQ_DECIMAL
without a full PLL re-enable")

Sadly it looks like I forgot to cc:stable it :/

Jani, can you pick that up for -fixes and slap a cc:stable on it?

> 
> Signed-off-by: Gabríel Arthúr Pétursson <gabriel.petursson@jbtmarel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 10 ++++++++--
>  drivers/gpu/drm/i915/display/intel_cdclk.h |  1 +
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index a47736613f6e..5eba50fde396 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1267,6 +1267,8 @@ static void skl_sanitize_cdclk(struct intel_display *display)
>  	display->cdclk.hw.cdclk = 0;
>  	/* force full PLL disable + enable */
>  	display->cdclk.hw.vco = ~0;
> +	/* modesetting may require another cdclk programming */
> +	display->cdclk.hw.sanitized = true;
>  }
>  
>  static void skl_cdclk_init_hw(struct intel_display *display)
> @@ -2365,9 +2367,10 @@ static void bxt_sanitize_cdclk(struct intel_display *display)
>  
>  	/* force cdclk programming */
>  	display->cdclk.hw.cdclk = 0;
> -
>  	/* force full PLL disable + enable */
>  	display->cdclk.hw.vco = ~0;
> +	/* modesetting may require another cdclk programming */
> +	display->cdclk.hw.sanitized = true;
>  }
>  
>  static void bxt_cdclk_init_hw(struct intel_display *display)
> @@ -3598,9 +3601,10 @@ static int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
>  
>  int intel_cdclk_atomic_check(struct intel_atomic_state *state)
>  {
> +	struct intel_display *display = to_intel_display(state);
>  	const struct intel_cdclk_state *old_cdclk_state;
>  	struct intel_cdclk_state *new_cdclk_state;
> -	bool need_cdclk_calc = false;
> +	bool need_cdclk_calc = display->cdclk.hw.sanitized;
>  	int ret;
>  
>  	ret = intel_cdclk_modeset_checks(state, &need_cdclk_calc);
> @@ -3631,6 +3635,8 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state)
>  		ret = intel_modeset_calc_cdclk(state);
>  		if (ret)
>  			return ret;
> +
> +		display->cdclk.hw.sanitized = false;
>  	}
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index 1ff7d078b42c..bae2b7ea2da9 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -18,6 +18,7 @@ struct intel_display;
>  struct intel_cdclk_config {
>  	unsigned int cdclk, vco, ref, bypass;
>  	u8 voltage_level;
> +	bool sanitized;
>  	/* This field is only valid for Xe2LPD and above. */
>  	bool joined_mbus;
>  };
> -- 
> 2.54.0
> 

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

[Joonas Lahtinen]

Quoting Ville Syrjälä (2026-06-23 17:24:43)
> This smells like the same thing I already fixed with
> commit 3f9de66f8acb ("drm/i915/cdclk: Fix up CDCLK_FREQ_DECIMAL
> without a full PLL re-enable")
> 
> Sadly it looks like I forgot to cc:stable it :/
> 
> Jani, can you pick that up for -fixes and slap a cc:stable on it?

I'll include it in drm-intel-next-fixes, do we have a
Fixes: tag to add?

Regards, Joonas

Re: [PATCH] drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

[Ville Syrjälä]

On Wed, Jun 24, 2026 at 09:47:39AM +0300, Joonas Lahtinen wrote:
> Quoting Ville Syrjälä (2026-06-23 17:24:43)
> > This smells like the same thing I already fixed with
> > commit 3f9de66f8acb ("drm/i915/cdclk: Fix up CDCLK_FREQ_DECIMAL
> > without a full PLL re-enable")
> > 
> > Sadly it looks like I forgot to cc:stable it :/
> > 
> > Jani, can you pick that up for -fixes and slap a cc:stable on it?
> 
> I'll include it in drm-intel-next-fixes, do we have a
> Fixes: tag to add?

It's been basically broken forever so the correct fixes
tags probably aren't super useful.

However the nastier user visible regression happened in 
commit ba91b9eecb47 ("drm/i915/cdclk: Decouple cdclk from
state->modeset") which therefore might make sense as a
fixes tag. But if we put that in then I'm worried that it
won't get backported to older kernels. I guess we could throw
that in there along with the ancient commits that actually
introduced the problem.

So something along the lines of:
Fixes: ba91b9eecb47 ("drm/i915/cdclk: Decouple cdclk from state->modeset")
Fixes: d66a21947e21 ("drm/i915/bxt: Sanitize CDCLK to fix breakage during S4 resume")
Fixes: c73666f394fc ("drm/i915/skl: If needed sanitize bios programmed cdclk")

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/i915: Calculate CDCLK on modeset after sanitizing pre-os programming

[Joonas Lahtinen]

Quoting Ville Syrjälä (2026-06-24 13:31:43)
> On Wed, Jun 24, 2026 at 09:47:39AM +0300, Joonas Lahtinen wrote:
> > Quoting Ville Syrjälä (2026-06-23 17:24:43)
> > > This smells like the same thing I already fixed with
> > > commit 3f9de66f8acb ("drm/i915/cdclk: Fix up CDCLK_FREQ_DECIMAL
> > > without a full PLL re-enable")
> > > 
> > > Sadly it looks like I forgot to cc:stable it :/
> > > 
> > > Jani, can you pick that up for -fixes and slap a cc:stable on it?
> > 
> > I'll include it in drm-intel-next-fixes, do we have a
> > Fixes: tag to add?
> 
> It's been basically broken forever so the correct fixes
> tags probably aren't super useful.
> 
> However the nastier user visible regression happened in 
> commit ba91b9eecb47 ("drm/i915/cdclk: Decouple cdclk from
> state->modeset") which therefore might make sense as a
> fixes tag. But if we put that in then I'm worried that it
> won't get backported to older kernels. I guess we could throw
> that in there along with the ancient commits that actually
> introduced the problem.
> 
> So something along the lines of:
> Fixes: ba91b9eecb47 ("drm/i915/cdclk: Decouple cdclk from state->modeset")
> Fixes: d66a21947e21 ("drm/i915/bxt: Sanitize CDCLK to fix breakage during S4 resume")
> Fixes: c73666f394fc ("drm/i915/skl: If needed sanitize bios programmed cdclk")

Thanks, applied with those and Cc: stable with "# v4.5+" picking
the earliest version of the three.

Regards, Joonas

> 
> -- 
> Ville Syrjälä
> Intel