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

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