* [PATCH v2] media: intel/ipu6: Improve DWC PHY HSFREQRANGE band selection for overlapping ranges
[not found] <20260323154037.1404865-1-mnencia@kcore.it>
@ 2026-03-25 9:32 ` Marco Nenciarini
2026-04-01 7:11 ` Marco Nenciarini
2026-04-01 11:28 ` Sakari Ailus
0 siblings, 2 replies; 4+ messages in thread
From: Marco Nenciarini @ 2026-03-25 9:32 UTC (permalink / raw)
To: linux-media
Cc: Sakari Ailus, Bingbu Cao, Tianshu Qiu, Mauro Carvalho Chehab,
stable, mnencia
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] media: intel/ipu6: Improve DWC PHY HSFREQRANGE band selection for overlapping ranges
2026-03-25 9:32 ` [PATCH v2] media: intel/ipu6: Improve DWC PHY HSFREQRANGE band selection for overlapping ranges Marco Nenciarini
@ 2026-04-01 7:11 ` Marco Nenciarini
2026-04-01 11:28 ` Sakari Ailus
1 sibling, 0 replies; 4+ messages in thread
From: Marco Nenciarini @ 2026-04-01 7:11 UTC (permalink / raw)
To: linux-media; +Cc: sakari.ailus, bingbu.cao, tian.shu.qiu, mchehab, stable
Hi Sakari,
Gentle ping on this patch. Any feedback on v2?
Thanks,
Marco
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] media: intel/ipu6: Improve DWC PHY HSFREQRANGE band selection for overlapping ranges
2026-03-25 9:32 ` [PATCH v2] media: intel/ipu6: Improve DWC PHY HSFREQRANGE band selection for overlapping ranges Marco Nenciarini
2026-04-01 7:11 ` Marco Nenciarini
@ 2026-04-01 11:28 ` Sakari Ailus
2026-04-01 16:25 ` Marco Nenciarini
1 sibling, 1 reply; 4+ messages in thread
From: Sakari Ailus @ 2026-04-01 11:28 UTC (permalink / raw)
To: Marco Nenciarini
Cc: linux-media, Bingbu Cao, Tianshu Qiu, Mauro Carvalho Chehab,
stable
Hi Marco,
Thanks for the update.
On Wed, Mar 25, 2026 at 10:32:41AM +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).
>
> 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;
The function returns u16 and u16 is fine here, too...
> + 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;
Doesn't the condition below handle this already?
> +
> + if (mbps < freqranges[i].min || mbps > freqranges[i].max)
> + continue;
The frequencies in the array are consistently increasing so you can replace
this with:
if (mbps < freqranges[i].min)
continue;
if (mbps > freqranges[i].max)
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,
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] media: intel/ipu6: Improve DWC PHY HSFREQRANGE band selection for overlapping ranges
2026-04-01 11:28 ` Sakari Ailus
@ 2026-04-01 16:25 ` Marco Nenciarini
0 siblings, 0 replies; 4+ messages in thread
From: Marco Nenciarini @ 2026-04-01 16:25 UTC (permalink / raw)
To: linux-media, sakari.ailus
Cc: bingbu.cao, tian.shu.qiu, mchehab, stable, andriy.shevchenko
Hi Sakari,
Thanks for the review. All three points addressed in v3.
On the exact default_mbps match: yes, the range check covers it
since default_mbps always falls within [min, max] for every entry,
so the early return was redundant.
On the early exit: I adopted your suggestion to use separate
continue/break, but swapped the two. Since both min and max
increase monotonically through the table, mbps > max needs to
continue (later entries have higher max) while mbps < min can
break (no later entry can match).
Marco
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-01 16:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260323154037.1404865-1-mnencia@kcore.it>
2026-03-25 9:32 ` [PATCH v2] media: intel/ipu6: Improve DWC PHY HSFREQRANGE band selection for overlapping ranges Marco Nenciarini
2026-04-01 7:11 ` Marco Nenciarini
2026-04-01 11:28 ` Sakari Ailus
2026-04-01 16:25 ` Marco Nenciarini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox