Re: [PATCH v3] media: intel/ipu6: Improve DWC PHY HSFREQRANGE band selection for overlapping ranges

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, Apr 01, 2026 at 06:25:47PM +0200, Marco Nenciarini wrote:
> The get_hsfreq_by_mbps() function searches the freqranges[] table
> backward (from highest to lowest index). Because adjacent frequency
> bands overlap, a data rate that falls in the overlap region always
> lands on the higher-indexed band.
> 
> For data rates up to 1500 Mbps (index 42) every band uses
> osc_freq_target 335. Starting at index 43 (1461-1640 Mbps) the
> osc_freq_target drops to 208. A sensor running at 1498 Mbps sits in
> the overlap between index 42 (1414-1588, osc 335) and index 43
> (1461-1640, osc 208). The backward search picks index 43, programming
> the lower osc_freq_target of 208 instead of the optimal 335.
> 
> This causes DDL lock instability and CSI-2 CRC errors on affected
> configurations, such as the OmniVision OV08X40 sensor on Intel Arrow
> Lake platforms (Dell Pro Max 16).
> 
> Rewrite get_hsfreq_by_mbps() to select the optimal band:
> 
> 1. Among bands whose min/max range covers the data rate, prefer
>    the one with the higher osc_freq_target.
> 2. If osc_freq_target is equal, prefer the band whose default_mbps
>    is closest to the requested rate.
> 
> Since the frequency ranges are monotonically increasing, the loop
> exits early once min exceeds the requested rate.
> 
> For 1498 Mbps this now correctly selects index 42 (osc_freq_target
> 335, range 1414-1588) instead of index 43 (osc_freq_target 208,
> range 1461-1640).

Below just a couple of remarks, no need to be addressed, JFYI.

...

>  static u16 get_hsfreq_by_mbps(u32 mbps)
>  {
> -	unsigned int i = DPHY_FREQ_RANGE_NUM;
> -
> -	while (i--) {
> -		if (freqranges[i].default_mbps == mbps ||
> -		    (mbps >= freqranges[i].min && mbps <= freqranges[i].max))
> -			return i;
> +	u16 best = DPHY_FREQ_RANGE_INVALID_INDEX;
> +	unsigned int i;
> +
> +	for (i = 0; i < DPHY_FREQ_RANGE_NUM; i++) {
> +		if (mbps > freqranges[i].max)
> +			continue;
> +
> +		if (mbps < freqranges[i].min)
> +			break;

Wondering if this can use bsearch() algo or any linear ranges.

> +		if (best == DPHY_FREQ_RANGE_INVALID_INDEX ||
> +		    freqranges[i].osc_freq_target >
> +		    freqranges[best].osc_freq_target ||
> +		    (freqranges[i].osc_freq_target ==
> +		     freqranges[best].osc_freq_target &&
> +		     abs((int)mbps - (int)freqranges[i].default_mbps) <
> +		     abs((int)mbps - (int)freqranges[best].default_mbps)))

Note, abs(INT_MIN) is UB, I hope this won't be the case IRL in this code.

> +			best = i;
>  	}
>  
> -	return DPHY_FREQ_RANGE_INVALID_INDEX;
> +	return best;
>  }

-- 
With Best Regards,
Andy Shevchenko