[PATCH 0/2] media: v4l: Add Broadcom sand format to the list of V4L formats

[PATCH 0/2] media: v4l: Add Broadcom sand format to the list of V4L formats

[John Cox]

This is in preparation for attempting to upstream the Rpi H265 decoder
as these formats are the only ones the hardware can decode to. They are
a column format rather than a tile format but I've added them to the
list of tiled formats as that seems the closest match.

V4L2_PIX_FMT_NV12_C128 matches DRM format NV12 with modifier
DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(ch) and
V4L2_PIX_FMT_P030_C128 matches DRM format P030 with the same modifier.

John Cox (2):
  media: v4l: Add Broadcom sand formats to videodev2.h
  media: v4l: Add documentation for Broadcom sand formats

 .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
 include/uapi/linux/videodev2.h                |   2 +
 2 files changed, 194 insertions(+)

-- 
2.37.2

[PATCH 1/2] media: v4l: Add Broadcom sand formats to videodev2.h

[John Cox]

Add fourccs for Broadcom 8 and 10-bit packed 128 byte column formats to
videodev2.h

Signed-off-by: John Cox <jc@kynesim.co.uk>
---
 include/uapi/linux/videodev2.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 1befd181a4cc..a836322ae5d8 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -656,6 +656,8 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_P010_4L4 v4l2_fourcc('T', '0', '1', '0') /* 12  Y/CbCr 4:2:0 10-bit 4x4 macroblocks */
 #define V4L2_PIX_FMT_NV12_8L128       v4l2_fourcc('A', 'T', '1', '2') /* Y/CbCr 4:2:0 8x128 tiles */
 #define V4L2_PIX_FMT_NV12_10BE_8L128  v4l2_fourcc_be('A', 'X', '1', '2') /* Y/CbCr 4:2:0 10-bit 8x128 tiles */
+#define V4L2_PIX_FMT_NV12_C128        v4l2_fourcc('C', 'N', '1', '2') /* Y/CbCr 4:2:0 128 byte columns */
+#define V4L2_PIX_FMT_P030_C128        v4l2_fourcc('C', 'N', '3', '0') /* Y/CbCr 4:2:0 10-bit packed 128 byte columns */
 
 /* Tiled YUV formats, non contiguous planes */
 #define V4L2_PIX_FMT_NV12MT  v4l2_fourcc('T', 'M', '1', '2') /* 12  Y/CbCr 4:2:0 64x32 tiles */
-- 
2.37.2

[PATCH 2/2] media: v4l: Add documentation for Broadcom sand formats

[John Cox]

Add documentation for for the Broadcom sand formats to pixfmt-yuv-planar.

Signed-off-by: John Cox <jc@kynesim.co.uk>
---
 .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
 1 file changed, 192 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
index f1d5bb7b806d..c1dd0856f497 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
@@ -123,6 +123,20 @@ All components are stored with the same number of bits per component.
       - Cb, Cr
       - Yes
       - 4x4 tiles
+    * - V4L2_PIX_FMT_NV12_C128
+      - 'CN12'
+      - 8
+      - 4:2:0
+      - Cb, Cr
+      - Yes
+      - 128 byte columns
+    * - V4L2_PIX_FMT_P030_C128
+      - 'CN30'
+      - 10
+      - 4:2:0
+      - Cb, Cr
+      - Yes
+      - 128 byte columns
     * - V4L2_PIX_FMT_NV16
       - 'NV16'
       - 8
@@ -277,6 +291,8 @@ of the luma plane.
 .. _V4L2-PIX-FMT-NV12M-10BE-8L128:
 .. _V4L2-PIX-FMT-NV12-10BE-8L128:
 .. _V4L2-PIX-FMT-MM21:
+.. _V4L2-PIX-FMT-NV12-C128:
+.. _V4L2-PIX-FMT-P030-C128:
 
 Tiled NV12
 ----------
@@ -364,6 +380,182 @@ two non-contiguous planes.
 
     Example V4L2_PIX_FMT_NV12MT memory layout of tiles
 
+``V4L2_PIX_FMT_NV21_C128`` store 8 bit luma and chroma data in 128 byte
+columns. Chroma data follows luma in each column. Height, which must be
+a multiple of 2 (h in the table below) determines the offset to the start
+of chroma data. Overall (luma + chroma)
+column height (ch in the table below) is also required and this is obtained
+by dividing the sizeimage by bytesperline. bytesperline is width rounded up
+to the next multiple of the column width (128).
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths: 15 10 10 10 10 4 10 10 10 10
+
+    * - start + 0:
+      - Y'\ :sub:`0,0`
+      - Y'\ :sub:`0,1`
+      - Y'\ :sub:`0,2`
+      - Y'\ :sub:`0,3`
+      - ...
+      - Y'\ :sub:`0,124`
+      - Y'\ :sub:`0,125`
+      - Y'\ :sub:`0,126`
+      - Y'\ :sub:`0,127`
+    * - start + 128:
+      - Y'\ :sub:`1,0`
+      - Y'\ :sub:`1,1`
+      - Y'\ :sub:`1,2`
+      - Y'\ :sub:`1,3`
+      - ...
+      - Y'\ :sub:`1,124`
+      - Y'\ :sub:`1,125`
+      - Y'\ :sub:`1,126`
+      - Y'\ :sub:`1,127`
+    * - start + 256:
+      - Y'\ :sub:`2,0`
+      - Y'\ :sub:`2,1`
+      - Y'\ :sub:`2,2`
+      - Y'\ :sub:`2,3`
+      - ...
+      - Y'\ :sub:`2,124`
+      - Y'\ :sub:`2,125`
+      - Y'\ :sub:`2,126`
+      - Y'\ :sub:`2,127`
+    * - ...
+      - ...
+      - ...
+      - ...
+      - ...
+      - ...
+      - ...
+      - ...
+    * - start + ((h-1) * 128):
+      - Y'\ :sub:`h-1,0`
+      - Y'\ :sub:`h-1,1`
+      - Y'\ :sub:`h-1,2`
+      - Y'\ :sub:`h-1,3`
+      - ...
+      - Y'\ :sub:`h-1,124`
+      - Y'\ :sub:`h-1,125`
+      - Y'\ :sub:`h-1,126`
+      - Y'\ :sub:`h-1,127`
+    * - start + ((h) * 128):
+      - Cb\ :sub:`0,0`
+      - Cr\ :sub:`0,0`
+      - Cb\ :sub:`0,1`
+      - Cr\ :sub:`0,1`
+      - ...
+      - Cb\ :sub:`0,62`
+      - Cr\ :sub:`0,62`
+      - Cb\ :sub:`0,63`
+      - Cr\ :sub:`0,63`
+    * - start + ((h+1) * 128):
+      - Cb\ :sub:`1,0`
+      - Cr\ :sub:`1,0`
+      - Cb\ :sub:`1,1`
+      - Cr\ :sub:`1,1`
+      - ...
+      - Cb\ :sub:`1,62`
+      - Cr\ :sub:`1,62`
+      - Cb\ :sub:`1,63`
+      - Cr\ :sub:`1,63`
+    * - ...
+      - ...
+      - ...
+      - ...
+      - ...
+      - ...
+      - ...
+      - ...
+    * - start + ((h+(h/2)-1) * 128):
+      - Cb\ :sub:`(h/2)-1,0`
+      - Cr\ :sub:`(h/2)-1,0`
+      - Cb\ :sub:`(h/2)-1,1`
+      - Cr\ :sub:`(h/2)-1,1`
+      - ...
+      - Cb\ :sub:`(h/2)-1,62`
+      - Cr\ :sub:`(h/2)-1,62`
+      - Cb\ :sub:`(h/2)-1,63`
+      - Cr\ :sub:`(h/2)-1,63`
+    * - start + (ch * 128):
+      - Y'\ :sub:`0,128`
+      - Y'\ :sub:`0,129`
+      - Y'\ :sub:`0,130`
+      - Y'\ :sub:`0,131`
+      - ...
+      - Y'\ :sub:`0,252`
+      - Y'\ :sub:`0,253`
+      - Y'\ :sub:`0,254`
+      - Y'\ :sub:`0,255`
+    * - ...
+      - ...
+      - ...
+      - ...
+      - ...
+      - ...
+      - ...
+      - ...
+
+``V4L2_PIX_FMT_P030_C128`` uses the same 128 byte column structure as
+``V4L2_PIX_FMT_NV12_C128``, but encodes 10-bit YUV.
+3 10-bit values are packed into 4 bytes as bits 9:0, 19:10, and 29:20, with
+bits 30 & 31 unused. For the luma plane, bits 9:0 are Y0, 19:10 are Y1, and
+29:20 are Y2. For the chroma plane the samples always come in pairs of Cr
+and Cb, so it needs to be considered 6 values packed in 8 bytes. bytesperline
+is ((width + 95)/96)*128
+
+Bit-packed representation - Luma:
+
+.. flat-table::
+    :header-rows:  1
+    :stub-columns: 0
+
+    * - byte
+      - value(s)
+    * - 0
+      - Y'\ :sub:`00[7:0]`
+    * - 1
+      - Y'\ :sub:`01[5:0]`\  (bits 7--2)
+      - Y'\ :sub:`00[9:8]`\  (bits 1--0)
+    * - 2
+      - Y'\ :sub:`02[3:0]`\  (bits 7--4)
+      - Y'\ :sub:`01[9:6]`\  (bits 3--0)
+    * - 3
+      - unused (bits 7--6)
+      - Y'\ :sub:`02[9:4]`\  (bits 5--0)
+
+Bit-packed representation - Chroma:
+
+.. flat-table::
+    :header-rows:  1
+    :stub-columns: 0
+
+    * - byte
+      - value(s)
+    * - 0
+      - Cb\ :sub:`00[7:0]`
+    * - 1
+      - Cr\ :sub:`00[5:0]`\  (bits 7--2)
+      - Cb\ :sub:`00[9:8]`\  (bits 1--0)
+    * - 2
+      - Cb\ :sub:`01[3:0]`\  (bits 7--4)
+      - Cr\ :sub:`00[9:6]`\  (bits 3--0)
+    * - 3
+      - unused (bits 7--6)
+      - Cb\ :sub:`02[9:4]`\  (bits 5--0)
+    * - 4
+      - Cr\ :sub:`01[7:0]`
+    * - 5
+      - Cb\ :sub:`02[5:0]`\  (bits 7--2)
+      - Cr\ :sub:`01[9:8]`\  (bits 1--0)
+    * - 6
+      - Cr\ :sub:`02[3:0]`\  (bits 7--4)
+      - Cb\ :sub:`02[9:6]`\  (bits 3--0)
+    * - 7
+      - unused (bits 7--6)
+      - Cr\ :sub:`02[9:4]`\  (bits 5--0)
 
 .. _V4L2-PIX-FMT-NV16:
 .. _V4L2-PIX-FMT-NV61:
