Re: [PATCH v8 03/38] media: uapi: Add generic serial metadata mbus formats

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

Thank you for the patch.

On Wed, Mar 13, 2024 at 09:24:41AM +0200, Sakari Ailus wrote:
> Add generic serial metadata mbus formats. These formats describe data
> width and packing but not the content itself. The reason for specifying
> such formats is that the formats as such are fairly device specific but
> they are still handled by CSI-2 receiver drivers that should not be aware
> of device specific formats. What makes generic metadata formats possible
> is that these formats are parsed by software only, after capturing the
> data to system memory.
> 
> Also add a definition for "Data unit" to cover what is essentially a pixel
> but is not image data.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  .../userspace-api/media/glossary.rst          |   9 +
>  .../media/v4l/subdev-formats.rst              | 258 ++++++++++++++++++
>  include/uapi/linux/media-bus-format.h         |   9 +
>  3 files changed, 276 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/glossary.rst b/Documentation/userspace-api/media/glossary.rst
> index ef0ab601b5bf..7078141894c5 100644
> --- a/Documentation/userspace-api/media/glossary.rst
> +++ b/Documentation/userspace-api/media/glossary.rst
> @@ -25,6 +25,15 @@ Glossary
>  
>  	See :ref:`cec`.
>  
> +.. _media-glossary-data-unit:
> +
> +    Data unit
> +
> +	Unit of data transported by a bus. On parallel buses, the data unit
> +	consists of one or more related samples while on serial buses the data
> +	unit is logical. If the data unit is image data, it may also be called a
> +	pixel.

I'm pretty sure nobody will be able to understand what this means, but I
don't have a better proposal at the moment.

> +
>      Device Driver
>  	Part of the Linux Kernel that implements support for a hardware
>  	component.
> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> index eb3cd20b0cf2..cbd475f7cae9 100644
> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> @@ -8306,3 +8306,261 @@ The following table lists the existing metadata formats.
>  	both sides of the link and the bus format is a fixed
>  	metadata format that is not configurable from userspace.
>  	Width and height will be set to 0 for this format.
> +
> +Generic Serial Metadata Formats
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Generic serial metadata formats are used on serial buses where the actual data
> +content is more or less device specific but the data is transmitted and received
> +by multiple devices that do not process the data in any way, simply writing
> +it to system memory for processing in software at the end of the pipeline.
> +
> +The more specific variant describing the actual data is used on the internal
> +source pad of the originating sub-device.

Maybe this paragraph would be best added in the patch that adds the
specific metadata formats, you could then mention one of them as an
example:

The exact format of the data generated by the device is reported on the
internal source pad of the originating sub-device, using one of the more
specific metadata formats such as MEDIA_BUS_FMT_CCS_EMBEDDED.

> +
> +"b" in an array cell signifies a byte of data, followed by the number of the bit

s/bit$/byte/

> +and finally the bit number in subscript. "X" indicates a padding bit.

