Re: [PATCH v3 19/31] media: Documentation: Add GS_ROUTING documentation

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

Hi Jacopo,

Thanks for writing the documentation for this!

The text is nice and informative; I have a few suggestions below.

On Tue, Mar 05, 2019 at 07:51:38PM +0100, Jacopo Mondi wrote:
> Add documentation for VIDIOC_SUBDEV_G/S_ROUTING ioctl and add
> description of multiplexed media pads and internal routing to the
> V4L2-subdev documentation section.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/media/uapi/v4l/dev-subdev.rst   |  90 +++++++++++
>  Documentation/media/uapi/v4l/user-func.rst    |   1 +
>  .../uapi/v4l/vidioc-subdev-g-routing.rst      | 142 ++++++++++++++++++
>  3 files changed, 233 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/vidioc-subdev-g-routing.rst
> 
> diff --git a/Documentation/media/uapi/v4l/dev-subdev.rst b/Documentation/media/uapi/v4l/dev-subdev.rst
> index 2c2768c7343b..b9fbb5d2caec 100644
> --- a/Documentation/media/uapi/v4l/dev-subdev.rst
> +++ b/Documentation/media/uapi/v4l/dev-subdev.rst
> @@ -36,6 +36,8 @@ will feature a character device node on which ioctls can be called to
>  
>  -  negotiate image formats on individual pads
>  
> +-  inspect and modify internal data routing between pads of the same entity
> +
>  Sub-device character device nodes, conventionally named
>  ``/dev/v4l-subdev*``, use major number 81.
>  
> @@ -461,3 +463,91 @@ source pads.
>      :maxdepth: 1
>  
>      subdev-formats
> +
> +
> +Multiplexed media pads and internal routing
> +-------------------------------------------
> +
> +Subdevice drivers might expose the internal connections between media pads of an

s/might/may/

> +entity by providing a routing table that applications can inspect and manipulate

s/providing/exposing/

> +to change the internal routing between sink and source pads' data connection
> +endpoints. A routing table is described by a struct
> +:c:type:`v4l2_subdev_routing`, which contains ``num_routes`` route entries, each
> +one represented by a struct :c:type:`v4l2_subdev_route`.
> +
> +Data routes do not just connect one pad to another in an entity, but they refer
> +instead to the ``streams`` a media pad provides. Streams are data connection
> +endpoints in a media pad. Multiplexed media pads expose multiple ``streams``
> +which represent, when the underlying hardware technology allows that, logical
> +data streams transported over a single physical media bus.

How about s/streams/flows/ for this instance?

> +
> +One of the most notable examples of logical stream multiplexing techniques is

s/One of the most notable examples/A noteworthy example/

> +represented by the data interleaving mechanism implemented by mean of Virtual
> +Channels identifiers as defined by the MIPI CSI-2 media bus specifications. A

s/identifiers //

> +subdevice that implements support for Virtual Channel data interleaving might
> +expose up to 4 data ``streams``, one for each available Virtual Channel, on the
> +source media pad that represents a CSI-2 connection endpoint.
> +
> +Routes are defined as potential data connections between a ``(sink_pad,
> +sink_stream)`` pair and a ``(source_pad, source_stream)`` one, where
> +``sink_pad`` and ``source_pad`` are the indexes of two media pads part of the
> +same media entity, and ``sink_stream`` and ``source_stream`` are the identifiers
> +of the data streams to be connected in the media pads. Media pads that do not
> +support data multiplexing expose a single stream, usually with identifier 0.
> +
> +Routes are reported to applications in a routing table which can be
> +inspected and manipulated using the :ref:`routing <VIDIOC_SUBDEV_G_ROUTING>`
> +ioctls.
> +
> +Routes can be activated and deactivated by setting or clearing the
> +V4L2_SUBDEV_ROUTE_FL_ACTIVE flag in the ``flags`` field of struct
> +:c:type:`v4l2_subdev_route`.
> +
> +Subdev driver might create routes that cannot be modified by applications. Such

