Re: [PATCH 0/7] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

Re: [PATCH 0/7] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Sergey Lebedev]

Peter,

Tested on a second machine: Surface Pro 11 for Business, Intel Lunar Lake,
IPU7 — same SMO55F0 HID as your Surface Pro 9, one IPU generation later.
Two results worth having before you respin, one good and one blocking.

The good one: the series applies and builds clean, and the sensor works on
IPU7. All seven patches applied in order to an Ubuntu 7.0.0 tree with no
conflicts (its vd55g1.c is 2061 lines, exactly what your diffstat deletes),
and media/i2c, media/pci/intel and int3472 rebuilt without a warning.
With firmware present the driver probes, the bridge finds the sensor and
the subdev registers:

  intel-ipu7: Found supported sensor SMO55F0:00
  intel_ipu7_isys: bind vd55g 1-0060 nlanes is 1 port is 1
  entity 233: vd55g 1-0060 (1 pad, 1 link, 0 routes)

So nothing here is IPU6-specific. That is the part I could add that you
could not.

The blocking one: without that firmware nobody can use VD55G0 at all.

  vd55g i2c-SMO55F0:00: Direct firmware load for vd55g0-cut1.bin failed
                        with error -2
  vd55g i2c-SMO55F0:00: Failed to load required firmware vd55g0-cut1.bin: -2

Probe fails and the camera disappears. The driver has a fallback for
exactly this case, but only the older part gets one:

  static const struct firmware vd55g1_builtin_fw = {
          .data = vd55g1_patch_array, .size = ARRAY_SIZE(vd55g1_patch_array)
  };
  ...
  if (ret == -ENOENT && fw_info->builtin_fw)

vd55g0_fw_maps[] sets fw_name and leaves builtin_fw NULL, and neither
vd55g0-cut1.bin nor vd55g0-cut2.bin is in linux-firmware. So the series
works for you and for nobody else, which I assume is not the intent. Either
a built-in like VD55G1's, or the files submitted to linux-firmware, or at
minimum a note in the cover letter saying where they come from.

To get past it I used the same ST patch arrays that André Gilerson's
out-of-tree VD55G0 driver compiles in (vd55g0_patches.h, "Copyright (C)
2024 STMicroelectronics SA"), written out as a file. cut1 is 6868 bytes and
its header parses correctly against struct vd55g_patch_header — 0x0681,
version 2.11 — so the format is right. That is how I tested, not a
distribution path I am proposing.

Second, and this one reaches further than this series. Benjamin's warning
about libcamera is concrete and it is a regression for machines that work
today. libcamera keys the sensor-properties database, the soft-ISP helper
and the tuning file on the media entity name, which is the driver name:

  WARN  No static properties available for 'vd55g'
  WARN  'vd55g 1-0060': No sensor delays found in static properties
  WARN  Configuration file 'vd55g.yaml' not found for IPA module 'simple'
  WARN  IPASoft: Failed to create camera sensor helper for vd55g

and the installed libcamera 0.7.0 does carry a vd55g1 entry — `strings
libcamera.so.0.7 | grep -x vd55g1` finds it. So renaming the driver does
not only fail to help VD55G0; it silently takes the properties and the
helper away from VD55G1 users who have them now. Whatever is decided about
one driver or two, that rename needs libcamera patches landing alongside,
or the entity name keeping its old form.

Two smaller things, neither chased:

  - `supply vcore not found, using dummy regulator` persists here despite
    4/7's SMO55F0 mapping. It was not the blocker, so I did not dig.
  - 3/7 advertises 380 MHz for SMO55F0. This machine has been running 402 MHz
    ("MIPI output freq is 804MHz / 2") from the out-of-tree driver. Your
    driver takes the endpoint's value rather than validating against a list,
    so it accepted 380 without complaint, and I have not established which is
    correct for either model. Worth knowing that two people measured
    different numbers for the same HID on different Surfaces.

Happy to run v2 here when you have one, and to report anything specific you
would like measured on IPU7.

Sergey

Re: [PATCH 0/7] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Benjamin Mugnier]

Hi Sergey,

