Re: [PATCH v3 10/10] media: uapi: v4l: Document source routes

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Hans,

Thank you for the review.

On Tue, Aug 08, 2023 at 10:55:24AM +0200, Hans Verkuil wrote:
> On 08/08/2023 09:55, Sakari Ailus wrote:
> > Document how internal pads are used on source routes.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  .../userspace-api/media/v4l/dev-subdev.rst    | 179 ++++++++++++++++++
> >  1 file changed, 179 insertions(+)
> > 
> > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > index b023918177b5..27b0fe2dc83a 100644
> > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > @@ -551,6 +551,27 @@ A stream at a specific point in the media pipeline is identified by the
> >  sub-device and a (pad, stream) pair. For sub-devices that do not support
> >  multiplexed streams the 'stream' field is always 0.
> >  
> > +.. _v4l2-subdev-source-routes:
> > +
> > +Internal pads and source routes
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Cases where a single sub-device source pad is traversed by multiple streams one
> > +or more of which originate from within the sub-device itself are special as
> > +there is no external sink pad for such routes. In those cases, the sources of
> > +the internally generated streams are represented by internal sink pads, which
> > +are sink pads that have the :ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>`
> > +pad flag set.
> > +
> > +Internal pads have all the properties of an external pad, including formats and
> > +selections. The format in this case is the source format of the stream. An
> > +internal pad always has a single stream only (0).
> > +
> > +/Source routes/ are routes from an internal sink pad to a(n external) source
> 
> '/Source routes/' appears just like that in the generated documentation, that's
> probably not what you intended.

This was intended to be *Source routes*. I'll address it in v4.

> 
> > +pad. Generally source routes are not modifiable but they can be activated and
> > +deactivated using the :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE
> > +<v4l2-subdev-routing-flags>` flag, depending on driver capabilities.
> > +
> >  Interaction between routes, streams, formats and selections
> >  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >  
> > @@ -666,3 +687,161 @@ A common way to accomplish this is to start from the sensors and propagate the
> >  configurations along the stream towards the receiver,
> >  using :ref:`VIDIOC_SUBDEV_S_FMT <VIDIOC_SUBDEV_G_FMT>` ioctls to configure each
> >  stream endpoint in each sub-device.
> > +
> > +Internal pads setup example
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +A simple example of a multiplexed stream setup might be as follows:
> > +
> > +- A CCS camera sensor source sub-device, with one sink pad (0), one source pad
> 
> Explain what 'CCS' means: probably a link to MIPI CCS is sufficient, but for newbies
> 'CCS' doesn't provide any information.
> 
> > +  (1), an internal sink pad (2) that represents the source of embedded
> 
> So since the INTERNAL flag is associated with a sink pad, this is now called an
> 'internal sink' instead of 'internal source' as it was before. I agree with that,
> but note that the phrase 'internal source' is still used in several places,
> including the cover letter. For the next version you post, please check for that
> and change the terminology since mixing the two is very confusing :-)

Are you referring to this patch? There don't seem to be any references to
"intenal source pads" left.

> 
> > +  data. There are two routes, one from the sink pad to the source, and another
> > +  from the internal sink pad to the source pad. The embedded data stream needs
> > +  to be enabled by activating the related route. The configuration of the rest
> > +  of the CCS sub-devices is omitted from this example.
> > +
> > +- Multiplexer bridge (Bridge). The bridge has one sink pad, connected to the
> > +  sensor (pad 0), and one source pad (pad 1), which outputs two streams.
> > +
> > +- Receiver in the SoC (Receiver). The receiver has a single sink pad (pad 0),
> > +  connected to the bridge, and two source pads (pads 1-2), going to the DMA
> > +  engine. The receiver demultiplexes the incoming streams to the source pads.
> > +
> > +- DMA Engines in the SoC (DMA Engine), one for each stream. Each DMA engine is
> > +  connected to a single source pad in the receiver.
> > +
> > +The sensor, the bridge and the receiver are modeled as V4L2 sub-devices,
> > +exposed to userspace via /dev/v4l-subdevX device nodes. The DMA engines are
> > +modeled as V4L2 devices, exposed to userspace via /dev/videoX nodes.
> > +
> > +To configure this pipeline, the userspace must take the following steps:
> > +
> > +1) Set up media links between entities: connect the sensors to the bridge,
> > +   bridge to the receiver, and the receiver to the DMA engines. This step does
> > +   not differ from normal non-multiplexed media controller setup.
> > +
> > +2) Configure routing
> > +
> > +.. flat-table:: Camera sensor
> > +    :header-rows: 1
> > +
> > +    * - Sink Pad/Stream
> > +      - Source Pad/Stream
> > +      - Routing Flags
> > +      - Comments
> > +    * - 0/0
> > +      - 1/0
> > +      - V4L2_SUBDEV_ROUTE_FL_ACTIVE
> > +      - Pixel data stream from the sink pad
> 
> Huh? It is a sensor subdev, so aren't both pixel data and metadata
> coming from an internal sink?

