[PATCH] drm/i915: Don't set min_cdclk in the initial crtc_state

Marius Hoch posted 1 patch 3 days ago
drivers/gpu/drm/i915/display/intel_modeset_setup.c | 5 -----
1 file changed, 5 deletions(-)
[PATCH] drm/i915: Don't set min_cdclk in the initial crtc_state
Posted by Marius Hoch 3 days ago
Setting the min_cdclk this early means that intel_cdclk_atomic_check
(called via intel_atomic_check) will not pick up the initial min_cdclk, as
there is no change between the old and new atomic states. This is
problematic, especially on Gemini Lake, where the picture gets unstable if
the CDCLK is too low (see vlv_dsi_min_cdclk).

This was introduced in 7a8d9cfa6db0, which states that the min_cdclk must
be set before calling intel_compute_global_watermarks. However, as the
only place that calls intel_compute_global_watermarks is
intel_atomic_check, right after setting the min_cdclk on new_crtc_state,
there is no need to set the min_cdclk initially.

This surfaced as a bug on my IdeaPad Duet 3 after ba91b9eecb47, leading
to the screen output being completely garbled initially (when asking for
the dm-crypt passphrase). It recovers after the passphrase prompt, as this
only affects the initial state.

Tested on an IdeaPad Duet 3 10IGL5-LTE (with UHD Graphics 605).

Cc: stable@vger.kernel.org
Fixes: 7a8d9cfa6db0 ("drm/i915: Compute per-crtc min_cdclk earlier")
Signed-off-by: Marius Hoch <mail@mariushoch.de>
---
 drivers/gpu/drm/i915/display/intel_modeset_setup.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index 4086f16a12bf..9278856375e9 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -865,11 +865,6 @@ static void intel_modeset_readout_hw_state(struct intel_display *display)
 				    crtc_state->plane_min_cdclk[plane->id]);
 		}
 
-		crtc_state->min_cdclk = intel_crtc_min_cdclk(crtc_state);
-
-		drm_dbg_kms(display->drm, "[CRTC:%d:%s] min_cdclk %d kHz\n",
-			    crtc->base.base.id, crtc->base.name, crtc_state->min_cdclk);
-
 		intel_pmdemand_update_port_clock(display, pmdemand_state, pipe,
 						 crtc_state->port_clock);
 	}
-- 
2.54.0
Re: [PATCH] drm/i915: Don't set min_cdclk in the initial crtc_state
Posted by Ville Syrjälä 2 days, 21 hours ago
On Thu, May 21, 2026 at 08:07:12PM +0200, Marius Hoch wrote:
> Setting the min_cdclk this early means that intel_cdclk_atomic_check
> (called via intel_atomic_check) will not pick up the initial min_cdclk, as
> there is no change between the old and new atomic states.

If there is no change then there is no need to change the CDCLK.
It's hard to say what you're really trying to work around here.

Please a file a new bug at
https://gitlab.freedesktop.org/drm/intel/issues/new and
attach the full dmesg from boot with 'log_buf_len=4M drm.debug=0xe'
passed to he kernel cmdline.

> This is
> problematic, especially on Gemini Lake, where the picture gets unstable if
> the CDCLK is too low (see vlv_dsi_min_cdclk).
> 
> This was introduced in 7a8d9cfa6db0, which states that the min_cdclk must
> be set before calling intel_compute_global_watermarks. However, as the
> only place that calls intel_compute_global_watermarks is
> intel_atomic_check, right after setting the min_cdclk on new_crtc_state,
> there is no need to set the min_cdclk initially.
> 
> This surfaced as a bug on my IdeaPad Duet 3 after ba91b9eecb47, leading
> to the screen output being completely garbled initially (when asking for
> the dm-crypt passphrase). It recovers after the passphrase prompt, as this
> only affects the initial state.
> 
> Tested on an IdeaPad Duet 3 10IGL5-LTE (with UHD Graphics 605).
> 
> Cc: stable@vger.kernel.org
> Fixes: 7a8d9cfa6db0 ("drm/i915: Compute per-crtc min_cdclk earlier")
> Signed-off-by: Marius Hoch <mail@mariushoch.de>
> ---
>  drivers/gpu/drm/i915/display/intel_modeset_setup.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index 4086f16a12bf..9278856375e9 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -865,11 +865,6 @@ static void intel_modeset_readout_hw_state(struct intel_display *display)
>  				    crtc_state->plane_min_cdclk[plane->id]);
>  		}
>  
> -		crtc_state->min_cdclk = intel_crtc_min_cdclk(crtc_state);
> -
> -		drm_dbg_kms(display->drm, "[CRTC:%d:%s] min_cdclk %d kHz\n",
> -			    crtc->base.base.id, crtc->base.name, crtc_state->min_cdclk);
> -
>  		intel_pmdemand_update_port_clock(display, pmdemand_state, pipe,
>  						 crtc_state->port_clock);
>  	}
> -- 
> 2.54.0

