All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Frank Li <Frank.Li@nxp.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	Christian Hemp <c.hemp@phytec.de>,
	Stefan Riedmueller <s.riedmueller@phytec.de>,
	Jacopo Mondi <jacopo@jmondi.org>,
	Loic Poulain <loic.poulain@oss.qualcomm.com>,
	Bryan O'Donoghue <bod@kernel.org>,
	Dong Aisheng <aisheng.dong@nxp.com>,
	Guoniu Zhou <guoniu.zhou@nxp.com>,
	linux-media@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Robert Chiras <robert.chiras@nxp.com>
Subject: Re: [PATCH v2 6/6] media: nxp: imx8-isi: Add additional 32-bit RGB format support
Date: Tue, 21 Jul 2026 03:21:41 +0300	[thread overview]
Message-ID: <20260721002141.GF50424@killaraus.ideasonboard.com> (raw)
In-Reply-To: <20260720-isi-v2-6-45845bc5d4fa@oss.nxp.com>

Hi Guoniu, Robert,

Thank you for the patch.

On Mon, Jul 20, 2026 at 11:34:08AM +0800, Guoniu Zhou wrote:
> Add support for additional 32-bit RGB pixel formats (BGRA32, RGBA32,
> BGRX32, RGBX32, ARGB2101010) and extend existing ABGR32 format with
> full memory-to-memory capabilities to meet Android requirements.

What are those Android requirements ?

> All formats support capture, M2M input, and M2M output operations,
> enabling complete format conversion pipelines.
> 
> Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> ---
> Changes in v2:
> - Add Reviewed-by tag from Frank Li
> ---
>  .../media/platform/nxp/imx8-isi/imx8-isi-video.c   | 59 +++++++++++++++++++++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
> 
> 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 05b51b98344b..ef638af350fe 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
> @@ -160,12 +160,69 @@ static const struct mxc_isi_format_info mxc_isi_formats[] = {
>  	}, {
>  		.mbus_code	= MEDIA_BUS_FMT_RGB888_1X24,
>  		.fourcc		= V4L2_PIX_FMT_ABGR32,
> -		.type		= MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_CAP,
> +		.type		= MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
> +				| MXC_ISI_VIDEO_M2M_CAP,
> +		.isi_in_format	= CHNL_MEM_RD_CTRL_IMG_TYPE_XRGB8,

This doesn't seem right. The value is the same as for
V4L2_PIX_FMT_XBGR32. As far as I understand, the ISI will ignore the
alpha bits (MSBs in the 32-bit data).

This is why the V4L2_PIX_FMT_ABGR32 format doesn't set
MXC_ISI_VIDEO_M2M_OUT: to be supported as an output format (input to the
ISI), we would need a hardware mode where the alpha bits are read from
memory and used by the ISI. I recommend dropping this part of the patch.

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

Same comment here, this format should not have MXC_ISI_VIDEO_M2M_OUT
set.

> +		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_RGBA8888,
> +		.mem_planes	= 1,
> +		.color_planes	= 1,
> +		.depth		= { 32 },
> +		.encoding	= MXC_ISI_ENC_RGB,
> +	}, {
> +		.mbus_code	= MEDIA_BUS_FMT_RGB888_1X24,
> +		.fourcc		= V4L2_PIX_FMT_RGBA32,
> +		.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,

Same here too.

> +		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_ABGR8888,
> +		.mem_planes	= 1,
> +		.color_planes	= 1,
> +		.depth		= { 32 },
> +		.encoding	= MXC_ISI_ENC_RGB,
> +	}, {
> +		.mbus_code	= MEDIA_BUS_FMT_RGB888_1X24,
> +		.fourcc		= V4L2_PIX_FMT_BGRX32,
> +		.type		= MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
> +				| MXC_ISI_VIDEO_M2M_CAP,
> +		.isi_in_format	= CHNL_MEM_RD_CTRL_IMG_TYPE_RGBX8,
> +		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_RGBX888,
> +		.mem_planes	= 1,
> +		.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,
> +	}, {
> +		.mbus_code	= MEDIA_BUS_FMT_RGB888_1X24,
> +		.fourcc		= V4L2_PIX_FMT_ARGB2101010,
> +		.type		= MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
> +				| MXC_ISI_VIDEO_M2M_CAP,
> +		.isi_in_format	= CHNL_MEM_RD_CTRL_IMG_TYPE_A2RGB10,
> +		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_A2RGB10,
> +		.mem_planes	= 1,
> +		.color_planes	= 1,
> +		.depth		= { 32 },
> +		.encoding	= MXC_ISI_ENC_RGB,

The rest looks good. I have tested all the new formats, both on the
input and output side, and have not noticed any issue.

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	},
>  	/*
>  	 * RAW formats

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2026-07-21  0:21 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20  3:34 [PATCH v2 0/6] imx8-isi: Bug fixes and format support enhancements Guoniu Zhou
2026-07-20  3:34 ` [PATCH v2 1/6] media: nxp: imx8-isi: Fix stream ID validation bypass in crossbar routing Guoniu Zhou
2026-07-20  3:43   ` sashiko-bot
2026-07-20 15:09   ` Frank Li
2026-07-20 17:30   ` Laurent Pinchart
2026-07-20  3:34 ` [PATCH v2 2/6] media: nxp: imx8-isi: Add stream ID validation for " Guoniu Zhou
2026-07-20  3:42   ` sashiko-bot
2026-07-20 15:11   ` Frank Li
2026-07-20 17:32     ` Laurent Pinchart
2026-07-20  3:34 ` [PATCH v2 3/6] media: nxp: imx8-isi: Fix per-stream reference counting for multiplexed streams Guoniu Zhou
2026-07-20  3:42   ` sashiko-bot
2026-07-20 15:34   ` Frank Li
2026-07-20 21:03   ` Laurent Pinchart
2026-07-20  3:34 ` [PATCH v2 4/6] media: nxp: imx8-isi: Add 16-bit raw Bayer format support guoniu.zhou
2026-07-20 21:15   ` Laurent Pinchart
2026-07-20  3:34 ` [PATCH v2 5/6] media: nxp: imx8-isi: Correct color map between V4L2 and ISI Guoniu Zhou
2026-07-20 15:37   ` Frank Li
2026-07-20 17:40   ` Laurent Pinchart
2026-07-20 23:18     ` Laurent Pinchart
2026-07-20  3:34 ` [PATCH v2 6/6] media: nxp: imx8-isi: Add additional 32-bit RGB format support Guoniu Zhou
2026-07-21  0:21   ` Laurent Pinchart [this message]
2026-07-21  0:32 ` [PATCH v2 0/6] imx8-isi: Bug fixes and format support enhancements Laurent Pinchart
2026-07-21  0:35   ` Laurent Pinchart

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=20260721002141.GF50424@killaraus.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=Frank.Li@nxp.com \
    --cc=aisheng.dong@nxp.com \
    --cc=bod@kernel.org \
    --cc=c.hemp@phytec.de \
    --cc=festevam@gmail.com \
    --cc=guoniu.zhou@nxp.com \
    --cc=guoniu.zhou@oss.nxp.com \
    --cc=imx@lists.linux.dev \
    --cc=jacopo@jmondi.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=loic.poulain@oss.qualcomm.com \
    --cc=mchehab@kernel.org \
    --cc=robert.chiras@nxp.com \
    --cc=s.hauer@pengutronix.de \
    --cc=s.riedmueller@phytec.de \
    /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.