| Message ID | 20240313072516.241106-4-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Generic line based metadata support, internal pads | expand |
On 13/03/2024 09:24, 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. The CCS spec talks about legacy packing and optimized packing for 16+ bit formats. You cover only the "legacy" ones here. Did you look at those? Tomi
Moi, On Thu, Mar 14, 2024 at 09:30:50AM +0200, Tomi Valkeinen wrote: > On 13/03/2024 09:24, 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. > > The CCS spec talks about legacy packing and optimized packing for 16+ bit > formats. You cover only the "legacy" ones here. Did you look at those? The reason is that the bus data layout of the new packing at higher bit depth matches with packing at lower bit depths (half to be precise). That's why there's no need to define formats for the new packing methods at higher bit depths (the driver simply uses the packing at half of the bit depth).
On 19/03/2024 15:27, Sakari Ailus wrote: > Moi, > > On Thu, Mar 14, 2024 at 09:30:50AM +0200, Tomi Valkeinen wrote: >> On 13/03/2024 09:24, 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. >> >> The CCS spec talks about legacy packing and optimized packing for 16+ bit >> formats. You cover only the "legacy" ones here. Did you look at those? > > The reason is that the bus data layout of the new packing at higher bit > depth matches with packing at lower bit depths (half to be precise). That's > why there's no need to define formats for the new packing methods at higher > bit depths (the driver simply uses the packing at half of the bit depth). Hmm. If we're capturing 10-bit raw format, say, with the width of 640 pixels, we'll configure the video stream format according to those. For the embedded stream, we'll set it to V4L2_META_FMT_GENERIC_CSI2_10 and 640 width, right? If we're capturing 20-bit raw, we'll configure the video stream format again accordingly, width to 640, and 20 bit fourcc/mbus code. If the embedded stream uses the "legacy" packing, we'll set the format to V4L2_META_FMT_GENERIC_CSI2_20 with width of 640, right? But if it's using packed format for the embedded stream, we set the format to V4L2_META_FMT_GENERIC_CSI2_10 and width to 1280? Considering that the video and (line-based) embedded data come from the same source, I'd expect the widths to be the same. Tomi
On Tue, Mar 19, 2024 at 04:20:35PM +0200, Tomi Valkeinen wrote: > On 19/03/2024 15:27, Sakari Ailus wrote: > > On Thu, Mar 14, 2024 at 09:30:50AM +0200, Tomi Valkeinen wrote: > >> On 13/03/2024 09:24, 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. > >> > >> The CCS spec talks about legacy packing and optimized packing for 16+ bit > >> formats. You cover only the "legacy" ones here. Did you look at those? > > > > The reason is that the bus data layout of the new packing at higher bit > > depth matches with packing at lower bit depths (half to be precise). That's > > why there's no need to define formats for the new packing methods at higher > > bit depths (the driver simply uses the packing at half of the bit depth). > > Hmm. If we're capturing 10-bit raw format, say, with the width of 640 > pixels, we'll configure the video stream format according to those. For > the embedded stream, we'll set it to V4L2_META_FMT_GENERIC_CSI2_10 and > 640 width, right? > > If we're capturing 20-bit raw, we'll configure the video stream format > again accordingly, width to 640, and 20 bit fourcc/mbus code. If the > embedded stream uses the "legacy" packing, we'll set the format to > V4L2_META_FMT_GENERIC_CSI2_20 with width of 640, right? > > But if it's using packed format for the embedded stream, we set the > format to V4L2_META_FMT_GENERIC_CSI2_10 and width to 1280? > > Considering that the video and (line-based) embedded data come from the > same source, I'd expect the widths to be the same. I don't have a strong objection against multiplying the width, but we need to figure out the impact on other kernel space components, as well as on userspace. I suppose the media bus code for the embedded data stream on the sensor source pad when using optimized packing and capturing RAW20 images would be MEDIA_BUS_FMT_META_10 ? In that case I think the sensor driver should be able to handle the width calculations on its own, and the value would just be propagated by userspace.
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 */
On Wed, Mar 20, 2024 at 12:33:48AM +0200, Laurent Pinchart wrote: > On Tue, Mar 19, 2024 at 04:20:35PM +0200, Tomi Valkeinen wrote: > > On 19/03/2024 15:27, Sakari Ailus wrote: > > > On Thu, Mar 14, 2024 at 09:30:50AM +0200, Tomi Valkeinen wrote: > > >> On 13/03/2024 09:24, 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. > > >> > > >> The CCS spec talks about legacy packing and optimized packing for 16+ bit > > >> formats. You cover only the "legacy" ones here. Did you look at those? > > > > > > The reason is that the bus data layout of the new packing at higher bit > > > depth matches with packing at lower bit depths (half to be precise). That's > > > why there's no need to define formats for the new packing methods at higher > > > bit depths (the driver simply uses the packing at half of the bit depth). > > > > Hmm. If we're capturing 10-bit raw format, say, with the width of 640 > > pixels, we'll configure the video stream format according to those. For > > the embedded stream, we'll set it to V4L2_META_FMT_GENERIC_CSI2_10 and > > 640 width, right? > > > > If we're capturing 20-bit raw, we'll configure the video stream format > > again accordingly, width to 640, and 20 bit fourcc/mbus code. If the > > embedded stream uses the "legacy" packing, we'll set the format to > > V4L2_META_FMT_GENERIC_CSI2_20 with width of 640, right? > > > > But if it's using packed format for the embedded stream, we set the > > format to V4L2_META_FMT_GENERIC_CSI2_10 and width to 1280? > > > > Considering that the video and (line-based) embedded data come from the > > same source, I'd expect the widths to be the same. > > I don't have a strong objection against multiplying the width, but we > need to figure out the impact on other kernel space components, as well > as on userspace. I suppose the media bus code for the embedded data > stream on the sensor source pad when using optimized packing and > capturing RAW20 images would be MEDIA_BUS_FMT_META_10 ? In that case I > think the sensor driver should be able to handle the width calculations > on its own, and the value would just be propagated by userspace. This should be documented somewhere in this series by the way (not in this patch).
Moi, On Tue, Mar 19, 2024 at 04:20:35PM +0200, Tomi Valkeinen wrote: > On 19/03/2024 15:27, Sakari Ailus wrote: > > Moi, > > > > On Thu, Mar 14, 2024 at 09:30:50AM +0200, Tomi Valkeinen wrote: > > > On 13/03/2024 09:24, 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. > > > > > > The CCS spec talks about legacy packing and optimized packing for 16+ bit > > > formats. You cover only the "legacy" ones here. Did you look at those? > > > > The reason is that the bus data layout of the new packing at higher bit > > depth matches with packing at lower bit depths (half to be precise). That's > > why there's no need to define formats for the new packing methods at higher > > bit depths (the driver simply uses the packing at half of the bit depth). > > Hmm. If we're capturing 10-bit raw format, say, with the width of 640 > pixels, we'll configure the video stream format according to those. For the > embedded stream, we'll set it to V4L2_META_FMT_GENERIC_CSI2_10 and 640 > width, right? > > If we're capturing 20-bit raw, we'll configure the video stream format again > accordingly, width to 640, and 20 bit fourcc/mbus code. If the embedded > stream uses the "legacy" packing, we'll set the format to > V4L2_META_FMT_GENERIC_CSI2_20 with width of 640, right? > > But if it's using packed format for the embedded stream, we set the format > to V4L2_META_FMT_GENERIC_CSI2_10 and width to 1280? The driver sets the embedded data sub-device format apart from the mbus code. The width would be 1280 in this case, yes, and fewer embedded data lines could be needed. > > Considering that the video and (line-based) embedded data come from the same > source, I'd expect the widths to be the same. The width in bytes (with the packing) of embedded data is still the same as for the image data. Pixels don't matter in this case.
Hi Laurent, On Wed, Mar 20, 2024 at 01:00:48AM +0200, Laurent Pinchart wrote: > On Wed, Mar 20, 2024 at 12:33:48AM +0200, Laurent Pinchart wrote: > > On Tue, Mar 19, 2024 at 04:20:35PM +0200, Tomi Valkeinen wrote: > > > On 19/03/2024 15:27, Sakari Ailus wrote: > > > > On Thu, Mar 14, 2024 at 09:30:50AM +0200, Tomi Valkeinen wrote: > > > >> On 13/03/2024 09:24, 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. > > > >> > > > >> The CCS spec talks about legacy packing and optimized packing for 16+ bit > > > >> formats. You cover only the "legacy" ones here. Did you look at those? > > > > > > > > The reason is that the bus data layout of the new packing at higher bit > > > > depth matches with packing at lower bit depths (half to be precise). That's > > > > why there's no need to define formats for the new packing methods at higher > > > > bit depths (the driver simply uses the packing at half of the bit depth). > > > > > > Hmm. If we're capturing 10-bit raw format, say, with the width of 640 > > > pixels, we'll configure the video stream format according to those. For > > > the embedded stream, we'll set it to V4L2_META_FMT_GENERIC_CSI2_10 and > > > 640 width, right? > > > > > > If we're capturing 20-bit raw, we'll configure the video stream format > > > again accordingly, width to 640, and 20 bit fourcc/mbus code. If the > > > embedded stream uses the "legacy" packing, we'll set the format to > > > V4L2_META_FMT_GENERIC_CSI2_20 with width of 640, right? > > > > > > But if it's using packed format for the embedded stream, we set the > > > format to V4L2_META_FMT_GENERIC_CSI2_10 and width to 1280? > > > > > > Considering that the video and (line-based) embedded data come from the > > > same source, I'd expect the widths to be the same. > > > > I don't have a strong objection against multiplying the width, but we > > need to figure out the impact on other kernel space components, as well > > as on userspace. I suppose the media bus code for the embedded data > > stream on the sensor source pad when using optimized packing and > > capturing RAW20 images would be MEDIA_BUS_FMT_META_10 ? In that case I > > think the sensor driver should be able to handle the width calculations > > on its own, and the value would just be propagated by userspace. > > This should be documented somewhere in this series by the way (not in > this patch). This could go to the CCS driver documentation. I modified the last paragraph and added a new one: ------8<----------- Devices supporting embedded data for bit depths greater than or equal to 16 may support more dense packing or legacy single metadata byte per data unit, or both of these. The supported embedded data formats can be enumerated and configured on stream 1 of the source pad (1) of the CCS source sub-device. The use of the denser packing results in embedded data lines being longer than the pixel data in data units since the data units are smaller. In bytes the embedded data lines are still not longer than the image data lines. ------8<----------- I believe the reason for the specs requiring embedded data lines not being longer (in bytes) is most likely that some hardware may have issues reciving the data otherwise for various reasons.
Hi Laurent, On Wed, Mar 20, 2024 at 12:59:48AM +0200, Laurent Pinchart wrote: > 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. I'd really like to get rid of the "internal source" pads as the naming is really confusing (it's present still in this version but not in many locations). They're sink pads after all, so I'd call them such. In a few locations there's text that explains they do represent sources of data within the sub-device itself. > > > + > > +"b" in an array cell signifies a byte of data, followed by the number of the bit > > s/bit$/byte/ Uh, yes. > > > +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'. This is documentation, not C source code where we'd want to denote a single character. Double quotes should thus be used instead. I'll switch to lower case X. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thank you!
On Wed, Mar 20, 2024 at 08:48:17AM +0000, Sakari Ailus wrote: > On Wed, Mar 20, 2024 at 01:00:48AM +0200, Laurent Pinchart wrote: > > On Wed, Mar 20, 2024 at 12:33:48AM +0200, Laurent Pinchart wrote: > > > On Tue, Mar 19, 2024 at 04:20:35PM +0200, Tomi Valkeinen wrote: > > > > On 19/03/2024 15:27, Sakari Ailus wrote: > > > > > On Thu, Mar 14, 2024 at 09:30:50AM +0200, Tomi Valkeinen wrote: > > > > >> On 13/03/2024 09:24, 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. > > > > >> > > > > >> The CCS spec talks about legacy packing and optimized packing for 16+ bit > > > > >> formats. You cover only the "legacy" ones here. Did you look at those? > > > > > > > > > > The reason is that the bus data layout of the new packing at higher bit > > > > > depth matches with packing at lower bit depths (half to be precise). That's > > > > > why there's no need to define formats for the new packing methods at higher > > > > > bit depths (the driver simply uses the packing at half of the bit depth). > > > > > > > > Hmm. If we're capturing 10-bit raw format, say, with the width of 640 > > > > pixels, we'll configure the video stream format according to those. For > > > > the embedded stream, we'll set it to V4L2_META_FMT_GENERIC_CSI2_10 and > > > > 640 width, right? > > > > > > > > If we're capturing 20-bit raw, we'll configure the video stream format > > > > again accordingly, width to 640, and 20 bit fourcc/mbus code. If the > > > > embedded stream uses the "legacy" packing, we'll set the format to > > > > V4L2_META_FMT_GENERIC_CSI2_20 with width of 640, right? > > > > > > > > But if it's using packed format for the embedded stream, we set the > > > > format to V4L2_META_FMT_GENERIC_CSI2_10 and width to 1280? > > > > > > > > Considering that the video and (line-based) embedded data come from the > > > > same source, I'd expect the widths to be the same. > > > > > > I don't have a strong objection against multiplying the width, but we > > > need to figure out the impact on other kernel space components, as well > > > as on userspace. I suppose the media bus code for the embedded data > > > stream on the sensor source pad when using optimized packing and > > > capturing RAW20 images would be MEDIA_BUS_FMT_META_10 ? In that case I > > > think the sensor driver should be able to handle the width calculations > > > on its own, and the value would just be propagated by userspace. > > > > This should be documented somewhere in this series by the way (not in > > this patch). > > This could go to the CCS driver documentation. I modified the last > paragraph and added a new one: > > ------8<----------- > Devices supporting embedded data for bit depths greater than or equal to 16 may > support more dense packing or legacy single metadata byte per data unit, or both > of these. The supported embedded data formats can be enumerated and configured > on stream 1 of the source pad (1) of the CCS source sub-device. > > The use of the denser packing results in embedded data lines being longer than > the pixel data in data units since the data units are smaller. In bytes the > embedded data lines are still not longer than the image data lines. Please document explicitly that e.g. V4L2_META_FMT_GENERIC_CSI2_10 is used for the RAW20 denser packing (in a general way that covers the other formats too). You should also explain more explicitly that the width is doubled in the relevant uAPI data structures. This is not limited to CCS but is applicable to other sensors too, so I'd like that documentation to be in a more generic place. > ------8<----------- > > I believe the reason for the specs requiring embedded data lines not being > longer (in bytes) is most likely that some hardware may have issues > reciving the data otherwise for various reasons.
Hi Sakari, On Wed, Mar 20, 2024 at 04:23:01PM +0000, Sakari Ailus wrote: > On Wed, Mar 20, 2024 at 12:59:48AM +0200, Laurent Pinchart wrote: > > 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. > > I'd really like to get rid of the "internal source" pads as the naming is > really confusing (it's present still in this version but not in many > locations). They're sink pads after all, so I'd call them such. In a few > locations there's text that explains they do represent sources of data > within the sub-device itself. I thought we had agreed to use the term "internal sink pad", but I saw a patch in this series that consistently makes use of "internal source pad". Both are confusing for different reasons, but regardless of which one we pick (that discussion belongs to the other patch that documents the internal source pads), please make sure to use the same terms consistently through the series in v9. > > > + > > > +"b" in an array cell signifies a byte of data, followed by the number of the bit > > > > s/bit$/byte/ > > Uh, yes. > > > > +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'. > > This is documentation, not C source code where we'd want to denote a single > character. Double quotes should thus be used instead. We seem to use single quotes in format documentations, I'd continue doing so for consistency. > I'll switch to lower case X. > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thank you!
On 20/03/2024 00:33, Laurent Pinchart wrote: > On Tue, Mar 19, 2024 at 04:20:35PM +0200, Tomi Valkeinen wrote: >> On 19/03/2024 15:27, Sakari Ailus wrote: >>> On Thu, Mar 14, 2024 at 09:30:50AM +0200, Tomi Valkeinen wrote: >>>> On 13/03/2024 09:24, 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. >>>> >>>> The CCS spec talks about legacy packing and optimized packing for 16+ bit >>>> formats. You cover only the "legacy" ones here. Did you look at those? >>> >>> The reason is that the bus data layout of the new packing at higher bit >>> depth matches with packing at lower bit depths (half to be precise). That's >>> why there's no need to define formats for the new packing methods at higher >>> bit depths (the driver simply uses the packing at half of the bit depth). >> >> Hmm. If we're capturing 10-bit raw format, say, with the width of 640 >> pixels, we'll configure the video stream format according to those. For >> the embedded stream, we'll set it to V4L2_META_FMT_GENERIC_CSI2_10 and >> 640 width, right? >> >> If we're capturing 20-bit raw, we'll configure the video stream format >> again accordingly, width to 640, and 20 bit fourcc/mbus code. If the >> embedded stream uses the "legacy" packing, we'll set the format to >> V4L2_META_FMT_GENERIC_CSI2_20 with width of 640, right? >> >> But if it's using packed format for the embedded stream, we set the >> format to V4L2_META_FMT_GENERIC_CSI2_10 and width to 1280? >> >> Considering that the video and (line-based) embedded data come from the >> same source, I'd expect the widths to be the same. > > I don't have a strong objection against multiplying the width, but we > need to figure out the impact on other kernel space components, as well > as on userspace. I suppose the media bus code for the embedded data > stream on the sensor source pad when using optimized packing and > capturing RAW20 images would be MEDIA_BUS_FMT_META_10 ? In that case I > think the sensor driver should be able to handle the width calculations > on its own, and the value would just be propagated by userspace. Yes, I think it works. I just find it more logical if the widths of both the video and embedded streams are the same (which is the case for all other embedded formats). However, even the CCS spec says "for RAW16, RAW20, and/or RAW24 Visible pixels, top-embedded data may instead be more optimally packed using RAW8, RAW10, and/or RAW12 pixels", so that's in line with what Sakari suggests. Although the spec specifically says "top-embedded", so does it mean that the optimized data is not allowed for bottom data? Tomi
Moi, On Fri, Mar 22, 2024 at 08:50:00AM +0200, Tomi Valkeinen wrote: > On 20/03/2024 00:33, Laurent Pinchart wrote: > > On Tue, Mar 19, 2024 at 04:20:35PM +0200, Tomi Valkeinen wrote: > > > On 19/03/2024 15:27, Sakari Ailus wrote: > > > > On Thu, Mar 14, 2024 at 09:30:50AM +0200, Tomi Valkeinen wrote: > > > > > On 13/03/2024 09:24, 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. > > > > > > > > > > The CCS spec talks about legacy packing and optimized packing for 16+ bit > > > > > formats. You cover only the "legacy" ones here. Did you look at those? > > > > > > > > The reason is that the bus data layout of the new packing at higher bit > > > > depth matches with packing at lower bit depths (half to be precise). That's > > > > why there's no need to define formats for the new packing methods at higher > > > > bit depths (the driver simply uses the packing at half of the bit depth). > > > > > > Hmm. If we're capturing 10-bit raw format, say, with the width of 640 > > > pixels, we'll configure the video stream format according to those. For > > > the embedded stream, we'll set it to V4L2_META_FMT_GENERIC_CSI2_10 and > > > 640 width, right? > > > > > > If we're capturing 20-bit raw, we'll configure the video stream format > > > again accordingly, width to 640, and 20 bit fourcc/mbus code. If the > > > embedded stream uses the "legacy" packing, we'll set the format to > > > V4L2_META_FMT_GENERIC_CSI2_20 with width of 640, right? > > > > > > But if it's using packed format for the embedded stream, we set the > > > format to V4L2_META_FMT_GENERIC_CSI2_10 and width to 1280? > > > > > > Considering that the video and (line-based) embedded data come from the > > > same source, I'd expect the widths to be the same. > > > > I don't have a strong objection against multiplying the width, but we > > need to figure out the impact on other kernel space components, as well > > as on userspace. I suppose the media bus code for the embedded data > > stream on the sensor source pad when using optimized packing and > > capturing RAW20 images would be MEDIA_BUS_FMT_META_10 ? In that case I > > think the sensor driver should be able to handle the width calculations > > on its own, and the value would just be propagated by userspace. > > Yes, I think it works. I just find it more logical if the widths of both the > video and embedded streams are the same (which is the case for all other > embedded formats). > > However, even the CCS spec says "for RAW16, RAW20, and/or RAW24 Visible > pixels, top-embedded data may instead be more optimally packed using RAW8, > RAW10, and/or RAW12 pixels", so that's in line with what Sakari suggests. > Although the spec specifically says "top-embedded", so does it mean that the > optimized data is not allowed for bottom data? I haven't read the spec regarding this but I don't think there should be anything that prevents it: it's the embedded data format. The CCS driver patches only adds support for the top embedded data anyway.
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. + 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. + +"b" in an array cell signifies a byte of data, followed by the number of the bit +and finally the bit number in subscript. "X" indicates a padding bit. + +.. _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 */
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(+)