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

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

[Marco Nenciarini]

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).

Fixes: 1e7eeb301696 ("media: intel/ipu6: add the CSI2 DPHY implementation")
Cc: stable@vger.kernel.org
Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Changes in v3:
- Changed best variable type from int to u16 to match the function
  return type (Sakari Ailus).
- Removed the exact default_mbps match early return since the range
  check already covers it (Sakari Ailus).
- Added early break when mbps < min, since frequencies are
  monotonically increasing (Sakari Ailus).

Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Bingbu Cao <bingbu.cao@intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 .../media/pci/intel/ipu6/ipu6-isys-dwc-phy.c  | 26 ++++++++++++++-----
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c b/drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c
index db2874843..237906cb1 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c
@@ -288,15 +288,27 @@ static const struct dwc_dphy_freq_range freqranges[DPHY_FREQ_RANGE_NUM] = {
 
 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;
+
+		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)))
+			best = i;
 	}
 
-	return DPHY_FREQ_RANGE_INVALID_INDEX;
+	return best;
 }
 
 static int ipu6_isys_dwc_phy_config(struct ipu6_isys *isys,
-- 
2.47.3

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

[Andy Shevchenko]

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

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

[Marco Nenciarini]

On 02-Apr-26 11:54, Andy Shevchenko wrote:
> Below just a couple of remarks, no need to be addressed, JFYI.

Thanks for the review.

> > +		if (mbps > freqranges[i].max)
> > +			continue;
> > +
> > +		if (mbps < freqranges[i].min)
> > +			break;
> 
> Wondering if this can use bsearch() algo or any linear ranges.

The overlapping ranges and the multi-criteria selection (osc_freq_target
first, then closest default_mbps) make it hard to fit into a single
comparator. The forward scan with early break seemed like the most
straightforward approach.

> > +		     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.

Good point. The values here are well within range, but an unsigned
abs_diff() helper would be cleaner. I will address it if a v4 is needed
for other reasons.

Marco

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

[Marco Nenciarini]

Hi Sakari,

Gentle ping on this. v3 addresses all your feedback from v2 (u16 for
best, dropped exact default_mbps match, early break when min exceeds
the requested rate).

Andy also had a look and had no requested changes.

Do you have any further comments, or is this good to go?

Thanks,
Marco

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

[Sakari Ailus]

Hi Marco,

On Tue, Apr 07, 2026 at 09:12:55PM +0200, Marco Nenciarini wrote:
> Hi Sakari,
> 
> Gentle ping on this. v3 addresses all your feedback from v2 (u16 for
> best, dropped exact default_mbps match, early break when min exceeds
> the requested rate).
> 
> Andy also had a look and had no requested changes.
> 
> Do you have any further comments, or is this good to go?

The patch looks good to me. I'll take it once we have rc1 in the media
tree.

-- 
Kind regards,

Sakari Ailus