Re: [PATCH v8 2/2] Added Digiteq Automotive MGB4 driver documentation

From: Martin Tůma
Date: Thu Aug 10 2023 - 11:37:49 EST


On 10. 08. 23 14:43, Hans Verkuil wrote:
On 8/10/23 13:54, Martin Tůma wrote:
On 26. 07. 23 12:40, Hans Verkuil wrote:
On 04/07/2023 15:13, tumic@xxxxxxxxxx wrote:
From: Martin Tůma <martin.tuma@xxxxxxxxxxxxxxxxxxxxx>

The "admin-guide" documentation for the Digiteq Automotive MGB4 driver.

Signed-off-by: Martin Tůma <martin.tuma@xxxxxxxxxxxxxxxxxxxxx>
---
Documentation/admin-guide/media/mgb4.rst | 369 ++++++++++++++++++
.../admin-guide/media/pci-cardlist.rst | 1 +
.../admin-guide/media/v4l-drivers.rst | 1 +
3 files changed, 371 insertions(+)
create mode 100644 Documentation/admin-guide/media/mgb4.rst

diff --git a/Documentation/admin-guide/media/mgb4.rst b/Documentation/admin-guide/media/mgb4.rst
new file mode 100644
index 000000000000..e1bb708a2265
--- /dev/null
+++ b/Documentation/admin-guide/media/mgb4.rst
@@ -0,0 +1,369 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+====================
+mgb4 sysfs interface
+====================
+
+The mgb4 driver provides a sysfs interface, that is used to configure video
+stream related parameters (some of them must be set properly before the v4l2
+device can be opened) and obtain the video device/stream status.
+
+There are two types of parameters - global / PCI card related, found under
+``/sys/class/video4linux/videoX/device`` and module specific found under
+``/sys/class/video4linux/videoX``.
+
+
+Global (PCI card) parameters
+============================
+
+**module_type** (R):
+ Module type.
+
+ | 0 - No module present
+ | 1 - FPDL3
+ | 2 - GMSL
+
+**module_version** (R):
+ Module version number. Zero in case of a missing module.
+
+**fw_type** (R):
+ Firmware type.
+
+ | 1 - FPDL3
+ | 2 - GMSL
+
+**fw_version** (R):
+ Firmware version number.
+
+**serial_number** (R):
+ Card serial number. The format is::
+
+ PRODUCT-REVISION-SERIES-SERIAL
+
+ where each component is a 8b number.
+
+
+Common FPDL3/GMSL input parameters
+==================================
+
+**input_id** (R):
+ Input number ID, zero based.
+
+**oldi_lane_width** (RW):
+ Number of deserializer output lanes.
+
+ | 0 - single
+ | 1 - dual
+
+**color_mapping** (RW):
+ Mapping of the incoming bits in the signal to the colour bits of the pixels.
+
+ | 0 - OLDI/JEIDA
+ | 1 - SPWG/VESA
+
+**link_status** (R):
+ Video link status. If the link is locked, chips are properly connected and
+ communicating at the same speed and protocol. The link can be locked without
+ an active video stream.
+
+ A value of 0 is equivalent to the V4L2_IN_ST_NO_SYNC flag of the V4L2
+ VIDIOC_ENUMINPUT status bits.
+
+ | 0 - unlocked
+ | 1 - locked
+
+**stream_status** (R):
+ Video stream status. A stream is detected if the link is locked, the input
+ pixel clock is running and the DE signal is moving.
+
+ A value of 0 is equivalent to the V4L2_IN_ST_NO_SIGNAL flag of the V4L2
+ VIDIOC_ENUMINPUT status bits.
+
+ | 0 - not detected
+ | 1 - detected
+
+**video_width** (R):
+ Video stream width. This is the actual width as detected by the HW.
+
+ The value is identical to what VIDIOC_QUERY_DV_TIMINGS returns in the width
+ field of the v4l2_bt_timings struct.
+
+**video_height** (R):
+ Video stream height. This is the actual height as detected by the HW.
+
+ The value is identical to what VIDIOC_QUERY_DV_TIMINGS returns in the height
+ field of the v4l2_bt_timings struct.
+
+**vsync_status** (R):
+ The type of VSYNC pulses as detected by the video format detector.
+
+ The value is equivalent to the flags returned by VIDIOC_QUERY_DV_TIMINGS in
+ the polarities field of the v4l2_bt_timings struct.
+
+ | 0 - active low
+ | 1 - active high
+ | 2 - not available
+
+**hsync_status** (R):
+ The type of HSYNC pulses as detected by the video format detector.
+
+ The value is equivalent to the flags returned by VIDIOC_QUERY_DV_TIMINGS in
+ the polarities field of the v4l2_bt_timings struct.
+
+ | 0 - active low
+ | 1 - active high
+ | 2 - not available
+
+**vsync_gap_length** (RW):
+ If the incoming video signal does not contain synchronization VSYNC and
+ HSYNC pulses, these must be generated internally in the FPGA to achieve
+ the correct frame ordering. This value indicates, how many "empty" pixels

Pixels or lines? This is vsync, so lines would be more logical.

Even if the hardware wants pixels, perhaps the driver should use lines and
translate it to pixels. It's much easier for userspace to work with lines.


According to our HW engineers, this is properly documented. I do not
have the full insight to the "signal parameters" topic, so my answers
here will be just some kind of "free" translation of what I got. The
justification here was, that the vsync gap length (in our case/HW)
represents something slightly different than you may think.

+ (pixels with deasserted Data Enable signal) are necessary to generate the
+ internal VSYNC pulse.
+
+**hsync_gap_length** (RW):
+ If the incoming video signal does not contain synchronization VSYNC and
+ HSYNC pulses, these must be generated internally in the FPGA to achieve
+ the correct frame ordering. This value indicates, how many "empty" pixels
+ (pixels with deasserted Data Enable signal) are necessary to generate the
+ internal HSYNC pulse. The value must be greater than 1 and smaller than
+ vsync_gap_length.

Does this make sense? vsync_gap_length can be many video lines, which makes
not sense for hsync_gap_length.

I wonder if it isn't easier to just change this to v/hsync_blanking_length
(lines for vsync, pixels for hsync) to indicate the length of the blanking
periods, and then let the driver pick a suitable hsync/vsync position.


Dtto.

So the problem here is that if I don't understand what is meant here, how will
a user of this driver be able to understand it?

I think it would be better to give one or two examples of devices and their
configuration. Or refer to freely available documentation, if that is available.


Those two parameters are very rare to be used and it is expected that users with the corresponding HW (signal source/displays) will either be experts on that topic or they will simply get the parameters as "magic numbers" from us after we test the HW and obtain the correct values. As far as I can decipher the info from the HW engineers, this is a quiet non-standard parameter that is hard to find in any documentation, but for some "exotic" systems it is required for the mgb4 to work with the corresponding HW.

Also note that v9 doesn't mentioned default values for these two properties.


I have not put the defaults here because in most cases the "default" is that this value is not used (the signal has VSYNC/HSYNC). And when it is needed, the value has to be changed to fit the HW anyway as chances to match any default value are almost zero. All other values are the defaults of a particular display/entertainment system (Škoda Octavia III)* but here I myself (unlike the HW people) have not even access to any HW to test it...

One can however always read-out the value if he desperately needs the default value.

M.

* Officially this is the reference system that shall be quiet accessible for the customers as this infotainment system is quite widespread in the VW concern, but the real reason is, it is the system laying on my desk ;-)