s/S/A s/

> +routes are identified by the presence of the V4L2_SUBDEV_ROUTE_FL_IMMUTABLE
> +flag in the ``flags`` field of struct :c:type:`v4l2_subdev_route`.
> +
> +As an example, the routing table of a source pad which supports two logical
> +video streams and can be connected to two sink pads is here below described.
> +
> +.. flat-table::
> +    :widths:       1 2 1
> +
> +    * - Pad Index
> +      - Function
> +      - Number of streams
> +    * - 0
> +      - SINK
> +      - 1
> +    * - 1
> +      - SINK
> +      - 1
> +    * - 2
> +      - SOURCE
> +      - 2
> +
> +In such an example, the source media pad will report a routing table with 4
> +entries, one entry for each possible ``(sink_pad, sink_stream) - (source_pad,
> +source_stream)`` combination.
> +
> +.. flat-table:: routing table
> +    :widths:       2 1 2
> +
> +    * - Sink Pad/Sink Stream
> +      - ->
> +      - Source Pad/Source Stream
> +    * - 0/0
> +      - ->
> +      - 2/0
> +    * - 0/0
> +      - ->
> +      - 2/1
> +    * - 1/0
> +      - ->
> +      - 2/0
> +    * - 1/0
> +      - ->
> +      - 2/1
> +
> +Subdev drivers are free to decide how many routes an application can enable on
> +a media pad at the same time, and refuse to enable or disable specific routes.
> diff --git a/Documentation/media/uapi/v4l/user-func.rst b/Documentation/media/uapi/v4l/user-func.rst
> index ca0ef21d77fe..0166446f4ab4 100644
> --- a/Documentation/media/uapi/v4l/user-func.rst
> +++ b/Documentation/media/uapi/v4l/user-func.rst
> @@ -77,6 +77,7 @@ Function Reference
>      vidioc-subdev-g-crop
>      vidioc-subdev-g-fmt
>      vidioc-subdev-g-frame-interval
> +    vidioc-subdev-g-routing
>      vidioc-subdev-g-selection
>      vidioc-subscribe-event
>      func-mmap
> diff --git a/Documentation/media/uapi/v4l/vidioc-subdev-g-routing.rst b/Documentation/media/uapi/v4l/vidioc-subdev-g-routing.rst
> new file mode 100644
> index 000000000000..8b592722c477
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/vidioc-subdev-g-routing.rst
> @@ -0,0 +1,142 @@
> +.. Permission is granted to copy, distribute and/or modify this
> +.. document under the terms of the GNU Free Documentation License,
> +.. Version 1.1 or any later version published by the Free Software
> +.. Foundation, with no Invariant Sections, no Front-Cover Texts
> +.. and no Back-Cover Texts. A copy of the license is included at
> +.. Documentation/media/uapi/fdl-appendix.rst.
> +..
> +.. TODO: replace it to GFDL-1.1-or-later WITH no-invariant-sections
> +
> +.. _VIDIOC_SUBDEV_G_ROUTING:
> +
> +******************************************************
> +ioctl VIDIOC_SUBDEV_G_ROUTING, VIDIOC_SUBDEV_S_ROUTING
> +******************************************************
> +
> +Name
> +====
> +
> +VIDIOC_SUBDEV_G_ROUTING - VIDIOC_SUBDEV_S_ROUTING - Get or set routing between streams of media pads in a media entity.
> +
> +
> +Synopsis
> +========
> +
> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_G_ROUTING, struct v4l2_subdev_routing *argp )
> +    :name: VIDIOC_SUBDEV_G_ROUTING
> +
> +.. c:function:: int ioctl( int fd, VIDIOC_SUBDEV_S_ROUTING, struct v4l2_subdev_routing *argp )
> +    :name: VIDIOC_SUBDEV_S_ROUTING
> +
> +
> +Arguments
> +=========
> +
> +``fd``
> +    File descriptor returned by :ref:`open() <func-open>`.
> +
> +``argp``
> +    Pointer to struct :c:type:`v4l2_subdev_routing`.
> +
> +
> +Description
> +===========
> +
> +These ioctls are used to get and set the routing informations associated to
> +media pads in a media entity. Routing informations are used to enable or disable

