RE: [PATCH v4 3/3] media: nxp: imx8-isi: Add additional 32-bit RGB format support

Hi Frank,

> -----Original Message-----
> From: Frank Li (OSS) <[email protected]>
> Sent: Friday, July 24, 2026 11:31 PM
> To: [email protected]
> Cc: G.N. Zhou (OSS) <[email protected]>; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v4 3/3] media: nxp: imx8-isi: Add additional 32-bit RGB
> format support
> 
> On Fri, Jul 24, 2026 at 08:03:42AM +0000, [email protected] wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to
> consider:
> > - [High] V4L2_PIX_FMT_RGBX32 and V4L2_PIX_FMT_XBGR32 share the same
> isi_in_format, which will result in swapped color channels during M2M reads.
> > --
> >
> > --- Patch [3]: [PATCH v4 3/3] media: nxp: imx8-isi: Add additional
> > 32-bit RGB format support --- commit
> > 60dac66057b0040d017034c0952f79ec30cec366
> > Author: Guoniu Zhou <[email protected]>
> >
> > media: nxp: imx8-isi: Add additional 32-bit RGB format support
> >
> > This commit adds support for additional 32-bit RGB pixel formats
> > (BGRA32, RGBA32, BGRX32, RGBX32, ARGB2101010) to the i.MX8 ISI driver.
> > Formats with an alpha channel only support capture, as the ISI 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
> > > --- 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[] = {
> >
> > [ ... ]
> >
> > > +	}, {
> > > +		.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,
> >
> > [Severity: High]
> > Does this mapping for V4L2_PIX_FMT_RGBX32 swap the red and blue
> > channels during M2M reads?
> >
> > The newly added V4L2_PIX_FMT_RGBX32 format (memory layout R,G,B,X)
> > maps to
> > CHNL_MEM_RD_CTRL_IMG_TYPE_XBGR8 for DMA reads.
> >
> > However, earlier in mxc_isi_formats[], the pre-existing
> > V4L2_PIX_FMT_XBGR32 format (memory layout B,G,R,X) uses this exact same
> configuration:
> 
> 
> Is this true?

No, there is patch to fix the earlier incorrect mapping.
https://lore.kernel.org/linux-media/[email protected]/

Best Regards
G.N Zhou

> 
> Frank
> >
> > 	{
> > 		.mbus_code	= MEDIA_BUS_FMT_RGB888_1X24,
> > 		.fourcc		= V4L2_PIX_FMT_XBGR32,
> > 		.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,
> >         ...
> > 	}
> >
> > Since these two formats have opposite red and blue channel locations
> > in memory, can the hardware DMA controller correctly interpret both
> > layouts using the identical input format configuration?
> >
> > It looks like this could result in corrupted color conversions for one
> > of these formats during M2M operations.
> >
> > > +		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_XBGR888,
> > > +		.mem_planes	= 1,
> > > +		.color_planes	= 1,
> > > +		.depth		= { 32 },
> > > +		.encoding	= MXC_ISI_ENC_RGB,
> >
> > [ ... ]
> >
> > --
> > Sashiko AI review *
> > https://sashiko.dev/#/patchset/[email protected].
> > com?part=3