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

[PATCH] media: intel/ipu6: Fix 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).

Add a check after get_hsfreq_by_mbps() that shifts to the previous
band when it also covers the requested data rate and provides a higher
osc_freq_target, ensuring stable DDL lock at boundary data rates.

Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Bingbu Cao <bingbu.cao@intel.com>
Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
 .../media/pci/intel/ipu6/ipu6-isys-dwc-phy.c  | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

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 db28748..450e7a2 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c
@@ -317,6 +317,27 @@ static int ipu6_isys_dwc_phy_config(struct ipu6_isys *isys,
 		return -EINVAL;
 	}
 
+	/*
+	 * The overlapping frequency ranges combined with the backward
+	 * search in get_hsfreq_by_mbps() can select a band where the
+	 * data rate sits at the lower edge and osc_freq_target drops
+	 * (335 -> 208 at index 43). When the previous band also covers
+	 * this rate and has a higher osc_freq_target, prefer it for
+	 * better DDL lock stability. This fixes CRC errors seen at
+	 * 1498 Mbps (e.g. OV08X40 on Arrow Lake).
+	 */
+	if (index > 0 &&
+	    mbps >= freqranges[index - 1].min &&
+	    mbps <= freqranges[index - 1].max &&
+	    freqranges[index - 1].osc_freq_target >
+	    freqranges[index].osc_freq_target) {
+		dev_dbg(dev, "Dphy %u: shift band %u->%u for %u mbps (osc %u->%u)",
+			phy_id, index, index - 1, mbps,
+			freqranges[index].osc_freq_target,
+			freqranges[index - 1].osc_freq_target);
+		index--;
+	}
+
 	dwc_dphy_write_mask(isys, phy_id, IPU6_DWC_DPHY_HSFREQRANGE,
 			    freqranges[index].hsfreq, 0, 7);
 
-- 
2.47.3

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

[Sakari Ailus]

Hi Marco,

Thanks for the patch.

On Mon, Mar 23, 2026 at 04:40:37PM +0100, 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).
> 
> Add a check after get_hsfreq_by_mbps() that shifts to the previous
> band when it also covers the requested data rate and provides a higher
> osc_freq_target, ensuring stable DDL lock at boundary data rates.
> 
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Bingbu Cao <bingbu.cao@intel.com>
> Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
> ---
>  .../media/pci/intel/ipu6/ipu6-isys-dwc-phy.c  | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> 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 db28748..450e7a2 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c
> @@ -317,6 +317,27 @@ static int ipu6_isys_dwc_phy_config(struct ipu6_isys *isys,
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * The overlapping frequency ranges combined with the backward
> +	 * search in get_hsfreq_by_mbps() can select a band where the
> +	 * data rate sits at the lower edge and osc_freq_target drops
> +	 * (335 -> 208 at index 43). When the previous band also covers
> +	 * this rate and has a higher osc_freq_target, prefer it for
> +	 * better DDL lock stability. This fixes CRC errors seen at
> +	 * 1498 Mbps (e.g. OV08X40 on Arrow Lake).
> +	 */
> +	if (index > 0 &&
> +	    mbps >= freqranges[index - 1].min &&
> +	    mbps <= freqranges[index - 1].max &&
> +	    freqranges[index - 1].osc_freq_target >
> +	    freqranges[index].osc_freq_target) {
> +		dev_dbg(dev, "Dphy %u: shift band %u->%u for %u mbps (osc %u->%u)",
> +			phy_id, index, index - 1, mbps,
> +			freqranges[index].osc_freq_target,
> +			freqranges[index - 1].osc_freq_target);
> +		index--;
> +	}

Instead of adding this workaround, can we change the selection algorithm
instead? I'm not sure where the table comes from (datasheet somewhere
maybe?), but it looks like there indeed are many options for most
frequencies. The current algorithm appears just crude.

What I might do is to select either the exact (default_mbps) frequency and
if there's no exact match, then pick the one that has higher
osc_freq_target (while still satisfying min/max limits) or if the two are
the same, then the one that has closer default_freq.

I can write a patch, too, if you like.

> +
>  	dwc_dphy_write_mask(isys, phy_id, IPU6_DWC_DPHY_HSFREQRANGE,
>  			    freqranges[index].hsfreq, 0, 7);
>  

-- 
Kind regards,

Sakari Ailus

[PATCH v2] 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. Prefer an exact default_mbps match (returned immediately).
2. Among bands whose min/max range covers the data rate, prefer
   the one with the higher osc_freq_target.
3. If osc_freq_target is equal, prefer the band whose default_mbps
   is closest to 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
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Bingbu Cao <bingbu.cao@intel.com>
Signed-off-by: Marco Nenciarini <mnencia@kcore.it>
---
Changes in v2:
- Rewrote get_hsfreq_by_mbps() with a proper selection algorithm instead
  of patching after the call, as suggested by Sakari Ailus.
- Added Fixes tag and Cc stable.

 .../media/pci/intel/ipu6/ipu6-isys-dwc-phy.c  | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 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 db28748..4c9e50c 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;
+	int best = DPHY_FREQ_RANGE_INVALID_INDEX;
+	unsigned int i;
 
-	while (i--) {
-		if (freqranges[i].default_mbps == mbps ||
-		    (mbps >= freqranges[i].min && mbps <= freqranges[i].max))
+	for (i = 0; i < DPHY_FREQ_RANGE_NUM; i++) {
+		if (freqranges[i].default_mbps == mbps)
 			return i;
+
+		if (mbps < freqranges[i].min || mbps > freqranges[i].max)
+			continue;
+
+		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