* [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