We use a lower-case x in pixfmt-rgb.rst, I would do the same here. We
also use single quotes there, turning "b" and "x" into 'b' and 'x'.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +.. _media-bus-format-generic-meta:
> +
> +.. cssclass: longtable
> +
> +.. flat-table:: Generic Serial Metadata Formats
> +    :header-rows:  2
> +    :stub-columns: 0
> +
> +    * - Identifier
> +      - Code
> +      -
> +      - :cspan:`23` Data organization within bus ``Data unit
> +	<media-glossary-data-unit>``
> +    * -
> +      -
> +      - Bit
> +      - 23
> +      - 22
> +      - 21
> +      - 20
> +      - 19
> +      - 18
> +      - 17
> +      - 16
> +      - 15
> +      - 14
> +      - 13
> +      - 12
> +      - 11
> +      - 10
> +      - 9
> +      - 8
> +      - 7
> +      - 6
> +      - 5
> +      - 4
> +      - 3
> +      - 2
> +      - 1
> +      - 0
> +    * .. _MEDIA-BUS-FMT-META-8:
> +
> +      - MEDIA_BUS_FMT_META_8
> +      - 0x8001
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      - b0\ :sub:`7`
> +      - b0\ :sub:`6`
> +      - b0\ :sub:`5`
> +      - b0\ :sub:`4`
> +      - b0\ :sub:`3`
> +      - b0\ :sub:`2`
> +      - b0\ :sub:`1`
> +      - b0\ :sub:`0`
> +    * .. _MEDIA-BUS-FMT-META-10:
> +
> +      - MEDIA_BUS_FMT_META_10
> +      - 0x8002
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      - b0\ :sub:`7`
> +      - b0\ :sub:`6`
> +      - b0\ :sub:`5`
> +      - b0\ :sub:`4`
> +      - b0\ :sub:`3`
> +      - b0\ :sub:`2`
> +      - b0\ :sub:`1`
> +      - b0\ :sub:`0`
> +      - X
> +      - X
> +    * .. _MEDIA-BUS-FMT-META-12:
> +
> +      - MEDIA_BUS_FMT_META_12
> +      - 0x8003
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      - b0\ :sub:`7`
> +      - b0\ :sub:`6`
> +      - b0\ :sub:`5`
> +      - b0\ :sub:`4`
> +      - b0\ :sub:`3`
> +      - b0\ :sub:`2`
> +      - b0\ :sub:`1`
> +      - b0\ :sub:`0`
> +      - X
> +      - X
> +      - X
> +      - X
> +    * .. _MEDIA-BUS-FMT-META-14:
> +
> +      - MEDIA_BUS_FMT_META_14
> +      - 0x8004
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      - b0\ :sub:`7`
> +      - b0\ :sub:`6`
> +      - b0\ :sub:`5`
> +      - b0\ :sub:`4`
> +      - b0\ :sub:`3`
> +      - b0\ :sub:`2`
> +      - b0\ :sub:`1`
> +      - b0\ :sub:`0`
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +    * .. _MEDIA-BUS-FMT-META-16:
> +
> +      - MEDIA_BUS_FMT_META_16
> +      - 0x8005
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      - b0\ :sub:`7`
> +      - b0\ :sub:`6`
> +      - b0\ :sub:`5`
> +      - b0\ :sub:`4`
> +      - b0\ :sub:`3`
> +      - b0\ :sub:`2`
> +      - b0\ :sub:`1`
> +      - b0\ :sub:`0`
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +    * .. _MEDIA-BUS-FMT-META-20:
> +
> +      - MEDIA_BUS_FMT_META_20
> +      - 0x8006
> +      -
> +      -
> +      -
> +      -
> +      -
> +      - b0\ :sub:`7`
> +      - b0\ :sub:`6`
> +      - b0\ :sub:`5`
> +      - b0\ :sub:`4`
> +      - b0\ :sub:`3`
> +      - b0\ :sub:`2`
> +      - b0\ :sub:`1`
> +      - b0\ :sub:`0`
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +    * .. _MEDIA-BUS-FMT-META-24:
> +
> +      - MEDIA_BUS_FMT_META_24
> +      - 0x8007
> +      -
> +      - b0\ :sub:`7`
> +      - b0\ :sub:`6`
> +      - b0\ :sub:`5`
> +      - b0\ :sub:`4`
> +      - b0\ :sub:`3`
> +      - b0\ :sub:`2`
> +      - b0\ :sub:`1`
> +      - b0\ :sub:`0`
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> +      - X
> diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
> index f05f747e444d..d4c1d991014b 100644
> --- a/include/uapi/linux/media-bus-format.h
> +++ b/include/uapi/linux/media-bus-format.h
> @@ -174,4 +174,13 @@
>   */
>  #define MEDIA_BUS_FMT_METADATA_FIXED		0x7001
>  
> +/* Generic line based metadata formats for serial buses. Next is 0x8008. */
> +#define MEDIA_BUS_FMT_META_8			0x8001
> +#define MEDIA_BUS_FMT_META_10			0x8002
> +#define MEDIA_BUS_FMT_META_12			0x8003
> +#define MEDIA_BUS_FMT_META_14			0x8004
> +#define MEDIA_BUS_FMT_META_16			0x8005
> +#define MEDIA_BUS_FMT_META_20			0x8006
> +#define MEDIA_BUS_FMT_META_24			0x8007
> +
>  #endif /* __LINUX_MEDIA_BUS_FORMAT_H */

-- 
Regards,

Laurent Pinchart