Re: [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

Re: [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Sergey Lebedev]

Peter,

Benjamin suggested we get in touch, so: hello, and I should have written
first. I missed this series when it went out - I was not on the Cc and I
was not reading the list for work next to my own - and on 20 September I
sent ST a separate vd55g0.c off-list, two days after you posted, without
mentioning that somebody was already doing it here. That one is mine.

Two things are yours to take wherever the driver ends up.

The illuminator, your 05/11. GPIO 1 is right, and it can be measured
rather than assumed. Each GPIO was strobed in turn on a Surface Pro 11
(Intel), watched at the bezel through a phone camera by a person
counting, then measured as frame brightness against recorded timestamps:

  GPIO 0   strobe on 19.74   off 19.76   -0.02
  GPIO 1   strobe on 51.93   off 40.97  +10.96
  GPIO 2   strobe on 19.51   off 19.51   +0.00
  GPIO 3   strobe on 22.53   off 24.87   -2.34

and three cycles of GPIO 1 alone with the others held off:
117.95/115.60/117.71 lit against 21.51/22.06/24.43 dark. Take it for the
commit message if it helps - somebody will ask why 1, and "assume"
invites the question.

Hardware. You tested on a Surface Pro 9 with IPU6; this is a Surface Pro
11 for Business (Intel) with IPU7, where the part is the infrared camera.
Say the word and v2 goes on it.

Your 01-05 are exactly the parts I do not have and would otherwise be
writing - ipu-bridge, int3472, the illuminator - and Benjamin has said he
will take them. If those land beside a vd55g0.c then the machine works,
which is a better end than either of us starting again.

It is not my file to give away, but if you want a share of the driver -
writing part of it, a maintainer line, or taking it over outright - say
so now rather than after it is posted. You were here first, and you have
hardware I do not.

Sergey

[PATCH v2 00/11] 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) to abstract sensor differences and adds support for
the VD55G0 monochrome sensor.

The VD55G0 is used as the IR face ID sensor in devices such as the
Microsoft Surface Pro 9, where it is found 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
  within safe levels.

Tested on a Microsoft Surface Pro 9 (an Intel IPU6 platform with ACPI
HID "SMO55F0") running kernel 7.2.0 (changes cherry-picked) and libcamera
0.7.0. Builds successfully on linux-media next.

Key changes from the previous series:
* The vd55g1 module is grown to support the extra hardware instead of
  replacing it with a completely new driver.
* The driver now includes a builtin patch array. External firmware loading
  is no longer required.
* Lower-level error messages are demoted to the debug level to keep kernel
  logs clean and actionable.
* Hardware registers are now fully abstracted, so operational logic never
  needs to touch a raw CCI address.
* Changes are broken into logical steps to improve clarity and make
  bisection possible.
* Rebased against linux-media next.

Peter Marshall (11):
  dt-bindings: media: i2c: st,vd55g1: Move allOf: after required:
  media: dt-bindings: i2c: vd55g1: Add vd55g0 compatible
  media: ipu-bridge: Add VD55G0 to the list of supported sensors
  platform/x86: int3472: Add VD55G0 supply GPIO mapping
  media: i2c: st-vd55g1: Default to illuminator on GPIO 1
  media: i2c: st,vd55g1: Handle virtual firmware graph endpoints
  media: i2c: st-vd55g1: Clean up module error reporting
  media: i2c: st-vd55g1: Unify frame timing calculations
  media: i2c: st-vd55g1: Abstract sensor models, revisions, and features
  media: i2c: st-vd55g1: Use indirect hardware register addressing
  media: i2c: st-vd55g1: Support VD55G0 global-shutter image sensor

 .../bindings/media/i2c/st,vd55g1.yaml         |   34 +-
 MAINTAINERS                                   |    1 +
 drivers/media/i2c/vd55g1.c                    | 1667 +++++++++++++----
 drivers/media/pci/intel/ipu-bridge.c          |    2 +
 drivers/platform/x86/intel/int3472/discrete.c |   18 +
 5 files changed, 1394 insertions(+), 328 deletions(-)

-- 
2.55.0

Re: [PATCH v2 00/11] media: i2c: st-vd55g1: Genericize driver and add VD55G0 support

[Benjamin Mugnier]

Hi Peter,

Thank you for your work.

Le 19/09/2026 à 00:16, Peter Marshall a écrit :
> This patch series refactors the STMicroelectronics VD55G1 and VD65G4 image
> sensor driver (vd55g1) to abstract sensor differences and adds support for
> the VD55G0 monochrome sensor.

I read through your patches more closely, and I still think this is not
the best approach. So for the driver refactoring this will be a NAK for
me. I'll gladly take the ACPI parts though.

Sergey is currently working on bringing the vd55g0 downstream driver in
upstream, which I find a better approach. Not sure if you're working
together, if not I recommend getting in touch as it may suit your needs ?

> 
> The VD55G0 is used as the IR face ID sensor in devices such as the
> Microsoft Surface Pro 9, where it is found 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
>   within safe levels.
> 
> Tested on a Microsoft Surface Pro 9 (an Intel IPU6 platform with ACPI
> HID "SMO55F0") running kernel 7.2.0 (changes cherry-picked) and libcamera
> 0.7.0. Builds successfully on linux-media next.
> 
> Key changes from the previous series:
> * The vd55g1 module is grown to support the extra hardware instead of
>   replacing it with a completely new driver.
> * The driver now includes a builtin patch array. External firmware loading
>   is no longer required.
> * Lower-level error messages are demoted to the debug level to keep kernel
>   logs clean and actionable.
> * Hardware registers are now fully abstracted, so operational logic never
>   needs to touch a raw CCI address.
> * Changes are broken into logical steps to improve clarity and make
>   bisection possible.
> * Rebased against linux-media next.
> 
> Peter Marshall (11):
>   dt-bindings: media: i2c: st,vd55g1: Move allOf: after required:
>   media: dt-bindings: i2c: vd55g1: Add vd55g0 compatible
>   media: ipu-bridge: Add VD55G0 to the list of supported sensors
>   platform/x86: int3472: Add VD55G0 supply GPIO mapping
>   media: i2c: st-vd55g1: Default to illuminator on GPIO 1
>   media: i2c: st,vd55g1: Handle virtual firmware graph endpoints
>   media: i2c: st-vd55g1: Clean up module error reporting
>   media: i2c: st-vd55g1: Unify frame timing calculations
>   media: i2c: st-vd55g1: Abstract sensor models, revisions, and features
>   media: i2c: st-vd55g1: Use indirect hardware register addressing
>   media: i2c: st-vd55g1: Support VD55G0 global-shutter image sensor
> 
>  .../bindings/media/i2c/st,vd55g1.yaml         |   34 +-
>  MAINTAINERS                                   |    1 +
>  drivers/media/i2c/vd55g1.c                    | 1667 +++++++++++++----
>  drivers/media/pci/intel/ipu-bridge.c          |    2 +
>  drivers/platform/x86/intel/int3472/discrete.c |   18 +
>  5 files changed, 1394 insertions(+), 328 deletions(-)
> 

-- 
Regards,
Benjamin