-- 
2.37.2

Re: [PATCH 0/2] media: v4l: Add Broadcom sand format to the list of V4L formats

[Nicolas Dufresne]

Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
> This is in preparation for attempting to upstream the Rpi H265 decoder
> as these formats are the only ones the hardware can decode to. They are
> a column format rather than a tile format but I've added them to the
> list of tiled formats as that seems the closest match.
> 
> V4L2_PIX_FMT_NV12_C128 matches DRM format NV12 with modifier
> DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(ch) and
> V4L2_PIX_FMT_P030_C128 matches DRM format P030 with the same modifier.

Cause pixel format matching is hard, P030 matches GStreamer NV12_10LE32, format
also found on Xilinx ZynMP CODECs (but without any modifiers so far).

This is just for curiosity, was there any software implementation of these
formats made available publicly ? or have they only been tested in conjunction
with an importing HW ?

> 
> John Cox (2):
>   media: v4l: Add Broadcom sand formats to videodev2.h
>   media: v4l: Add documentation for Broadcom sand formats
> 
>  .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
>  include/uapi/linux/videodev2.h                |   2 +
>  2 files changed, 194 insertions(+)
> 

Re: [PATCH 1/2] media: v4l: Add Broadcom sand formats to videodev2.h

[Nicolas Dufresne]

Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
> Add fourccs for Broadcom 8 and 10-bit packed 128 byte column formats to
> videodev2.h
> 
> Signed-off-by: John Cox <jc@kynesim.co.uk>
> ---
>  include/uapi/linux/videodev2.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 1befd181a4cc..a836322ae5d8 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -656,6 +656,8 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_P010_4L4 v4l2_fourcc('T', '0', '1', '0') /* 12  Y/CbCr 4:2:0 10-bit 4x4 macroblocks */
>  #define V4L2_PIX_FMT_NV12_8L128       v4l2_fourcc('A', 'T', '1', '2') /* Y/CbCr 4:2:0 8x128 tiles */
>  #define V4L2_PIX_FMT_NV12_10BE_8L128  v4l2_fourcc_be('A', 'X', '1', '2') /* Y/CbCr 4:2:0 10-bit 8x128 tiles */
> +#define V4L2_PIX_FMT_NV12_C128        v4l2_fourcc('C', 'N', '1', '2') /* Y/CbCr 4:2:0 128 byte columns */
> +#define V4L2_PIX_FMT_P030_C128        v4l2_fourcc('C', 'N', '3', '0') /* Y/CbCr 4:2:0 10-bit packed 128 byte columns */
>  
>  /* Tiled YUV formats, non contiguous planes */
>  #define V4L2_PIX_FMT_NV12MT  v4l2_fourcc('T', 'M', '1', '2') /* 12  Y/CbCr 4:2:0 64x32 tiles */

I would expect updates to v4l2-common.c and v4l2-ioctl.c to be in the same
patch. And then the driver should be using the helpers there whenever possible.

regards,
Nicolas

Re: [PATCH 2/2] media: v4l: Add documentation for Broadcom sand formats

[Nicolas Dufresne]

Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
> Add documentation for for the Broadcom sand formats to pixfmt-yuv-planar.

Not a review comment, but does SAND stand for anything you can share ?

> 
> Signed-off-by: John Cox <jc@kynesim.co.uk>
> ---
>  .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
>  1 file changed, 192 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> index f1d5bb7b806d..c1dd0856f497 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> @@ -123,6 +123,20 @@ All components are stored with the same number of bits per component.
>        - Cb, Cr
>        - Yes
>        - 4x4 tiles
> +    * - V4L2_PIX_FMT_NV12_C128
> +      - 'CN12'
> +      - 8
> +      - 4:2:0
> +      - Cb, Cr
> +      - Yes
> +      - 128 byte columns
> +    * - V4L2_PIX_FMT_P030_C128
> +      - 'CN30'
> +      - 10
> +      - 4:2:0
> +      - Cb, Cr
> +      - Yes
> +      - 128 byte columns
>      * - V4L2_PIX_FMT_NV16
>        - 'NV16'
>        - 8
> @@ -277,6 +291,8 @@ of the luma plane.
>  .. _V4L2-PIX-FMT-NV12M-10BE-8L128:
>  .. _V4L2-PIX-FMT-NV12-10BE-8L128:
>  .. _V4L2-PIX-FMT-MM21:
> +.. _V4L2-PIX-FMT-NV12-C128:
> +.. _V4L2-PIX-FMT-P030-C128:
>  
>  Tiled NV12
>  ----------
> @@ -364,6 +380,182 @@ two non-contiguous planes.
>  
>      Example V4L2_PIX_FMT_NV12MT memory layout of tiles
>  
> +``V4L2_PIX_FMT_NV21_C128`` store 8 bit luma and chroma data in 128 byte

I think you meant V4L2_PIX_FMT_NV12_C128 ?

> +columns. Chroma data follows luma in each column. Height, which must be
> +a multiple of 2 (h in the table below) determines the offset to the start
> +of chroma data. Overall (luma + chroma)
> +column height (ch in the table below) is also required and this is obtained
> +by dividing the sizeimage by bytesperline. bytesperline is width rounded up
> +to the next multiple of the column width (128).

Assuming I understood the description, unlike any other tiled formats, the Y and
UV columns are not being split in two plane, but instead Y and UV columns are
being interleaved. Am I understanding correctly ? I think the text could be
improved in this regard.

Note, I totally agree on using NV12 as reference, I see lower that CbCr is
effectively interleaved.

> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths: 15 10 10 10 10 4 10 10 10 10
> +
> +    * - start + 0:
> +      - Y'\ :sub:`0,0`
> +      - Y'\ :sub:`0,1`
> +      - Y'\ :sub:`0,2`
> +      - Y'\ :sub:`0,3`
> +      - ...
> +      - Y'\ :sub:`0,124`
> +      - Y'\ :sub:`0,125`
> +      - Y'\ :sub:`0,126`
> +      - Y'\ :sub:`0,127`
> +    * - start + 128:
> +      - Y'\ :sub:`1,0`
> +      - Y'\ :sub:`1,1`
> +      - Y'\ :sub:`1,2`
> +      - Y'\ :sub:`1,3`
> +      - ...
> +      - Y'\ :sub:`1,124`
> +      - Y'\ :sub:`1,125`
> +      - Y'\ :sub:`1,126`
> +      - Y'\ :sub:`1,127`
> +    * - start + 256:
> +      - Y'\ :sub:`2,0`
> +      - Y'\ :sub:`2,1`
> +      - Y'\ :sub:`2,2`
> +      - Y'\ :sub:`2,3`
> +      - ...
> +      - Y'\ :sub:`2,124`
> +      - Y'\ :sub:`2,125`
> +      - Y'\ :sub:`2,126`
> +      - Y'\ :sub:`2,127`
> +    * - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +    * - start + ((h-1) * 128):
> +      - Y'\ :sub:`h-1,0`
> +      - Y'\ :sub:`h-1,1`
> +      - Y'\ :sub:`h-1,2`
> +      - Y'\ :sub:`h-1,3`
> +      - ...
> +      - Y'\ :sub:`h-1,124`
> +      - Y'\ :sub:`h-1,125`
> +      - Y'\ :sub:`h-1,126`
> +      - Y'\ :sub:`h-1,127`
> +    * - start + ((h) * 128):
> +      - Cb\ :sub:`0,0`
> +      - Cr\ :sub:`0,0`
> +      - Cb\ :sub:`0,1`
> +      - Cr\ :sub:`0,1`
> +      - ...
> +      - Cb\ :sub:`0,62`
> +      - Cr\ :sub:`0,62`
> +      - Cb\ :sub:`0,63`
> +      - Cr\ :sub:`0,63`
> +    * - start + ((h+1) * 128):
> +      - Cb\ :sub:`1,0`
> +      - Cr\ :sub:`1,0`
> +      - Cb\ :sub:`1,1`
> +      - Cr\ :sub:`1,1`
> +      - ...
> +      - Cb\ :sub:`1,62`
> +      - Cr\ :sub:`1,62`
> +      - Cb\ :sub:`1,63`
> +      - Cr\ :sub:`1,63`
> +    * - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +    * - start + ((h+(h/2)-1) * 128):
> +      - Cb\ :sub:`(h/2)-1,0`
> +      - Cr\ :sub:`(h/2)-1,0`
> +      - Cb\ :sub:`(h/2)-1,1`
> +      - Cr\ :sub:`(h/2)-1,1`
> +      - ...
> +      - Cb\ :sub:`(h/2)-1,62`
> +      - Cr\ :sub:`(h/2)-1,62`
> +      - Cb\ :sub:`(h/2)-1,63`
> +      - Cr\ :sub:`(h/2)-1,63`
> +    * - start + (ch * 128):
> +      - Y'\ :sub:`0,128`
> +      - Y'\ :sub:`0,129`
> +      - Y'\ :sub:`0,130`
> +      - Y'\ :sub:`0,131`
> +      - ...
> +      - Y'\ :sub:`0,252`
> +      - Y'\ :sub:`0,253`
> +      - Y'\ :sub:`0,254`
> +      - Y'\ :sub:`0,255`
> +    * - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +      - ...
> +
> +``V4L2_PIX_FMT_P030_C128`` uses the same 128 byte column structure as
> +``V4L2_PIX_FMT_NV12_C128``, but encodes 10-bit YUV.
> +3 10-bit values are packed into 4 bytes as bits 9:0, 19:10, and 29:20, with
> +bits 30 & 31 unused. For the luma plane, bits 9:0 are Y0, 19:10 are Y1, and
> +29:20 are Y2. For the chroma plane the samples always come in pairs of Cr
> +and Cb, so it needs to be considered 6 values packed in 8 bytes. bytesperline
> +is ((width + 95)/96)*128

Maybe insert a phrase related to the use of the same columns pattern as
NV12_C128 ? Then from there, its easier to understand why this bytesperline
formula. I'm guessing that 96 is the number of pixels that fits 1 row of 128
bytes (128 / 4 * 3). I could not guess why you need 128 times that size though ?
Maybe I'm missing something ?

> +
> +Bit-packed representation - Luma:
> +
> +.. flat-table::
> +    :header-rows:  1
> +    :stub-columns: 0
> +
> +    * - byte
> +      - value(s)
> +    * - 0
> +      - Y'\ :sub:`00[7:0]`
> +    * - 1
> +      - Y'\ :sub:`01[5:0]`\  (bits 7--2)
> +      - Y'\ :sub:`00[9:8]`\  (bits 1--0)
> +    * - 2
> +      - Y'\ :sub:`02[3:0]`\  (bits 7--4)
> +      - Y'\ :sub:`01[9:6]`\  (bits 3--0)
> +    * - 3
> +      - unused (bits 7--6)
> +      - Y'\ :sub:`02[9:4]`\  (bits 5--0)
> +
> +Bit-packed representation - Chroma:
> +
> +.. flat-table::
> +    :header-rows:  1
> +    :stub-columns: 0
> +
> +    * - byte
> +      - value(s)
> +    * - 0
> +      - Cb\ :sub:`00[7:0]`
> +    * - 1
> +      - Cr\ :sub:`00[5:0]`\  (bits 7--2)
> +      - Cb\ :sub:`00[9:8]`\  (bits 1--0)
> +    * - 2
> +      - Cb\ :sub:`01[3:0]`\  (bits 7--4)
> +      - Cr\ :sub:`00[9:6]`\  (bits 3--0)
> +    * - 3
> +      - unused (bits 7--6)
> +      - Cb\ :sub:`02[9:4]`\  (bits 5--0)
> +    * - 4
> +      - Cr\ :sub:`01[7:0]`
> +    * - 5
> +      - Cb\ :sub:`02[5:0]`\  (bits 7--2)
> +      - Cr\ :sub:`01[9:8]`\  (bits 1--0)
> +    * - 6
> +      - Cr\ :sub:`02[3:0]`\  (bits 7--4)
> +      - Cb\ :sub:`02[9:6]`\  (bits 3--0)
> +    * - 7
> +      - unused (bits 7--6)
> +      - Cr\ :sub:`02[9:4]`\  (bits 5--0)
>  
>  .. _V4L2-PIX-FMT-NV16:
>  .. _V4L2-PIX-FMT-NV61:

Re: [PATCH 0/2] media: v4l: Add Broadcom sand format to the list of V4L formats

[John Cox]

Hi

>Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
>> This is in preparation for attempting to upstream the Rpi H265 decoder
>> as these formats are the only ones the hardware can decode to. They are
>> a column format rather than a tile format but I've added them to the
>> list of tiled formats as that seems the closest match.
>> 
>> V4L2_PIX_FMT_NV12_C128 matches DRM format NV12 with modifier
>> DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(ch) and
>> V4L2_PIX_FMT_P030_C128 matches DRM format P030 with the same modifier.
>
>Cause pixel format matching is hard, P030 matches GStreamer NV12_10LE32, format
>also found on Xilinx ZynMP CODECs (but without any modifiers so far).
>
>This is just for curiosity, was there any software implementation of these
>formats made available publicly ? or have they only been tested in conjunction
>with an importing HW ?

I'm unsure exactly what you are asking here.

I don't think that anyone other than RPi/Broadcom has used these formats
for anything. I've certainly written code that uses and converts them
that has been on my public github and has been used by RPi but I doubt
that is what you meant by "Publicly". V4L2_PIX_FMT_NV12_C128 is annoying
to use in s/w (though I have written s/w parts of a decoder that use
it), V4L2_PIX_FMT_P030_C128 is stupidly annoying to use in s/w and all
I've ever written is code to convert it to something more useable.

Does that answer the question?
 
>> John Cox (2):
>>   media: v4l: Add Broadcom sand formats to videodev2.h
>>   media: v4l: Add documentation for Broadcom sand formats
>> 
>>  .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
>>  include/uapi/linux/videodev2.h                |   2 +
>>  2 files changed, 194 insertions(+)
>> 

Re: [PATCH 2/2] media: v4l: Add documentation for Broadcom sand formats

[John Cox]

>Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
>> Add documentation for for the Broadcom sand formats to pixfmt-yuv-planar.
>
>Not a review comment, but does SAND stand for anything you can share ?

Honestly I don't know - I got the term from Broadcom's MMAL definitions.
 
