mirror of https://lore.kernel.org/lkml/
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®