Re: [PATCH 1/7] media: i2c: ov7251: Fix link frequency index

[Dave Stevenson]

Hi Antoine and D. Manresa

On Tue, 27 Oct 2026 at 10:27, D. Manresa <dmanresa@xxxxxxxxx> wrote:
>
> On Mon, Oct 26, 2026 at 08:10:26PM +0900, Antoine Pichot wrote:
> > Observed on a Surface Book 2, where ipu-bridge lists one link frequency,
> > 319.2 MHz, which is link_freq[1]: V4L2_CID_LINK_FREQ reads 240000000,
> > and the pixel rate and PLL registers are off the same way.

I'll agree that the link frequency isn't abided by, and your patch
does fix that. However testing on a DT based system (Raspberry Pi)
with a 24MHz extclk I'm also seeing issues.

I'm working with the datasheet from
https://static6.arrow.com/aropdfconversion/dedba0a4f2b52e9b6587341a0e31cd115a2c3a46/ov7750_ov7251_csp3_rev1f_ds_2.12.pdf

As I read it, the pixelrate is configured via PLL2. Those settings are
identical between the 2 link frequencies, so the pixel rate should be
48000000 in both cases. Without that I was getting 75fps from
libcamera when I asked for 100fps. 100 * 480 / 638.4 = 75.1fps.
The settings for PLL2 with a 24MHz extclk exactly follow the sample
PLL configuration in table 2.10 of the datasheet. For 24MHz the
multiplier is 0x28 / 40dec vs 0x32 / 50dec for 19.2MHz, resulting in
the same rate.

The settings for PLL1 seem to be all over the shop if the datasheet is correct.

With a 24MHz extclk and measuring with an oscilloscope, the "240MHz
link freq" actually runs at 400MHz. The "319.2MHz link frequency" runs
at 454MHz.
If I follow the datasheet, then the 240MHz mode does fall out at 400MHz
For the 319.2MHz mode I reckon that the VCO is well out of spec -
2.128GHz when the limit is 800MHz.

The numbers for the 19.2MHz modes look sane in their ratios, but I
can't test them.
The 240MHz link freq does a prediv of 3, multiply by 75, and divider
of 1, which keeps things in range, and 19.2 / 3 * 75 / 1 = 480MPix/s,
or 240MHz.
The 319.2MHz link frequency uses a prediv of 1 and multiply by 0x85 /
133 gives a VCO frequency of 2.55GHz, when again the limit is 800MHz.
That then gets divided by 4 afterwards for a supposed result of
638.4MPix/s or 319.2MHz.

@D. Manresa it'd be worth trying using a prediv instead of (post)div:
static const struct ov7251_pll1_cfg ov7251_pll1_cfg_19_2_mhz_319_2_mhz = {
    .pre_div = 0x04,
    .mult = 0x85,
    .div = 0x01,
    .pix_div = 0x0a,
    .mipi_div = 0x05,
};

So I'll agree the patch is correct, but it'll show up other failures
that were currently masked. I'm not fussed about the 24MHz mode, but
it sounds like there are devices using the 19.2MHz / 319.2MHz mode.

(I've also noted that the driver allows V4L2_CID_VBLANK to go down to
1, when 40 or below seem to give corrupt images for me. Implementing
set_frame_interval for a raw sensor is also wrong, particularly when
V4L2_CID_VBLANK is also implemented. I'll see if I can find some time
to patch it up).

Hope that helps.
  Dave

> The diagnosis holds on a Surface Pro 7+ (IPU6 Tiger Lake, 8086:9a19).
> ipu-bridge gives this machine the same single frequency as yours:
>
>   IPU_SENSOR_CONFIG("INT347E", 1, 319200000)
>
> and unpatched, the sensor reports
>
>   link_frequency ... value=0 (240000000)
>   pixel_rate ... value=48000000
>
> exactly the mismatch you describe.
>
> Applying the patch makes this machine worse, though, and since it carries
> Cc: stable I think that is worth resolving before it goes in.
>
> The fix does not only correct a reported value: it reprograms the PLL and
> moves the link from 480 to 638.4 Mbps.
>
>                      unpatched v6.19     + this patch
>   link_frequency     0 (240000000)       1 (319200000)
>   pixel_rate         48000000            63840000
>   PLL1 0x30b1/b3/b4  0x01 / 0x4b / 0x03  0x04 / 0x85 / 0x01
>
> The IPU6 receiver here does not hold that link. 150-frame captures of the
> 640x480 30 fps mode, counting dropped buffers from v4l2-ctl and csi2-5
> error lines from dmesg (including the "callbacks suppressed" lines):
>
>   unpatched  run 1  dropped 23   csi2 errors ~7216
>   unpatched  run 2  dropped 23   csi2 errors 13277
>   unpatched  run 3  dropped 13   csi2 errors 4722
>   patched    run 1  dropped 106  csi2 errors 120788
>   patched    run 3  stalled: 50 frames, then none for 60 s
>
> The errors are DPHY fatal error, SOT sync error, inter-frame short/long
> packet discarded and frame sync error. The frames that do arrive look
> normal, so this is the link and not the image.
>
> Two things to be honest about. This unit is not a clean baseline: even
> unpatched, mainline already logs thousands of CSI-2 errors per run at
> 30 fps, and the out-of-tree Surface driver sidesteps the whole area with
> a vts_boost parameter -- at its production VTS of 3448, 15 fps, I measure
> zero CSI-2 errors. And the sample is small: three runs each, one unit.
> What makes me report it anyway is that the one-line difference was
> consistent in every run, that it introduced hard stalls I never saw
> unpatched, and that going back to a build with only that line reverted
> gave 568 consecutive frames with zero CSI-2 errors.
>
> One hypothesis tried and failed, so you do not repeat it: the out-of-tree
> Surface driver notes that the vendor runs the 319.2 MHz PLL with the MIPI
> divider at 0x01 rather than 0x05. Building this patch with mipi_div
> changed to 0x01 in the 19_2_mhz_319_2_mhz entry was worse, not better --
> 3 frames in about four minutes. So the divider alone is not the missing
> piece. The vendor path also uses VTS 522, which I did not try.
>
> So either ipu-bridge's advertised link frequency is wrong for this SKU,
> or mainline's 319.2 MHz PLL entry needs more than the index fix to be
> usable. I have the machine for a few more days and can run whatever would
> help decide which.
>
> The rest of the series tests well here; I have replied to the cover letter
> with that.
>
> D. Manresa
>
> (Written with AI assistance, directed and reviewed by me.)
>