>> Signed-off-by: John Cox <jc@kynesim.co.uk>
>> ---
>>  .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
>>  1 file changed, 192 insertions(+)
>> 
>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
>> index f1d5bb7b806d..c1dd0856f497 100644
>> --- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
>> @@ -123,6 +123,20 @@ All components are stored with the same number of bits per component.
>>        - Cb, Cr
>>        - Yes
>>        - 4x4 tiles
>> +    * - V4L2_PIX_FMT_NV12_C128
>> +      - 'CN12'
>> +      - 8
>> +      - 4:2:0
>> +      - Cb, Cr
>> +      - Yes
>> +      - 128 byte columns
>> +    * - V4L2_PIX_FMT_P030_C128
>> +      - 'CN30'
>> +      - 10
>> +      - 4:2:0
>> +      - Cb, Cr
>> +      - Yes
>> +      - 128 byte columns
>>      * - V4L2_PIX_FMT_NV16
>>        - 'NV16'
>>        - 8
>> @@ -277,6 +291,8 @@ of the luma plane.
>>  .. _V4L2-PIX-FMT-NV12M-10BE-8L128:
>>  .. _V4L2-PIX-FMT-NV12-10BE-8L128:
>>  .. _V4L2-PIX-FMT-MM21:
>> +.. _V4L2-PIX-FMT-NV12-C128:
>> +.. _V4L2-PIX-FMT-P030-C128:
>>  
>>  Tiled NV12
>>  ----------
>> @@ -364,6 +380,182 @@ two non-contiguous planes.
>>  
>>      Example V4L2_PIX_FMT_NV12MT memory layout of tiles
>>  
>> +``V4L2_PIX_FMT_NV21_C128`` store 8 bit luma and chroma data in 128 byte
>
>I think you meant V4L2_PIX_FMT_NV12_C128 ?
Yes - will fix

>> +columns. Chroma data follows luma in each column. Height, which must be
>> +a multiple of 2 (h in the table below) determines the offset to the start
>> +of chroma data. Overall (luma + chroma)
>> +column height (ch in the table below) is also required and this is obtained
>> +by dividing the sizeimage by bytesperline. bytesperline is width rounded up
>> +to the next multiple of the column width (128).
>
>Assuming I understood the description, unlike any other tiled formats, the Y and
>UV columns are not being split in two plane, but instead Y and UV columns are
>being interleaved. Am I understanding correctly ? I think the text could be
>improved in this regard.

I'll see what I can do

Its a somewhat confusing format. You get a 128-byte wide, full picture Y
height column of luma, optional padding, followed by a 128-byte wide,
full chroma height column of interleaved CrCb, more optional padding,
then the start of the next luma column.

The thing that makes it uniqely annoying as far as representation is
concerned is that in conventional terms it effectively has a fixed pitch
or bytesperline of 128 (i.e. you always add 128 to get from x,y to
x,y+1) and it has a "vertical pitch" to go between columns which doesn't
fit into any normal picture description.

>Note, I totally agree on using NV12 as reference, I see lower that CbCr is
>effectively interleaved.
>
>> +
>> +.. flat-table::
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths: 15 10 10 10 10 4 10 10 10 10
>> +
>> +    * - start + 0:
>> +      - Y'\ :sub:`0,0`
>> +      - Y'\ :sub:`0,1`
>> +      - Y'\ :sub:`0,2`
>> +      - Y'\ :sub:`0,3`
>> +      - ...
>> +      - Y'\ :sub:`0,124`
>> +      - Y'\ :sub:`0,125`
>> +      - Y'\ :sub:`0,126`
>> +      - Y'\ :sub:`0,127`
>> +    * - start + 128:
>> +      - Y'\ :sub:`1,0`
>> +      - Y'\ :sub:`1,1`
>> +      - Y'\ :sub:`1,2`
>> +      - Y'\ :sub:`1,3`
>> +      - ...
>> +      - Y'\ :sub:`1,124`
>> +      - Y'\ :sub:`1,125`
>> +      - Y'\ :sub:`1,126`
>> +      - Y'\ :sub:`1,127`
>> +    * - start + 256:
>> +      - Y'\ :sub:`2,0`
>> +      - Y'\ :sub:`2,1`
>> +      - Y'\ :sub:`2,2`
>> +      - Y'\ :sub:`2,3`
>> +      - ...
>> +      - Y'\ :sub:`2,124`
>> +      - Y'\ :sub:`2,125`
>> +      - Y'\ :sub:`2,126`
>> +      - Y'\ :sub:`2,127`
>> +    * - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +    * - start + ((h-1) * 128):
>> +      - Y'\ :sub:`h-1,0`
>> +      - Y'\ :sub:`h-1,1`
>> +      - Y'\ :sub:`h-1,2`
>> +      - Y'\ :sub:`h-1,3`
>> +      - ...
>> +      - Y'\ :sub:`h-1,124`
>> +      - Y'\ :sub:`h-1,125`
>> +      - Y'\ :sub:`h-1,126`
>> +      - Y'\ :sub:`h-1,127`
>> +    * - start + ((h) * 128):
>> +      - Cb\ :sub:`0,0`
>> +      - Cr\ :sub:`0,0`
>> +      - Cb\ :sub:`0,1`
>> +      - Cr\ :sub:`0,1`
>> +      - ...
>> +      - Cb\ :sub:`0,62`
>> +      - Cr\ :sub:`0,62`
>> +      - Cb\ :sub:`0,63`
>> +      - Cr\ :sub:`0,63`
>> +    * - start + ((h+1) * 128):
>> +      - Cb\ :sub:`1,0`
>> +      - Cr\ :sub:`1,0`
>> +      - Cb\ :sub:`1,1`
>> +      - Cr\ :sub:`1,1`
>> +      - ...
>> +      - Cb\ :sub:`1,62`
>> +      - Cr\ :sub:`1,62`
>> +      - Cb\ :sub:`1,63`
>> +      - Cr\ :sub:`1,63`
>> +    * - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +    * - start + ((h+(h/2)-1) * 128):
>> +      - Cb\ :sub:`(h/2)-1,0`
>> +      - Cr\ :sub:`(h/2)-1,0`
>> +      - Cb\ :sub:`(h/2)-1,1`
>> +      - Cr\ :sub:`(h/2)-1,1`
>> +      - ...
>> +      - Cb\ :sub:`(h/2)-1,62`
>> +      - Cr\ :sub:`(h/2)-1,62`
>> +      - Cb\ :sub:`(h/2)-1,63`
>> +      - Cr\ :sub:`(h/2)-1,63`
>> +    * - start + (ch * 128):
>> +      - Y'\ :sub:`0,128`
>> +      - Y'\ :sub:`0,129`
>> +      - Y'\ :sub:`0,130`
>> +      - Y'\ :sub:`0,131`
>> +      - ...
>> +      - Y'\ :sub:`0,252`
>> +      - Y'\ :sub:`0,253`
>> +      - Y'\ :sub:`0,254`
>> +      - Y'\ :sub:`0,255`
>> +    * - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +      - ...
>> +
>> +``V4L2_PIX_FMT_P030_C128`` uses the same 128 byte column structure as
>> +``V4L2_PIX_FMT_NV12_C128``, but encodes 10-bit YUV.
>> +3 10-bit values are packed into 4 bytes as bits 9:0, 19:10, and 29:20, with
>> +bits 30 & 31 unused. For the luma plane, bits 9:0 are Y0, 19:10 are Y1, and
>> +29:20 are Y2. For the chroma plane the samples always come in pairs of Cr
>> +and Cb, so it needs to be considered 6 values packed in 8 bytes. bytesperline
>> +is ((width + 95)/96)*128
>
>Maybe insert a phrase related to the use of the same columns pattern as
>NV12_C128 ? Then from there, its easier to understand why this bytesperline
>formula. I'm guessing that 96 is the number of pixels that fits 1 row of 128
>bytes (128 / 4 * 3). I could not guess why you need 128 times that size though ?
>Maybe I'm missing something ?

96 is indeed the number of pixels that fits in one 128 byte column.

(width+95)/96 is the number of columns required, mutiplying it by 128
gets you a "pitch" that when mutiplied by height gives you the number of
bytes occupied by the luma plane. This is mostly to keep software that
expects width <= bytesperline happy.

