[PATCH v5 0/2] media: nxp: imx8-isi: Add virtual channel and frame descriptor support

[PATCH v5 0/2] media: nxp: imx8-isi: Add virtual channel and frame descriptor support

[Guoniu Zhou]

This patch series enhances the i.MX ISI driver's with virtual channel
support and adds frame descriptor capabilities to the crossbar subdevice.

Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v5:
- Rebase to latest media/next
- Swap patch order
- Return -EPIPE instead of -EINVAL for stream configuration errors
- Clear VC_ID_1 after generic mask to follow generic-then-conditional order
- Pass vc as function parameter instead of storing in pipe structure.
- Drop get_frame_desc fallback as crossbar now implements the operation
- Remove redundant num_entries check in mxc_isi_get_vc().
- Set vc to 0 for M2M as it doesn't support virtual channels.
- Use v4l2_subdev_get_frame_desc_passthrough helper
- Rewrote commit message
- Link to v4: https://lore.kernel.org/r/20260508-isi_vc-v4-0-feee39c63939@oss.nxp.com

Changes in v4:
- Rebase to latest media/next(previous dependency now in mainline)
- Fix VC boundary check: use num_vc (virtual channels count) instead of
  num_channels (ISI pipelines count)
- Set VC to 0 when frame descriptor has no entries
- Move platform-specific comments to block style to fix line length warnings
- Use %d instead of %u for ret variable in error messages
- Fix potential -ENOIOCTLCMD leak by resetting ret to 0 on continue
- See each patch's changelog for details
- Link to v3: https://lore.kernel.org/r/20260328-isi_vc-v3-0-a03b9a6fe117@oss.nxp.com

Changes in v3:
- Rebased on latest media/next
- Add num_vc field to platform data to indicate VC support
- Clear VC_ID_1 bit after reading CHNL_CTRL for proper VC switching
- Set VC_ID_1 only on platforms with num_vc > 4
- Improve mxc_isi_get_vc() error handling
- Add back CHNL_CTRL_BLANK_PXL and document platform-specific register fields
- Add xbar get_frame_desc() implementation (feedback from Laurent Pinchart)
- Link to v2: https://lore.kernel.org/r/20260310-isi_vc-v2-1-acbf77db8e6f@nxp.com

Changes in v2:
- Add Rb tag from Frank Li
- Fix typo in comment(s/support/supports/)
- Update commit log to include more details about ISI virtual channel support
  on different platform
- Include bitfield.h file to fix following build error
  drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h:23:65: error: implicit declaration of function ‘FIELD_PREP’ [-Wimplicit-function-declaration]
- Link to v1: https://lore.kernel.org/r/20260309-isi_vc-v1-1-fd0b8035d1cd@nxp.com

Changes in v1:
- Depends on https://lore.kernel.org/linux-media/20251105-isi_imx95-v3-2-3987533cca1c@nxp.com/

---
Guoniu Zhou (1):
      media: nxp: imx8-isi: Add virtual channel support

Guoniu.zhou (1):
      media: imx8-isi: crossbar: Add get_frame_desc operation

 .../media/platform/nxp/imx8-isi/imx8-isi-core.c    |  3 ++
 .../media/platform/nxp/imx8-isi/imx8-isi-core.h    |  2 +
 .../platform/nxp/imx8-isi/imx8-isi-crossbar.c      |  1 +
 drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c  | 17 +++++++-
 drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c |  2 +-
 .../media/platform/nxp/imx8-isi/imx8-isi-pipe.c    | 50 +++++++++++++++++++++-
 .../media/platform/nxp/imx8-isi/imx8-isi-regs.h    | 12 ++++--
 7 files changed, 80 insertions(+), 7 deletions(-)
---
base-commit: 2c8fe1f14240d75f2002e16b2b69c5c2d27ed41c
change-id: 20260309-isi_vc-285fd815140e

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

[PATCH v5 1/2] media: imx8-isi: crossbar: Add get_frame_desc operation

[Guoniu Zhou]

From: "Guoniu.zhou" <guoniu.zhou@nxp.com>

Implement the get_frame_desc pad operation for the crossbar subdev using
the v4l2_subdev_get_frame_desc_passthrough() helper. This allows the
crossbar to properly propagate frame descriptors from its sink pads to
its source pads, which is necessary for proper stream configuration in
multiplexed streams scenarios.

