From: Frank Li <Frank.li@oss.nxp.com>
To: sashiko-reviews@lists.linux.dev
Cc: Guoniu Zhou <guoniu.zhou@oss.nxp.com>,
imx@lists.linux.dev, Frank.Li@kernel.org, media-ci@linuxtv.org
Subject: Re: [PATCH v4 3/3] media: nxp: imx8-isi: Add additional 32-bit RGB format support
Date: Fri, 24 Jul 2026 10:30:30 -0500 [thread overview]
Message-ID: <amOFFm253o4lwhs1@SMW015318> (raw)
In-Reply-To: <20260724080342.998CE1F000E9@smtp.kernel.org>
On Fri, Jul 24, 2026 at 08:03:42AM +0000, sashiko-bot@kernel.org 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 <guoniu.zhou@oss.nxp.com>
>
> 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/20260724-isi-v4-0-27df7b5f6060@oss.nxp.com?part=3
prev parent reply other threads:[~2026-07-24 15:30 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-24 7:52 [PATCH v4 0/3] imx8-isi: Format support enhancements Guoniu Zhou
2026-07-24 7:52 ` [PATCH v4 1/3] media: nxp: imx8-isi: Implement per-stream reference counting for multiplexed streams Guoniu Zhou
2026-07-24 8:02 ` sashiko-bot
2026-07-24 15:29 ` Frank Li
2026-07-24 7:52 ` [PATCH v4 2/3] media: nxp: imx8-isi: Add 16-bit raw Bayer format support guoniu.zhou
2026-07-24 7:52 ` [PATCH v4 3/3] media: nxp: imx8-isi: Add additional 32-bit RGB " Guoniu Zhou
2026-07-24 8:03 ` sashiko-bot
2026-07-24 15:30 ` Frank Li [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=amOFFm253o4lwhs1@SMW015318 \
--to=frank.li@oss.nxp.com \
--cc=Frank.Li@kernel.org \
--cc=guoniu.zhou@oss.nxp.com \
--cc=imx@lists.linux.dev \
--cc=media-ci@linuxtv.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.