Re: [PATCH 0/4] media: Add the ST VD55G0 global shutter sensor

[Fernando Rimoli <fernandorimoli11@gmail.com>]

Hi Sergey,

Here is a run of the series on IPU6: a Surface Pro 9 (Alder Lake-P,
8086:465d), whose infrared camera is the same SMO55F0. The sensor
reports revision 0x1111, cut 1, and your built-in cut 1 patch is
identical to the one I had been using. In short, it works, and I have
three findings from the run.

The setup is not media next, so let me say how far it is from it. This
machine boots a 6.19-based kernel. I took 2/4 as posted and made three
local changes for the test:

 - dropped the struct v4l2_subdev_client_info argument from set_fmt and
   get_selection, which 6.19 does not have;
 - renamed "dovdd" to "dvdd" in the ACPI supply names, because the
   6.19-based int3472 here registers this sensor's two GPIO supplies as
   avdd and dvdd, so dovdd got the dummy regulator. Hans's int3472
   series reworks these names, so it is worth checking against whatever
   lands;
 - fell back to GPIO 1 when there is no st,leds, since 4/4 has no
   Surface Pro 9 entry (see my reply to 4/4).

I ran it at the 396 MHz from 3/4, with a cold boot to load the bridge,
and at 600 MHz, which is what this machine has used until now.

What works, at both frequencies:

 - probe, bind, and v4l2-compliance 44/44 on the subdev, starting from
   both auto and manual exposure;
 - all twelve combinations of R8, R10 and R10_CSI2P with 644x604,
   640x600, 640x480 and 320x240 stream 90 of 90 frames through libcamera
   with --strict-formats. R8 at 320x240 runs at about a third of the
   rate (15.93 fps at 396 MHz). That is the IPU6 limit on narrow lines
   in its packed capture path, not the driver; R10 and R10_CSI2P at
   320x240 run at full rate;
 - the strobe, through led_mode;
 - the system relay this machine uses for face unlock, which arms manual
   exposure and the strobe: it delivers frames through the driver,
   though see 3 below.

1. The Surface Pro 9 has the same clock problem as your Surface Pro 11.
At 396 MHz every mode runs at 47.31 fps where the controls give 59.14,
the same numbers you reported on Hao's thread. At 600 MHz it is 57.20
against 71.49. The SSDB says 24 MHz, INT3472 enables the clock through
the _DSM with 1, and everything the sensor derives runs at four fifths.
The previous driver on this machine behaves the same way at both
frequencies, so the cause is the clock.

Unlike on your machine, Hao's v2 does not fix it here. From a cold boot
with it applied, nothing changes: 57.20 against 71.49 at 600 MHz, with
the clock reported at 24 MHz, so the patch does pass 3 to the _DSM. This
firmware seems to give the sensor 19.2 MHz whatever INT3472 asks for. I
have reported it on Hao's thread.

That matters for 3/4. On this machine 396 MHz puts the sensor's MIPI
output at 633.6 Mbps, below its 750 Mbps minimum. It streams anyway, at
every mode above, but it is out of specification, and 600 MHz (960 Mbps
here) is not. I had kept this machine at 600 MHz because 380 looked 17%
slower, and that turned out to be this clock, not the link frequency, so
the frame rate is no argument either way. The specification is, though,
as long as the Surface Pro 9 clock cannot be fixed. Since the driver
needs a multiple of the clock it believes it has, a separate entry for
this IPU (8086:465d) through the per-IPU match in ipu-bridge would let
it run in specification without changing yours.

2. Nothing bounds the exposure while the strobe is on, and the
illuminator has an overcurrent limit that a long exposure trips. Your
maximum is frame length minus 64. At 640x480 Y8 and analogue gain 8,
ramping the exposure with led_mode set to flash gives (frame mean, with
the same exposure and the LED off in brackets; the room light differed
between the first run and the other two):

  frame 1860 lines, 600 MHz:  lit through 1200 lines (158.2 [119.9])
                              out at 1300 (123.0 [118.3])
  frame 1860 lines, 396 MHz:  lit through 1200 (108.0 [27.2])
                              out at 1300 (42.4 [28.4])
  frame 3480 lines, 396 MHz:  lit through 2100 (159.7)
                              partly at 2400 (97.6 [42.0])
                              out at 2700 (39.9)

So it gives out at about two thirds of the frame length both times:
65-70% for the short frame, 60-78% for the long one. The longest fully
lit pulse above is about 24 ms, against about 13.6 ms at the cutoff for
the 1860-line frame, so it is the duty cycle that matters, not the pulse
length. It recovers by itself: the LED comes back as soon as the
exposure is lowered. A ceiling proportional to the frame length while
led_mode is flash would keep auto exposure out of that region. Half the
frame length, which Peter used, leaves a margin.

3. On IPU6 the stream needs a CPU latency bound, or frames come out
corrupted. Through the relay, with a static scene, the frame mean jumped
between 150 and 230, and the log showed

  intel_ipu6_isys.isys intel_ipu6.isys.40: csi2-3 error: Transfer FIFO overflow
  intel_ipu6_isys.isys intel_ipu6.isys.40: csi2-3 error: Inter-frame long packet discarded

while holding a 50 us request on /dev/cpu_dma_latency for the same
capture gave 60 of 60 frames with a mean of 150 and a deviation of 0.4.
That is not this driver's to fix: the ISYS asks for 300 us
(ISYS_PM_QOS_VALUE), and Peter has a change to 50 for it in his tree,
not yet posted. But anyone trying this driver on IPU6 without that
change will see it, and it looks like a sensor problem when it is not.
The previous driver on this machine hid it by holding its own PM QoS
request while streaming.

I will add a Tested-by for the Surface Pro 9 on v2, if it has an entry
for it.

Fernando