All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Calliari <calliarifelipe@gmail.com>
To: sakari.ailus@linux.intel.com
Cc: bod@kernel.org, calliarifelipe@gmail.com, hansg@kernel.org,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	mchehab@kernel.org
Subject: Re: [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock
Date: Tue,  8 Sep 2026 23:55:05 -0300	[thread overview]
Message-ID: <20260909025505.89752-1-calliarifelipe@gmail.com> (raw)
In-Reply-To: <ap_CGQTdNFysTLot@kekkonen.localdomain>

Hi Sakari,

On Tue, Sep 08, 2026 at 11:06:49AM +0300, Sakari Ailus wrote:
> Please don't use a hard-coded value here. Instead, calculate the pixel
> rate.
>
> Registers 0x0304 and 0x0315 (both 16-bit) control the PLL multipliers for
> OP and VT PLLs, respectively. You could also change the multipliers to
> arrive in a frequency close to the previous configuration. The values would
> be 0x28a and 0x1b1, respectively. I don't have the sensor so I can't test
> this. The pixel rate would be a bit off, 400,307929 MHz, assuming the
> previous value was exactly 400 MHz. This would also require adding the
> frequency to the IPU bridge.

Thanks -- re-programming the multipliers is the right approach and it
does work, just not with those particular values. I tested both on the
affected hardware (Samsung Galaxy Book3, OV02C10 on a confirmed 26 MHz
external clock, verified via /sys/kernel/debug/clk/clk_summary:
INT3472:01-clk = 26000000).

With 0x0304/0x0305 = 0x028a and 0x0315/0x0316 = 0x01b1 (each pair
big-endian, low address = high byte, as elsewhere in this driver) the
sensor produces no output at all: a 3-frame v4l2-ctl --stream-mmap
capture hangs with 0 bytes written, no I2C errors are logged, and
intel_ipu6_isys logs "stream stop time out" / "stream close time out"
on teardown. Recovering the sensor needed an i2c unbind/rebind.

What does work is scaling the multipliers the driver already programs.
The mode tables set the OP multiplier to 0x0190 = 400 for a 19.2 MHz
clock: the common table writes 0x0304 = 0x01, and the per-lane tables
then override 0x0305 = 0x90. Scaling that by 19.2/26 gives
400 * 19.2 / 26 = 295.4, i.e. 0x0127, applied to both multipliers when
the external clock is 26 MHz -- written after the per-lane table in
enable_streams():

    {0x0304, 0x01},
    {0x0305, 0x27},
    {0x0315, 0x01},
    {0x0316, 0x27},

Measured on the Galaxy Book3, which runs the sensor on two CSI-2 data
lanes (0x3016 reads back 0x32):

 - Reading the registers back over i2c while streaming gives 0x0127 for
   both the OP and the VT multiplier.
 - The frame rate is 29.94 fps, timing a 400-frame capture against a
   100-frame one so that pipeline startup cancels out (13.71 s vs
   3.69 s). The same hardware ran at ~40 fps with the unmodified tables.
 - The exported controls agree with that: hblank 352 and vblank 1236
   make a 2280 x 2328 frame, which at pixel_rate 160000000 works out to
   30.14 fps against the 29.94 measured. link_frequency reads 400000000.
 - No stream stop/close timeouts.

295 * 26 / 19.2 = 399.5 MHz, i.e. within 0.13% of the nominal 400 MHz,
so the 400 MHz entry the IPU bridge already advertises stays accurate.
That means v2 can drop the second, hard-coded link-frequency entry
altogether: there is a single 400 MHz entry again,
v4l2_link_freq_to_bitmap() matches it directly with no index fixup, and
no IPU bridge change is needed. pixel_rate stays derived from the link
frequency as before.

I can't account for 0x28a from here: 650 * 26 / 19.2 = 880 MHz, which is
far outside the D-PHY range the receiver is configured for and would
explain the missing signal. 650 would only line up if the starting
multiplier were ~880, whereas the tables program 400 once the per-lane
0x0305 = 0x90 override is applied. If you meant a different baseline, or
a different register pairing, I'm happy to test that too.

Two things worth flagging. The mode tables never write 0x0315 (only
0x0316 = 0x90), so I can't say what the VT multiplier's high byte was
beforehand -- the driver now writes both bytes explicitly, and the
readback above is after that write. And the single CSI-2 "frame sync
error" logged at stream start is still there with the link back at
~400 MHz, so it isn't caused by the faster clock, as the v1 commit
message speculated.

Unless you'd rather have it done differently, I'll send a v2 of this
patch with the PLL re-programming above and the 541.667 MHz entry
dropped.

Thanks,
Felipe

  reply	other threads:[~2026-09-09  2:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <Tms-fUHfw0sPf1-YX_rgSNq7z1MNyvtfCImHXhxAUQAoTq-fwKvZF-p8a3ozCxAJ7kJCo-lKEU_JSkfUJy6oIA==@protonmail.internalid>
2026-09-05  3:07 ` [PATCH 1/3] media: ov02c10: Drop duplicate register write Felipe Calliari
2026-09-05  3:07   ` [PATCH 2/3] media: ov02c10: Implement get_selection Felipe Calliari
2026-09-08  9:07     ` Bryan O'Donoghue
2026-09-05  3:07   ` [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock Felipe Calliari
2026-09-08  8:06     ` Sakari Ailus
2026-09-09  2:55       ` Felipe Calliari [this message]
2026-09-08  9:21     ` Bryan O'Donoghue
2026-09-08  9:04   ` [PATCH 1/3] media: ov02c10: Drop duplicate register write Bryan O'Donoghue

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260909025505.89752-1-calliarifelipe@gmail.com \
    --to=calliarifelipe@gmail.com \
    --cc=bod@kernel.org \
    --cc=hansg@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.