Signed-off-by: Guoniu.zhou <guoniu.zhou@nxp.com>
---
Changes in v5:
- Use v4l2_subdev_get_frame_desc_passthrough helper
- Rewrote commit message

Changes in v4:
- Use %d instead of %u for ret variable in error messages
- Fix potential -ENOIOCTLCMD leak by resetting ret to 0 on continue

Changes in v3:
- New patch added based on feedback from Laurent Pinchart
---
 drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c | 1 +
 1 file changed, 1 insertion(+)

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 605a45124103..0b593aed618b 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -404,6 +404,7 @@ static const struct v4l2_subdev_pad_ops mxc_isi_crossbar_subdev_pad_ops = {
 	.enum_mbus_code = mxc_isi_crossbar_enum_mbus_code,
 	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = mxc_isi_crossbar_set_fmt,
+	.get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
 	.set_routing = mxc_isi_crossbar_set_routing,
 	.enable_streams = mxc_isi_crossbar_enable_streams,
 	.disable_streams = mxc_isi_crossbar_disable_streams,

-- 
2.34.1

[PATCH v5 2/2] media: nxp: imx8-isi: Add virtual channel support

[Guoniu Zhou]

From: Guoniu Zhou <guoniu.zhou@nxp.com>

The ISI supports different numbers of virtual channels depending on the
platform. i.MX95 supports 8 virtual channels, and i.MX8QXP/QM support 4
virtual channels. They are used in multiple camera use cases, such as
surround view. Other platforms (such as i.MX8/MN/MP/ULP/91/93) don't
support virtual channels, and the VC_ID bits are marked as read-only.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v5:
- Return -EPIPE instead of -EINVAL for stream configuration errors
- Clear VC_ID_1 after generic mask to follow generic-then-conditional order
- Pass vc as function parameter instead of storing in pipe structure.
- Drop get_frame_desc fallback as crossbar now implements the operation
- Remove redundant num_entries check in mxc_isi_get_vc().
- Set vc to 0 for M2M as it doesn't support virtual channels.

Changes in v4:
- Fix VC boundary check: use num_vc (virtual channels count) instead of
  num_channels (ISI pipelines count)
- Set VC to 0 when frame descriptor has no entries
- Move platform-specific comments to block style to fix line length warnings

Changes in v3:
- Add num_vc field to platform data to indicate VC support
- Clear VC_ID_1 bit after reading CHNL_CTRL for proper VC switching
- Set VC_ID_1 only on platforms with num_vc > 4
- Improve mxc_isi_get_vc() error handling
- Add back CHNL_CTRL_BLANK_PXL and document platform-specific register fields
---
 .../media/platform/nxp/imx8-isi/imx8-isi-core.c    |  3 ++
 .../media/platform/nxp/imx8-isi/imx8-isi-core.h    |  2 +
 drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c  | 17 +++++++-
 drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c |  2 +-
 .../media/platform/nxp/imx8-isi/imx8-isi-pipe.c    | 50 +++++++++++++++++++++-
 .../media/platform/nxp/imx8-isi/imx8-isi-regs.h    | 12 ++++--
 6 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 4bf8570e1b9e..837ac7046cf2 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -318,6 +318,7 @@ static const struct mxc_isi_plat_data mxc_imx95_data = {
 	.model			= MXC_ISI_IMX95,
 	.num_ports		= 4,
 	.num_channels		= 8,
+	.num_vc			= 8,
 	.reg_offset		= 0x10000,
 	.ier_reg		= &mxc_imx8_isi_ier_v2,
 	.set_thd		= &mxc_imx8_isi_thd_v1,
@@ -329,6 +330,7 @@ static const struct mxc_isi_plat_data mxc_imx8qm_data = {
 	.model			= MXC_ISI_IMX8QM,
 	.num_ports		= 5,
 	.num_channels		= 8,
+	.num_vc			= 4,
 	.reg_offset		= 0x10000,
 	.ier_reg		= &mxc_imx8_isi_ier_qm,
 	.set_thd		= &mxc_imx8_isi_thd_v1,
@@ -340,6 +342,7 @@ static const struct mxc_isi_plat_data mxc_imx8qxp_data = {
 	.model			= MXC_ISI_IMX8QXP,
 	.num_ports		= 5,
 	.num_channels		= 6,
+	.num_vc			= 4,
 	.reg_offset		= 0x10000,
 	.ier_reg		= &mxc_imx8_isi_ier_v2,
 	.set_thd		= &mxc_imx8_isi_thd_v1,
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 14d63ec36416..2957119c81f2 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -169,6 +169,7 @@ struct mxc_isi_plat_data {
 	enum model model;
 	unsigned int num_ports;
 	unsigned int num_channels;
+	unsigned int num_vc;		/* Number of VCs, 0 = no VC support */
 	unsigned int reg_offset;
 	const struct mxc_isi_ier_reg  *ier_reg;
 	const struct mxc_isi_set_thd *set_thd;
@@ -377,6 +378,7 @@ void mxc_isi_channel_unchain(struct mxc_isi_pipe *pipe);
 
 void mxc_isi_channel_config(struct mxc_isi_pipe *pipe,
 			    enum mxc_isi_input_id input,
+			    unsigned int vc,
 			    const struct v4l2_area *in_size,
 			    const struct v4l2_area *scale,
 			    const struct v4l2_rect *crop,
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c
index 0187d4ab97e8..a98d7bec731d 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c
@@ -301,6 +301,7 @@ static void mxc_isi_channel_set_panic_threshold(struct mxc_isi_pipe *pipe)
 
 static void mxc_isi_channel_set_control(struct mxc_isi_pipe *pipe,
 					enum mxc_isi_input_id input,
+					unsigned int vc,
 					bool bypass)
 {
 	u32 val;
@@ -312,6 +313,10 @@ static void mxc_isi_channel_set_control(struct mxc_isi_pipe *pipe,
 		 CHNL_CTRL_SRC_TYPE_MASK | CHNL_CTRL_MIPI_VC_ID_MASK |
 		 CHNL_CTRL_SRC_INPUT_MASK);
 
+	/* Clear the VC_ID_1 bit on platforms supporting more than 4 VCs. */
+	if (pipe->isi->pdata->num_vc > 4)
+		val &= ~CHNL_CTRL_VC_ID_1_MASK;
+
 	/*
 	 * If no scaling or color space conversion is needed, bypass the
 	 * channel.
@@ -338,7 +343,14 @@ static void mxc_isi_channel_set_control(struct mxc_isi_pipe *pipe,
 	} else {
 		val |= CHNL_CTRL_SRC_TYPE(CHNL_CTRL_SRC_TYPE_DEVICE);
 		val |= CHNL_CTRL_SRC_INPUT(input);
-		val |= CHNL_CTRL_MIPI_VC_ID(0); /* FIXME: For CSI-2 only */
+		val |= CHNL_CTRL_MIPI_VC_ID(vc); /* FIXME: For CSI-2 only */
+
+		/*
+		 * On platforms with more than 4 VCs (i.MX95), the VC ID is
+		 * split across VC_ID_0 (bits 7:6) and VC_ID_1 (bit 16).
+		 */
+		if (pipe->isi->pdata->num_vc > 4)
+			val |= CHNL_CTRL_VC_ID_1(vc >> 2);
 	}
 
 	mxc_isi_write(pipe, CHNL_CTRL, val);
@@ -348,6 +360,7 @@ static void mxc_isi_channel_set_control(struct mxc_isi_pipe *pipe,
 
 void mxc_isi_channel_config(struct mxc_isi_pipe *pipe,
 			    enum mxc_isi_input_id input,
+			    unsigned int vc,
 			    const struct v4l2_area *in_size,
 			    const struct v4l2_area *scale,
 			    const struct v4l2_rect *crop,
@@ -374,7 +387,7 @@ void mxc_isi_channel_config(struct mxc_isi_pipe *pipe,
 	mxc_isi_channel_set_panic_threshold(pipe);
 
 	/* Channel control */
-	mxc_isi_channel_set_control(pipe, input, csc_bypass && scaler_bypass);
+	mxc_isi_channel_set_control(pipe, input, vc, csc_bypass && scaler_bypass);
 }
 
 void mxc_isi_channel_set_input_format(struct mxc_isi_pipe *pipe,
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c
index a39ad7a1ab18..291907ef44cb 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c
@@ -144,7 +144,7 @@ static void mxc_isi_m2m_device_run(void *priv)
 			.height = ctx->queues.cap.format.height,
 		};
 
-		mxc_isi_channel_config(m2m->pipe, MXC_ISI_INPUT_MEM,
+		mxc_isi_channel_config(m2m->pipe, MXC_ISI_INPUT_MEM, 0,
 				       &in_size, &scale, &crop,
 				       ctx->queues.out.info->encoding,
 				       ctx->queues.cap.info->encoding);
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 a41c51dd9ce0..03e0115b5b5a 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
@@ -232,6 +232,47 @@ static inline struct mxc_isi_pipe *to_isi_pipe(struct v4l2_subdev *sd)
 	return container_of(sd, struct mxc_isi_pipe, sd);
 }
 
+static int mxc_isi_get_vc(struct mxc_isi_pipe *pipe)
+{
+	struct mxc_isi_crossbar *xbar = &pipe->isi->crossbar;
+	struct device *dev = pipe->isi->dev;
+	struct v4l2_mbus_frame_desc fd = { };
+	unsigned int source_pad = xbar->num_sinks + pipe->id;
+	unsigned int max_vc;
+	unsigned int i;
+	int ret;
+
+	ret = v4l2_subdev_call(&xbar->sd, pad, get_frame_desc,
+			       source_pad, &fd);
+	if (ret < 0) {
+		dev_err(dev, "Failed to get source frame desc from pad %u\n",
+			source_pad);
+		return ret;
+	}
+
+	/* Find stream 0 in the frame descriptor */
+	for (i = 0; i < fd.num_entries; i++) {
+		if (fd.entry[i].stream == 0)
+			break;
+	}
+
+	if (i == fd.num_entries) {
+		dev_err(dev, "Failed to find stream from source frame desc\n");
+		return -EPIPE;
+	}
+
+	max_vc = pipe->isi->pdata->num_vc ? : 1;
+
+	/* Check virtual channel range */
+	if (fd.entry[i].bus.csi2.vc >= max_vc) {
+		dev_err(dev, "Virtual channel %u exceeds maximum %u\n",
+			fd.entry[i].bus.csi2.vc, max_vc - 1);
+		return -EPIPE;
+	}
+
+	return fd.entry[i].bus.csi2.vc;
+}
+
 int mxc_isi_pipe_enable(struct mxc_isi_pipe *pipe)
 {
 	struct mxc_isi_crossbar *xbar = &pipe->isi->crossbar;
@@ -244,6 +285,7 @@ int mxc_isi_pipe_enable(struct mxc_isi_pipe *pipe)
 	struct v4l2_subdev *sd = &pipe->sd;
 	struct v4l2_area in_size, scale;
 	struct v4l2_rect crop;
+	unsigned int vc;
 	u32 input;
 	int ret;
 
@@ -280,8 +322,14 @@ int mxc_isi_pipe_enable(struct mxc_isi_pipe *pipe)
 
 	v4l2_subdev_unlock_state(state);
 
+	ret = mxc_isi_get_vc(pipe);
+	if (ret < 0)
+		return ret;
+
+	vc = ret;
+
 	/* Configure the ISI channel. */
-	mxc_isi_channel_config(pipe, input, &in_size, &scale, &crop,
+	mxc_isi_channel_config(pipe, input, vc, &in_size, &scale, &crop,
 			       sink_info->encoding, src_info->encoding);
 
 	mxc_isi_channel_enable(pipe);
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h
index 1b65eccdf0da..e795f4daf3ff 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-regs.h
@@ -6,6 +6,7 @@
 #ifndef __IMX8_ISI_REGS_H__
 #define __IMX8_ISI_REGS_H__
 
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 
 /* ISI Registers Define  */
@@ -19,9 +20,14 @@
 #define CHNL_CTRL_CHAIN_BUF_NO_CHAIN				0
 #define CHNL_CTRL_CHAIN_BUF_2_CHAIN				1
 #define CHNL_CTRL_SW_RST					BIT(24)
-#define CHNL_CTRL_BLANK_PXL(n)					((n) << 16)
-#define CHNL_CTRL_BLANK_PXL_MASK				GENMASK(23, 16)
-#define CHNL_CTRL_MIPI_VC_ID(n)					((n) << 6)
+/*
+ * CHNL_CTRL_BLANK_PXL: i.MX8{QM,QXP} only
+ * CHNL_CTRL_VC_ID_1, CHNL_CTRL_VC_ID_1_MASK: i.MX95 only
+ */
+#define CHNL_CTRL_BLANK_PXL(n)					FIELD_PREP(GENMASK(23, 16), (n))
+#define CHNL_CTRL_VC_ID_1(n)					FIELD_PREP(BIT(16), (n))
+#define CHNL_CTRL_VC_ID_1_MASK					BIT(16)
+#define CHNL_CTRL_MIPI_VC_ID(n)					FIELD_PREP(GENMASK(7, 6), (n))
 #define CHNL_CTRL_MIPI_VC_ID_MASK				GENMASK(7, 6)
 #define CHNL_CTRL_SRC_TYPE(n)					((n) << 4)
 #define CHNL_CTRL_SRC_TYPE_MASK					BIT(4)

-- 
2.34.1

Re: [PATCH v5 2/2] media: nxp: imx8-isi: Add virtual channel support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Pipeline enablement unconditionally fails if the upstream subdevice does not implement the `get_frame_desc` operation.
- [Medium] The driver unconditionally accesses CSI-2 specific fields in the frame descriptor without validating the frame descriptor type.
--

commit 4d8a62dc345af085b3b0a217c07143648e85fd35
Author: Guoniu Zhou <guoniu.zhou@nxp.com>
Subject: media: nxp: imx8-isi: Add virtual channel support

This commit introduces virtual channel (VC) support to the i.MX8 ISI driver.
It retrieves the VC from the remote upstream subdevice's frame descriptor and
applies the required register configurations based on the platform's maximum
supported VCs.

> 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 a41c51dd9ce0f..03e0115b5b5a8 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c

[ ... ]

> +static int mxc_isi_get_vc(struct mxc_isi_pipe *pipe)
> +{
> +	struct mxc_isi_crossbar *xbar = &pipe->isi->crossbar;
> +	struct device *dev = pipe->isi->dev;
> +	struct v4l2_mbus_frame_desc fd = { };
> +	unsigned int source_pad = xbar->num_sinks + pipe->id;
> +	unsigned int max_vc;
> +	unsigned int i;
> +	int ret;
> +
> +	ret = v4l2_subdev_call(&xbar->sd, pad, get_frame_desc,
> +			       source_pad, &fd);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to get source frame desc from pad %u\n",
> +			source_pad);
> +		return ret;
> +	}

[Severity: High]
If the remote upstream subdevice does not implement the optional
get_frame_desc operation, v4l2_subdev_call() will return -ENOIOCTLCMD.

Does this mean pipeline enablement will now unconditionally fail for any
sensor lacking get_frame_desc support?

Prior to this patch, the VC was implicitly hardcoded to 0, which worked
seamlessly for parallel and legacy CSI-2 sensors. Would it be better to
explicitly check for -ENOIOCTLCMD (and potentially -EOPNOTSUPP) here and
gracefully fallback to a default VC of 0 to prevent this regression?

> +
> +	/* Find stream 0 in the frame descriptor */
> +	for (i = 0; i < fd.num_entries; i++) {
> +		if (fd.entry[i].stream == 0)
> +			break;
> +	}
> +
> +	if (i == fd.num_entries) {
> +		dev_err(dev, "Failed to find stream from source frame desc\n");
> +		return -EPIPE;
> +	}
> +
> +	max_vc = pipe->isi->pdata->num_vc ? : 1;
> +
> +	/* Check virtual channel range */
> +	if (fd.entry[i].bus.csi2.vc >= max_vc) {

[Severity: Medium]
This code directly accesses fd.entry[i].bus.csi2.vc, but is it guaranteed
that the frame descriptor type is V4L2_MBUS_FRAME_DESC_TYPE_CSI2?

Since the i.MX8 ISI also supports parallel cameras, fd.type could be
V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL, making the access to bus.csi2
semantically invalid. 

Should there be a check to validate fd.type before accessing the CSI-2
specific union members to avoid potential undefined behavior?

> +		dev_err(dev, "Virtual channel %u exceeds maximum %u\n",
> +			fd.entry[i].bus.csi2.vc, max_vc - 1);
> +		return -EPIPE;
> +	}
> +
> +	return fd.entry[i].bus.csi2.vc;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-isi_vc-v5-0-a38eb4fcd58e@oss.nxp.com?part=2

Re: (subset) [PATCH v5 0/2] media: nxp: imx8-isi: Add virtual channel and frame descriptor support

[Frank.Li]

From: Frank Li <Frank.Li@nxp.com>


On Thu, 21 May 2026 17:10:03 +0800, Guoniu Zhou wrote:
> This patch series enhances the i.MX ISI driver's with virtual channel
> support and adds frame descriptor capabilities to the crossbar subdevice.

Applied, thanks!

[1/2] media: imx8-isi: crossbar: Add get_frame_desc operation
      commit: 3e15a3510908c990ee352aa206d5f9c23d4b216e

Best regards,
-- 
Frank Li <Frank.Li@nxp.com>

Re: [PATCH v5 2/2] media: nxp: imx8-isi: Add virtual channel support

[Frank Li]

On Thu, May 21, 2026 at 10:06:31AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Pipeline enablement unconditionally fails if the upstream subdevice does not implement the `get_frame_desc` operation.
> - [Medium] The driver unconditionally accesses CSI-2 specific fields in the frame descriptor without validating the frame descriptor type.
> --
>
> commit 4d8a62dc345af085b3b0a217c07143648e85fd35
> Author: Guoniu Zhou <guoniu.zhou@nxp.com>
> Subject: media: nxp: imx8-isi: Add virtual channel support
>
> This commit introduces virtual channel (VC) support to the i.MX8 ISI driver.
> It retrieves the VC from the remote upstream subdevice's frame descriptor and
> applies the required register configurations based on the platform's maximum
> supported VCs.
>
> > 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 a41c51dd9ce0f..03e0115b5b5a8 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
>
> [ ... ]
>
> > +static int mxc_isi_get_vc(struct mxc_isi_pipe *pipe)
> > +{
> > +	struct mxc_isi_crossbar *xbar = &pipe->isi->crossbar;
> > +	struct device *dev = pipe->isi->dev;
> > +	struct v4l2_mbus_frame_desc fd = { };
> > +	unsigned int source_pad = xbar->num_sinks + pipe->id;
> > +	unsigned int max_vc;
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	ret = v4l2_subdev_call(&xbar->sd, pad, get_frame_desc,
> > +			       source_pad, &fd);
> > +	if (ret < 0) {
> > +		dev_err(dev, "Failed to get source frame desc from pad %u\n",
> > +			source_pad);
> > +		return ret;
> > +	}
>
> [Severity: High]
> If the remote upstream subdevice does not implement the optional
> get_frame_desc operation, v4l2_subdev_call() will return -ENOIOCTLCMD.
>
> Does this mean pipeline enablement will now unconditionally fail for any
> sensor lacking get_frame_desc support?
>
> Prior to this patch, the VC was implicitly hardcoded to 0, which worked
> seamlessly for parallel and legacy CSI-2 sensors. Would it be better to
> explicitly check for -ENOIOCTLCMD (and potentially -EOPNOTSUPP) here and
> gracefully fallback to a default VC of 0 to prevent this regression?
>
> > +
> > +	/* Find stream 0 in the frame descriptor */
> > +	for (i = 0; i < fd.num_entries; i++) {
> > +		if (fd.entry[i].stream == 0)
> > +			break;
> > +	}
> > +
> > +	if (i == fd.num_entries) {
> > +		dev_err(dev, "Failed to find stream from source frame desc\n");
> > +		return -EPIPE;
> > +	}
> > +
> > +	max_vc = pipe->isi->pdata->num_vc ? : 1;
> > +
> > +	/* Check virtual channel range */
> > +	if (fd.entry[i].bus.csi2.vc >= max_vc) {
>
> [Severity: Medium]
> This code directly accesses fd.entry[i].bus.csi2.vc, but is it guaranteed
> that the frame descriptor type is V4L2_MBUS_FRAME_DESC_TYPE_CSI2?
>
> Since the i.MX8 ISI also supports parallel cameras, fd.type could be
> V4L2_MBUS_FRAME_DESC_TYPE_PARALLEL, making the access to bus.csi2
> semantically invalid.
>
> Should there be a check to validate fd.type before accessing the CSI-2
> specific union members to avoid potential undefined behavior?


Look like make sense,

Can you double check it?

Frank

>
> > +		dev_err(dev, "Virtual channel %u exceeds maximum %u\n",
> > +			fd.entry[i].bus.csi2.vc, max_vc - 1);
> > +		return -EPIPE;
> > +	}
> > +
> > +	return fd.entry[i].bus.csi2.vc;
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260521-isi_vc-v5-0-a38eb4fcd58e@oss.nxp.com?part=2

Re: (subset) [PATCH v5 0/2] media: nxp: imx8-isi: Add virtual channel and frame descriptor support

[Laurent Pinchart]

On Mon, Jun 29, 2026 at 03:42:31PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> 
> On Thu, 21 May 2026 17:10:03 +0800, Guoniu Zhou wrote:
> > This patch series enhances the i.MX ISI driver's with virtual channel
> > support and adds frame descriptor capabilities to the crossbar subdevice.
> 
> Applied, thanks!
> 
> [1/2] media: imx8-isi: crossbar: Add get_frame_desc operation
>       commit: 3e15a3510908c990ee352aa206d5f9c23d4b216e

Is this a mistake ? Patch 1/2 has no R-b tag, and you're not listed as
maintainer for this driver.

-- 
Regards,

Laurent Pinchart

Re: (subset) [PATCH v5 0/2] media: nxp: imx8-isi: Add virtual channel and frame descriptor support

[Frank Li]

On Mon, Jun 29, 2026 at 11:23:02PM +0300, Laurent Pinchart wrote:
> On Mon, Jun 29, 2026 at 03:42:31PM -0400, Frank.Li@oss.nxp.com wrote:
> > From: Frank Li <Frank.Li@nxp.com>
> >
> >
> > On Thu, 21 May 2026 17:10:03 +0800, Guoniu Zhou wrote:
> > > This patch series enhances the i.MX ISI driver's with virtual channel
> > > support and adds frame descriptor capabilities to the crossbar subdevice.
> >
> > Applied, thanks!
> >
> > [1/2] media: imx8-isi: crossbar: Add get_frame_desc operation
> >       commit: 3e15a3510908c990ee352aa206d5f9c23d4b216e
>
> Is this a mistake ? Patch 1/2 has no R-b tag, and you're not listed as
> maintainer for this driver.

Sorry, I missed checking Maintainer files, in media summit, agree on I pick
imx's media drivers, but forget finalize the file\dir list. Can you help
summery which files\dir I should take care?

If you have concern about this patch, I can drop it.

Frank


>
> --
> Regards,
>
> Laurent Pinchart

Re: (subset) [PATCH v5 0/2] media: nxp: imx8-isi: Add virtual channel and frame descriptor support

[Bryan O'Donoghue]

On 30/06/2026 17:20, Frank Li wrote:
> On Mon, Jun 29, 2026 at 11:23:02PM +0300, Laurent Pinchart wrote:
>> On Mon, Jun 29, 2026 at 03:42:31PM -0400, Frank.Li@oss.nxp.com wrote:
>>> From: Frank Li <Frank.Li@nxp.com>
>>>
>>>
>>> On Thu, 21 May 2026 17:10:03 +0800, Guoniu Zhou wrote:
>>>> This patch series enhances the i.MX ISI driver's with virtual channel
>>>> support and adds frame descriptor capabilities to the crossbar subdevice.
>>>
>>> Applied, thanks!
>>>
>>> [1/2] media: imx8-isi: crossbar: Add get_frame_desc operation
>>>        commit: 3e15a3510908c990ee352aa206d5f9c23d4b216e
>>
>> Is this a mistake ? Patch 1/2 has no R-b tag, and you're not listed as
>> maintainer for this driver.
> 
> Sorry, I missed checking Maintainer files, in media summit, agree on I pick
> imx's media drivers, but forget finalize the file\dir list. Can you help
> summery which files\dir I should take care?
> 
> If you have concern about this patch, I can drop it.
> 
> Frank
You should set yourself up here: 
https://patchwork.linuxtv.org/project/linux-media/list/

That way you can coordinate with other maintainers on which files get 
reviewed by whom, see what the RB and ci status is.

Its pretty essential.

---
bod

Re: (subset) [PATCH v5 0/2] media: nxp: imx8-isi: Add virtual channel and frame descriptor support

[Frank Li]

On Tue, Jun 30, 2026 at 11:20:53PM +0100, Bryan O'Donoghue wrote:
> On 30/06/2026 17:20, Frank Li wrote:
> > On Mon, Jun 29, 2026 at 11:23:02PM +0300, Laurent Pinchart wrote:
> > > On Mon, Jun 29, 2026 at 03:42:31PM -0400, Frank.Li@oss.nxp.com wrote:
> > > > From: Frank Li <Frank.Li@nxp.com>
> > > >
> > > >
> > > > On Thu, 21 May 2026 17:10:03 +0800, Guoniu Zhou wrote:
> > > > > This patch series enhances the i.MX ISI driver's with virtual channel
> > > > > support and adds frame descriptor capabilities to the crossbar subdevice.
> > > >
> > > > Applied, thanks!
> > > >
> > > > [1/2] media: imx8-isi: crossbar: Add get_frame_desc operation
> > > >        commit: 3e15a3510908c990ee352aa206d5f9c23d4b216e
> > >
> > > Is this a mistake ? Patch 1/2 has no R-b tag, and you're not listed as
> > > maintainer for this driver.
> >
> > Sorry, I missed checking Maintainer files, in media summit, agree on I pick
> > imx's media drivers, but forget finalize the file\dir list. Can you help
> > summery which files\dir I should take care?
> >
> > If you have concern about this patch, I can drop it.
> >
> > Frank
> You should set yourself up here:
> https://patchwork.linuxtv.org/project/linux-media/list/

I just register it.

"Registration successful!

A confirmation email has been sent to frank.li@kernel.org. You'll need to visit the link provided in that email to confirm your registration."

I have not recieved such confirm email. Anything wrong?

Frank


>
> That way you can coordinate with other maintainers on which files get
> reviewed by whom, see what the RB and ci status is.
>
> Its pretty essential.
>
> ---
> bod

Re: (subset) [PATCH v5 0/2] media: nxp: imx8-isi: Add virtual channel and frame descriptor support

[Frank Li]

On Wed, Jul 01, 2026 at 10:51:32AM -0500, Frank Li wrote:
> On Tue, Jun 30, 2026 at 11:20:53PM +0100, Bryan O'Donoghue wrote:
> > On 30/06/2026 17:20, Frank Li wrote:
> > > On Mon, Jun 29, 2026 at 11:23:02PM +0300, Laurent Pinchart wrote:
> > > > On Mon, Jun 29, 2026 at 03:42:31PM -0400, Frank.Li@oss.nxp.com wrote:
> > > > > From: Frank Li <Frank.Li@nxp.com>
> > > > >
> > > > >
> > > > > On Thu, 21 May 2026 17:10:03 +0800, Guoniu Zhou wrote:
> > > > > > This patch series enhances the i.MX ISI driver's with virtual channel
> > > > > > support and adds frame descriptor capabilities to the crossbar subdevice.
> > > > >
> > > > > Applied, thanks!
> > > > >
> > > > > [1/2] media: imx8-isi: crossbar: Add get_frame_desc operation
> > > > >        commit: 3e15a3510908c990ee352aa206d5f9c23d4b216e
> > > >
> > > > Is this a mistake ? Patch 1/2 has no R-b tag, and you're not listed as
> > > > maintainer for this driver.
> > >
> > > Sorry, I missed checking Maintainer files, in media summit, agree on I pick
> > > imx's media drivers, but forget finalize the file\dir list. Can you help
> > > summery which files\dir I should take care?
> > >
> > > If you have concern about this patch, I can drop it.
> > >
> > > Frank
> > You should set yourself up here:
> > https://patchwork.linuxtv.org/project/linux-media/list/
>
> I just register it.
>
> "Registration successful!
>
> A confirmation email has been sent to frank.li@kernel.org. You'll need to visit the link provided in that email to confirm your registration."
>
> I have not recieved such confirm email. Anything wrong?

I just got it. But I am not in delegate list yet.

Frank

>
> Frank
>
>
> >
> > That way you can coordinate with other maintainers on which files get
> > reviewed by whom, see what the RB and ci status is.
> >
> > Its pretty essential.
> >
> > ---
> > bod
>