[PATCH v3 0/3] media: Add OmniVision OV32C4 sensor driver

[Robert Bozik]

Hi,

v3 of the OV32C4 sensor driver. v2 is at
https://lore.kernel.org/linux-media/20260828132104.21473-1-robertbozik@xxxxxxxxx/

Changes in v3 are Sakari's review of v1, carried out in full; the
per-patch changelogs have the detail. The three that are more than
mechanical:

 - The line length. OV32C4_SCLK and the scaling function are gone; the
   mode carries .ppl directly, in pixels in the units of PIXEL_RATE, as
   you said it should be. The numbers come out identical, checked
   against the controls before and after: hblank 816, vblank 774,
   exposure max 2582, pixel rate 320000000.

 - The chip id retries are dropped. Measured over 31 power-ups of the
   sensor - one cold boot and 30 unbind/bind cycles - the first read
   answered every time, so the power-up sequence is sufficient on its
   own.

 - .get_frame_desc() is dropped, and the argument I made for it in v1
   does not hold. I re-measured it properly this time: the same scene,
   60 frames, with the op and without it. Both give 10 "Received packet
   is too long" errors and an identical picture. The receiver reports
   that extra long packet either way; the op never suppressed it, and I
   had attributed to it something it does not do.

One measurement I would rather report than sit on, on dropping the
endpoint check. On the 7.0 kernel this machine runs,
v4l2_fwnode_endpoint_alloc_parse() returns -EINVAL rather than
-EPROBE_DEFER when the endpoint is not there yet, and the probe is then
not retried, so the sensor does not bind at boot when it loses the race
with ipu-bridge - which it does on roughly half the boots here. The
deferring path with the comment naming the IPU bridge is in
media_stage. So the check is redundant against the tree this is merged
into, which is why it is gone, and I mention the older behaviour only as
data.

The companion chip, which was the first of the two questions in v2: it
is not a VCM, and my guess that it might be one with an integrated LDO
was wrong. Measured, with the sensor powered and the VCM driver unbound:

    w2@0x3e 0x10 0x01  r1@0x3e   ->  0x04     (what we write, read back)
    w2@0x3e 0x10 0x00  r8@0x3e   ->  00 04 00 00 00 00 00 00
    w2@0x3e 0x10 0x00 r32@0x3e   ->  a sparse block at 0x1010-0x1018
    w2@0x3e 0x00 0x00 r32@0x3e   ->  all zeroes

It has a 16-bit addressed register file and it remembers writes, so it
is not a dw9714, which is a write-only DAC with no register addressing
at all. Where the name comes from is ipu-bridge: it takes vcmtype
straight out of the ACPI SSDB and indexes ipu_vcm_types[] with it
without checking anything, and dw9714 has no id register, so its driver
binds unconditionally. The chip has no ACPI device of its own - the
subdev ends up with 0 pads and 0 links - and I found no id register in
it, so I cannot name it. The vendor Windows driver issues the same write
from its sensor driver and has no separate driver for it either.

So the position is unchanged but better founded: the write has to happen
or the sensor does not answer on I2C, and there is no handle for the
chip in the device model other than that mis-named VCM client. I still
agree it does not belong in a sensor driver. What I do not know is the
shape you want, since a regulator would mean a driver for a chip nobody
can name, and something would first have to stop ipu-bridge from
claiming the address as a VCM. Guidance welcome; I am happy to do the
work.

Still open from your review: the power-up timings. "These are 0 and 5
ms, respectively" - I did not follow, and the question stands. The
values in the patch are 5 ms after the supply and 20 ms after reset,
arrived at during bring-up; there is no datasheet.

The rest of the series is as before. It adds a driver for the OmniVision
OV32C4, a 32 megapixel RGBC CMOS image sensor. It ships as the
under-display camera in the Lenovo Yoga Slim 9 14ILL10, where it is
enumerated through ACPI (_HID "OVTI32C4") and feeds an Intel IPU7. The
last patch adds the sensor to ipu-bridge; without it the bridge builds
no fwnode graph for the sensor and the driver never binds.