Le 07/09/2026 à 22:59, Sergey Lebedev a écrit :
> Peter,
> 
> Tested on a second machine: Surface Pro 11 for Business, Intel Lunar Lake,
> IPU7 — same SMO55F0 HID as your Surface Pro 9, one IPU generation later.
> Two results worth having before you respin, one good and one blocking.
> 
> The good one: the series applies and builds clean, and the sensor works on
> IPU7. All seven patches applied in order to an Ubuntu 7.0.0 tree with no
> conflicts (its vd55g1.c is 2061 lines, exactly what your diffstat deletes),
> and media/i2c, media/pci/intel and int3472 rebuilt without a warning.
> With firmware present the driver probes, the bridge finds the sensor and
> the subdev registers:
> 
>   intel-ipu7: Found supported sensor SMO55F0:00
>   intel_ipu7_isys: bind vd55g 1-0060 nlanes is 1 port is 1
>   entity 233: vd55g 1-0060 (1 pad, 1 link, 0 routes)
> 
> So nothing here is IPU6-specific. That is the part I could add that you
> could not.
> 
> The blocking one: without that firmware nobody can use VD55G0 at all.
> 
>   vd55g i2c-SMO55F0:00: Direct firmware load for vd55g0-cut1.bin failed
>                         with error -2
>   vd55g i2c-SMO55F0:00: Failed to load required firmware vd55g0-cut1.bin: -2
> 
> Probe fails and the camera disappears. The driver has a fallback for
> exactly this case, but only the older part gets one:
> 
>   static const struct firmware vd55g1_builtin_fw = {
>           .data = vd55g1_patch_array, .size = ARRAY_SIZE(vd55g1_patch_array)
>   };
>   ...
>   if (ret == -ENOENT && fw_info->builtin_fw)
> 
> vd55g0_fw_maps[] sets fw_name and leaves builtin_fw NULL, and neither
> vd55g0-cut1.bin nor vd55g0-cut2.bin is in linux-firmware. So the series
> works for you and for nobody else, which I assume is not the intent. Either
> a built-in like VD55G1's, or the files submitted to linux-firmware, or at
> minimum a note in the cover letter saying where they come from.
> 
> To get past it I used the same ST patch arrays that André Gilerson's
> out-of-tree VD55G0 driver compiles in (vd55g0_patches.h, "Copyright (C)
> 2024 STMicroelectronics SA"), written out as a file. cut1 is 6868 bytes and

I'm not aware of André's work. Could you link it to me ?

The only vd55g0 out of tree driver I know is our own version on
Github[1]. Is it a fork of it ?

[1] https://github.com/STMicroelectronics/vd55g0-linux-driver

> its header parses correctly against struct vd55g_patch_header — 0x0681,
> version 2.11 — so the format is right. That is how I tested, not a
> distribution path I am proposing.
> 
> Second, and this one reaches further than this series. Benjamin's warning
> about libcamera is concrete and it is a regression for machines that work
> today. libcamera keys the sensor-properties database, the soft-ISP helper
> and the tuning file on the media entity name, which is the driver name:
> 
>   WARN  No static properties available for 'vd55g'
>   WARN  'vd55g 1-0060': No sensor delays found in static properties
>   WARN  Configuration file 'vd55g.yaml' not found for IPA module 'simple'
>   WARN  IPASoft: Failed to create camera sensor helper for vd55g
> 
> and the installed libcamera 0.7.0 does carry a vd55g1 entry — `strings
> libcamera.so.0.7 | grep -x vd55g1` finds it. So renaming the driver does
> not only fail to help VD55G0; it silently takes the properties and the
> helper away from VD55G1 users who have them now. Whatever is decided about
> one driver or two, that rename needs libcamera patches landing alongside,
> or the entity name keeping its old form.
> 
> Two smaller things, neither chased:
> 
>   - `supply vcore not found, using dummy regulator` persists here despite
>     4/7's SMO55F0 mapping. It was not the blocker, so I did not dig.
>   - 3/7 advertises 380 MHz for SMO55F0. This machine has been running 402 MHz
>     ("MIPI output freq is 804MHz / 2") from the out-of-tree driver. Your
>     driver takes the endpoint's value rather than validating against a list,
>     so it accepted 380 without complaint, and I have not established which is
>     correct for either model. Worth knowing that two people measured
>     different numbers for the same HID on different Surfaces.
> 
> Happy to run v2 here when you have one, and to report anything specific you
> would like measured on IPU7.
> 
> Sergey
> 

-- 
Regards,
Benjamin

Re: [PATCH 0/7] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Sergey Lebedev]

Benjamin,

