Re: [PATCH v2 1/6] dt-bindings: media: video-interfaces: add support for Virtual Channel IDs

On 2/21/25 10:38 AM, Sakari Ailus wrote:

Hi Cosmin,

Thanks for the patches.

On Fri, Feb 21, 2025 at 01:08:09AM +0200, Cosmin Tanislav wrote:

quoted

Multi-camera systems often have issues with receiving video streams
from multiple cameras at the same time because the cameras use the same
Virtual Channel IDs.

CSI bridges might not support remapping the Virtual Channel IDs, making
it impossible to receive the separate video streams at the same
time, while the CSI receiver is able to de-mux streams based on VC IDs.

Cameras sometimes have support for changing the VC IDs they output
themselves.

For a practical example, GMSL2 deserializer chips do not support VC ID
remapping in tunnel mode, and neither do the serializers. Allowing the
cameras to have their VC IDs configured would allow multi-camera setups
to use tunnel mode.

We've tried to avoid having virtual channels in firmware and in UAPI,
I'm not yet entirely convinced we need to depart from the established
practices. Let's see. Apart from that, please see my comments below.

Can you think if any other way of handling this? The most useful way
would be to have it accessible at runtime so that devices upstream of
the cameras could assign the VC IDs dynamically.

This would be useful when having more cameras than the maximum supported
number of VC IDs (4, without extended VC IDs), and streaming from them
selectively.

For example, for 8 cameras, you'd have to prepare your VC IDs in advance
to fit the streaming selection you want to make. If the cameras 0 to 7
have the VC IDs 0, 1, 2, 3, 0, 1, 2, 3, you wouldn't be able to stream
camera 0 together with camera 4.

Dynamic configuration of the VC IDs would solve that usecase since it
would assign VC IDs based on the routed streams.

v4l2_subdev_pad_ops has a .set_frame_desc() that could be used to apply
an updated v4l2_mbus_frame_desc, after retrieving it using
.get_frame_desc(). Cameras that don't support VC ID remapping would just
modify the v4l2_mbus_frame_desc to restore the VC ID to the original one
(similar to how set_fmt() can modify the passed in format to what it
supports) and the caller would have to handle that situation how it sees
fit.

Does that sound better than sticking the VC ID in device tree?

quoted

Add support for specifying these Virtual Channel IDs in Video Interface
Endpoints. The supported values are 0 to 3, with a maximum of 4 values.
Although the CSI-2 specification allows for up to 32 virtual channels,
most hardware doesn't support more than 4. This can be extended later
if need be.

Signed-off-by: Cosmin Tanislav <redacted>
---
  .../devicetree/bindings/media/video-interfaces.yaml   | 11 +++++++++++
  1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.yaml b/Documentation/devicetree/bindings/media/video-interfaces.yaml
index 038e85b45befa..414b5fa8f3472 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.yaml
+++ b/Documentation/devicetree/bindings/media/video-interfaces.yaml

@@ -231,6 +231,17 @@ properties:
        shall be interpreted as 0 (ABC). This property is valid for CSI-2 C-PHY
        busses only.
  
+  vc-ids:

Other properties aren't using abbreviations, at least most of them. How
about "virtual-channels"?

quoted

+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 4

Shouldn't this be 32?

quoted

+    items:
+      maximum: 3

31 here, too.

quoted

+    description:
+      An array of Virtual Channel IDs. These are unsigned integers that specify

I'd leave out the explanation on the data type. It's redundant.

quoted

+      the VC IDs used by the device for its data streams. This property is valid
+      for MIPI CSI-2 only.
+
    strobe:
      $ref: /schemas/types.yaml#/definitions/uint32
      enum: [ 0, 1 ]