Re: [PATCH v5 4/4] media: nxp: imx8-isi: Add additional 32-bit RGB format support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect M2M input format mapping for V4L2_PIX_FMT_XBGR32 causes Red and Blue channels to be swapped.
--

--- Patch [4]: [PATCH v5 4/4] media: nxp: imx8-isi: Add additional 32-bit RGB format support ---
commit 14f1b0edd57844d1618923f42c7d14b79400e9d8
Author: Guoniu Zhou <[email protected]>
Subject: media: nxp: imx8-isi: Add additional 32-bit RGB format support

This commit adds support for additional 32-bit RGB pixel formats,
including BGRA32, RGBA32, BGRX32, RGBX32, and ARGB2101010. Formats with
an alpha channel are configured to only support capture since the hardware
ignores alpha bits when reading from memory.

> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> index 92647ee1e7933..646a7c3d12639 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> @@ -166,6 +166,57 @@ static const struct mxc_isi_format_info mxc_isi_formats[] = {
>  		.color_planes	= 1,
>  		.depth		= { 32 },
>  		.encoding	= MXC_ISI_ENC_RGB,

[ ... ]

> +	}, {
> +		.mbus_code	= MEDIA_BUS_FMT_RGB888_1X24,
> +		.fourcc		= V4L2_PIX_FMT_RGBX32,
> +		.type		= MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
> +				| MXC_ISI_VIDEO_M2M_CAP,
> +		.isi_in_format	= CHNL_MEM_RD_CTRL_IMG_TYPE_XBGR8,
> +		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_XBGR888,
> +		.mem_planes	= 1,
> +		.color_planes	= 1,
> +		.depth		= { 32 },
> +		.encoding	= MXC_ISI_ENC_RGB,

[Severity: High]
This is a pre-existing issue, but I noticed the newly added
V4L2_PIX_FMT_RGBX32 format maps its isi_in_format to
CHNL_MEM_RD_CTRL_IMG_TYPE_XBGR8. This happens to be the exact same mapping
used for the pre-existing V4L2_PIX_FMT_XBGR32 format.

Since V4L2_PIX_FMT_XBGR32 and V4L2_PIX_FMT_RGBX32 have swapped Red and
Blue byte orders in memory, they shouldn't map to the same hardware input
format.

Does the pre-existing V4L2_PIX_FMT_XBGR32 format in mxc_isi_formats
incorrectly use CHNL_MEM_RD_CTRL_IMG_TYPE_XBGR8 instead of
CHNL_MEM_RD_CTRL_IMG_TYPE_XRGB8? This would cause the Red and Blue channels
to be swapped when a userspace application uses V4L2_PIX_FMT_XBGR32 for the
M2M output queue.

> +	}, {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4