>> +
>> +Bit-packed representation - Luma:
>> +
>> +.. flat-table::
>> +    :header-rows:  1
>> +    :stub-columns: 0
>> +
>> +    * - byte
>> +      - value(s)
>> +    * - 0
>> +      - Y'\ :sub:`00[7:0]`
>> +    * - 1
>> +      - Y'\ :sub:`01[5:0]`\  (bits 7--2)
>> +      - Y'\ :sub:`00[9:8]`\  (bits 1--0)
>> +    * - 2
>> +      - Y'\ :sub:`02[3:0]`\  (bits 7--4)
>> +      - Y'\ :sub:`01[9:6]`\  (bits 3--0)
>> +    * - 3
>> +      - unused (bits 7--6)
>> +      - Y'\ :sub:`02[9:4]`\  (bits 5--0)
>> +
>> +Bit-packed representation - Chroma:
>> +
>> +.. flat-table::
>> +    :header-rows:  1
>> +    :stub-columns: 0
>> +
>> +    * - byte
>> +      - value(s)
>> +    * - 0
>> +      - Cb\ :sub:`00[7:0]`
>> +    * - 1
>> +      - Cr\ :sub:`00[5:0]`\  (bits 7--2)
>> +      - Cb\ :sub:`00[9:8]`\  (bits 1--0)
>> +    * - 2
>> +      - Cb\ :sub:`01[3:0]`\  (bits 7--4)
>> +      - Cr\ :sub:`00[9:6]`\  (bits 3--0)
>> +    * - 3
>> +      - unused (bits 7--6)
>> +      - Cb\ :sub:`02[9:4]`\  (bits 5--0)
>> +    * - 4
>> +      - Cr\ :sub:`01[7:0]`
>> +    * - 5
>> +      - Cb\ :sub:`02[5:0]`\  (bits 7--2)
>> +      - Cr\ :sub:`01[9:8]`\  (bits 1--0)
>> +    * - 6
>> +      - Cr\ :sub:`02[3:0]`\  (bits 7--4)
>> +      - Cb\ :sub:`02[9:6]`\  (bits 3--0)
>> +    * - 7
>> +      - unused (bits 7--6)
>> +      - Cr\ :sub:`02[9:4]`\  (bits 5--0)
>>  
>>  .. _V4L2-PIX-FMT-NV16:
>>  .. _V4L2-PIX-FMT-NV61:

Re: [PATCH 1/2] media: v4l: Add Broadcom sand formats to videodev2.h

[John Cox]

Hi

>Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
>> Add fourccs for Broadcom 8 and 10-bit packed 128 byte column formats to
>> videodev2.h
>> 
>> Signed-off-by: John Cox <jc@kynesim.co.uk>
>> ---
>>  include/uapi/linux/videodev2.h | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 1befd181a4cc..a836322ae5d8 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -656,6 +656,8 @@ struct v4l2_pix_format {
>>  #define V4L2_PIX_FMT_P010_4L4 v4l2_fourcc('T', '0', '1', '0') /* 12  Y/CbCr 4:2:0 10-bit 4x4 macroblocks */
>>  #define V4L2_PIX_FMT_NV12_8L128       v4l2_fourcc('A', 'T', '1', '2') /* Y/CbCr 4:2:0 8x128 tiles */
>>  #define V4L2_PIX_FMT_NV12_10BE_8L128  v4l2_fourcc_be('A', 'X', '1', '2') /* Y/CbCr 4:2:0 10-bit 8x128 tiles */
>> +#define V4L2_PIX_FMT_NV12_C128        v4l2_fourcc('C', 'N', '1', '2') /* Y/CbCr 4:2:0 128 byte columns */
>> +#define V4L2_PIX_FMT_P030_C128        v4l2_fourcc('C', 'N', '3', '0') /* Y/CbCr 4:2:0 10-bit packed 128 byte columns */
>>  
>>  /* Tiled YUV formats, non contiguous planes */
>>  #define V4L2_PIX_FMT_NV12MT  v4l2_fourcc('T', 'M', '1', '2') /* 12  Y/CbCr 4:2:0 64x32 tiles */
>
>I would expect updates to v4l2-common.c and v4l2-ioctl.c to be in the same
>patch. And then the driver should be using the helpers there whenever possible.

Fair point - I'll fix that.

What is the correct .bpp for 3 10-bit pixels packed into 4 bytes in the
v4l2_format_info?

Regards

John Cox

Re: [PATCH 2/2] media: v4l: Add documentation for Broadcom sand formats

[Dave Stevenson]

Hi Nicolas (and John)

On Thu, 9 Feb 2023 at 18:54, John Cox <jc@kynesim.co.uk> wrote:
>
> >Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
> >> Add documentation for for the Broadcom sand formats to pixfmt-yuv-planar.
> >
> >Not a review comment, but does SAND stand for anything you can share ?
>
> Honestly I don't know - I got the term from Broadcom's MMAL definitions.

Broadcom acquired Sand Video back in 2004, and it was the format that
they used in their video codec IP that was then integrated into
VideoCore. Internally it was always referred to as "Sand Video
Format", or just Sand.

  Dave

https://www.eetimes.com/broadcom-pays-77-5-million-for-sand-video/

> >> Signed-off-by: John Cox <jc@kynesim.co.uk>
> >> ---
> >>  .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
> >>  1 file changed, 192 insertions(+)
> >>
> >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> >> index f1d5bb7b806d..c1dd0856f497 100644
> >> --- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> >> @@ -123,6 +123,20 @@ All components are stored with the same number of bits per component.
> >>        - Cb, Cr
> >>        - Yes
> >>        - 4x4 tiles
> >> +    * - V4L2_PIX_FMT_NV12_C128
> >> +      - 'CN12'
> >> +      - 8
> >> +      - 4:2:0
> >> +      - Cb, Cr
> >> +      - Yes
> >> +      - 128 byte columns
> >> +    * - V4L2_PIX_FMT_P030_C128
> >> +      - 'CN30'
> >> +      - 10
> >> +      - 4:2:0
> >> +      - Cb, Cr
> >> +      - Yes
> >> +      - 128 byte columns
> >>      * - V4L2_PIX_FMT_NV16
> >>        - 'NV16'
> >>        - 8
> >> @@ -277,6 +291,8 @@ of the luma plane.
> >>  .. _V4L2-PIX-FMT-NV12M-10BE-8L128:
> >>  .. _V4L2-PIX-FMT-NV12-10BE-8L128:
> >>  .. _V4L2-PIX-FMT-MM21:
> >> +.. _V4L2-PIX-FMT-NV12-C128:
> >> +.. _V4L2-PIX-FMT-P030-C128:
> >>
> >>  Tiled NV12
> >>  ----------
> >> @@ -364,6 +380,182 @@ two non-contiguous planes.
> >>
> >>      Example V4L2_PIX_FMT_NV12MT memory layout of tiles
> >>
> >> +``V4L2_PIX_FMT_NV21_C128`` store 8 bit luma and chroma data in 128 byte
> >
> >I think you meant V4L2_PIX_FMT_NV12_C128 ?
> Yes - will fix
>
> >> +columns. Chroma data follows luma in each column. Height, which must be
> >> +a multiple of 2 (h in the table below) determines the offset to the start
> >> +of chroma data. Overall (luma + chroma)
> >> +column height (ch in the table below) is also required and this is obtained
> >> +by dividing the sizeimage by bytesperline. bytesperline is width rounded up
> >> +to the next multiple of the column width (128).
> >
> >Assuming I understood the description, unlike any other tiled formats, the Y and
> >UV columns are not being split in two plane, but instead Y and UV columns are
> >being interleaved. Am I understanding correctly ? I think the text could be
> >improved in this regard.
>
> I'll see what I can do
>
> Its a somewhat confusing format. You get a 128-byte wide, full picture Y
> height column of luma, optional padding, followed by a 128-byte wide,
> full chroma height column of interleaved CrCb, more optional padding,
> then the start of the next luma column.
>
> The thing that makes it uniqely annoying as far as representation is
> concerned is that in conventional terms it effectively has a fixed pitch
> or bytesperline of 128 (i.e. you always add 128 to get from x,y to
> x,y+1) and it has a "vertical pitch" to go between columns which doesn't
> fit into any normal picture description.
>
> >Note, I totally agree on using NV12 as reference, I see lower that CbCr is
> >effectively interleaved.
> >
> >> +
> >> +.. flat-table::
> >> +    :header-rows:  0
> >> +    :stub-columns: 0
> >> +    :widths: 15 10 10 10 10 4 10 10 10 10
> >> +
> >> +    * - start + 0:
> >> +      - Y'\ :sub:`0,0`
> >> +      - Y'\ :sub:`0,1`
> >> +      - Y'\ :sub:`0,2`
> >> +      - Y'\ :sub:`0,3`
> >> +      - ...
> >> +      - Y'\ :sub:`0,124`
> >> +      - Y'\ :sub:`0,125`
> >> +      - Y'\ :sub:`0,126`
> >> +      - Y'\ :sub:`0,127`
> >> +    * - start + 128:
> >> +      - Y'\ :sub:`1,0`
> >> +      - Y'\ :sub:`1,1`
> >> +      - Y'\ :sub:`1,2`
> >> +      - Y'\ :sub:`1,3`
> >> +      - ...
> >> +      - Y'\ :sub:`1,124`
> >> +      - Y'\ :sub:`1,125`
> >> +      - Y'\ :sub:`1,126`
> >> +      - Y'\ :sub:`1,127`
> >> +    * - start + 256:
> >> +      - Y'\ :sub:`2,0`
> >> +      - Y'\ :sub:`2,1`
> >> +      - Y'\ :sub:`2,2`
> >> +      - Y'\ :sub:`2,3`
> >> +      - ...
> >> +      - Y'\ :sub:`2,124`
> >> +      - Y'\ :sub:`2,125`
> >> +      - Y'\ :sub:`2,126`
> >> +      - Y'\ :sub:`2,127`
> >> +    * - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +    * - start + ((h-1) * 128):
> >> +      - Y'\ :sub:`h-1,0`
> >> +      - Y'\ :sub:`h-1,1`
> >> +      - Y'\ :sub:`h-1,2`
> >> +      - Y'\ :sub:`h-1,3`
> >> +      - ...
> >> +      - Y'\ :sub:`h-1,124`
> >> +      - Y'\ :sub:`h-1,125`
> >> +      - Y'\ :sub:`h-1,126`
> >> +      - Y'\ :sub:`h-1,127`
> >> +    * - start + ((h) * 128):
> >> +      - Cb\ :sub:`0,0`
> >> +      - Cr\ :sub:`0,0`
> >> +      - Cb\ :sub:`0,1`
> >> +      - Cr\ :sub:`0,1`
> >> +      - ...
> >> +      - Cb\ :sub:`0,62`
> >> +      - Cr\ :sub:`0,62`
> >> +      - Cb\ :sub:`0,63`
> >> +      - Cr\ :sub:`0,63`
> >> +    * - start + ((h+1) * 128):
> >> +      - Cb\ :sub:`1,0`
> >> +      - Cr\ :sub:`1,0`
> >> +      - Cb\ :sub:`1,1`
> >> +      - Cr\ :sub:`1,1`
> >> +      - ...
> >> +      - Cb\ :sub:`1,62`
> >> +      - Cr\ :sub:`1,62`
> >> +      - Cb\ :sub:`1,63`
> >> +      - Cr\ :sub:`1,63`
> >> +    * - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +    * - start + ((h+(h/2)-1) * 128):
> >> +      - Cb\ :sub:`(h/2)-1,0`
> >> +      - Cr\ :sub:`(h/2)-1,0`
> >> +      - Cb\ :sub:`(h/2)-1,1`
> >> +      - Cr\ :sub:`(h/2)-1,1`
> >> +      - ...
> >> +      - Cb\ :sub:`(h/2)-1,62`
> >> +      - Cr\ :sub:`(h/2)-1,62`
> >> +      - Cb\ :sub:`(h/2)-1,63`
> >> +      - Cr\ :sub:`(h/2)-1,63`
> >> +    * - start + (ch * 128):
> >> +      - Y'\ :sub:`0,128`
> >> +      - Y'\ :sub:`0,129`
> >> +      - Y'\ :sub:`0,130`
> >> +      - Y'\ :sub:`0,131`
> >> +      - ...
> >> +      - Y'\ :sub:`0,252`
> >> +      - Y'\ :sub:`0,253`
> >> +      - Y'\ :sub:`0,254`
> >> +      - Y'\ :sub:`0,255`
> >> +    * - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +
> >> +``V4L2_PIX_FMT_P030_C128`` uses the same 128 byte column structure as
> >> +``V4L2_PIX_FMT_NV12_C128``, but encodes 10-bit YUV.
> >> +3 10-bit values are packed into 4 bytes as bits 9:0, 19:10, and 29:20, with
> >> +bits 30 & 31 unused. For the luma plane, bits 9:0 are Y0, 19:10 are Y1, and
> >> +29:20 are Y2. For the chroma plane the samples always come in pairs of Cr
> >> +and Cb, so it needs to be considered 6 values packed in 8 bytes. bytesperline
> >> +is ((width + 95)/96)*128
> >
> >Maybe insert a phrase related to the use of the same columns pattern as
> >NV12_C128 ? Then from there, its easier to understand why this bytesperline
> >formula. I'm guessing that 96 is the number of pixels that fits 1 row of 128
> >bytes (128 / 4 * 3). I could not guess why you need 128 times that size though ?
> >Maybe I'm missing something ?
>
> 96 is indeed the number of pixels that fits in one 128 byte column.
>
> (width+95)/96 is the number of columns required, mutiplying it by 128
> gets you a "pitch" that when mutiplied by height gives you the number of
> bytes occupied by the luma plane. This is mostly to keep software that
> expects width <= bytesperline happy.
>
> >> +
> >> +Bit-packed representation - Luma:
> >> +
> >> +.. flat-table::
> >> +    :header-rows:  1
> >> +    :stub-columns: 0
> >> +
> >> +    * - byte
> >> +      - value(s)
> >> +    * - 0
> >> +      - Y'\ :sub:`00[7:0]`
> >> +    * - 1
> >> +      - Y'\ :sub:`01[5:0]`\  (bits 7--2)
> >> +      - Y'\ :sub:`00[9:8]`\  (bits 1--0)
> >> +    * - 2
> >> +      - Y'\ :sub:`02[3:0]`\  (bits 7--4)
> >> +      - Y'\ :sub:`01[9:6]`\  (bits 3--0)
> >> +    * - 3
> >> +      - unused (bits 7--6)
> >> +      - Y'\ :sub:`02[9:4]`\  (bits 5--0)
> >> +
> >> +Bit-packed representation - Chroma:
> >> +
> >> +.. flat-table::
> >> +    :header-rows:  1
> >> +    :stub-columns: 0
> >> +
> >> +    * - byte
> >> +      - value(s)
> >> +    * - 0
> >> +      - Cb\ :sub:`00[7:0]`
> >> +    * - 1
> >> +      - Cr\ :sub:`00[5:0]`\  (bits 7--2)
> >> +      - Cb\ :sub:`00[9:8]`\  (bits 1--0)
> >> +    * - 2
> >> +      - Cb\ :sub:`01[3:0]`\  (bits 7--4)
> >> +      - Cr\ :sub:`00[9:6]`\  (bits 3--0)
> >> +    * - 3
> >> +      - unused (bits 7--6)
> >> +      - Cb\ :sub:`02[9:4]`\  (bits 5--0)
> >> +    * - 4
> >> +      - Cr\ :sub:`01[7:0]`
> >> +    * - 5
> >> +      - Cb\ :sub:`02[5:0]`\  (bits 7--2)
> >> +      - Cr\ :sub:`01[9:8]`\  (bits 1--0)
> >> +    * - 6
> >> +      - Cr\ :sub:`02[3:0]`\  (bits 7--4)
> >> +      - Cb\ :sub:`02[9:6]`\  (bits 3--0)
> >> +    * - 7
> >> +      - unused (bits 7--6)
> >> +      - Cr\ :sub:`02[9:4]`\  (bits 5--0)
> >>
> >>  .. _V4L2-PIX-FMT-NV16:
> >>  .. _V4L2-PIX-FMT-NV61:

Re: [PATCH 0/2] media: v4l: Add Broadcom sand format to the list of V4L formats

[Nicolas Dufresne]

Le jeudi 09 février 2023 à 18:21 +0000, John Cox a écrit :
> Hi
> 
> > Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
> > > This is in preparation for attempting to upstream the Rpi H265 decoder
> > > as these formats are the only ones the hardware can decode to. They are
> > > a column format rather than a tile format but I've added them to the
> > > list of tiled formats as that seems the closest match.
> > > 
> > > V4L2_PIX_FMT_NV12_C128 matches DRM format NV12 with modifier
> > > DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(ch) and
> > > V4L2_PIX_FMT_P030_C128 matches DRM format P030 with the same modifier.
> > 
> > Cause pixel format matching is hard, P030 matches GStreamer NV12_10LE32, format
> > also found on Xilinx ZynMP CODECs (but without any modifiers so far).
> > 
> > This is just for curiosity, was there any software implementation of these
> > formats made available publicly ? or have they only been tested in conjunction
> > with an importing HW ?
> 
> I'm unsure exactly what you are asking here.
> 
> I don't think that anyone other than RPi/Broadcom has used these formats
> for anything. I've certainly written code that uses and converts them
> that has been on my public github and has been used by RPi but I doubt
> that is what you meant by "Publicly". V4L2_PIX_FMT_NV12_C128 is annoying
> to use in s/w (though I have written s/w parts of a decoder that use
> it), V4L2_PIX_FMT_P030_C128 is stupidly annoying to use in s/w and all
> I've ever written is code to convert it to something more useable.
> 
> Does that answer the question?

Well, whatever you have and you can share a link to would be nice, it does help
reviewing your doc. But I think I understand what it is from your doc so far, so
nothing to worry about.

As a side note, for boards that are readily available in KernelCI, I often
implement slow path converter in GStreamer so we can run it through fluster and
catch any regressions. It is very minimal regression tests simply using what ITU
made publicly available.

>  
> > > John Cox (2):
> > >   media: v4l: Add Broadcom sand formats to videodev2.h
> > >   media: v4l: Add documentation for Broadcom sand formats
> > > 
> > >  .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
> > >  include/uapi/linux/videodev2.h                |   2 +
> > >  2 files changed, 194 insertions(+)
> > > 

Re: [PATCH 2/2] media: v4l: Add documentation for Broadcom sand formats

[Nicolas Dufresne]

Le jeudi 09 février 2023 à 18:54 +0000, John Cox a écrit :
> > Maybe insert a phrase related to the use of the same columns pattern as
> > NV12_C128 ? Then from there, its easier to understand why this bytesperline
> > formula. I'm guessing that 96 is the number of pixels that fits 1 row of 128
> > bytes (128 / 4 * 3). I could not guess why you need 128 times that size
> > though ?
> > Maybe I'm missing something ?
> 
> 96 is indeed the number of pixels that fits in one 128 byte column.
> 
> (width+95)/96 is the number of columns required, mutiplying it by 128
> gets you a "pitch" that when mutiplied by height gives you the number of
> bytes occupied by the luma plane. This is mostly to keep software that
> expects width <= bytesperline happy.

Thanks, now that you say it, its kind of obvious, its pixel to byte conversion.
And I agree this is probably the best emulation we can do, and documenting it is
important. Notion of stride is also a bit fuzzy with other tile formats. And I
suspect this convention is also that one use on DRM side ?

Nicolas

Re: [PATCH 0/2] media: v4l: Add Broadcom sand format to the list of V4L formats

[John Cox]

>Le jeudi 09 février 2023 à 18:21 +0000, John Cox a écrit :
>> Hi
>> 
>> > Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
>> > > This is in preparation for attempting to upstream the Rpi H265 decoder
>> > > as these formats are the only ones the hardware can decode to. They are
>> > > a column format rather than a tile format but I've added them to the
>> > > list of tiled formats as that seems the closest match.
>> > > 
>> > > V4L2_PIX_FMT_NV12_C128 matches DRM format NV12 with modifier
>> > > DRM_FORMAT_MOD_BROADCOM_SAND128_COL_HEIGHT(ch) and
>> > > V4L2_PIX_FMT_P030_C128 matches DRM format P030 with the same modifier.
>> > 
>> > Cause pixel format matching is hard, P030 matches GStreamer NV12_10LE32, format
>> > also found on Xilinx ZynMP CODECs (but without any modifiers so far).
>> > 
>> > This is just for curiosity, was there any software implementation of these
>> > formats made available publicly ? or have they only been tested in conjunction
>> > with an importing HW ?
>> 
>> I'm unsure exactly what you are asking here.
>> 
>> I don't think that anyone other than RPi/Broadcom has used these formats
>> for anything. I've certainly written code that uses and converts them
>> that has been on my public github and has been used by RPi but I doubt
>> that is what you meant by "Publicly". V4L2_PIX_FMT_NV12_C128 is annoying
>> to use in s/w (though I have written s/w parts of a decoder that use
>> it), V4L2_PIX_FMT_P030_C128 is stupidly annoying to use in s/w and all
>> I've ever written is code to convert it to something more useable.
>> 
>> Does that answer the question?
>
>Well, whatever you have and you can share a link to would be nice, it does help
>reviewing your doc. But I think I understand what it is from your doc so far, so
>nothing to worry about.

There are more files including arvm7 & v8 neon converters but this has C
converters out of sand8/30 to YUV420/NV12/VUV420P10: 

https://github.com/jc-kynesim/rpi-ffmpeg/blob/test/5.1.2/main/libavutil/rpi_sand_fns.c

which should give you what you need. V4L2_PIX_FMT_NV12_C128 =
AV_PIX_FMT_RPI4_8, V4L2_PIX_FMT_P030_C128 = AV_PIX_FMT_RPI4_10.

Ignore code referencing AV_PIX_FMT_SAND64_10, that format is now
obsolete (10 bits in 16, so 64 pixels per col) and was only used by the
Pi3 gpu assisted H265 s/w decode.

>As a side note, for boards that are readily available in KernelCI, I often
>implement slow path converter in GStreamer so we can run it through fluster and
>catch any regressions. It is very minimal regression tests simply using what ITU
>made publicly available.

Oh absolutely. Whilst I admit I haven't done proper fate integration
(yet) I do run a test script against all the ITU conformance streams (+
a few others that had bad cases missed by ITU) on a regular basis.

Regards

John Cox
  
>> > > John Cox (2):
>> > >   media: v4l: Add Broadcom sand formats to videodev2.h
>> > >   media: v4l: Add documentation for Broadcom sand formats
>> > > 
>> > >  .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
>> > >  include/uapi/linux/videodev2.h                |   2 +
>> > >  2 files changed, 194 insertions(+)
>> > > 

Re: [PATCH 1/2] media: v4l: Add Broadcom sand formats to videodev2.h

[Nicolas Dufresne]

Le jeudi 09 février 2023 à 19:06 +0000, John Cox a écrit :
> Hi
> 
> > Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
> > > Add fourccs for Broadcom 8 and 10-bit packed 128 byte column formats to
> > > videodev2.h
> > > 
> > > Signed-off-by: John Cox <jc@kynesim.co.uk>
> > > ---
> > >  include/uapi/linux/videodev2.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > > index 1befd181a4cc..a836322ae5d8 100644
> > > --- a/include/uapi/linux/videodev2.h
> > > +++ b/include/uapi/linux/videodev2.h
> > > @@ -656,6 +656,8 @@ struct v4l2_pix_format {
> > >  #define V4L2_PIX_FMT_P010_4L4 v4l2_fourcc('T', '0', '1', '0') /* 12  Y/CbCr 4:2:0 10-bit 4x4 macroblocks */
> > >  #define V4L2_PIX_FMT_NV12_8L128       v4l2_fourcc('A', 'T', '1', '2') /* Y/CbCr 4:2:0 8x128 tiles */
> > >  #define V4L2_PIX_FMT_NV12_10BE_8L128  v4l2_fourcc_be('A', 'X', '1', '2') /* Y/CbCr 4:2:0 10-bit 8x128 tiles */
> > > +#define V4L2_PIX_FMT_NV12_C128        v4l2_fourcc('C', 'N', '1', '2') /* Y/CbCr 4:2:0 128 byte columns */
> > > +#define V4L2_PIX_FMT_P030_C128        v4l2_fourcc('C', 'N', '3', '0') /* Y/CbCr 4:2:0 10-bit packed 128 byte columns */
> > >  
> > >  /* Tiled YUV formats, non contiguous planes */
> > >  #define V4L2_PIX_FMT_NV12MT  v4l2_fourcc('T', 'M', '1', '2') /* 12  Y/CbCr 4:2:0 64x32 tiles */
> > 
> > I would expect updates to v4l2-common.c and v4l2-ioctl.c to be in the same
> > patch. And then the driver should be using the helpers there whenever possible.
> 
> Fair point - I'll fix that.
> 
> What is the correct .bpp for 3 10-bit pixels packed into 4 bytes in the
> v4l2_format_info?

Good question, maybe this can be done with the fractional bpp support. I must
admit, I didn't think about padded cases, I was handling 10bit fully packed over
5 bytes.

https://lore.kernel.org/linux-arm-kernel/20230103170058.810597-3-benjamin.gaignard@collabora.com/
My case ended working with:

 { .format = V4L2_PIX_FMT_NV12_10LE40_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2 },

Question is what do we do about comp_planes, if we kind of fake it to be 2, then maybe this would work.

 { .format = V4L2_PIX_FMT_P030_C128, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 4, 8, 0, 0 }, .bpp_div = { 3, 3, 1, 1 }, .hdiv = 2, .vdiv = 2 },

For weird format, this is a bit of hacky, all we want is to get the right
stride, and the offset part is not used for mem_planes = 1 formats.

let me know,
Nicolas

> 
> Regards
> 
> John Cox

Re: [PATCH 1/2] media: v4l: Add Broadcom sand formats to videodev2.h

[John Cox]

Hi

>Le jeudi 09 février 2023 à 19:06 +0000, John Cox a écrit :
>> Hi
>> 
>> > Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
>> > > Add fourccs for Broadcom 8 and 10-bit packed 128 byte column formats to
>> > > videodev2.h
>> > > 
>> > > Signed-off-by: John Cox <jc@kynesim.co.uk>
>> > > ---
>> > >  include/uapi/linux/videodev2.h | 2 ++
>> > >  1 file changed, 2 insertions(+)
>> > > 
>> > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> > > index 1befd181a4cc..a836322ae5d8 100644
>> > > --- a/include/uapi/linux/videodev2.h
>> > > +++ b/include/uapi/linux/videodev2.h
>> > > @@ -656,6 +656,8 @@ struct v4l2_pix_format {
>> > >  #define V4L2_PIX_FMT_P010_4L4 v4l2_fourcc('T', '0', '1', '0') /* 12  Y/CbCr 4:2:0 10-bit 4x4 macroblocks */
>> > >  #define V4L2_PIX_FMT_NV12_8L128       v4l2_fourcc('A', 'T', '1', '2') /* Y/CbCr 4:2:0 8x128 tiles */
>> > >  #define V4L2_PIX_FMT_NV12_10BE_8L128  v4l2_fourcc_be('A', 'X', '1', '2') /* Y/CbCr 4:2:0 10-bit 8x128 tiles */
>> > > +#define V4L2_PIX_FMT_NV12_C128        v4l2_fourcc('C', 'N', '1', '2') /* Y/CbCr 4:2:0 128 byte columns */
>> > > +#define V4L2_PIX_FMT_P030_C128        v4l2_fourcc('C', 'N', '3', '0') /* Y/CbCr 4:2:0 10-bit packed 128 byte columns */
>> > >  
>> > >  /* Tiled YUV formats, non contiguous planes */
>> > >  #define V4L2_PIX_FMT_NV12MT  v4l2_fourcc('T', 'M', '1', '2') /* 12  Y/CbCr 4:2:0 64x32 tiles */
>> > 
>> > I would expect updates to v4l2-common.c and v4l2-ioctl.c to be in the same
>> > patch. And then the driver should be using the helpers there whenever possible.
>> 
>> Fair point - I'll fix that.
>> 
>> What is the correct .bpp for 3 10-bit pixels packed into 4 bytes in the
>> v4l2_format_info?
>
>Good question, maybe this can be done with the fractional bpp support. I must
>admit, I didn't think about padded cases, I was handling 10bit fully packed over
>5 bytes.
>
>https://lore.kernel.org/linux-arm-kernel/20230103170058.810597-3-benjamin.gaignard@collabora.com/
>My case ended working with:
>
> { .format = V4L2_PIX_FMT_NV12_10LE40_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2 },
>
>Question is what do we do about comp_planes, if we kind of fake it to be 2, then maybe this would work.
>
> { .format = V4L2_PIX_FMT_P030_C128, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 4, 8, 0, 0 }, .bpp_div = { 3, 3, 1, 1 }, .hdiv = 2, .vdiv = 2 },
>
>For weird format, this is a bit of hacky, all we want is to get the right
>stride, and the offset part is not used for mem_planes = 1 formats.

I'm happy with whatever the consensus says is "right".

What tree/branch should I be patching against? Code to support the above
doesn't seem to be in git://linuxtv.org/media_tree:master which is what
I was using.

Thanks

John Cox

>let me know,
>Nicolas
>
>> 
>> Regards
>> 
>> John Cox

Re: [PATCH 1/2] media: v4l: Add Broadcom sand formats to videodev2.h

[Nicolas Dufresne]

Le dimanche 12 février 2023 à 18:49 +0000, John Cox a écrit :
> Hi
> 
> > Le jeudi 09 février 2023 à 19:06 +0000, John Cox a écrit :
> > > Hi
> > > 
> > > > Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
> > > > > Add fourccs for Broadcom 8 and 10-bit packed 128 byte column formats to
> > > > > videodev2.h
> > > > > 
> > > > > Signed-off-by: John Cox <jc@kynesim.co.uk>
> > > > > ---
> > > > >  include/uapi/linux/videodev2.h | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > > 
> > > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > > > > index 1befd181a4cc..a836322ae5d8 100644
> > > > > --- a/include/uapi/linux/videodev2.h
> > > > > +++ b/include/uapi/linux/videodev2.h
> > > > > @@ -656,6 +656,8 @@ struct v4l2_pix_format {
> > > > >  #define V4L2_PIX_FMT_P010_4L4 v4l2_fourcc('T', '0', '1', '0') /* 12  Y/CbCr 4:2:0 10-bit 4x4 macroblocks */
> > > > >  #define V4L2_PIX_FMT_NV12_8L128       v4l2_fourcc('A', 'T', '1', '2') /* Y/CbCr 4:2:0 8x128 tiles */
> > > > >  #define V4L2_PIX_FMT_NV12_10BE_8L128  v4l2_fourcc_be('A', 'X', '1', '2') /* Y/CbCr 4:2:0 10-bit 8x128 tiles */
> > > > > +#define V4L2_PIX_FMT_NV12_C128        v4l2_fourcc('C', 'N', '1', '2') /* Y/CbCr 4:2:0 128 byte columns */
> > > > > +#define V4L2_PIX_FMT_P030_C128        v4l2_fourcc('C', 'N', '3', '0') /* Y/CbCr 4:2:0 10-bit packed 128 byte columns */
> > > > >  
> > > > >  /* Tiled YUV formats, non contiguous planes */
> > > > >  #define V4L2_PIX_FMT_NV12MT  v4l2_fourcc('T', 'M', '1', '2') /* 12  Y/CbCr 4:2:0 64x32 tiles */
> > > > 
> > > > I would expect updates to v4l2-common.c and v4l2-ioctl.c to be in the same
> > > > patch. And then the driver should be using the helpers there whenever possible.
> > > 
> > > Fair point - I'll fix that.
> > > 
> > > What is the correct .bpp for 3 10-bit pixels packed into 4 bytes in the
> > > v4l2_format_info?
> > 
> > Good question, maybe this can be done with the fractional bpp support. I must
> > admit, I didn't think about padded cases, I was handling 10bit fully packed over
> > 5 bytes.
> > 
> > https://lore.kernel.org/linux-arm-kernel/20230103170058.810597-3-benjamin.gaignard@collabora.com/
> > My case ended working with:
> > 
> > { .format = V4L2_PIX_FMT_NV12_10LE40_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> > 
> > Question is what do we do about comp_planes, if we kind of fake it to be 2, then maybe this would work.
> > 
> > { .format = V4L2_PIX_FMT_P030_C128, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 4, 8, 0, 0 }, .bpp_div = { 3, 3, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> > 
> > For weird format, this is a bit of hacky, all we want is to get the right
> > stride, and the offset part is not used for mem_planes = 1 formats.
> 
> I'm happy with whatever the consensus says is "right".
> 
> What tree/branch should I be patching against? Code to support the above
> doesn't seem to be in git://linuxtv.org/media_tree:master which is what
> I was using.

Not yet, this patch is part of "[PATCH v3 00/13] AV1 stateless decoder for
RK3588" serie from Benjamin, which now needs to be rebased on V5 of the API.

I see two options, you simply pick it as part of your patchset, worst case the
other serie get merge and we skip it or I split it out of AV1 so we can get it
merged sooner. I can probably make it on its own, as  it is used to enable the
native (reference frame) format Hantro produces, including VP9, which is wrongly
mapped to P010_4L4 at the moment (not even sure this format exist).

Nicolas

> 
> Thanks
> 
> John Cox
> 
> > let me know,
> > Nicolas
> > 
> > > 
> > > Regards
> > > 
> > > John Cox