[05/16] media: v4l2-common: Fix NV15_4L4 format info block height

Message ID 20260518102451.417971-6-paulk@sys-base.io (mailing list archive)
State New
Headers
Series media: sun6i-csi/isp MC-centric support and cleanups |

Commit Message

Paul Kocialkowski May 18, 2026, 10:24 a.m. UTC
The NV15_4L4 format is specified as a 4x4 format, not 4x1.
In addition the block size should not take subsampling in account,
so specify it as 4x4 for both luma and chroma.

Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
---
 drivers/media/v4l2-core/v4l2-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

sashiko-bot@kernel.org May 18, 2026, 11:14 a.m. UTC | #1
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Other tiled formats (MT2110T, MT2110R, NV12MT, NV12MT_16X16) incorrectly take subsampling into account in their block_w and block_h definitions, causing chroma plane under-allocation.
- [High] Formats with bpp_div > 1 (e.g., NV15, NV20, Bayer 10P) lack block_w definitions, causing integer truncation in stride calculations.
--

(Note: The format of this report is altered due to recitation restrictions.
The original patch code is not quoted inline; instead, a free-form summary
of the findings is provided.)

Commit: 04995fa68d4b9c2dd80539724de182a617de7bac
Author: Paul Kocialkowski <paulk@sys-base.io>
Subject: media: v4l2-common: Fix NV15_4L4 format info block height

This commit corrects the NV15_4L4 format info by specifying it as a 4x4
format and ensuring the block size does not take subsampling into account
for luma and chroma.

You noted in the commit message that block size should not take subsampling
into account. Does this mean other tiled formats like MT2110T, MT2110R,
NV12MT, and NV12MT_16X16 are currently incorrect?

Looking at their definitions in v4l2_format_info(), they seem to divide
their chroma block dimensions by the subsampling factor. For instance,
MT2110T defines block_w as { 16, 8, 0, 0 } and block_h as { 32, 16, 0, 0 }.

Because v4l2_format_plane_stride() and v4l2_format_plane_height() apply
these alignments to the luma width and height before dividing by hdiv and
vdiv, an unaligned dimension like width 8 and height 16 would correctly
align the luma plane to 16x32. However, for the chroma plane, it would
align the luma dimensions to 8x16 before dividing by the subsampling
factors.

Could this result in the chroma plane being allocated only half of a
macroblock's required dimensions, leading to out-of-bounds memory writes
when the hardware writes full tiles?

Related to the macroblock sizing logic, do formats with bpp_div greater
than 1 need explicit block_w definitions to prevent integer truncation
during stride calculations?

Linear packed formats like NV15, NV20, SBGGR10P, and the RAW_CRU variants
pack multiple pixels into a fractional number of bytes. For example, NV15
has a bpp_div of 4. Because they currently lack a block_w definition, they
default to an alignment of 1.

If an unaligned width is passed to v4l2_format_plane_stride(), the formula
(aligned_width / hdiv) * bpp / bpp_div truncates. Using NV15 with a width
of 2 calculates a stride of (2 * 5) / 4 = 2 bytes. However, two 10-bit
pixels require 20 bits (2.5 bytes), so the stride rounds down.

Could this allocate less memory per line than the requested pixels require,
potentially causing buffer overflows since hardware typically writes in
multiples of bpp_div blocks?
  
Nicolas Dufresne May 19, 2026, 3:16 p.m. UTC | #2
Le lundi 18 mai 2026 à 12:24 +0200, Paul Kocialkowski a écrit :
> The NV15_4L4 format is specified as a 4x4 format, not 4x1.
> In addition the block size should not take subsampling in account,
> so specify it as 4x4 for both luma and chroma.
> 
> Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 554c591e1113..77a0daa92c2b 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -309,7 +309,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>  		/* Tiled YUV formats */
>  		{ .format = V4L2_PIX_FMT_NV12_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
>  		{ .format = V4L2_PIX_FMT_NV15_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,
> -		  .block_w = { 4, 2, 0, 0 }, .block_h = { 1, 1, 0, 0 }},
> +		  .block_w = { 4, 4, 0, 0 }, .block_h = { 4, 4, 0, 0 }},

Only the block_h is broken. The block_w is in "pixels" which for the UV plane is
component pairs. So both set of tiles have 5bytes stride. But since the second
set, the UV tiles, are interleaved, they only have 2 pairs of UV per row. So to
me the correct fix is:

+		  .block_w = { 4, 2, 0, 0 }, .block_h = { 4, 4, 0, 0 }},

If its not the case for the camera pipeline, then a new format is needed, since
this format should perfectly match NV15 + VIVANTE_TILED in the DRM world.

regards,
Nicolas

>  		{ .format = V4L2_PIX_FMT_P010_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
>  
>  		/* YUV planar formats, non contiguous variant */
  
Paul Kocialkowski May 19, 2026, 8:33 p.m. UTC | #3
Hi Nicolas,

Thanks for the review!

Le Tue 19 May 26, 11:16, Nicolas Dufresne a écrit :
> Le lundi 18 mai 2026 à 12:24 +0200, Paul Kocialkowski a écrit :
> > The NV15_4L4 format is specified as a 4x4 format, not 4x1.
> > In addition the block size should not take subsampling in account,
> > so specify it as 4x4 for both luma and chroma.
> > 
> > Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> > ---
> >  drivers/media/v4l2-core/v4l2-common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > index 554c591e1113..77a0daa92c2b 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -309,7 +309,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
> >  		/* Tiled YUV formats */
> >  		{ .format = V4L2_PIX_FMT_NV12_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> >  		{ .format = V4L2_PIX_FMT_NV15_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,
> > -		  .block_w = { 4, 2, 0, 0 }, .block_h = { 1, 1, 0, 0 }},
> > +		  .block_w = { 4, 4, 0, 0 }, .block_h = { 4, 4, 0, 0 }},
> 
> Only the block_h is broken. The block_w is in "pixels" which for the UV plane is
> component pairs. So both set of tiles have 5bytes stride. But since the second
> set, the UV tiles, are interleaved, they only have 2 pairs of UV per row. So to
> me the correct fix is:
> 
> +		  .block_w = { 4, 2, 0, 0 }, .block_h = { 4, 4, 0, 0 }},
> 
> If its not the case for the camera pipeline, then a new format is needed, since
> this format should perfectly match NV15 + VIVANTE_TILED in the DRM world.

Ah yes I think you're right, I lost sight that these are semi-planar
formats with two components per chroma memory plane.

Good catch, thanks!

All the best,

Paul

> regards,
> Nicolas
> 
> >  		{ .format = V4L2_PIX_FMT_P010_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
> >  
> >  		/* YUV planar formats, non contiguous variant */
  

Patch

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 554c591e1113..77a0daa92c2b 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -309,7 +309,7 @@  const struct v4l2_format_info *v4l2_format_info(u32 format)
 		/* Tiled YUV formats */
 		{ .format = V4L2_PIX_FMT_NV12_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
 		{ .format = V4L2_PIX_FMT_NV15_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,
-		  .block_w = { 4, 2, 0, 0 }, .block_h = { 1, 1, 0, 0 }},
+		  .block_w = { 4, 4, 0, 0 }, .block_h = { 4, 4, 0, 0 }},
 		{ .format = V4L2_PIX_FMT_P010_4L4, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 2, 4, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
 
 		/* YUV planar formats, non contiguous variant */