[v1,1/3] media: sun6i-csi: capture: Align size to 16 for NV12_16L16
Commit Message
The NV12_16L16 format is made of 16x16 tiles, so both the width and
the height must be a multiple of 16. The format preparation always
aligned dimensions to 2 pixels, whatever the pixel format, which
allowed sizes covering partial tiles.
Select the alignment from the pixel format, and do so after the
pixel format fallback so that the final format is the one used.
Assisted-by: Claude:claude-sonnet-5-5
Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
---
.../sunxi/sun6i-csi/sun6i_csi_capture.c | 20 +++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
Comments
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: sun6i-csi: incorrect bytesperline for NV12_16L16 format
--
commit 476de016f26cd243aac004fdf752c97c70ae693b
Author: Arash Golgol <arash.golgol@gmail.com>
media: sun6i-csi: capture: Align size to 16 for NV12_16L16
This commit introduces alignment handling specific to pixel formats in the
sun6i-csi capture driver. It sets the required alignment based on the format
and applies it during format preparation.
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> index d90abba213098..f55edfd0b174a 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
[ ... ]
> @@ -665,17 +671,19 @@ static void sun6i_csi_capture_format_prepare(struct v4l2_format *format)
> {
> struct v4l2_pix_format *pix_format = &format->fmt.pix;
> const struct v4l2_format_info *info;
> - unsigned int width, height;
> -
> - v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
> - SUN6I_CSI_CAPTURE_WIDTH_MAX, 1,
> - &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
> - SUN6I_CSI_CAPTURE_HEIGHT_MAX, 1, 0);
> + unsigned int width, height, align;
>
> if (!sun6i_csi_capture_format_find(pix_format->pixelformat))
> pix_format->pixelformat =
> sun6i_csi_capture_formats[0].pixelformat;
>
> + align = sun6i_csi_capture_format_align(pix_format->pixelformat);
> +
> + v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
> + SUN6I_CSI_CAPTURE_WIDTH_MAX, align,
> + &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
> + SUN6I_CSI_CAPTURE_HEIGHT_MAX, align, 0);
> +
> width = pix_format->width;
> height = pix_format->height;
>
> info = v4l2_format_info(pix_format->pixelformat);
>
> switch (pix_format->pixelformat) {
> case V4L2_PIX_FMT_NV12_16L16:
> pix_format->bytesperline = width * 12 / 8;
[Severity: Medium]
Does this code violate the V4L2 API contract by incorrectly hardcoding
bytesperline to 1.5x width as a shortcut to calculate the correct sizeimage?
The V4L2 specification requires bytesperline to reflect the stride of the
largest plane, which for NV12 is just width. By exposing a stride of
1.5 * width, userspace applications may compute out-of-bounds plane offsets,
potentially leading to memory corruption or crashes within the userspace
client.
This is supported by sun6i_csi_setup_format(), which configures the
hardware using the correct luma and chroma strides of width internally:
drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c:sun6i_csi_setup_format() {
...
switch (pixelformat) {
...
case V4L2_PIX_FMT_NV12_16L16:
luma_line = width;
chroma_line = width;
break;
...
}
@@ -41,6 +41,12 @@ void sun6i_csi_capture_format(struct sun6i_csi_device *csi_dev,
*field = csi_dev->capture.format.fmt.pix.field;
}
+static unsigned int sun6i_csi_capture_format_align(u32 pixelformat)
+{
+ /* Returns log2 of the required alignment. */
+ return pixelformat == V4L2_PIX_FMT_NV12_16L16 ? 4 : 1;
+}
+
/* Format */
static const struct sun6i_csi_capture_format sun6i_csi_capture_formats[] = {
@@ -665,17 +671,19 @@ static void sun6i_csi_capture_format_prepare(struct v4l2_format *format)
{
struct v4l2_pix_format *pix_format = &format->fmt.pix;
const struct v4l2_format_info *info;
- unsigned int width, height;
-
- v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
- SUN6I_CSI_CAPTURE_WIDTH_MAX, 1,
- &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
- SUN6I_CSI_CAPTURE_HEIGHT_MAX, 1, 0);
+ unsigned int width, height, align;
if (!sun6i_csi_capture_format_find(pix_format->pixelformat))
pix_format->pixelformat =
sun6i_csi_capture_formats[0].pixelformat;
+ align = sun6i_csi_capture_format_align(pix_format->pixelformat);
+
+ v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
+ SUN6I_CSI_CAPTURE_WIDTH_MAX, align,
+ &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
+ SUN6I_CSI_CAPTURE_HEIGHT_MAX, align, 0);
+
width = pix_format->width;
height = pix_format->height;