[PATCH v5 0/4] imx8-isi: Format support enhancements

[PATCH v5 0/4] imx8-isi: Format support enhancements

[Guoniu Zhou]

This is a follow-up series to the imx8-isi bug fixes and format support
patches [1]. Patches 1/6, 2/6, and 5/6 from v2 have been merged into the
maintainer's branch (pinchart/for-next/media/nxp). This series contains
the remaining 3 patches rebased on top of the merged changes, plus a new
bug fix patch.

Patch 1 fixes BIT() usage on 64-bit stream masks which could cause incorrect
results on 32-bit architectures when stream IDs are 32 or greater.

Patch 2 implements per-stream reference counting in the crossbar to properly
enable multiple streams from different virtual channels on the same input pad.

Patch 3 adds support for 16-bit raw Bayer formats (SBGGR16, SGBRG16,
SGRBG16, SRGGB16) and Y16 (16-bit monochrome).

Patch 4 extends RGB format support by adding BGRA32, RGBA32, BGRX32, RGBX32,
and ARGB2101010 formats.

[1] https://lore.kernel.org/r/20260720-isi-v2-0-45845bc5d4fa@oss.nxp.com

Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
Changes in v5:
- Add new patch 1/4: Use BIT_ULL() for 64-bit stream masks to fix potential
  issues on 32-bit architectures (Sashiko)
- Patch numbering updated: v4 1/3 -> v5 2/4, v4 2/3 -> v5 3/4, v4 3/3 -> v5 4/4
- Link to v4: https://lore.kernel.org/r/20260724-isi-v4-0-27df7b5f6060@oss.nxp.com

Changes in v4:
- [1/3] Make enable/disable symmetric by checking stream_in_use
- [1/3] Add mxc_isi_crossbar_stream_in_use() helper for both paths
- Link to v3: https://lore.kernel.org/r/20260723-isi-v3-0-594919eae847@oss.nxp.com

Changes in v3:
- Rebased on maintainer's branch which already includes v2 patches 1/6,
  2/6, and 5/6 (stream ID validation fixes and color map correction)
- Series reduced from 6 patches to 3 patches (remaining unmerged patches)
- Patch numbering updated: v2 3/6 -> v3 1/3, v2 4/6 -> v3 2/3, v2 6/6 -> v3 3/3
- [1/3] Change from "Fix" to "Implement" as multi-stream support is for newer
  SoCs not present when the driver was merged (Laurent)
- [1/3] Remove Fixes and Cc: stable tags (Laurent)
- [1/3] Store input/input_stream in mxc_isi_pipe instead of using per-stream
  counters array, avoiding arbitrary 64-entry limit (Laurent)
- [1/3] Use UINT_MAX to mark pipe as inactive to distinguish from valid pad 0
- [2/3] Remove "high-end" from description as RAW16 is common now (Laurent)
- [2/3] Add Y16 (16-bit monochrome) format support (Laurent)
- [2/3] Add Reviewed-by tag from Laurent
- [3/3] Drop M2M_OUT for ABGR32/BGRA32/RGBA32 as ISI ignores alpha bits (Laurent)
- [3/3] Remove Android requirements from commit message as alpha formats
  cannot be used as M2M input (Laurent)
- [3/3] Add Tested-by tag from Laurent Pinchart
- Link to v2: https://lore.kernel.org/r/20260720-isi-v2-0-45845bc5d4fa@oss.nxp.com

Changes in v2:
- [1/6] Split v1 patch 1/5 into two patches: this patch fixes the core
  for_each_active_route() bug, next patch adds additional stream
  validation (Frank Li)
- [2/6] New patch split from v1 1/5: adds stream ID validation on top of
  for_each_active_route() fix (Frank Li)
- [2/6] Remove incorrect sink_stream validation
- [3/6] Use fixed-size array for enabled_count instead of dynamic allocation
- [3/6] Use BIT_ULL() macro for u64 bitmask operations
- [3/6] Use MXC_ISI_MAX_STREAMS (64) as loop boundary instead of num_sources
- [3/6] Remove mxc_isi_stream_counters_alloc/free functions
- [4/6] Add Reviewed-by tag from Frank Li
- [5/6] Reword commit description for clarity (Frank Li)
- [6/6] Add Reviewed-by tag from Frank Li
- Link to v1: https://lore.kernel.org/r/20260629-isi-v1-0-deebfdb1b07b@oss.nxp.com