The CCS driver exposes three sub-devices, so in this case the sink pad
isn't internal. Almost (?) all other sensor drivers have a single
sub-device only, perhaps I'll change the example to align with one of
those.

> 
> I think you are actually describing the internal subdev of a CCS
> sensor here, but that is not clear from the description at all.
> 
> Since CCS sensors are relatively common (I think), it might be
> good to give a fully fledged example.
> 
> But suppose you have a non-CCS sensor that generates the pixel data
> and metadata, then you would have two internal sink pads and one
> source pad for two streams, right?
> 
> And very simple sensors that currently just have a single source pad
> could be describes as having an internal sink pad connected to the
> source pad by a fixed route. But we don't do that, because it is
> overkill. Correct? If so, then this might be useful information to add
> to answer the question why some sensors have internal sinks and others
> do not.
> 
> > +    * - 2/0
> > +      - 1/1
> > +      - **V4L2_SUBDEV_ROUTE_FL_ACTIVE**
> > +      - Metadata stream from the internal sink pad
> > +
> > +.. flat-table:: Bridge routing table
> > +    :header-rows: 1
> > +
> > +    * - Sink Pad/Stream
> > +      - Source Pad/Stream
> > +      - Routing Flags
> > +      - Comments
> > +    * - 0/0
> > +      - 1/0
> > +      - V4L2_SUBDEV_ROUTE_FL_ACTIVE
> > +      - Pixel data stream from camera sensor
> > +    * - 0/1
> > +      - 1/1
> > +      - V4L2_SUBDEV_ROUTE_FL_ACTIVE
> > +      - Metadata stream from camera sensor
> > +
> > +.. flat-table:: Receiver routing table
> > +    :header-rows:  1
> > +
> > +    * - Sink Pad/Stream
> > +      - Source Pad/Stream
> > +      - Routing Flags
> > +      - Comments
> > +    * - 0/0
> > +      - 1/0
> > +      - V4L2_SUBDEV_ROUTE_FL_ACTIVE
> > +      - Pixel data stream from camera sensor
> > +    * - 0/1
> > +      - 2/0
> > +      - V4L2_SUBDEV_ROUTE_FL_ACTIVE
> > +      - Metadata stream from camera sensor
> > +
> > +The options available in sensor's routing configuration are dictated by hardware
> > +capabilities: typically camera sensors always produce image data stream while
> > +the embedded data stream typically can be either enabled or disabled.
> > +
> > +3) Configure formats and selections
> > +
> > +This example assumes that the formats are propagated from sink pad to the source
> > +pad as-is. The tables contain fields of both struct v4l2_subdev_format and
> > +struct v4l2_mbus_framefmt. The full configuration of CCS camera sensor is out of
> > +scope of this example.
> > +
> > +.. flat-table:: Formats set on the sub-devices. Bold values are set, others are
> > +                static or propagated.
> > +    :header-rows: 1
> > +    :fill-cells:
> > +
> > +    * - Sub-device
> > +      - Pad/Stream
> > +      - Width
> > +      - Height
> > +      - Code
> > +    * - :rspan:`3` Camera sensor sub-device (CCS source sub-device)
> > +      - 0/0
> > +      - 640
> > +      - 480
> > +      - MEDIA_BUS_FMT_SGRBG10
> > +    * - 1/0
> > +      - 640
> > +      - 480
> > +      - **MEDIA_BUS_FMT_SGRBG10**
> > +    * - 2/0
> > +      - 640
> > +      - 2
> > +      - MEDIA_BUS_FMT_CCS_EMBEDDED_10
> > +    * - 1/1
> > +      - 640
> > +      - 2
> > +      - MEDIA_BUS_FMT_META_10
> > +    * - :rspan:`3` Bridge
> > +      - 0/0
> > +      - **640**
> > +      - **480**
> > +      - **MEDIA_BUS_FMT_SGRBG10**
> > +    * - 1/0
> > +      - 640
> > +      - 480
> > +      - MEDIA_BUS_FMT_SGRBG10
> > +    * - 0/1
> > +      - **640**
> > +      - **2**
> > +      - **MEDIA_BUS_FMT_META_10**
> > +    * - 1/1
> > +      - 640
> > +      - 2
> > +      - MEDIA_BUS_FMT_META_10
> > +    * - :rspan:`3` Receiver
> > +      - 0/0
> > +      - **640**
> > +      - **480**
> > +      - **MEDIA_BUS_FMT_SGRBG10**
> > +    * - 1/0
> > +      - 640
> > +      - 480
> > +      - MEDIA_BUS_FMT_SGRBG10
> > +    * - 0/1
> > +      - **640**
> > +      - **2**
> > +      - **MEDIA_BUS_FMT_META_10**
> > +    * - 2/0
> > +      - 640
> > +      - 2
> > +      - MEDIA_BUS_FMT_META_10
> > +
> > +The embedded data format does not need to be configured as the format is
> > +dictated by the pixel data format in this case.

-- 
Kind regards,

Sakari Ailus