Re: [RFC 3/7] media: uapi: v4l: Document source routes

[Hans Verkuil <hverkuil@xs4all.nl>]

On 05/05/2023 23:52, 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     | 18 ++++++++++++++++++
>  .../media/v4l/vidioc-subdev-g-routing.rst      |  5 +++++
>  include/uapi/linux/v4l2-subdev.h               |  6 +++++-
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> index a4f1df7093e8..395e06d2f0f2 100644
> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> @@ -551,6 +551,24 @@ 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:
> +
> +Source routes
> +^^^^^^^^^^^^^
> +
> +Cases where a single sub-device pad is a source of multiple streams are special
> +as there is no sink pad for such a route. In those cases, an internal pad is
> +used as the sink pad. Such pads have the :ref:`MEDIA_PAD_FL_INTERNAL_SOURCE
> +<MEDIA-PAD-FL-INTERNAL-SOURCE>` flag set.

add: instead of the <MEDIA-PAD-FL-SINK> flag.

> +
> +Internal pads have all the properties of a sink pad in such case, 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).
> +
> +Generally source routes are not modifiable but they can be activated and
> +inactivated using the :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE

inactivated -> deactivated

> +<v4l2-subdev-routing-flags>` flag, depending on driver capabilities.
> +

Again, this really needs one or more examples.

>  Interaction between routes, streams, formats and selections
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> index 68ca343c3b44..e00622071b64 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> @@ -94,6 +94,11 @@ for all the route entries and call ``VIDIOC_SUBDEV_G_ROUTING`` again.
>      * - __u32
>        - ``sink_pad``
>        - Sink pad number.
> +    * - __u32
> +      - ``internal_source_pad``
> +      - Internal source pad number. For pads with :ref:`internal source pad flag
> +	<MEDIA-PAD-FL-INTERNAL-SOURCE>`, for use with :ref:`source routes
> +	<v4l2-subdev-source-routes>`.
>      * - __u32
>        - ``sink_stream``
>        - Sink pad stream number.
> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> index 4a195b68f28f..703e3a1f199b 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -203,6 +203,7 @@ struct v4l2_subdev_capability {
>   * struct v4l2_subdev_route - A route inside a subdev
>   *
>   * @sink_pad: the sink pad index
> + * @internal_source_pad: the internal source pad
>   * @sink_stream: the sink stream identifier
>   * @source_pad: the source pad index
>   * @source_stream: the source stream identifier
> @@ -210,7 +211,10 @@ struct v4l2_subdev_capability {
>   * @reserved: drivers and applications must zero this array
>   */
>  struct v4l2_subdev_route {
> -	__u32 sink_pad;
> +	union {
> +		__u32 sink_pad;
> +		__u32 internal_source_pad;

Nobody will understand this. I think this should still be kept as
sink_pad. If the INTERNAL_SOURCE flag is changed to SINK_FROM_INTERNAL_SOURCE
(or something along those lines), then 'sink_pad' still makes sense
and you don't need this change.

It's still a sink pad, just a sink pad with a special property in that it
receives the data from an internal source.

> +	};
>  	__u32 sink_stream;
>  	__u32 source_pad;
>  	__u32 source_stream;

Regards,

	Hans