Yes — it is a fork of yours, and André says so himself in the header he
kept:

  https://github.com/AndreGilerson/linux-surface-kernel
  branch v7.0-surface-devel, drivers/media/i2c/vd55g0.c

  // SPDX-License-Identifier: GPL-2.0
  /*
   * Driver for VD55G0 global shutter sensor family driver
   *
   * Copyright (C) 2024 STMicroelectronics SA
   *
   * Adapter with small changes by André Gilerson
   */

Against your master it is 127 changed lines out of 2114, and all 59
vd55g0_* function names are the same set — nothing added, nothing removed.
vd55g0_patches.h differs in four places: his attribution line, uint ->
unsigned int, and two structs made static. The firmware arrays are
byte-identical, cut1_patch 6868 bytes and cut2_patch 260.

That last point matters for this series. The blob I wrote to
/lib/firmware/vd55g0-cut1.bin to get past the missing-firmware failure was
taken from his header, so it is your firmware, published by you under
GPL-2.0 in your own repository. Peter's driver needs those bytes and
cannot get them from anywhere in-tree; you are the one who can say how
they should reach linux-firmware, or whether VD55G0 should keep a built-in
array the way VD55G1 does.

What the 127 lines actually change, since it is a fair summary of what a
laptop needed that your driver did not have:

  - ACPI. Yours is DT-only; his adds acpi.h and an ACPI match on SMO55F0.
  - Supply names. Yours asks for VCORE, VDDIO and VANA. His asks for avdd
    and dovdd, two rather than three, with the comment that INT3472 on the
    Surface Pro 11 provides GPIO type 0x0B as avdd and 0x10 as dovdd.
  - SGRBG aliases beside the mono formats, commented "allows IPU7 Bayer
    pipeline".
  - illuminator GPIO handling.
  - an unknown revision warns and continues instead of returning -EINVAL.

