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

[D. Manresa <dmanresa@gmail.com>]

Hi,

On Tue, Oct 27, 2026 at 10:26:37PM +0900, Antoine Pichot wrote:
> Could you please try the two patches below, the first alone and then
> both, with your 150-frame measurement?

Done. The swap fixes the link completely on this receiver. Details
below, but first a correction to my own numbers, because they are wrong
in the archive.

Correction to the dropped-buffer figures in my previous mail
------------------------------------------------------------

I read v4l2-ctl's output wrong. It prints a line roughly once a second,
and "dropped buffers: N" is the drops in that interval, not a running
total; the suffix is omitted entirely when there were none. The column I
posted was the last line of each run, so it was the drops in the final
second, not the drops in the run. Summed over the run the figures are
some fifty times larger. The CSI-2 error counts came from dmesg and are
unaffected.

With that fixed, 1/7 alone at 30 fps, three 150-frame runs:

  dropped (sum over run)   1181    388    926
  CSI-2 error lines        >=4996  >=1931 >=3785
  elapsed for 150 frames   47.0 s  18.8 s 38.4 s

A clean run of the same capture takes 5.2 s. Error totals remain lower
bounds: the printk is rate-limited and I add the "callbacks suppressed"
counts, but the suppressions of the last partial interval are never
printed.

The divider swap
----------------

Your 0001 removes the problem here. Every run: 150 of 150 frames in
5.20 to 5.24 s at 30.00 fps, zero dropped buffers, zero CSI-2 error
lines. Thirteen runs. Readback over i2c mid-stream, with 0x0100 = 0x01
confirming the sensor was streaming:

  build          30b0 30b1 30b3 30b4 30b5 4837  link_freq  pixel_rate
  unpatched      0x0a 0x01 0x4b 0x03 0x05 0x19  240000000  48000000
  1/7            0x0a 0x04 0x85 0x01 0x05 0x19  319200000  63840000
  1/7+0001       0x0a 0x01 0x85 0x04 0x05 0x19  319200000  63840000
  1/7+0001+0002  0x0a 0x01 0x85 0x04 0x05 0x1f  319200000  63840000

The only difference from 1/7 alone is the two dividers, with the link
frequency and pixel rate unchanged, so this is the VCO as you say and
not the link speed.

0002 adds nothing measurable: ten runs, the same perfect result as 0001
alone. 0x4837 = 0x1f is confirmed in the hardware, so that is a real
null and not an unapplied patch. It does no harm either.

Why I ran more than three of each
---------------------------------

Because on this machine each stream start is a lottery and three runs
cannot see it. A run is either perfectly clean or uniformly broken for
its whole duration, with nothing in between, and the unpatched driver
began failing later in the session too, so results taken in blocks are
confounded by drift.

So I interleaved: unpatched, 1/7, 1/7+0001, 1/7+0001+0002, one 150-frame
run of each per cycle, swapping the module every time, seven cycles.

  build            runs with any CSI-2 error   stalls
  unpatched                1 of 7                 1
  1/7                      4 of 7                 3
  1/7+0001                 0 of 7                 0
  1/7+0001+0002            0 of 7                 0

Pooled with the block runs: unpatched 3 bad of 13, 1/7 7 bad of 10,
1/7+0001 0 of 13, 1/7+0001+0002 0 of 10. The +0001 build was also
re-checked immediately after the worst runs of the session and was still
clean, so this is not simply a good patch of time.

One caveat for the record: this receiver intermittently fails to lock
even with a completely unpatched driver, 3 of 13 runs at 30 fps
including two 60-second stalls. The Pro 7+ has a baseline instability of
its own. 1/7 makes it far more likely; 0001 removed it entirely in this
sample.

So v2 with the PLL table fix before the index fix, both Cc: stable,
matches what I measure here. Send it and I will run it on this machine
while it is still with me.

2/7 at 60 and 90 fps, without 1/7
---------------------------------

The error count follows frame starts, not the link. The link is 240 MHz
/ 480 Mbps at all three rates (0x30b3 = 0x4b, 0x30b4 = 0x03 confirmed by
readback), and vblank follows the mode as 2/7 intends: 1244, 380, 92.

One module load, then 30, 60 and 90 fps interleaved, five cycles. That
block fell entirely in the bad regime, which is what makes the rates
comparable:

  rate    errors per frame start   errors per second   mean dropped/run
  30 fps          3.62                   104                 867
  60 fps          3.70                   204                 517
  90 fps          2.72                   222                 262

Per second the count doubles from 30 to 60 fps over an identical link.
Per frame start the three collapse onto each other, and 90 fps is about
25% lower per frame while carrying three times the payload of 30 fps.
The bad 1/7 runs at 319.2 MHz sit in the same range, 3.5 to 4.6 errors
per frame, so the per-frame cost of a bad lock is the same at both link
frequencies.

That reads as a failure to lock at stream start which then persists for
the whole stream, rather than anything accumulating with transported
bits. It is also why errors per run vary by two orders of magnitude
while errors per frame do not.

On the cover letter
-------------------

6/7: I would rather re-run it than have the tag carried across a rewrite
I did not test. Send me the v2 version and I will repeat the clamp sweep
and the photometry while the machine is still here. If v2 goes out
before that, carry it, since you say the result is unchanged, but I
would rather earn it.

Thank you for the pixel_rate explanation. I had taken 63840000 at face
value.

7/7: I will answer on the orientation in a separate mail.

D. Manresa

(Written with AI assistance, directed and reviewed by me.)