[PATCH] media: v4l2: Add support for NV12M tiled variants to v4l2_format_info()

[PATCH] media: v4l2: Add support for NV12M tiled variants to v4l2_format_info()

[Marek Szyprowski]

Commit 6f1466123d73 ("media: s5p-mfc: Add YV12 and I420 multiplanar
format support") added support for the new formats to s5p-mfc driver,
what in turn required some internal calls to the v4l2_format_info()
function while setting up formats. This in turn broke support for the
"old" tiled NV12MT* formats, which are not recognized by this function.
Fix this by adding those variants of NV12M pixel format to
v4l2_format_info() function database.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index df40756ac8f6..0829d7028ef7 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -323,6 +323,10 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
 		{ .format = V4L2_PIX_FMT_NV61M,   .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_P012M,   .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
 
+		/* Tiled YUV formats, non contiguous variant */
+		{ .format = V4L2_PIX_FMT_NV12MT,        .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
+		{ .format = V4L2_PIX_FMT_NV12MT_16X16,  .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
+
 		/* Bayer RGB formats */
 		{ .format = V4L2_PIX_FMT_SBGGR8,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_SGBRG8,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 1 },
-- 
2.34.1

Re: [PATCH] media: v4l2: Add support for NV12M tiled variants to v4l2_format_info()

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 3013 bytes --]

Hi Marek,

Le jeudi 10 juillet 2025 à 09:51 +0200, Marek Szyprowski a écrit :
> Commit 6f1466123d73 ("media: s5p-mfc: Add YV12 and I420 multiplanar
> format support") added support for the new formats to s5p-mfc driver,
> what in turn required some internal calls to the v4l2_format_info()
> function while setting up formats. This in turn broke support for the
> "old" tiled NV12MT* formats, which are not recognized by this function.
> Fix this by adding those variants of NV12M pixel format to
> v4l2_format_info() function database.


Since you mention something broke, should this come with a Fixes: tag ?

> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index df40756ac8f6..0829d7028ef7 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -323,6 +323,10 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>  		{ .format = V4L2_PIX_FMT_NV61M,   .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_P012M,   .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
>  
> +		/* Tiled YUV formats, non contiguous variant */

Since these are tiled, it should not leave the block_w/block_h to 0. NV12_4L4/P010_4L4
are the broken example, we missed, and the drivers using them actually uses bigger
alignment. And that makes me notice MT2110T/R are not in the tiled group :-(


> +		{ .format = V4L2_PIX_FMT_NV12MT,        .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },

So this one is special, 32x32 tiles, but they come in pair row wise, so the data
must be aligned 64x32.

		  .block_w = { 64, 32, 0, 0 }, .block_h = { 32, 16, 0, 0 }},

> +		{ .format = V4L2_PIX_FMT_NV12MT_16X16,  .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },

		  .block_w = { 16, 8, 0, 0 }, .block_h = { 16, 8, 0, 0 }},

Please double check, the block is in pixels, so on interleave plane, 1 pixels
contains 2 bytes. The stride (bytesperline) should endup the same for both
planes.

regards,
Nicolas

> +
>  		/* Bayer RGB formats */
>  		{ .format = V4L2_PIX_FMT_SBGGR8,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_SGBRG8,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 1 },

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] media: v4l2: Add support for NV12M tiled variants to v4l2_format_info()

[Marek Szyprowski]

On 10.07.2025 17:37, Nicolas Dufresne wrote:
> Le jeudi 10 juillet 2025 à 09:51 +0200, Marek Szyprowski a écrit :
>> Commit 6f1466123d73 ("media: s5p-mfc: Add YV12 and I420 multiplanar
>> format support") added support for the new formats to s5p-mfc driver,
>> what in turn required some internal calls to the v4l2_format_info()
>> function while setting up formats. This in turn broke support for the
>> "old" tiled NV12MT* formats, which are not recognized by this function.
>> Fix this by adding those variants of NV12M pixel format to
>> v4l2_format_info() function database.
> Since you mention something broke, should this come with a Fixes: tag ?

I wasn't sure that such change in generic v4l2 code deserves a tag 
pointing to the commit in the niche driver.

>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   drivers/media/v4l2-core/v4l2-common.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
>> index df40756ac8f6..0829d7028ef7 100644
>> --- a/drivers/media/v4l2-core/v4l2-common.c
>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>> @@ -323,6 +323,10 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>>   		{ .format = V4L2_PIX_FMT_NV61M,   .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 },
>>   		{ .format = V4L2_PIX_FMT_P012M,   .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
>>   
>> +		/* Tiled YUV formats, non contiguous variant */
> Since these are tiled, it should not leave the block_w/block_h to 0. NV12_4L4/P010_4L4
> are the broken example, we missed, and the drivers using them actually uses bigger
> alignment. And that makes me notice MT2110T/R are not in the tiled group :-(

I assume that these formats would require separate cleanup.

>> +		{ .format = V4L2_PIX_FMT_NV12MT,        .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> So this one is special, 32x32 tiles, but they come in pair row wise, so the data
> must be aligned 64x32.
>
> 		  .block_w = { 64, 32, 0, 0 }, .block_h = { 32, 16, 0, 0 }},
>
>> +		{ .format = V4L2_PIX_FMT_NV12MT_16X16,  .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> 		  .block_w = { 16, 8, 0, 0 }, .block_h = { 16, 8, 0, 0 }},
>
> Please double check, the block is in pixels, so on interleave plane, 1 pixels
> contains 2 bytes. The stride (bytesperline) should endup the same for both
> planes.

Right, I missed those block_w/h restrictions, thanks for pointing them. 
I assumed that most parameters will be the same as for the generic 
V4L2_PIX_FMT_NV12 case as those formats only differs by macroblock pixel 
layout.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] media: v4l2: Add support for NV12M tiled variants to v4l2_format_info()

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 3759 bytes --]

Le vendredi 11 juillet 2025 à 11:11 +0200, Marek Szyprowski a écrit :
> On 10.07.2025 17:37, Nicolas Dufresne wrote:
> > Le jeudi 10 juillet 2025 à 09:51 +0200, Marek Szyprowski a écrit :
> > > Commit 6f1466123d73 ("media: s5p-mfc: Add YV12 and I420 multiplanar
> > > format support") added support for the new formats to s5p-mfc driver,
> > > what in turn required some internal calls to the v4l2_format_info()
> > > function while setting up formats. This in turn broke support for the
> > > "old" tiled NV12MT* formats, which are not recognized by this function.
> > > Fix this by adding those variants of NV12M pixel format to
> > > v4l2_format_info() function database.
> > Since you mention something broke, should this come with a Fixes: tag ?
> 
> I wasn't sure that such change in generic v4l2 code deserves a tag 
> pointing to the commit in the niche driver.

I don't have enough context to know how many Samsung chips are affected, that being said, whatever we do, stable AI bot will likely come to the conclusion it should be backported.

> 
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > ---
> > >   drivers/media/v4l2-core/v4l2-common.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > > index df40756ac8f6..0829d7028ef7 100644
> > > --- a/drivers/media/v4l2-core/v4l2-common.c
> > > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > > @@ -323,6 +323,10 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
> > >   		{ .format = V4L2_PIX_FMT_NV61M,   .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 },
> > >   		{ .format = V4L2_PIX_FMT_P012M,   .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> > >   
> > > +		/* Tiled YUV formats, non contiguous variant */
> > Since these are tiled, it should not leave the block_w/block_h to 0. NV12_4L4/P010_4L4
> > are the broken example, we missed, and the drivers using them actually uses bigger
> > alignment. And that makes me notice MT2110T/R are not in the tiled group :-(
> 
> I assume that these formats would require separate cleanup.

Correct, this is just me being overly verbose, I will take care a little later,
there is no urgency there.

> 
> > > +		{ .format = V4L2_PIX_FMT_NV12MT,        .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> > So this one is special, 32x32 tiles, but they come in pair row wise, so the data
> > must be aligned 64x32.
> > 
> > 		  .block_w = { 64, 32, 0, 0 }, .block_h = { 32, 16, 0, 0 }},
> > 
> > > +		{ .format = V4L2_PIX_FMT_NV12MT_16X16,  .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> > 		  .block_w = { 16, 8, 0, 0 }, .block_h = { 16, 8, 0, 0 }},
> > 
> > Please double check, the block is in pixels, so on interleave plane, 1 pixels
> > contains 2 bytes. The stride (bytesperline) should endup the same for both
> > planes.
> 
> Right, I missed those block_w/h restrictions, thanks for pointing them. 
> I assumed that most parameters will be the same as for the generic 
> V4L2_PIX_FMT_NV12 case as those formats only differs by macroblock pixel 
> layout.

Correct, these formats are relatively simple tiling and the inner format is
identical to NV12. So the delta is going to be mem_planes being 2, and a
block_w/h.

> 
> Best regards

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]