Re: [PATCH v4 3/3] media: nxp: imx8-isi: Add additional 32-bit RGB format support
Frank Li <[email protected]>
| Newsgroups | dev.linux.lists.imx,dev.linux.lists.sashiko-reviews |
|---|---|
| Message-ID | <amOFFm253o4lwhs1@SMW015318> |
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? 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]?part=3