The driver supports 3264x1840 at 30 fps, 10-bit Bayer, 4 CSI-2 lanes at
a 400 MHz link frequency, with exposure, analogue gain, digital gain,
vblank, hblank and flip controls, runtime PM and .get_selection.

Tested on the machine above: the sensor probes, streams continuously at
a measured 30.00 fps, and the frames arrive complete (60 frames =
720691200 bytes = 60 * 3264 * 1840 * 2). The full path up to a processed
image was exercised with libcamera's software ISP.

There is no public datasheet for this sensor, so a note on where the
numbers come from. The comments that carried this in v2 are gone from
the code, as you asked, so it is here:

  - The mode register table is the verbatim initialisation sequence from
    the vendor Windows driver: 1787 writes, strictly ascending, copied
    1:1 with nothing added or reordered.
  - The register meanings the controls depend on (exposure 0x3500,
    analogue gain 0x3508, digital gain 0x350a, VTS 0x380e, and the rule
    exposure_max = VTS - 32) were read out of the same binary and then
    confirmed against the values the chip reports.
  - The derived timings were checked against reality: the computed
    320000000 / (4080 * 2614) = 30.005 fps matches the measured 30.00.
  - The 6560x4928 pixel array and the 6528x4896 active area reported by
    .get_selection follow from the window registers of the mode table
    and agree with the vendor's published product brief.
  - The gain ranges were measured on the sensor, not inherited. The
    vendor driver clamps gain a layer above and carries no limits of its
    own, and the obvious donor - ov13b10, same registers - puts analogue
    unity at 0x80, which is wrong here. Analogue response is exactly
    proportional between 0x100 and 0x7c0 (1x to 7.75x, 0x100 also being
    the power-up value); digital gain is proportional with 1024 as unity
    and clips to black one step above 16383.
  - Flip handling was measured the same way. This sensor preserves the
    Bayer order across mirror and flip, so the driver only toggles the
    bits and the media bus code never changes. ov13b10 compensates the
    crop window by one pixel on the same registers to undo a Bayer
    shift; doing that here introduces one rather than removing it.

Tooling disclosure, as asked for by Documentation/process/generated-content.rst:
this series was written with the help of an AI coding assistant (Claude,
Anthropic; claude-opus-5 for the early work, claude-fable-5 for the rest)
in an extended interactive session. The assistant drafted the driver
source, the binding and this cover letter from my descriptions of the
hardware and of the vendor driver; every register meaning, gain range and
timing in it was measured by me on the sensor as described above, I ran
all of the tests, and I have reviewed and understand all of the code and
take responsibility for it. The mode register table was copied 1:1 from
the vendor driver, not generated. Static checks used: checkpatch.pl
--strict, sparse (C=1), W=1 and dt_binding_check. The individual patches
carry Assisted-by tags.

The series applies to media_stage.git; base-commit is below.

Thanks,
Robert

Robert Bozik (3):
  dt-bindings: media: i2c: Add OmniVision OV32C4
  media: i2c: Add driver for OmniVision OV32C4
  media: ipu-bridge: Add OmniVision OV32C4

 .../bindings/media/i2c/ovti,ov32c4.yaml       |  105 +
 MAINTAINERS                                   |    8 +
 drivers/media/i2c/Kconfig                     |   10 +
 drivers/media/i2c/Makefile                    |    1 +
 drivers/media/i2c/ov32c4.c                    | 2664 +++++++++++++++++
 drivers/media/pci/intel/ipu-bridge.c          |    2 +
 6 files changed, 2790 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov32c4.yaml
 create mode 100644 drivers/media/i2c/ov32c4.c


base-commit: 4900cad020c0580dfb1be27776ff10a4ef110cfa
-- 
2.53.0