* [PATCH 1/3] media: ov02c10: Drop duplicate register write @ 2026-09-05 3:07 ` Felipe Calliari 2026-09-05 3:07 ` [PATCH 2/3] media: ov02c10: Implement get_selection Felipe Calliari ` (3 more replies) 0 siblings, 4 replies; 14+ messages in thread From: Felipe Calliari @ 2026-09-05 3:07 UTC (permalink / raw) To: linux-media Cc: Sakari Ailus, Hans de Goede, Bryan O'Donoghue, Mauro Carvalho Chehab, linux-kernel, Felipe Calliari sensor_1928x1092_30fps_setting[] writes {0x395f, 0x00} twice in a row. The second write is redundant; drop it. No functional change. Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com> --- drivers/media/i2c/ov02c10.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c index cf93d3603..6220461fd 100644 --- a/drivers/media/i2c/ov02c10.c +++ b/drivers/media/i2c/ov02c10.c @@ -209,7 +209,6 @@ static const struct reg_sequence sensor_1928x1092_30fps_setting[] = { {0x395d, 0x05}, {0x395e, 0x02}, {0x395f, 0x00}, - {0x395f, 0x00}, {0x3960, 0x00}, {0x3961, 0x00}, {0x3962, 0x00}, -- 2.55.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] media: ov02c10: Implement get_selection 2026-09-05 3:07 ` [PATCH 1/3] media: ov02c10: Drop duplicate register write Felipe Calliari @ 2026-09-05 3:07 ` 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 ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Felipe Calliari @ 2026-09-05 3:07 UTC (permalink / raw) To: linux-media Cc: Sakari Ailus, Hans de Goede, Bryan O'Donoghue, Mauro Carvalho Chehab, linux-kernel, Felipe Calliari The driver does not implement .get_selection, so userspace cannot query the sensor's native size or active crop rectangle. libcamera reports "Unable to get rectangle N on pad 0/0: Inappropriate ioctl for device" and "The sensor kernel driver needs to be fixed" (see Documentation/sensor_driver_requirements.rst). Implement .get_selection returning the fixed geometry of the sensor: - V4L2_SEL_TGT_NATIVE_SIZE / V4L2_SEL_TGT_CROP_BOUNDS: the full 1928x1092 pixel array. - V4L2_SEL_TGT_CROP / V4L2_SEL_TGT_CROP_DEFAULT: the 1920x1080 active area, offset by (4, 6), matching the readout window programmed in sensor_1928x1092_30fps_setting[] (registers 0x3800-0x3807). The crop is fixed, so sd_state is not consulted. Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com> --- drivers/media/i2c/ov02c10.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c index 6220461fd..114db38c0 100644 --- a/drivers/media/i2c/ov02c10.c +++ b/drivers/media/i2c/ov02c10.c @@ -10,6 +10,7 @@ #include <linux/pm_runtime.h> #include <linux/regmap.h> #include <media/v4l2-cci.h> +#include <media/v4l2-common.h> #include <media/v4l2-ctrls.h> #include <media/v4l2-device.h> #include <media/v4l2-fwnode.h> @@ -18,6 +19,13 @@ #define OV02C10_MCLK 19200000 #define OV02C10_RGB_DEPTH 10 +#define OV02C10_NATIVE_WIDTH 1928 +#define OV02C10_NATIVE_HEIGHT 1092 +#define OV02C10_ACTIVE_WIDTH 1920 +#define OV02C10_ACTIVE_HEIGHT 1080 +#define OV02C10_ACTIVE_LEFT 4 +#define OV02C10_ACTIVE_TOP 6 + #define OV02C10_REG_CHIP_ID CCI_REG16(0x300a) #define OV02C10_CHIP_ID 0x5602 @@ -767,11 +775,36 @@ static const struct v4l2_subdev_video_ops ov02c10_video_ops = { .s_stream = v4l2_subdev_s_stream_helper, }; +static int ov02c10_get_selection(struct v4l2_subdev *sd, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_selection *sel) +{ + switch (sel->target) { + case V4L2_SEL_TGT_NATIVE_SIZE: + case V4L2_SEL_TGT_CROP_BOUNDS: + sel->r.top = 0; + sel->r.left = 0; + sel->r.width = OV02C10_NATIVE_WIDTH; + sel->r.height = OV02C10_NATIVE_HEIGHT; + return 0; + case V4L2_SEL_TGT_CROP: + case V4L2_SEL_TGT_CROP_DEFAULT: + sel->r.top = OV02C10_ACTIVE_TOP; + sel->r.left = OV02C10_ACTIVE_LEFT; + sel->r.width = OV02C10_ACTIVE_WIDTH; + sel->r.height = OV02C10_ACTIVE_HEIGHT; + return 0; + } + + return -EINVAL; +} + static const struct v4l2_subdev_pad_ops ov02c10_pad_ops = { .set_fmt = ov02c10_set_format, .get_fmt = v4l2_subdev_get_fmt, .enum_mbus_code = ov02c10_enum_mbus_code, .enum_frame_size = ov02c10_enum_frame_size, + .get_selection = ov02c10_get_selection, .enable_streams = ov02c10_enable_streams, .disable_streams = ov02c10_disable_streams, }; -- 2.55.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] media: ov02c10: Implement get_selection 2026-09-05 3:07 ` [PATCH 2/3] media: ov02c10: Implement get_selection Felipe Calliari @ 2026-09-08 9:07 ` Bryan O'Donoghue 0 siblings, 0 replies; 14+ messages in thread From: Bryan O'Donoghue @ 2026-09-08 9:07 UTC (permalink / raw) To: Felipe Calliari, linux-media Cc: Sakari Ailus, Hans de Goede, Mauro Carvalho Chehab, linux-kernel On 05/09/2026 04:07, Felipe Calliari wrote: > The driver does not implement .get_selection, so userspace cannot query > the sensor's native size or active crop rectangle. libcamera reports > "Unable to get rectangle N on pad 0/0: Inappropriate ioctl for device" > and "The sensor kernel driver needs to be fixed" (see > Documentation/sensor_driver_requirements.rst). > > Implement .get_selection returning the fixed geometry of the sensor: > > - V4L2_SEL_TGT_NATIVE_SIZE / V4L2_SEL_TGT_CROP_BOUNDS: the full > 1928x1092 pixel array. > - V4L2_SEL_TGT_CROP / V4L2_SEL_TGT_CROP_DEFAULT: the 1920x1080 active > area, offset by (4, 6), matching the readout window programmed in > sensor_1928x1092_30fps_setting[] (registers 0x3800-0x3807). > > The crop is fixed, so sd_state is not consulted. > > Signed-off-by: Felipe Calliari<calliarifelipe@gmail.com> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock 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-05 3:07 ` Felipe Calliari 2026-09-08 8:06 ` Sakari Ailus ` (2 more replies) 2026-09-08 9:04 ` [PATCH 1/3] media: ov02c10: Drop duplicate register write Bryan O'Donoghue 2026-09-23 14:42 ` [PATCH v2 0/3] media: ov02c10: get_selection and 26 MHz clock support Felipe Calliari 3 siblings, 3 replies; 14+ messages in thread From: Felipe Calliari @ 2026-09-05 3:07 UTC (permalink / raw) To: linux-media Cc: Sakari Ailus, Hans de Goede, Bryan O'Donoghue, Mauro Carvalho Chehab, linux-kernel, Felipe Calliari Several Meteor Lake / Lunar Lake designs (e.g. the Samsung Galaxy Book3/4 series) wire the OV02C10 to a 26 MHz external clock instead of the 19.2 MHz assumed so far. The IPU6 ipu-bridge forwards the rate from the ACPI SSDB verbatim as the "clock-frequency" property, so probe() just rejects it today: ov02c10 i2c-OVTI02C1:00: external clock 26000000 is not supported Rename OV02C10_MCLK to OV02C10_MCLK_19_2MHZ, add OV02C10_MCLK_26MHZ and accept both. The PLL register tables are the 19.2 MHz ones; OmniVision's 26 MHz PLL programming is not publicly available. With a 26 MHz input the same dividers make every internal clock, and therefore the MIPI link, run 26/19.2 = 1.3542x faster: a ~541.7 MHz link and ~40 fps instead of the nominal 400 MHz / 30 fps. Rather than leave link-frequency and pixel-rate describing the 19.2 MHz case, add a second V4L2_CID_LINK_FREQ menu entry (400 MHz * 26 / 19.2) and select it when the external clock is 26 MHz. pixel-rate is derived from the link frequency and scales with it, so the frame rate and exposure times reported to userspace match the hardware, and the IPU6 CSI-2 receiver programs its D-PHY high-speed frequency range and bandwidth budget for the rate the sensor actually transmits. The ipu-bridge fwnode only lists the nominal 400 MHz link frequency (keyed by ACPI HID, not by clock rate), so v4l2_link_freq_to_bitmap() still matches on the 400 MHz entry and the 541.7 MHz index is selected explicitly for the 26 MHz case. On a Meteor Lake test machine a single CSI-2 "frame sync error" may still be logged by the IPU6 receiver at stream start, after which capture runs cleanly; this looks like a sensor PLL settling transient and is not addressed here. While touching the clock check, terminate its error string with a newline. Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com> --- drivers/media/i2c/ov02c10.c | 45 ++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c index 114db38c0..5184eb1cc 100644 --- a/drivers/media/i2c/ov02c10.c +++ b/drivers/media/i2c/ov02c10.c @@ -16,7 +16,16 @@ #include <media/v4l2-fwnode.h> #define OV02C10_LINK_FREQ_400MHZ 400000000ULL -#define OV02C10_MCLK 19200000 +/* + * The PLL register tables target a 19.2 MHz input clock. On boards that + * clock the sensor at 26 MHz the same dividers yield a 26/19.2 = 1.3542x + * faster MIPI link (and frame rate). OmniVision's 26 MHz PLL values are + * not public, so rather than re-normalise the link the driver advertises + * the real, scaled link frequency: 400 MHz * 26 / 19.2 = 541.667 MHz. + */ +#define OV02C10_LINK_FREQ_541MHZ 541666667ULL +#define OV02C10_MCLK_19_2MHZ 19200000 +#define OV02C10_MCLK_26MHZ 26000000 #define OV02C10_RGB_DEPTH 10 #define OV02C10_NATIVE_WIDTH 1928 @@ -345,8 +354,14 @@ static const char * const ov02c10_test_pattern_menu[] = { "Color Bar type 4", }; +enum { + OV02C10_LINK_FREQ_400MHZ_IDX, /* 19.2 MHz external clock */ + OV02C10_LINK_FREQ_541MHZ_IDX, /* 26 MHz external clock */ +}; + static const s64 link_freq_menu_items[] = { - OV02C10_LINK_FREQ_400MHZ, + [OV02C10_LINK_FREQ_400MHZ_IDX] = OV02C10_LINK_FREQ_400MHZ, + [OV02C10_LINK_FREQ_541MHZ_IDX] = OV02C10_LINK_FREQ_541MHZ, }; static const struct ov02c10_mode supported_modes[] = { @@ -396,6 +411,9 @@ struct ov02c10 { /* MIPI lane info */ u32 link_freq_index; u8 mipi_lanes; + + /* External (sensor) clock rate, Hz */ + u32 xvclk_freq; }; static inline struct ov02c10 *to_ov02c10(struct v4l2_subdev *subdev) @@ -507,7 +525,8 @@ static int ov02c10_init_controls(struct ov02c10 *ov02c10) ov02c10->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr, &ov02c10_ctrl_ops, V4L2_CID_LINK_FREQ, - ov02c10->link_freq_index, 0, + ov02c10->link_freq_index, + ov02c10->link_freq_index, link_freq_menu_items); if (ov02c10->link_freq) ov02c10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; @@ -875,6 +894,21 @@ static int ov02c10_check_hwcfg(struct ov02c10 *ov02c10) /* v4l2_link_freq_to_bitmap() guarantees at least 1 bit is set */ ov02c10->link_freq_index = ffs(link_freq_bitmap) - 1; + /* + * The IPU6 ipu-bridge always describes the nominal 19.2 MHz link + * (400 MHz) in the fwnode, keyed by ACPI HID, even on boards that + * clock the sensor at 26 MHz. There the real link frequency is + * 26/19.2 higher; advertise it so the CSI-2 receiver programs its + * D-PHY frequency band and bandwidth budget for the rate the sensor + * actually transmits. + */ + if (ov02c10->xvclk_freq == OV02C10_MCLK_26MHZ) + ov02c10->link_freq_index = OV02C10_LINK_FREQ_541MHZ_IDX; + + dev_dbg(dev, "%u Hz external clock, link freq %lld Hz\n", + ov02c10->xvclk_freq, + link_freq_menu_items[ov02c10->link_freq_index]); + if (bus_cfg.bus.mipi_csi2.num_data_lanes != 1 && bus_cfg.bus.mipi_csi2.num_data_lanes != 2) { ret = dev_err_probe(dev, -EINVAL, @@ -924,10 +958,11 @@ static int ov02c10_probe(struct i2c_client *client) "failed to get imaging clock\n"); freq = clk_get_rate(ov02c10->img_clk); - if (freq != OV02C10_MCLK) + if (freq != OV02C10_MCLK_19_2MHZ && freq != OV02C10_MCLK_26MHZ) return dev_err_probe(ov02c10->dev, -EINVAL, - "external clock %lu is not supported", + "external clock %lu is not supported\n", freq); + ov02c10->xvclk_freq = freq; v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops); -- 2.55.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock 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 2026-09-08 9:21 ` Bryan O'Donoghue 2026-09-22 6:35 ` Tomas Moro 2 siblings, 1 reply; 14+ messages in thread From: Sakari Ailus @ 2026-09-08 8:06 UTC (permalink / raw) To: Felipe Calliari Cc: linux-media, Hans de Goede, Bryan O'Donoghue, Mauro Carvalho Chehab, linux-kernel Hi Felipe, On Sat, Sep 05, 2026 at 12:07:32AM -0300, Felipe Calliari wrote: > Several Meteor Lake / Lunar Lake designs (e.g. the Samsung Galaxy Book3/4 > series) wire the OV02C10 to a 26 MHz external clock instead of the > 19.2 MHz assumed so far. The IPU6 ipu-bridge forwards the rate from the > ACPI SSDB verbatim as the "clock-frequency" property, so probe() just > rejects it today: > > ov02c10 i2c-OVTI02C1:00: external clock 26000000 is not supported > > Rename OV02C10_MCLK to OV02C10_MCLK_19_2MHZ, add OV02C10_MCLK_26MHZ and > accept both. > > The PLL register tables are the 19.2 MHz ones; OmniVision's 26 MHz PLL > programming is not publicly available. With a 26 MHz input the same > dividers make every internal clock, and therefore the MIPI link, run > 26/19.2 = 1.3542x faster: a ~541.7 MHz link and ~40 fps instead of the > nominal 400 MHz / 30 fps. Rather than leave link-frequency and > pixel-rate describing the 19.2 MHz case, add a second > V4L2_CID_LINK_FREQ menu entry (400 MHz * 26 / 19.2) and select it when > the external clock is 26 MHz. pixel-rate is derived from the link > frequency and scales with it, so the frame rate and exposure times > reported to userspace match the hardware, and the IPU6 CSI-2 receiver > programs its D-PHY high-speed frequency range and bandwidth budget for > the rate the sensor actually transmits. > > The ipu-bridge fwnode only lists the nominal 400 MHz link frequency > (keyed by ACPI HID, not by clock rate), so v4l2_link_freq_to_bitmap() > still matches on the 400 MHz entry and the 541.7 MHz index is selected > explicitly for the 26 MHz case. 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. Either the pixel rate or the link frequency exported by the driver is probably wrong. -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock 2026-09-08 8:06 ` Sakari Ailus @ 2026-09-09 2:55 ` Felipe Calliari 0 siblings, 0 replies; 14+ messages in thread From: Felipe Calliari @ 2026-09-09 2:55 UTC (permalink / raw) To: sakari.ailus Cc: bod, calliarifelipe, hansg, linux-kernel, linux-media, mchehab 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock 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-08 9:21 ` Bryan O'Donoghue 2026-09-22 6:35 ` Tomas Moro 2 siblings, 0 replies; 14+ messages in thread From: Bryan O'Donoghue @ 2026-09-08 9:21 UTC (permalink / raw) To: Felipe Calliari, linux-media Cc: Sakari Ailus, Hans de Goede, Mauro Carvalho Chehab, linux-kernel On 05/09/2026 04:07, Felipe Calliari wrote: > Several Meteor Lake / Lunar Lake designs (e.g. the Samsung Galaxy Book3/4 > series) wire the OV02C10 to a 26 MHz external clock instead of the > 19.2 MHz assumed so far. The IPU6 ipu-bridge forwards the rate from the > ACPI SSDB verbatim as the "clock-frequency" property, so probe() just > rejects it today: > > ov02c10 i2c-OVTI02C1:00: external clock 26000000 is not supported > > Rename OV02C10_MCLK to OV02C10_MCLK_19_2MHZ, add OV02C10_MCLK_26MHZ and > accept both. > > The PLL register tables are the 19.2 MHz ones; OmniVision's 26 MHz PLL > programming is not publicly available. With a 26 MHz input the same > dividers make every internal clock, and therefore the MIPI link, run > 26/19.2 = 1.3542x faster: a ~541.7 MHz link and ~40 fps instead of the > nominal 400 MHz / 30 fps. Rather than leave link-frequency and > pixel-rate describing the 19.2 MHz case, add a second > V4L2_CID_LINK_FREQ menu entry (400 MHz * 26 / 19.2) and select it when > the external clock is 26 MHz. pixel-rate is derived from the link > frequency and scales with it, so the frame rate and exposure times > reported to userspace match the hardware, and the IPU6 CSI-2 receiver > programs its D-PHY high-speed frequency range and bandwidth budget for > the rate the sensor actually transmits. > > The ipu-bridge fwnode only lists the nominal 400 MHz link frequency > (keyed by ACPI HID, not by clock rate), so v4l2_link_freq_to_bitmap() > still matches on the 400 MHz entry and the 541.7 MHz index is selected > explicitly for the 26 MHz case. > > On a Meteor Lake test machine a single CSI-2 "frame sync error" may > still be logged by the IPU6 receiver at stream start, after which > capture runs cleanly; this looks like a sensor PLL settling transient > and is not addressed here. > > While touching the clock check, terminate its error string with a > newline. > > Signed-off-by: Felipe Calliari<calliarifelipe@gmail.com> I don't see how this gets handled with Intel's downstream work https://github.com/intel/ipu6-drivers/blob/master/drivers/media/i2c/ov02c10.c So perhaps it just isn't handled. Seems a bit odd that several ODMs would use the 26MHz clock but there is no update to the ipu sensors drivers to match. Absent the real init sequences for a 26 MHz xvclk, this solution is better than nothing. Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock 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-08 9:21 ` Bryan O'Donoghue @ 2026-09-22 6:35 ` Tomas Moro 2 siblings, 0 replies; 14+ messages in thread From: Tomas Moro @ 2026-09-22 6:35 UTC (permalink / raw) To: calliarifelipe Cc: bod, hansg, linux-kernel, linux-media, mchehab, sakari.ailus, Mars-Wave Hi Felipe, First of all, thank you for the good work on this. I tested this series (v1, all three patches) on a Samsung Galaxy Book3 Pro, whose board feeds the sensor 26 MHz. Without the series the sensor does not probe: ov02c10 i2c-OVTI02C1:00: error -EINVAL: external clock 26000000 is not supported ov02c10 i2c-OVTI02C1:00: probe with driver ov02c10 failed with error -22 Hardware DMI: SAMSUNG ELECTRONICS CO., LTD. / 960XFG / NP960XFG-KC2IT BIOS P07RGU.330.240529.ZQ, i7-1360P, IPU6 [8086:a75d] sensor ACPI HID OVTI02C1, clock 26 MHz (clk_summary) Software 7.2.6 (linux-cachyos, clang + ThinLTO). ov02c10.c there is identical to v7.2.6 stable, the three patches applied. Only ov02c10 was rebuilt (out of tree). In-tree intel_ipu6 + intel_ipu6_isys. libcamera 0.7.2, simple pipeline, GPU soft ISP. Results probe OK controls link_frequency 541666667, pixel_rate 216666666, 2 lanes; get_selection bounds 1928x1092, crop 1920x1080 streaming 1920x1092 at 40.70 fps, steady, no drops over several 400-frame runs. That is ~35 % above 30 fps, which matches the link frequency. Raw SGRBG10 1928x1092 capture works too. stream start one "csi2-0 error: Frame sync error" every time first buffer the first dequeued buffer is always empty orientation image is rotated 180 degrees. This model is not in the ipu-bridge upside-down DMI list; strings above. s2idle 7 cycles (rtcwake -m freeze), idle and right after streaming: all resumed, sensor still bound, capture works. s2idle while refused by isys, not a hang becuse the running stream is streaming wedged afterwards, a new open works: intel_ipu6_isys.isys intel_ipu6.isys.40: PM: dpm_run_callback(): isys_suspend [intel_ipu6_isys] returns -16 intel_ipu6_isys.isys intel_ipu6.isys.40: PM: failed to suspend: error -16 PM: Some devices failed to suspend, or early wake event detected PM: suspend exit intel_ipu6_isys.isys intel_ipu6.isys.40: stream stop time out intel_ipu6_isys.isys intel_ipu6.isys.40: stream close time out Same tests with out-of-tree intel-ipu6-psys loaded and bound (intel/ipu6-drivers 71bddb5, built unmodified against this kernel): identical results, 4 more s2idle cycles all resumed. The s2idle hang from intel/ipu6-drivers#381 does not reproduce here on 7.2.6. Footnote, unrelated to this series: I saw one hard lockup (no trace) on the first stream after rmmod of intel-ipu6-psys. I think that taking it out might have given me a corrupt state, had to reboot. Happy to test v2. Tested-by: Mars-Wave <tmorolias@gmail.com> # Samsung Galaxy Book3 Pro NP960XFG (github.com/MarsWave) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] media: ov02c10: Drop duplicate register write 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-05 3:07 ` [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock Felipe Calliari @ 2026-09-08 9:04 ` Bryan O'Donoghue 2026-09-23 14:42 ` [PATCH v2 0/3] media: ov02c10: get_selection and 26 MHz clock support Felipe Calliari 3 siblings, 0 replies; 14+ messages in thread From: Bryan O'Donoghue @ 2026-09-08 9:04 UTC (permalink / raw) To: Felipe Calliari, linux-media Cc: Sakari Ailus, Hans de Goede, Mauro Carvalho Chehab, linux-kernel On 05/09/2026 04:07, Felipe Calliari wrote: > sensor_1928x1092_30fps_setting[] writes {0x395f, 0x00} twice in a row. > The second write is redundant; drop it. No functional change. > > Signed-off-by: Felipe Calliari<calliarifelipe@gmail.com> > --- > drivers/media/i2c/ov02c10.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c > index cf93d3603..6220461fd 100644 > --- a/drivers/media/i2c/ov02c10.c > +++ b/drivers/media/i2c/ov02c10.c > @@ -209,7 +209,6 @@ static const struct reg_sequence sensor_1928x1092_30fps_setting[] = { > {0x395d, 0x05}, > {0x395e, 0x02}, > {0x395f, 0x00}, > - {0x395f, 0x00}, Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 0/3] media: ov02c10: get_selection and 26 MHz clock support 2026-09-05 3:07 ` [PATCH 1/3] media: ov02c10: Drop duplicate register write Felipe Calliari ` (2 preceding siblings ...) 2026-09-08 9:04 ` [PATCH 1/3] media: ov02c10: Drop duplicate register write Bryan O'Donoghue @ 2026-09-23 14:42 ` Felipe Calliari 2026-09-23 14:42 ` [PATCH v2 1/3] media: ov02c10: Drop duplicate register write Felipe Calliari ` (2 more replies) 3 siblings, 3 replies; 14+ messages in thread From: Felipe Calliari @ 2026-09-23 14:42 UTC (permalink / raw) To: linux-media Cc: Sakari Ailus, Hans de Goede, Bryan O'Donoghue, Mauro Carvalho Chehab, Tomas Moro, linux-kernel, Felipe Calliari Some Meteor Lake / Lunar Lake laptops (e.g. the Samsung Galaxy Book3/4 series) wire the OV02C10 to a 26 MHz external clock, which the driver rejects at probe today. This series adds support for it, together with two small changes found along the way. Patch 1 drops a duplicated register write, patch 2 implements .get_selection so libcamera can query the sensor geometry, and patch 3 accepts a 26 MHz clock and re-programs the OP and VT PLL multipliers so that the MIPI link stays at the nominal 400 MHz. Tested on a Samsung Galaxy Book3 (two CSI-2 lanes, 26 MHz clock): capture runs at a steady 30.01 fps by buffer timestamp, against ~40 fps with the unmodified register tables. v1 was also tested on a Galaxy Book3 Pro; that Tested-by is carried on patches 1 and 2 only, since patch 3 was rewritten. Changes in v2: - Rebased onto media next; patch 2 takes the new const struct v4l2_subdev_client_info * argument of .get_selection. - Patch 3: re-program the PLL multipliers instead of advertising a scaled 541.667 MHz link frequency (Sakari), and drop the second V4L2_CID_LINK_FREQ entry. No ipu-bridge change is needed any more. v1: https://lore.kernel.org/linux-media/20260905030732.39196-1-calliarifelipe@gmail.com/ Felipe Calliari (3): media: ov02c10: Drop duplicate register write media: ov02c10: Implement get_selection media: ov02c10: Accept a 26 MHz external clock drivers/media/i2c/ov02c10.c | 78 +++++++++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 4 deletions(-) base-commit: 0c795849fd70d6bdb99aea5dd08d9b43128612f6 -- 2.55.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] media: ov02c10: Drop duplicate register write 2026-09-23 14:42 ` [PATCH v2 0/3] media: ov02c10: get_selection and 26 MHz clock support Felipe Calliari @ 2026-09-23 14:42 ` Felipe Calliari 2026-09-23 14:42 ` [PATCH v2 2/3] media: ov02c10: Implement get_selection Felipe Calliari 2026-09-23 14:42 ` [PATCH v2 3/3] media: ov02c10: Accept a 26 MHz external clock Felipe Calliari 2 siblings, 0 replies; 14+ messages in thread From: Felipe Calliari @ 2026-09-23 14:42 UTC (permalink / raw) To: linux-media Cc: Sakari Ailus, Hans de Goede, Bryan O'Donoghue, Mauro Carvalho Chehab, Tomas Moro, linux-kernel, Felipe Calliari, Bryan O'Donoghue sensor_1928x1092_30fps_setting[] writes {0x395f, 0x00} twice in a row. The second write is redundant; drop it. No functional change. Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Tested-by: Mars-Wave <tmorolias@gmail.com> # Samsung Galaxy Book3 Pro NP960XFG (github.com/MarsWave) --- Changes in v2: - No change; picked up the Reviewed-by and Tested-by tags. drivers/media/i2c/ov02c10.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c index d622f5dca..730a40e94 100644 --- a/drivers/media/i2c/ov02c10.c +++ b/drivers/media/i2c/ov02c10.c @@ -209,7 +209,6 @@ static const struct reg_sequence sensor_1928x1092_30fps_setting[] = { {0x395d, 0x05}, {0x395e, 0x02}, {0x395f, 0x00}, - {0x395f, 0x00}, {0x3960, 0x00}, {0x3961, 0x00}, {0x3962, 0x00}, -- 2.55.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] media: ov02c10: Implement get_selection 2026-09-23 14:42 ` [PATCH v2 0/3] media: ov02c10: get_selection and 26 MHz clock support Felipe Calliari 2026-09-23 14:42 ` [PATCH v2 1/3] media: ov02c10: Drop duplicate register write Felipe Calliari @ 2026-09-23 14:42 ` Felipe Calliari 2026-09-23 14:42 ` [PATCH v2 3/3] media: ov02c10: Accept a 26 MHz external clock Felipe Calliari 2 siblings, 0 replies; 14+ messages in thread From: Felipe Calliari @ 2026-09-23 14:42 UTC (permalink / raw) To: linux-media Cc: Sakari Ailus, Hans de Goede, Bryan O'Donoghue, Mauro Carvalho Chehab, Tomas Moro, linux-kernel, Felipe Calliari, Bryan O'Donoghue The driver does not implement .get_selection, so userspace cannot query the sensor's native size or active crop rectangle. libcamera reports "Unable to get rectangle N on pad 0/0: Inappropriate ioctl for device" and "The sensor kernel driver needs to be fixed" (see Documentation/sensor_driver_requirements.rst). Implement .get_selection returning the fixed geometry of the sensor: - V4L2_SEL_TGT_NATIVE_SIZE / V4L2_SEL_TGT_CROP_BOUNDS: the full 1928x1092 pixel array. - V4L2_SEL_TGT_CROP / V4L2_SEL_TGT_CROP_DEFAULT: the 1920x1080 active area, offset by (4, 6), matching the readout window programmed in sensor_1928x1092_30fps_setting[] (registers 0x3800-0x3807). The crop is fixed, so sd_state is not consulted. Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Tested-by: Mars-Wave <tmorolias@gmail.com> # Samsung Galaxy Book3 Pro NP960XFG (github.com/MarsWave) --- Changes in v2: - Add the const struct v4l2_subdev_client_info * argument that .get_selection now takes in media next. - Picked up the Reviewed-by and Tested-by tags. drivers/media/i2c/ov02c10.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c index 730a40e94..cdccbdef3 100644 --- a/drivers/media/i2c/ov02c10.c +++ b/drivers/media/i2c/ov02c10.c @@ -10,6 +10,7 @@ #include <linux/pm_runtime.h> #include <linux/regmap.h> #include <media/v4l2-cci.h> +#include <media/v4l2-common.h> #include <media/v4l2-ctrls.h> #include <media/v4l2-device.h> #include <media/v4l2-fwnode.h> @@ -18,6 +19,13 @@ #define OV02C10_MCLK 19200000 #define OV02C10_RGB_DEPTH 10 +#define OV02C10_NATIVE_WIDTH 1928 +#define OV02C10_NATIVE_HEIGHT 1092 +#define OV02C10_ACTIVE_WIDTH 1920 +#define OV02C10_ACTIVE_HEIGHT 1080 +#define OV02C10_ACTIVE_LEFT 4 +#define OV02C10_ACTIVE_TOP 6 + #define OV02C10_REG_CHIP_ID CCI_REG16(0x300a) #define OV02C10_CHIP_ID 0x5602 @@ -768,11 +776,37 @@ static const struct v4l2_subdev_video_ops ov02c10_video_ops = { .s_stream = v4l2_subdev_s_stream_helper, }; +static int ov02c10_get_selection(struct v4l2_subdev *sd, + const struct v4l2_subdev_client_info *ci, + struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_selection *sel) +{ + switch (sel->target) { + case V4L2_SEL_TGT_NATIVE_SIZE: + case V4L2_SEL_TGT_CROP_BOUNDS: + sel->r.top = 0; + sel->r.left = 0; + sel->r.width = OV02C10_NATIVE_WIDTH; + sel->r.height = OV02C10_NATIVE_HEIGHT; + return 0; + case V4L2_SEL_TGT_CROP: + case V4L2_SEL_TGT_CROP_DEFAULT: + sel->r.top = OV02C10_ACTIVE_TOP; + sel->r.left = OV02C10_ACTIVE_LEFT; + sel->r.width = OV02C10_ACTIVE_WIDTH; + sel->r.height = OV02C10_ACTIVE_HEIGHT; + return 0; + } + + return -EINVAL; +} + static const struct v4l2_subdev_pad_ops ov02c10_pad_ops = { .set_fmt = ov02c10_set_format, .get_fmt = v4l2_subdev_get_fmt, .enum_mbus_code = ov02c10_enum_mbus_code, .enum_frame_size = ov02c10_enum_frame_size, + .get_selection = ov02c10_get_selection, .enable_streams = ov02c10_enable_streams, .disable_streams = ov02c10_disable_streams, }; -- 2.55.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] media: ov02c10: Accept a 26 MHz external clock 2026-09-23 14:42 ` [PATCH v2 0/3] media: ov02c10: get_selection and 26 MHz clock support Felipe Calliari 2026-09-23 14:42 ` [PATCH v2 1/3] media: ov02c10: Drop duplicate register write Felipe Calliari 2026-09-23 14:42 ` [PATCH v2 2/3] media: ov02c10: Implement get_selection Felipe Calliari @ 2026-09-23 14:42 ` Felipe Calliari 2026-09-23 20:54 ` Sakari Ailus 2 siblings, 1 reply; 14+ messages in thread From: Felipe Calliari @ 2026-09-23 14:42 UTC (permalink / raw) To: linux-media Cc: Sakari Ailus, Hans de Goede, Bryan O'Donoghue, Mauro Carvalho Chehab, Tomas Moro, linux-kernel, Felipe Calliari Several Meteor Lake / Lunar Lake designs (e.g. the Samsung Galaxy Book3/4 series) wire the OV02C10 to a 26 MHz external clock instead of the 19.2 MHz assumed so far. The IPU6 ipu-bridge forwards the rate from the ACPI SSDB verbatim as the "clock-frequency" property, so probe() just rejects it today: ov02c10 i2c-OVTI02C1:00: external clock 26000000 is not supported Rename OV02C10_MCLK to OV02C10_MCLK_19_2MHZ, add OV02C10_MCLK_26MHZ and accept both. The register tables program the OP PLL multiplier (0x0304/0x0305, 16-bit) to 0x0190 = 400 for a 19.2 MHz clock: the common table writes 0x0304 = 0x01 and the per-lane tables override 0x0305 = 0x90. Left alone on a 26 MHz clock the same dividers run every internal clock, and therefore the MIPI link, 26/19.2 = 1.3542x faster: a ~541.7 MHz link at ~40 fps instead of the nominal 400 MHz at ~30 fps. Re-program both PLL multipliers by the inverse factor instead, so that the link stays where the driver and the ipu-bridge fwnode already describe it: 400 * 19.2 / 26 = 295 = 0x0127, written to 0x0304/0x0305 and 0x0315/0x0316 after the per-lane table when the external clock is 26 MHz. That puts the link at 295 * 26 / 19.2 = 399.5 MHz, within 0.13% of the nominal 400 MHz, so link-frequency and pixel-rate stay accurate with the single existing menu entry and the ipu-bridge needs no change. Tested on a Samsung Galaxy Book3 (two CSI-2 data lanes, 26 MHz clock confirmed via clk_summary): the multiplier registers read back 0x0127 while streaming and capture runs at a steady 30.01 fps by buffer timestamp, against ~40 fps with the unmodified tables, matching the 30.14 fps that the exported hblank, vblank and pixel-rate describe. While touching the clock check, terminate its error string with a newline. Link: https://lore.kernel.org/linux-media/ap_CGQTdNFysTLot@kekkonen.localdomain/ Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com> --- Changes in v2: - Re-program the OP and VT PLL multipliers instead of advertising the scaled 541.667 MHz link frequency (Sakari). The suggested 0x28a / 0x1b1 produce no output on this hardware; 0x0127 -- the 0x0190 the tables already program, scaled by 19.2/26 -- does, and brings the frame rate back to the nominal ~30 fps. - Drop the second V4L2_CID_LINK_FREQ entry that v1 added: with the link back at ~400 MHz the existing single entry matches the ipu-bridge fwnode directly, so no ipu-bridge change is needed either. - Bryan's Reviewed-by and the Tested-by on v1 are not carried over, as the patch was rewritten. drivers/media/i2c/ov02c10.c | 43 ++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c index cdccbdef3..17208f2e6 100644 --- a/drivers/media/i2c/ov02c10.c +++ b/drivers/media/i2c/ov02c10.c @@ -16,7 +16,8 @@ #include <media/v4l2-fwnode.h> #define OV02C10_LINK_FREQ_400MHZ 400000000ULL -#define OV02C10_MCLK 19200000 +#define OV02C10_MCLK_19_2MHZ 19200000 +#define OV02C10_MCLK_26MHZ 26000000 #define OV02C10_RGB_DEPTH 10 #define OV02C10_NATIVE_WIDTH 1928 @@ -337,6 +338,23 @@ static const struct reg_sequence sensor_1928x1092_30fps_2lane_setting[] = { {0x3016, 0x32}, }; +/* + * The mode tables target a 19.2 MHz input clock, programming the OP PLL + * multiplier (0x0304/0x0305, 16-bit) to 0x0190 = 400 for a 400 MHz link at + * ~30 fps. A 26 MHz input clock instead runs every internal clock, and + * therefore the MIPI link, 26/19.2 = 1.3542x faster (~541.7 MHz, ~40 fps). + * Scaling both PLL multipliers by 19.2/26 -- 400 * 19.2 / 26 = 295 = 0x0127 + * -- puts the link back at 295 * 26 / 19.2 = 399.5 MHz, within 0.13% of the + * nominal 400 MHz, so the frame rate and the advertised link frequency both + * stay correct without a second link-frequency entry. + */ +static const struct reg_sequence sensor_pll_26mhz_setting[] = { + {0x0304, 0x01}, + {0x0305, 0x27}, + {0x0315, 0x01}, + {0x0316, 0x27}, +}; + static const char * const ov02c10_test_pattern_menu[] = { "Disabled", "Color Bar", @@ -396,6 +414,9 @@ struct ov02c10 { /* MIPI lane info */ u32 link_freq_index; u8 mipi_lanes; + + /* External (sensor) clock rate, Hz */ + u32 xvclk_freq; }; static inline struct ov02c10 *to_ov02c10(struct v4l2_subdev *subdev) @@ -619,6 +640,17 @@ static int ov02c10_enable_streams(struct v4l2_subdev *sd, goto out; } + if (ov02c10->xvclk_freq == OV02C10_MCLK_26MHZ) { + reg_sequence = sensor_pll_26mhz_setting; + sequence_length = ARRAY_SIZE(sensor_pll_26mhz_setting); + ret = regmap_multi_reg_write(ov02c10->regmap, + reg_sequence, sequence_length); + if (ret) { + dev_err(ov02c10->dev, "failed to write PLL settings\n"); + goto out; + } + } + ret = __v4l2_ctrl_handler_setup(ov02c10->sd.ctrl_handler); if (ret) goto out; @@ -877,6 +909,10 @@ static int ov02c10_check_hwcfg(struct ov02c10 *ov02c10) /* v4l2_link_freq_to_bitmap() guarantees at least 1 bit is set */ ov02c10->link_freq_index = ffs(link_freq_bitmap) - 1; + dev_dbg(dev, "%u Hz external clock, link freq %lld Hz\n", + ov02c10->xvclk_freq, + link_freq_menu_items[ov02c10->link_freq_index]); + if (bus_cfg.bus.mipi_csi2.num_data_lanes != 1 && bus_cfg.bus.mipi_csi2.num_data_lanes != 2) { ret = dev_err_probe(dev, -EINVAL, @@ -926,10 +962,11 @@ static int ov02c10_probe(struct i2c_client *client) "failed to get imaging clock\n"); freq = clk_get_rate(ov02c10->img_clk); - if (freq != OV02C10_MCLK) + if (freq != OV02C10_MCLK_19_2MHZ && freq != OV02C10_MCLK_26MHZ) return dev_err_probe(ov02c10->dev, -EINVAL, - "external clock %lu is not supported", + "external clock %lu is not supported\n", freq); + ov02c10->xvclk_freq = freq; v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops); -- 2.55.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] media: ov02c10: Accept a 26 MHz external clock 2026-09-23 14:42 ` [PATCH v2 3/3] media: ov02c10: Accept a 26 MHz external clock Felipe Calliari @ 2026-09-23 20:54 ` Sakari Ailus 0 siblings, 0 replies; 14+ messages in thread From: Sakari Ailus @ 2026-09-23 20:54 UTC (permalink / raw) To: Felipe Calliari Cc: linux-media, Hans de Goede, Bryan O'Donoghue, Mauro Carvalho Chehab, Tomas Moro, linux-kernel Hi Felipe, Thanks for the update. I think this patch still needs a bit more work, see below. On Wed, Sep 23, 2026 at 11:42:57AM -0300, Felipe Calliari wrote: > Several Meteor Lake / Lunar Lake designs (e.g. the Samsung Galaxy Book3/4 > series) wire the OV02C10 to a 26 MHz external clock instead of the > 19.2 MHz assumed so far. The IPU6 ipu-bridge forwards the rate from the > ACPI SSDB verbatim as the "clock-frequency" property, so probe() just > rejects it today: > > ov02c10 i2c-OVTI02C1:00: external clock 26000000 is not supported > > Rename OV02C10_MCLK to OV02C10_MCLK_19_2MHZ, add OV02C10_MCLK_26MHZ and > accept both. > > The register tables program the OP PLL multiplier (0x0304/0x0305, 16-bit) > to 0x0190 = 400 for a 19.2 MHz clock: the common table writes > 0x0304 = 0x01 and the per-lane tables override 0x0305 = 0x90. Left alone > on a 26 MHz clock the same dividers run every internal clock, and > therefore the MIPI link, 26/19.2 = 1.3542x faster: a ~541.7 MHz link at > ~40 fps instead of the nominal 400 MHz at ~30 fps. > > Re-program both PLL multipliers by the inverse factor instead, so that > the link stays where the driver and the ipu-bridge fwnode already > describe it: 400 * 19.2 / 26 = 295 = 0x0127, written to 0x0304/0x0305 > and 0x0315/0x0316 after the per-lane table when the external clock is > 26 MHz. That puts the link at 295 * 26 / 19.2 = 399.5 MHz, within 0.13% > of the nominal 400 MHz, so link-frequency and pixel-rate stay accurate > with the single existing menu entry and the ipu-bridge needs no change. Please do add that frequency; it's still different from what's supported now. If someone later on improves the driver and implements a PLL calculator for it, this will stop working. What about pixel rate? It's also affected, isn't it? > > Tested on a Samsung Galaxy Book3 (two CSI-2 data lanes, 26 MHz clock > confirmed via clk_summary): the multiplier registers read back 0x0127 > while streaming and capture runs at a steady 30.01 fps by buffer > timestamp, against ~40 fps with the unmodified tables, matching the > 30.14 fps that the exported hblank, vblank and pixel-rate describe. If the patch is adding support for a new link frequency, just say that. The rest goes to the cover letter. > > While touching the clock check, terminate its error string with a > newline. > > Link: https://lore.kernel.org/linux-media/ap_CGQTdNFysTLot@kekkonen.localdomain/ > Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com> > --- > > Changes in v2: > - Re-program the OP and VT PLL multipliers instead of advertising the > scaled 541.667 MHz link frequency (Sakari). The suggested 0x28a / 0x1b1 > produce no output on this hardware; 0x0127 -- the 0x0190 the tables > already program, scaled by 19.2/26 -- does, and brings the frame rate > back to the nominal ~30 fps. > - Drop the second V4L2_CID_LINK_FREQ entry that v1 added: with the link > back at ~400 MHz the existing single entry matches the ipu-bridge > fwnode directly, so no ipu-bridge change is needed either. > - Bryan's Reviewed-by and the Tested-by on v1 are not carried over, as > the patch was rewritten. > > drivers/media/i2c/ov02c10.c | 43 ++++++++++++++++++++++++++++++++++--- > 1 file changed, 40 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c > index cdccbdef3..17208f2e6 100644 > --- a/drivers/media/i2c/ov02c10.c > +++ b/drivers/media/i2c/ov02c10.c > @@ -16,7 +16,8 @@ > #include <media/v4l2-fwnode.h> > > #define OV02C10_LINK_FREQ_400MHZ 400000000ULL > -#define OV02C10_MCLK 19200000 > +#define OV02C10_MCLK_19_2MHZ 19200000 > +#define OV02C10_MCLK_26MHZ 26000000 > #define OV02C10_RGB_DEPTH 10 > > #define OV02C10_NATIVE_WIDTH 1928 > @@ -337,6 +338,23 @@ static const struct reg_sequence sensor_1928x1092_30fps_2lane_setting[] = { > {0x3016, 0x32}, > }; > > +/* > + * The mode tables target a 19.2 MHz input clock, programming the OP PLL > + * multiplier (0x0304/0x0305, 16-bit) to 0x0190 = 400 for a 400 MHz link at > + * ~30 fps. A 26 MHz input clock instead runs every internal clock, and > + * therefore the MIPI link, 26/19.2 = 1.3542x faster (~541.7 MHz, ~40 fps). > + * Scaling both PLL multipliers by 19.2/26 -- 400 * 19.2 / 26 = 295 = 0x0127 > + * -- puts the link back at 295 * 26 / 19.2 = 399.5 MHz, within 0.13% of the > + * nominal 400 MHz, so the frame rate and the advertised link frequency both > + * stay correct without a second link-frequency entry. > + */ > +static const struct reg_sequence sensor_pll_26mhz_setting[] = { > + {0x0304, 0x01}, > + {0x0305, 0x27}, > + {0x0315, 0x01}, > + {0x0316, 0x27}, > +}; This needs to be preceded by a patch splitting off these registers from the main register list. The result, in this patch, should be two register lists to choose from (or even better, a PLL calculator). > + > static const char * const ov02c10_test_pattern_menu[] = { > "Disabled", > "Color Bar", > @@ -396,6 +414,9 @@ struct ov02c10 { > /* MIPI lane info */ > u32 link_freq_index; > u8 mipi_lanes; > + > + /* External (sensor) clock rate, Hz */ > + u32 xvclk_freq; > }; > > static inline struct ov02c10 *to_ov02c10(struct v4l2_subdev *subdev) > @@ -619,6 +640,17 @@ static int ov02c10_enable_streams(struct v4l2_subdev *sd, > goto out; > } > > + if (ov02c10->xvclk_freq == OV02C10_MCLK_26MHZ) { > + reg_sequence = sensor_pll_26mhz_setting; > + sequence_length = ARRAY_SIZE(sensor_pll_26mhz_setting); > + ret = regmap_multi_reg_write(ov02c10->regmap, > + reg_sequence, sequence_length); > + if (ret) { > + dev_err(ov02c10->dev, "failed to write PLL settings\n"); > + goto out; > + } > + } > + > ret = __v4l2_ctrl_handler_setup(ov02c10->sd.ctrl_handler); > if (ret) > goto out; > @@ -877,6 +909,10 @@ static int ov02c10_check_hwcfg(struct ov02c10 *ov02c10) > /* v4l2_link_freq_to_bitmap() guarantees at least 1 bit is set */ > ov02c10->link_freq_index = ffs(link_freq_bitmap) - 1; > > + dev_dbg(dev, "%u Hz external clock, link freq %lld Hz\n", > + ov02c10->xvclk_freq, > + link_freq_menu_items[ov02c10->link_freq_index]); Maybe useful at development time, but hardly anymore. > + > if (bus_cfg.bus.mipi_csi2.num_data_lanes != 1 && > bus_cfg.bus.mipi_csi2.num_data_lanes != 2) { > ret = dev_err_probe(dev, -EINVAL, > @@ -926,10 +962,11 @@ static int ov02c10_probe(struct i2c_client *client) > "failed to get imaging clock\n"); > > freq = clk_get_rate(ov02c10->img_clk); > - if (freq != OV02C10_MCLK) > + if (freq != OV02C10_MCLK_19_2MHZ && freq != OV02C10_MCLK_26MHZ) > return dev_err_probe(ov02c10->dev, -EINVAL, > - "external clock %lu is not supported", > + "external clock %lu is not supported\n", > freq); > + ov02c10->xvclk_freq = freq; > > v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops); > -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-09-23 20:54 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[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
2026-09-08 9:21 ` Bryan O'Donoghue
2026-09-22 6:35 ` Tomas Moro
2026-09-08 9:04 ` [PATCH 1/3] media: ov02c10: Drop duplicate register write Bryan O'Donoghue
2026-09-23 14:42 ` [PATCH v2 0/3] media: ov02c10: get_selection and 26 MHz clock support Felipe Calliari
2026-09-23 14:42 ` [PATCH v2 1/3] media: ov02c10: Drop duplicate register write Felipe Calliari
2026-09-23 14:42 ` [PATCH v2 2/3] media: ov02c10: Implement get_selection Felipe Calliari
2026-09-23 14:42 ` [PATCH v2 3/3] media: ov02c10: Accept a 26 MHz external clock Felipe Calliari
2026-09-23 20:54 ` Sakari Ailus
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®