Re: [PATCH v1] drm/i915: Add fallback for CDCLK selection when min_cdclk is too high

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Oct 17, 2025 at 05:18:21PM +0530, Naladala Ramanaidu wrote:
> In cases where the requested minimum CDCLK exceeds all available
> values for the current reference clock, the CDCLK selection logic
> previously returned 0. This could result coverity division or
> modulo by zero issue.
> 
> This change introduces a fallback mechanism that selects the last
> valid CDCLK value associated with the current reference clock. A
> warning is logged to indicate that the minimum requirement could
> not be satisfied, and a safe fallback value is used instead.
> 
> Fixes: Coverity CID 2628056
> 
> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index e92e7fd9fd13..a90b602a40c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1551,17 +1551,21 @@ static int cdclk_divider(int cdclk, int vco, u16 waveform)
>  static int bxt_calc_cdclk(struct intel_display *display, int min_cdclk)
>  {
>  	const struct intel_cdclk_vals *table = display->cdclk.table;
> -	int i;
> +	int i, last_valid_cdclk = 0;
>  
> -	for (i = 0; table[i].refclk; i++)
> +	for (i = 0; table[i].refclk; i++) {
>  		if (table[i].refclk == display->cdclk.hw.ref &&
>  		    table[i].cdclk >= min_cdclk)
>  			return table[i].cdclk;
>  
> +		if (table[i].refclk == display->cdclk.hw.ref)
> +			last_valid_cdclk = table[i].cdclk;
> +	}
> +
>  	drm_WARN(display->drm, 1,
> -		 "Cannot satisfy minimum cdclk %d with refclk %u\n",
> -		 min_cdclk, display->cdclk.hw.ref);
> -	return 0;

Will never happen because we never pass in a min_cdclk that
high into this function.

At some point I was thinking of determining the max cdclk via
'calc_cdclk(INT_MAX)' to get rid of some of the magic numbers
in intel_update_max_cdclk(). But I think in some cases the table
will contain values >max_cdclk anyway, so that probably is
not going to fly anyway.

So without that I don't think we really need this much
complexity and you could just do:

- return 0;
+ return display->cdclk.max_cdclk_freq;

> +		 "Cannot satisfy minimum cdclk %d with refclk %u, falling back to %d\n",
> +		 min_cdclk, display->cdclk.hw.ref, last_valid_cdclk);
> +	return last_valid_cdclk;
>  }
>  
>  static int bxt_calc_cdclk_pll_vco(struct intel_display *display, int cdclk)
> -- 
> 2.43.0

-- 
Ville Syrjälä
Intel