The second of those explains something I reported yesterday and had not
chased: on this machine Peter's 4/7 leaves `supply vcore not found, using
dummy regulator`. His mapping gives the SMO55F0 GPIOs the names your
driver wants, while the machine's INT3472 appears to describe what André
named avdd and dovdd. One of the two mappings does not match this
hardware, and I have not established which — I can measure it here if that
is useful.

Two things I should retire from my own report, having now seen the
off-list exchange you added back. Peter had already withdrawn the rename
on 4 September, so the libcamera breakage I raised - the sensor-properties
and helper lookups keying on the entity name, which vd55g1 machines have
today - does not arise in a v2 that keeps the name. And the metadata point
you make to him is the same one from the other side: unknown to libcamera,
the sensor streams but on defaults.

What is not retired is the firmware. Nothing in the exchange touches it,
and it is the one thing that stops anyone but Peter using VD55G0 at all.

For contrast rather than contradiction: he reports the sensor is not quite
usable on his Surface Pro 9 because of IPU6 stream problems, short lines
and frames cut off when the CPU idles. I saw none of that on IPU7 - it
probed, bound and registered, and streamed 60 frames at 30 fps. Different
bridge, different result, which may be worth knowing when the two are
compared.

I should be clear that I am not speaking for André. I have written to him
separately about upstreaming his IMX681 driver, he answered that mainline
is his goal, and he is away until 28 September. If you want to reach him
directly his address is in the MODULE_AUTHOR line of that file.

Sergey

Re: [PATCH 0/7] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Sergey Lebedev]

Following up on the firmware blocker I raised on 7 September, since it is
still the thing that stops anyone but the author from using VD55G0. Relayed
with Peter Marshall's agreement.

Peter sourced the firmware the same way I did for testing: the version 2.11
patch array in Andre Gilerson's out-of-tree VD55G0 driver, which he reports is
identical to the one in ST's standalone driver. He has published the extracted
blobs for testing at

  https://github.com/petm5/vd55g-firmware

and would prefer submission to linux-firmware over a built-in array, so that
firmware can be updated independently of the kernel version. I agree that is
the better end state.

Worth adding, because it may lower the bar for getting there: the built-in
arrangement is already in the tree, from ST, for the sibling part.
drivers/media/i2c/vd55g1.c is Copyright (C) 2025 STMicroelectronics SA under
SPDX-License-Identifier: GPL-2.0, and it carries vd55g1_patch_array[] - 3512
bytes, firmware revision 2.9 by its own defines. That file has no
request_firmware path at all; the built-in array is its only mechanism.

Different sensor and a different binary from the VD55G0 one, which measures
6868 bytes at revision 2.11, so it is precedent for the arrangement rather
than for the blob. But it does mean a built-in for VD55G0 would not be asking
ST for something they have not already done one part number over, whereas
linux-firmware needs an explicit redistribution grant in WHENCE from the
copyright holder.

So the two routes are not equally hard today, and they are not exclusive: a
built-in now and linux-firmware when ST are ready is not a wasted step.

Whether GPL-2.0 on the surrounding code carries the blob with it is a licence
question and not one I will answer.

Benjamin, Sylvain - this is the one blocker on the series that nobody outside
ST can clear.

Sergey

[PATCH 0/7] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Peter Marshall]

This patch series refactors the STMicroelectronics VD55G1 and VD65G4 image
sensor driver (vd55g1) into a generic VD55G family driver (vd55g) and adds
support for the VD55G0 monochrome sensor.

The VD55G0 sensor is found in devices such as the Microsoft Surface Pro 9 under
ACPI HID `SMO55F0`. It features a native resolution of 644 x 604 pixels and
differs from later variants in the family in several ways:

* It uses a different native resolution and register addresses mapping,
  requiring specification of generational feature differences.
* It lacks support for spatial and temporal HDR.
* It requires an exposure duty cycle limit to keep the illuminator current safe.

Tested on a Microsoft Surface Pro 9 (an Intel IPU6 platform with ACPI
HID "SMO55F0") running libcamera.

Peter Marshall (7):
  media: i2c: vd55g: move patch array to a separate firmware header
  dt-bindings: media: i2c: st,vd55g: rename and add VD55G0 variant
  media: ipu-bridge: Add VD55G0 to the list of supported sensors
  platform/x86: int3472: Add VD55G0 supply GPIO mapping
  media: i2c: st-vd55g: Add generic driver and VD55G0 support
  media: i2c: st-vd55g: Remove legacy vd55g1 remnants
  MAINTAINERS: Update ST VD55G camera driver entry

 .../i2c/{st,vd55g1.yaml => st,vd55g.yaml}     |   30 +-
 MAINTAINERS                                   |    7 +-
 drivers/media/i2c/Kconfig                     |   11 +
 drivers/media/i2c/Makefile                    |    2 +-
 drivers/media/i2c/vd55g-fw.h                  |  305 +++
 drivers/media/i2c/vd55g.c                     | 2166 +++++++++++++++++
 drivers/media/i2c/vd55g1.c                    | 2061 ----------------
 drivers/media/pci/intel/ipu-bridge.c          |    2 +
 drivers/platform/x86/intel/int3472/discrete.c |   18 +
 9 files changed, 2531 insertions(+), 2071 deletions(-)
 rename Documentation/devicetree/bindings/media/i2c/{st,vd55g1.yaml => st,vd55g.yaml} (76%)
 create mode 100644 drivers/media/i2c/vd55g-fw.h
 create mode 100644 drivers/media/i2c/vd55g.c
 delete mode 100644 drivers/media/i2c/vd55g1.c

-- 
2.55.0

Re: [PATCH 0/7] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Benjamin Mugnier]

Hi Peter,

Le 02/09/2026 à 22:45, Peter Marshall a écrit :
> This patch series refactors the STMicroelectronics VD55G1 and VD65G4 image
> sensor driver (vd55g1) into a generic VD55G family driver (vd55g) and adds
> support for the VD55G0 monochrome sensor.
> 
> The VD55G0 sensor is found in devices such as the Microsoft Surface Pro 9 under
> ACPI HID `SMO55F0`. It features a native resolution of 644 x 604 pixels and
> differs from later variants in the family in several ways:
> 
> * It uses a different native resolution and register addresses mapping,
>   requiring specification of generational feature differences.
> * It lacks support for spatial and temporal HDR.
> * It requires an exposure duty cycle limit to keep the illuminator current safe.
> 
> Tested on a Microsoft Surface Pro 9 (an Intel IPU6 platform with ACPI
> HID "SMO55F0") running libcamera.

I'm sorry I haven't read through your whole serie yet.

First thing first what is the point in refactoring both drivers into the
same one ? I guess it should be addressed in your commit messages. To be
fair it even looks to add complexity because even if the sensors share
some IPs you might have notice they are in fact very different. I could
even argue that the vd56g3 is closer to the vd55g1 than the vd55g0 is.

Plus having your commits merged will also require changes in libcamera.

Otherwise I'm interested in your patches 3 and 4. If I read it correctly
they allow the Microsoft Surface Pro 9 to use the embedded vd55g0 right ?

> 
> Peter Marshall (7):
>   media: i2c: vd55g: move patch array to a separate firmware header
>   dt-bindings: media: i2c: st,vd55g: rename and add VD55G0 variant
>   media: ipu-bridge: Add VD55G0 to the list of supported sensors
>   platform/x86: int3472: Add VD55G0 supply GPIO mapping
>   media: i2c: st-vd55g: Add generic driver and VD55G0 support
>   media: i2c: st-vd55g: Remove legacy vd55g1 remnants
>   MAINTAINERS: Update ST VD55G camera driver entry
> 
>  .../i2c/{st,vd55g1.yaml => st,vd55g.yaml}     |   30 +-
>  MAINTAINERS                                   |    7 +-
>  drivers/media/i2c/Kconfig                     |   11 +
>  drivers/media/i2c/Makefile                    |    2 +-
>  drivers/media/i2c/vd55g-fw.h                  |  305 +++
>  drivers/media/i2c/vd55g.c                     | 2166 +++++++++++++++++
>  drivers/media/i2c/vd55g1.c                    | 2061 ----------------
>  drivers/media/pci/intel/ipu-bridge.c          |    2 +
>  drivers/platform/x86/intel/int3472/discrete.c |   18 +
>  9 files changed, 2531 insertions(+), 2071 deletions(-)
>  rename Documentation/devicetree/bindings/media/i2c/{st,vd55g1.yaml => st,vd55g.yaml} (76%)
>  create mode 100644 drivers/media/i2c/vd55g-fw.h
>  create mode 100644 drivers/media/i2c/vd55g.c
>  delete mode 100644 drivers/media/i2c/vd55g1.c
> 

-- 
Regards,
Benjamin

Re: [PATCH 0/7] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Benjamin Mugnier]

Hi Peter,

Adding back everyone to the CC list so they can see our discussion ;)

Le 04/09/2026 à 22:51, Peter Marshall a écrit :
> Hi Benjamin,
> 
> Thanks for your initial review.
> 
> I chose to merge the new hardware support into the existing driver to avoid code
> duplication and improve long-term maintainability. It looks like mainline is
> moving toward generic interfaces, and away from the "copy-and-modify" status quo
> common in vendor kernel trees.

I'm not aware of such thing in the mainline tree, we can take for
example imx sensors and every sensor tends to have it's own driver, but
I may be wrong.

> 
> The vd55g1 appears to be a generational successor of the vd55g0 and the two are
> similar enough that the driver only needs a few feature flags passed through
> the ACPI/OF match table to support both of them. The added abstraction does not
> obscure operational logic, although it might make it harder to verify according
> to the datasheets.

I tend to disagree, I still think it adds complexity and slowdown code
iteration as we will have to test both drivers at the same time to see
any regressions, which is not ideal.

I'd like to have an advice from Sakari and or Laurent on this matter,
what do you think ?

> 
> Patches 3 and 4 add the required identifiers for the Microsoft Surface Pro 9 to
> use its embedded vd55g0. The sensor isn't quite usable yet because of remaining
> issues in the IPU6 subsystem. Most notably the stream fails to start if the
> sensor sends less than a certain number of bytes per line (384 bytes or less
> on my hardware), and the frame gets cut off causing flickering when the CPU is
> idle. The latter issue was already documented by Hans de Goede on linux-media
> (Oct 2025).

Gotcha.

> 
> libcamera comes with generic defaults for unknown sensors and doesn't require
> any modifications to stream successfully.

Well yeah, the sensor will stream but will lack its medata to make it
stream cleanly. Please note that for Raspberry Pi this is a hard
requirement and it won't stream at without a cam_helper. Honestly that's
not that big of a deal and I consider it a side effect, we could fix
that easilly. I'm more concerned about the huge refactoring part.

> 
> Unfortunately, patch 5 in my series is not split into logical steps and does
> way too much at once. The renaming from vd55g1 to vd55g was unnecessary, and the
> following patches 6 and 7 should have been squashed into it as they represent
> cleanup steps instead of isolated logical changes. That makes this series
> completely unbisectable and not suitable for merging. The next version will

Indeed.

> focus on abstracting vd55g1.c in logical steps before supporting
> the vd55g0 revision.

Thank you for your work. I'm eager to see this.

> 
> Best regards,
> Peter Marshall

-- 
Regards,
Benjamin