---
Guoniu Zhou (3):
      media: nxp: imx8-isi: Use BIT_ULL() for 64-bit stream masks
      media: nxp: imx8-isi: Implement per-stream reference counting for multiplexed streams
      media: nxp: imx8-isi: Add additional 32-bit RGB format support

Laurentiu Palcu (1):
      media: nxp: imx8-isi: Add 16-bit raw Bayer format support

 .../media/platform/nxp/imx8-isi/imx8-isi-core.h    |  5 +-
 .../platform/nxp/imx8-isi/imx8-isi-crossbar.c      | 88 ++++++++++++++------
 .../media/platform/nxp/imx8-isi/imx8-isi-pipe.c    | 31 +++++++
 .../media/platform/nxp/imx8-isi/imx8-isi-video.c   | 96 ++++++++++++++++++++++
 4 files changed, 195 insertions(+), 25 deletions(-)
---
base-commit: 4d32fbba9d23f50d157ed4a37c9e8fbce61c904a
change-id: 20260626-isi-00f05b044ac9

Best regards,
-- 
Guoniu Zhou <guoniu.zhou@oss.nxp.com>

[PATCH v5 1/4] media: nxp: imx8-isi: Use BIT_ULL() for 64-bit stream masks

[Guoniu Zhou]

Use BIT_ULL() instead of BIT() for u64 stream masks to avoid incorrect
results on 32-bit architectures when stream IDs are 32 or greater.

Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
 drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
index 9f0231ca47a3..9db9c6e55c2c 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -157,10 +157,10 @@ mxc_isi_crossbar_xlate_streams(struct mxc_isi_crossbar *xbar,
 	 */
 	for_each_active_route(&state->routing, route) {
 		if (route->source_pad != source_pad ||
-		    !(source_streams & BIT(route->source_stream)))
+		    !(source_streams & BIT_ULL(route->source_stream)))
 			continue;
 
-		sink_streams |= BIT(route->sink_stream);
+		sink_streams |= BIT_ULL(route->sink_stream);
 		sink_pad = route->sink_pad;
 	}
 

-- 
2.34.1

[PATCH v5 2/4] media: nxp: imx8-isi: Implement per-stream reference counting for multiplexed streams

[Guoniu Zhou]

The ISI crossbar needs to properly enable multiple streams from different
virtual channels on the same input pad. Currently only the first stream
gets enabled in hardware, subsequent streams are silently ignored.

The driver uses a single enable_count per input to track the input state.
When enable_count is non-zero, the code assumes the input is already active
and skips calling v4l2_subdev_enable_streams() for additional streams:

  Call 1: enable_streams(stream 0)
    -> enable_count == 0, enable gasket and stream 0 in hardware
    -> enable_count = 1

  Call 2: enable_streams(stream 1)
    -> enable_count == 1, skip hardware enable
    -> enable_count = 2
    -> stream 1 never gets enabled

Similarly on disable, when enable_count reaches zero, ALL streams are
disabled regardless of which streams are actually still active.

Implement per-stream state tracking by storing the input index and stream
mask in the mxc_isi_pipe structure. On enable, record which input and
stream the pipe is receiving. On disable, clear the pipe's record and
check if any other pipe is still using the same input stream before
actually disabling it.

Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
Changes in v4:
- Make enable/disable symmetric by checking stream_in_use
- Add mxc_isi_crossbar_stream_in_use() helper for both paths

Changes in v3:
- Change from "Fix" to "Implement" as multi-stream support is for newer
  SoCs not present when the driver was merged (Laurent)
- Remove Fixes and Cc: stable tags (Laurent)
- Store input/input_stream in mxc_isi_pipe instead of using per-stream
  counters array, avoiding arbitrary 64-entry limit (Laurent)
