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

[Sakari Ailus <sakari.ailus@linux.intel.com>]

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