The routing is a property of an entity. How about

s/the routing informations associated to media pads in/routing/

s/R[^\.]+(?=\.)/The routing configuration determines the flows of data
inside an entity/

> +data connections between stream endpoints of multiplexed media pads.
> +
> +Drivers report their routing tables using VIDIOC_SUBDEV_G_ROUTING and
> +application use the information there reported to enable or disable data
> +connections between streams in a pad, by setting or clearing the

How about:

s/applications\K.*a pad/may enable or disable routes with the
VIDIOC_SUBDEV_S_ROUTING IOCTL,

> +V4L2_SUBDEV_ROUTE_FL_ACTIVE flag of ``flags`` field of a struct
> +:c:type:`v4l2_subdev_route`.
> +
> +When inspecting routes through VIDIOC_SUBDEV_G_ROUTING and the application
> +provided ``num_routes`` is not big enough to contain all the available routes
> +the subdevice exposes, drivers return the ENOSPC error code and adjust the
> +``num_routes`` value. Application should then reserve enough memory for all the

s/the\K$/value of the/
s/value\K\./field/

> +route entries and call VIDIOC_SUBDEV_G_ROUTING again.
> +
> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
> +
> +.. c:type:: v4l2_subdev_routing
> +
> +.. flat-table:: struct v4l2_subdev_routing
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       1 1 2
> +
> +    * - struct :c:type:`v4l2_subdev_route`
> +      - ``routes[]``
> +      - Array of struct :c:type:`v4l2_subdev_route` entries
> +    * - __u32
> +      - ``num_routes``
> +      - Number of entries of the routes array
> +    * - __u32
> +      - ``reserved``\ [5]
> +      - Reserved for future extensions. Applications and drivers must set
> +	the array to zero.
> +
> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
> +
> +.. c:type:: v4l2_subdev_route
> +
> +.. flat-table:: struct v4l2_subdev_route
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       1 1 2
> +
> +    * - __u32
> +      - ``sink_pad``
> +      - Sink pad number
> +    * - __u32
> +      - ``sink_stream``
> +      - Sink pad stream number
> +    * - __u32
> +      - ``source_stream``
> +      - Source pad stream number
> +    * - __u32
> +      - ``sink_stream``
> +      - Sink pad stream number
> +    * - __u32
> +      - ``flags``
> +      - Route enable/disable flags
> +	:ref:`v4l2_subdev_routing_flags <v4l2-subdev-routing-flags>`.
> +    * - __u32
> +      - ``reserved``\ [5]
> +      - Reserved for future extensions. Applications and drivers must set
> +	the array to zero.
> +
> +.. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
> +
> +.. _v4l2-subdev-routing-flags:
> +
> +.. flat-table:: enum v4l2_subdev_routing_flags
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       3 1 4
> +
> +    * - V4L2_SUBDEV_ROUTE_FL_ACTIVE
> +      - 0
> +      - The route is enabled. Set by applications.
> +    * - V4L2_SUBDEV_ROUTE_FL_IMMUTABLE
> +      - 1
> +      - The route is immutable. Set by the driver.
> +
> +Return Value
> +============
> +
> +On success 0 is returned, on error -1 and the ``errno`` variable is set
> +appropriately. The generic error codes are described at the
> +:ref:`Generic Error Codes <gen-errors>` chapter.
> +
> +ENOSPC
> +   The number of provided route entries is less than the available ones.
> +
> +EINVAL
> +   The sink or source pad identifiers reference a non-existing pad, or refernce
> +   pads of different types (ie. the sink_pad identifiers refers to a source pad)
> +   The sink or source stream identifiers reference a non-existing stream

s/The/or the/

> +   in the sink or source pad.

s/i/o/

> +

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com