- Use UINT_MAX to mark pipe as inactive to distinguish from valid pad 0

Changes in v2:
- Use fixed-size array for enabled_count instead of dynamic allocation
- Use BIT_ULL() macro for u64 bitmask operations
- Use MXC_ISI_MAX_STREAMS (64) as loop boundary instead of num_sources
- Remove mxc_isi_stream_counters_alloc/free functions
---
 .../media/platform/nxp/imx8-isi/imx8-isi-core.h    |  5 +-
 .../platform/nxp/imx8-isi/imx8-isi-crossbar.c      | 84 ++++++++++++++++------
 .../media/platform/nxp/imx8-isi/imx8-isi-pipe.c    |  1 +
 3 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
index 9bb4d430d15e..c07abdd6a1f0 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -186,7 +186,7 @@ struct mxc_isi_dma_buffer {
 };
 
 struct mxc_isi_input {
-	unsigned int			enable_count;
+	u64				enabled_streams;
 };
 
 struct mxc_isi_crossbar {
@@ -259,6 +259,9 @@ struct mxc_isi_pipe {
 	u8				acquired_res;
 	u8				chained_res;
 	bool				chained;
+
+	unsigned int			input;
+	u64				input_stream;
 };
 
 struct mxc_isi_m2m {
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
index 9db9c6e55c2c..eacc3bb28f9c 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -330,11 +330,26 @@ static int mxc_isi_crossbar_set_routing(struct v4l2_subdev *sd,
 	return __mxc_isi_crossbar_set_routing(sd, state, routing);
 }
 
+static bool mxc_isi_crossbar_stream_in_use(struct mxc_isi_crossbar *xbar,
+					   unsigned int sink_pad, u64 sink_streams)
+{
+	for (unsigned int i = 0; i < xbar->isi->pdata->num_channels; ++i) {
+		struct mxc_isi_pipe *pipe = &xbar->isi->pipes[i];
+
+		if (pipe->input == sink_pad &&
+		    pipe->input_stream == sink_streams)
+			return true;
+	}
+
+	return false;
+}
+
 static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
 					   struct v4l2_subdev_state *state,
 					   u32 pad, u64 streams_mask)
 {
 	struct mxc_isi_crossbar *xbar = to_isi_crossbar(sd);
+	struct mxc_isi_pipe *pipe = &xbar->isi->pipes[pad - xbar->num_sinks];
 	struct v4l2_subdev *remote_sd;
 	struct mxc_isi_input *input;
 	u64 sink_streams;
@@ -351,29 +366,44 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
 	input = &xbar->inputs[sink_pad];
 
 	/*
-	 * TODO: Track per-stream enable counts to support multiplexed
-	 * streams.
+	 * Check if any other pipe already receives the same input stream.
+	 * If so, just record this pipe's usage and return.
 	 */
-	if (!input->enable_count) {
+	if (mxc_isi_crossbar_stream_in_use(xbar, sink_pad, sink_streams)) {
+		pipe->input = sink_pad;
+		pipe->input_stream = sink_streams;
+		return 0;
+	}
+
+	/* Enable the gasket when the first stream is enabled for this input. */
+	if (!input->enabled_streams) {
 		ret = mxc_isi_crossbar_gasket_enable(xbar, state, remote_sd,
 						     remote_pad, sink_pad);
 		if (ret)
 			return ret;
+	}
 
-		ret = v4l2_subdev_enable_streams(remote_sd, remote_pad,
-						 sink_streams);
-		if (ret) {
-			dev_err(xbar->isi->dev,
-				"failed to enable streams 0x%llx on '%s':%u: %d\n",
-				sink_streams, remote_sd->name, remote_pad, ret);
-			mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
-			return ret;
-		}
+	ret = v4l2_subdev_enable_streams(remote_sd, remote_pad, sink_streams);
+	if (ret) {
+		dev_err(xbar->isi->dev,
+			"failed to enable streams 0x%llx on '%s':%u: %d\n",
+			sink_streams, remote_sd->name, remote_pad, ret);
+		goto err_gasket_disable;
 	}
 
-	input->enable_count++;
+	input->enabled_streams |= sink_streams;
+
+	/* Record the input and stream for this pipe. */
+	pipe->input = sink_pad;
+	pipe->input_stream = sink_streams;
 
 	return 0;
+
+err_gasket_disable:
+	if (!input->enabled_streams)
+		mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
+
+	return ret;
 }
 
 static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
@@ -381,6 +411,7 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
 					    u32 pad, u64 streams_mask)
 {
 	struct mxc_isi_crossbar *xbar = to_isi_crossbar(sd);
+	struct mxc_isi_pipe *pipe = &xbar->isi->pipes[pad - xbar->num_sinks];
 	struct v4l2_subdev *remote_sd;
 	struct mxc_isi_input *input;
 	u64 sink_streams;
@@ -396,18 +427,27 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
 
 	input = &xbar->inputs[sink_pad];
 
-	input->enable_count--;
+	/* Clear the input and stream for this pipe. */
+	pipe->input = UINT_MAX;
+	pipe->input_stream = 0;
 
-	if (!input->enable_count) {
-		ret = v4l2_subdev_disable_streams(remote_sd, remote_pad,
-						  sink_streams);
-		if (ret)
-			dev_err(xbar->isi->dev,
-				"failed to disable streams 0x%llx on '%s':%u: %d\n",
-				sink_streams, remote_sd->name, remote_pad, ret);
+	/*
+	 * Check if any other pipe receives the same input stream. If so we
+	 * can't disable it yet, so return immediately.
+	 */
+	if (mxc_isi_crossbar_stream_in_use(xbar, sink_pad, sink_streams))
+		return 0;
+
+	ret = v4l2_subdev_disable_streams(remote_sd, remote_pad, sink_streams);
+	if (ret)
+		dev_err(xbar->isi->dev,
+			"failed to disable streams 0x%llx on '%s':%u: %d\n",
+			sink_streams, remote_sd->name, remote_pad, ret);
+
+	input->enabled_streams &= ~sink_streams;
 
+	if (!input->enabled_streams)
 		mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
-	}
 
 	return ret;
 }
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
index 16085f23bc0b..c0ec59856374 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
@@ -816,6 +816,7 @@ int mxc_isi_pipe_init(struct mxc_isi_dev *isi, unsigned int id)
 	pipe->acquired_res = 0;
 	pipe->chained_res = 0;
 	pipe->chained = false;
+	pipe->input = UINT_MAX;
 
 	sd = &pipe->sd;
 	v4l2_subdev_init(sd, &mxc_isi_pipe_subdev_ops);

-- 
2.34.1

[PATCH v5 3/4] media: nxp: imx8-isi: Add 16-bit raw Bayer format support

[guoniu.zhou]

From: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

Add support for 16-bit raw Bayer formats (SBGGR16, SGBRG16, SGRBG16,
SRGGB16) to both the pipeline subdev and video capture interface.

These formats are used by image sensors that output 16-bit raw data,
enabling the ISI to capture full dynamic range from such sensors.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
Changes in v3:
- Remove "high-end" from description as RAW16 is common now (Laurent)
- Add Y16 (16-bit monochrome) format support (Laurent)
- Add Reviewed-by tag from Laurent

Changes in v2:
- Add Reviewed-by tag from Frank Li
---
 .../media/platform/nxp/imx8-isi/imx8-isi-pipe.c    | 30 +++++++++++++++
 .../media/platform/nxp/imx8-isi/imx8-isi-video.c   | 45 ++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
index c0ec59856374..934f7b356258 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
@@ -83,6 +83,12 @@ static const struct mxc_isi_bus_format_info mxc_isi_bus_formats[] = {
 		.pads		= BIT(MXC_ISI_PIPE_PAD_SINK)
 				| BIT(MXC_ISI_PIPE_PAD_SOURCE),
 		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_Y16_1X16,
+		.output		= MEDIA_BUS_FMT_Y16_1X16,
+		.pads		= BIT(MXC_ISI_PIPE_PAD_SINK)
+				| BIT(MXC_ISI_PIPE_PAD_SOURCE),
+		.encoding	= MXC_ISI_ENC_RAW,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR8_1X8,
 		.output		= MEDIA_BUS_FMT_SBGGR8_1X8,
@@ -179,6 +185,30 @@ static const struct mxc_isi_bus_format_info mxc_isi_bus_formats[] = {
 		.pads		= BIT(MXC_ISI_PIPE_PAD_SINK)
 				| BIT(MXC_ISI_PIPE_PAD_SOURCE),
 		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SBGGR16_1X16,
+		.output		= MEDIA_BUS_FMT_SBGGR16_1X16,
+		.pads		= BIT(MXC_ISI_PIPE_PAD_SINK)
+				| BIT(MXC_ISI_PIPE_PAD_SOURCE),
+		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGBRG16_1X16,
+		.output		= MEDIA_BUS_FMT_SGBRG16_1X16,
+		.pads		= BIT(MXC_ISI_PIPE_PAD_SINK)
+				| BIT(MXC_ISI_PIPE_PAD_SOURCE),
+		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGRBG16_1X16,
+		.output		= MEDIA_BUS_FMT_SGRBG16_1X16,
+		.pads		= BIT(MXC_ISI_PIPE_PAD_SINK)
+				| BIT(MXC_ISI_PIPE_PAD_SOURCE),
+		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SRGGB16_1X16,
+		.output		= MEDIA_BUS_FMT_SRGGB16_1X16,
+		.pads		= BIT(MXC_ISI_PIPE_PAD_SINK)
+				| BIT(MXC_ISI_PIPE_PAD_SOURCE),
+		.encoding	= MXC_ISI_ENC_RAW,
 	},
 	/* JPEG */
 	{
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 056603055c81..a6e044056762 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
@@ -212,6 +212,15 @@ static const struct mxc_isi_format_info mxc_isi_formats[] = {
 		.color_planes	= 1,
 		.depth		= { 16 },
 		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_Y16_1X16,
+		.fourcc		= V4L2_PIX_FMT_Y16,
+		.type		= MXC_ISI_VIDEO_CAP,
+		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_RAW16,
+		.mem_planes	= 1,
+		.color_planes	= 1,
+		.depth		= { 16 },
+		.encoding	= MXC_ISI_ENC_RAW,
 	}, {
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR8_1X8,
 		.fourcc		= V4L2_PIX_FMT_SBGGR8,
@@ -356,6 +365,42 @@ static const struct mxc_isi_format_info mxc_isi_formats[] = {
 		.color_planes	= 1,
 		.depth		= { 16 },
 		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SBGGR16_1X16,
+		.fourcc		= V4L2_PIX_FMT_SBGGR16,
+		.type		= MXC_ISI_VIDEO_CAP,
+		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_RAW16,
+		.mem_planes	= 1,
+		.color_planes	= 1,
+		.depth		= { 16 },
+		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGBRG16_1X16,
+		.fourcc		= V4L2_PIX_FMT_SGBRG16,
+		.type		= MXC_ISI_VIDEO_CAP,
+		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_RAW16,
+		.mem_planes	= 1,
+		.color_planes	= 1,
+		.depth		= { 16 },
+		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SGRBG16_1X16,
+		.fourcc		= V4L2_PIX_FMT_SGRBG16,
+		.type		= MXC_ISI_VIDEO_CAP,
+		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_RAW16,
+		.mem_planes	= 1,
+		.color_planes	= 1,
+		.depth		= { 16 },
+		.encoding	= MXC_ISI_ENC_RAW,
+	}, {
+		.mbus_code	= MEDIA_BUS_FMT_SRGGB16_1X16,
+		.fourcc		= V4L2_PIX_FMT_SRGGB16,
+		.type		= MXC_ISI_VIDEO_CAP,
+		.isi_out_format	= CHNL_IMG_CTRL_FORMAT_RAW16,
+		.mem_planes	= 1,
+		.color_planes	= 1,
+		.depth		= { 16 },
+		.encoding	= MXC_ISI_ENC_RAW,
 	},
 	/* JPEG */
 	{

-- 
2.34.1

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

[Guoniu Zhou]

Add support for additional 32-bit RGB pixel formats (BGRA32, RGBA32,
BGRX32, RGBX32, ARGB2101010).

Formats with alpha channel (BGRA32, RGBA32) only support capture as
ISI ignores alpha bits when reading from memory.

Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
Changes in v3:
- Drop M2M_OUT for ABGR32/BGRA32/RGBA32 as ISI ignores alpha bits (Laurent)
- Remove Android requirements from commit message as alpha formats
  cannot be used as M2M input (Laurent)
- Add Tested-by tag from Laurent Pinchart

Changes in v2:
- Add Reviewed-by tag from Frank Li
---
 .../media/platform/nxp/imx8-isi/imx8-isi-video.c   | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)

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 a6e044056762..f45c2aae59ce 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_BGRA32,
+		.type		= MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_CAP,
+		.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_CAP,
+		.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,
 	},
 	/*
 	 * RAW formats

-- 
2.34.1

Re: [PATCH v5 1/4] media: nxp: imx8-isi: Use BIT_ULL() for 64-bit stream masks

[sashiko-bot]

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

Pre-existing issues:
- [High] Routing validation checks the old routing state instead of the newly requested routing.
- [High] Stream translation silently overwrites sink pad and mixes streams from different inputs, violating hardware assumptions.
--

--- Patch [1]: [PATCH v5 1/4] media: nxp: imx8-isi: Use BIT_ULL() for 64-bit stream masks ---
commit 4ed3fcf5e835032b4860a0f0d335cd63bb0bee70
Author: Guoniu Zhou <guoniu.zhou@oss.nxp.com>

media: nxp: imx8-isi: Use BIT_ULL() for 64-bit stream masks

This commit replaces BIT() with BIT_ULL() for 64-bit stream masks to avoid
incorrect results on 32-bit architectures when stream IDs are 32 or greater.

> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> @@ -145,10 +145,10 @@ mxc_isi_crossbar_xlate_streams(struct mxc_isi_crossbar *xbar,
>  	 */
>  	for_each_active_route(&state->routing, route) {
>  		if (route->source_pad != source_pad ||
> -		    !(source_streams & BIT(route->source_stream)))
> +		    !(source_streams & BIT_ULL(route->source_stream)))
>  			continue;
>  
> -		sink_streams |= BIT(route->sink_stream);
> +		sink_streams |= BIT_ULL(route->sink_stream);
>  		sink_pad = route->sink_pad;

[Severity: High]
This isn't a regression introduced by this patch, but a pre-existing issue.
Could this stream translation silently overwrite the sink pad and mix streams
from different inputs?

In mxc_isi_crossbar_xlate_streams(), the loop iterates over all routes matching
the source pad and source_streams mask. It appears to blindly overwrite
sink_pad and bitwise-OR the sink_streams without checking if they originate
from the same sink pad.

If userspace sets up routing where streams from different sink pads are routed
to the same source pad on different stream ids, could this cause only the last
overwritten sink_pad to be returned?

This might lead to only one remote subdevice being enabled while abandoning
others, passing an invalidly mixed stream mask to the enabled device.

>  	}
>  

As an additional observation in this file:

[Severity: High]
This isn't a regression introduced by this patch, but a pre-existing issue.
Is the old routing state checked instead of the newly requested routing during
validation?

In __mxc_isi_crossbar_set_routing():

    for_each_active_route(&state->routing, route) {

When userspace calls VIDIOC_SUBDEV_S_ROUTING with an invalid route mapping the
memory input to a pipeline other than the first, does this validate the
request against the old routing table (&state->routing) rather than the new
routing array passed as a parameter?

Since the old table does not contain the invalid route yet, it appears this
check might pass incorrectly and allow impossible hardware configurations to
be accepted.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-isi-v5-0-1d22ab91602a@oss.nxp.com?part=1

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

[sashiko-bot]

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 <guoniu.zhou@oss.nxp.com>
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/20260728-isi-v5-0-1d22ab91602a@oss.nxp.com?part=4