-- 
Ville Syrjälä
Intel
Re: [PATCH] drm/i915: Don't set min_cdclk in the initial crtc_state
Posted by Marius Hoch 2 days, 5 hours ago
Thanks for the quick reply!

On 21/05/2026 23:13, Ville Syrjälä wrote:
> On Thu, May 21, 2026 at 08:07:12PM +0200, Marius Hoch wrote:
>> Setting the min_cdclk this early means that intel_cdclk_atomic_check
>> (called via intel_atomic_check) will not pick up the initial min_cdclk, as
>> there is no change between the old and new atomic states.
> If there is no change then there is no need to change the CDCLK.
As far as I see, the problem is that the minimal CDCLK is reflected in 
the initial crtc_state, but there is no direct connection to the actual 
(hardware) CDCLK, which can be lower at this point. I'll add some more 
details below.
> It's hard to say what you're really trying to work around here.
>
> Please a file a new bug at
> https://gitlab.freedesktop.org/drm/intel/issues/new and
> attach the full dmesg from boot with 'log_buf_len=4M drm.debug=0xe'
> passed to he kernel cmdline.
Makes sense, I've filed 
https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16209 for that.

As far as I see, the CDCLK is updated based on / for an atomic change in 
the following way: intel_atomic_check calls intel_cdclk_atomic_check to 
update the CDCLK iff needed. In order to find out if a CDCLK calc is 
needed, intel_cdclk_atomic_check in turn (via intel_cdclk_atomic_check 
and intel_crtcs_calc_min_cdclk) calls intel_cdclk_update_crtc_min_cdclk. 
In intel_cdclk_update_crtc_min_cdclk, the old and the new min_cdclk get 
compared, and if there's a change, the CDCLK will be recalculated.

The issue here is that we start out with the min_cdclk set (to 158400 in 
the GLK case) in the initial / old atomic state (which is not reflected 
in the actual / hardware state). Thus intel_cdclk_update_crtc_min_cdclk 
won't be able to detect a change here (as both the old and the new 
atomic state have the same min_cdclk), and thus won't indicate that the 
CDCLK needs to be recalculated (even though the actual CDCLK might be 
lower as min_cdclk, as can be seen on the dmesg attached in the ticket).

If you want to see this addressed in another way, I'm happy to provide 
another patch. Also I'm happy to provide more details, if needed.

>
>> This is
>> problematic, especially on Gemini Lake, where the picture gets unstable if
>> the CDCLK is too low (see vlv_dsi_min_cdclk).
See beb29980026f / 
https://lore.kernel.org/all/20190430125119.7478-1-stanislav.lisovskiy@intel.com/ 
and the tickets linked from there for context here. See also 
cf696856bc54, which fixed a regression related to this Gemini Lake quirk 
before.
>>
>> This was introduced in 7a8d9cfa6db0, which states that the min_cdclk must
>> be set before calling intel_compute_global_watermarks. However, as the
>> only place that calls intel_compute_global_watermarks is
>> intel_atomic_check, right after setting the min_cdclk on new_crtc_state,
>> there is no need to set the min_cdclk initially.
>>
>> This surfaced as a bug on my IdeaPad Duet 3 after ba91b9eecb47, leading
>> to the screen output being completely garbled initially (when asking for
>> the dm-crypt passphrase). It recovers after the passphrase prompt, as this
>> only affects the initial state.
I've bisected this problem to ba91b9eecb47 initially, and naively adding 
the intel_modeset_calc_cdclk back after intel_modeset_checks in 
intel_atomic_check also fixes this issue 
(https://github.com/mariushoch/linux/commit/5d083fe47a9c0c10afcdf5b5cff5683cbfb3fe22).

>>
>> Tested on an IdeaPad Duet 3 10IGL5-LTE (with UHD Graphics 605).
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 7a8d9cfa6db0 ("drm/i915: Compute per-crtc min_cdclk earlier")
>> Signed-off-by: Marius Hoch <mail@mariushoch.de>
>> ---
>>   drivers/gpu/drm/i915/display/intel_modeset_setup.c | 5 -----
>>   1 file changed, 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> index 4086f16a12bf..9278856375e9 100644
>> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
>> @@ -865,11 +865,6 @@ static void intel_modeset_readout_hw_state(struct intel_display *display)
>>   				    crtc_state->plane_min_cdclk[plane->id]);
>>   		}
>>   
>> -		crtc_state->min_cdclk = intel_crtc_min_cdclk(crtc_state);
>> -
>> -		drm_dbg_kms(display->drm, "[CRTC:%d:%s] min_cdclk %d kHz\n",
>> -			    crtc->base.base.id, crtc->base.name, crtc_state->min_cdclk);
>> -
>>   		intel_pmdemand_update_port_clock(display, pmdemand_state, pipe,
>>   						 crtc_state->port_clock);
>>   	}
>> -- 
>> 2.54.0