diff mbox series

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

Message ID 20230127153415.83126-2-jc@kynesim.co.uk
State Superseded
Headers show
Series media: v4l: Add Broadcom sand format to the list of V4L formats | expand

Commit Message

John Cox Jan. 27, 2023, 3:34 p.m. UTC
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(+)

Comments

Nicolas Dufresne Feb. 9, 2023, 6:20 p.m. UTC | #1
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
John Cox Feb. 9, 2023, 7:06 p.m. UTC | #2
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
Nicolas Dufresne Feb. 10, 2023, 4:17 p.m. UTC | #3
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
John Cox Feb. 12, 2023, 6:49 p.m. UTC | #4
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
Nicolas Dufresne Feb. 13, 2023, 4:25 p.m. UTC | #5
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
diff mbox series

Patch

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 */