* [PATCH v1 1/6] media: imx-mipi-csis: Add VC-related register fields
2025-11-07 1:58 [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Laurent Pinchart
@ 2025-11-07 1:58 ` Laurent Pinchart
2025-11-07 16:19 ` Frank Li
2025-11-07 1:58 ` [PATCH v1 2/6] media: imx-mipi-csis: Switch to .enable_streams() Laurent Pinchart
` (5 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 1:58 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team,
Pengutronix Kernel Team, imx, Frank Li, Stefan Klug, Sakari Ailus
The CMN_CTRL and ISPCFG registers have fiels related to virtual channel
handling. In CMN_CTRL, the INTERLEAVE_MODE field is 2 bits wide, and
controls filtering by DT, VC or both. The VC number is then set in the
ISPCFG.VIRTUAL_CHANNEL field.
Expand the definition of the register macros to support those features,
and set the VC ID to 0 explicitly instead of relying on the default
register value. This prepares for VC filtering but does not modify the
driver's behaviour.
While at it, use GENMASK in the last few register mask macros that don't
use it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/nxp/imx-mipi-csis.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index d5de7854f579..42b1ec8eed96 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -55,7 +55,11 @@
/* CSIS common control */
#define MIPI_CSIS_CMN_CTRL 0x04
#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(n) BIT((n) + 16)
-#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT BIT(10)
+#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_NONE (0 << 10)
+#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT (1 << 10)
+#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_VC (2 << 10)
+#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_VCDT (3 << 10)
+#define MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_MASK GENMASK(11, 10)
#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER(n) ((n) << 8)
#define MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK GENMASK(9, 8)
#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL BIT(2)
@@ -179,6 +183,8 @@
#define MIPI_CSIS_ISPCFG_PARALLEL BIT(11)
#define MIPI_CSIS_ISPCFG_DATAFORMAT(fmt) ((fmt) << 2)
#define MIPI_CSIS_ISPCFG_DATAFORMAT_MASK GENMASK(7, 2)
+#define MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(vc) ((vc) << 0)
+#define MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL_MASK GENMASK(1, 0)
/* ISP Image Resolution register */
#define MIPI_CSIS_ISP_RESOL_CH(n) (0x44 + (n) * 0x10)
@@ -588,7 +594,8 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
/* Color format */
val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(0));
val &= ~(MIPI_CSIS_ISPCFG_PARALLEL | MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK |
- MIPI_CSIS_ISPCFG_DATAFORMAT_MASK);
+ MIPI_CSIS_ISPCFG_DATAFORMAT_MASK |
+ MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL_MASK);
/*
* YUV 4:2:2 can be transferred with 8 or 16 bits per clock sample
@@ -607,6 +614,8 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
val |= MIPI_CSIS_ISPCFG_DATAFORMAT(csis_fmt->data_type);
+ val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
+
mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
/* Pixel resolution */
@@ -672,10 +681,14 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
u32 val;
val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
- val &= ~MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK;
+ val &= ~(MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK |
+ MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_MASK);
+
val |= MIPI_CSIS_CMN_CTRL_LANE_NUMBER(lanes - 1);
+
if (csis->info->version == MIPI_CSIS_V3_3)
val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
+
mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
__mipi_csis_set_format(csis, format, csis_fmt);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v1 1/6] media: imx-mipi-csis: Add VC-related register fields
2025-11-07 1:58 ` [PATCH v1 1/6] media: imx-mipi-csis: Add VC-related register fields Laurent Pinchart
@ 2025-11-07 16:19 ` Frank Li
2025-11-07 18:44 ` Laurent Pinchart
0 siblings, 1 reply; 26+ messages in thread
From: Frank Li @ 2025-11-07 16:19 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 03:58:08AM +0200, Laurent Pinchart wrote:
> The CMN_CTRL and ISPCFG registers have fiels related to virtual channel
> handling. In CMN_CTRL, the INTERLEAVE_MODE field is 2 bits wide, and
> controls filtering by DT, VC or both. The VC number is then set in the
> ISPCFG.VIRTUAL_CHANNEL field.
>
> Expand the definition of the register macros to support those features,
> and set the VC ID to 0 explicitly instead of relying on the default
> register value. This prepares for VC filtering but does not modify the
> driver's behaviour.
>
> While at it, use GENMASK in the last few register mask macros that don't
> use it.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx-mipi-csis.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
...
>
> val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> - val &= ~MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK;
> + val &= ~(MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK |
> + MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_MASK);
> +
> val |= MIPI_CSIS_CMN_CTRL_LANE_NUMBER(lanes - 1);
> +
> if (csis->info->version == MIPI_CSIS_V3_3)
> val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> +
above two empty line is not related this patch
Frank
> mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
>
> __mipi_csis_set_format(csis, format, csis_fmt);
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 1/6] media: imx-mipi-csis: Add VC-related register fields
2025-11-07 16:19 ` Frank Li
@ 2025-11-07 18:44 ` Laurent Pinchart
2025-11-07 20:37 ` Frank Li
0 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 18:44 UTC (permalink / raw)
To: Frank Li
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 11:19:53AM -0500, Frank Li wrote:
> On Fri, Nov 07, 2025 at 03:58:08AM +0200, Laurent Pinchart wrote:
> > The CMN_CTRL and ISPCFG registers have fiels related to virtual channel
> > handling. In CMN_CTRL, the INTERLEAVE_MODE field is 2 bits wide, and
> > controls filtering by DT, VC or both. The VC number is then set in the
> > ISPCFG.VIRTUAL_CHANNEL field.
> >
> > Expand the definition of the register macros to support those features,
> > and set the VC ID to 0 explicitly instead of relying on the default
> > register value. This prepares for VC filtering but does not modify the
> > driver's behaviour.
> >
> > While at it, use GENMASK in the last few register mask macros that don't
> > use it.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > drivers/media/platform/nxp/imx-mipi-csis.c | 19 ++++++++++++++++---
> > 1 file changed, 16 insertions(+), 3 deletions(-)
> >
> ...
> >
> > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > - val &= ~MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK;
> > + val &= ~(MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK |
> > + MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_MASK);
> > +
> > val |= MIPI_CSIS_CMN_CTRL_LANE_NUMBER(lanes - 1);
> > +
> > if (csis->info->version == MIPI_CSIS_V3_3)
> > val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> > +
>
> above two empty line is not related this patch
It's just a matter of giving the code more room to breathe. I can move
them to patch 6/6 if you prefer.
> > mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> >
> > __mipi_csis_set_format(csis, format, csis_fmt);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 1/6] media: imx-mipi-csis: Add VC-related register fields
2025-11-07 18:44 ` Laurent Pinchart
@ 2025-11-07 20:37 ` Frank Li
0 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-11-07 20:37 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 08:44:29PM +0200, Laurent Pinchart wrote:
> On Fri, Nov 07, 2025 at 11:19:53AM -0500, Frank Li wrote:
> > On Fri, Nov 07, 2025 at 03:58:08AM +0200, Laurent Pinchart wrote:
> > > The CMN_CTRL and ISPCFG registers have fiels related to virtual channel
> > > handling. In CMN_CTRL, the INTERLEAVE_MODE field is 2 bits wide, and
> > > controls filtering by DT, VC or both. The VC number is then set in the
> > > ISPCFG.VIRTUAL_CHANNEL field.
> > >
> > > Expand the definition of the register macros to support those features,
> > > and set the VC ID to 0 explicitly instead of relying on the default
> > > register value. This prepares for VC filtering but does not modify the
> > > driver's behaviour.
> > >
> > > While at it, use GENMASK in the last few register mask macros that don't
> > > use it.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > drivers/media/platform/nxp/imx-mipi-csis.c | 19 ++++++++++++++++---
> > > 1 file changed, 16 insertions(+), 3 deletions(-)
> > >
> > ...
> > >
> > > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > > - val &= ~MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK;
> > > + val &= ~(MIPI_CSIS_CMN_CTRL_LANE_NUMBER_MASK |
> > > + MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_MASK);
> > > +
> > > val |= MIPI_CSIS_CMN_CTRL_LANE_NUMBER(lanes - 1);
> > > +
> > > if (csis->info->version == MIPI_CSIS_V3_3)
> > > val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> > > +
> >
> > above two empty line is not related this patch
>
> It's just a matter of giving the code more room to breathe. I can move
> them to patch 6/6 if you prefer.
That will be great.
Frank
>
> > > mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> > >
> > > __mipi_csis_set_format(csis, format, csis_fmt);
>
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v1 2/6] media: imx-mipi-csis: Switch to .enable_streams()
2025-11-07 1:58 [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Laurent Pinchart
2025-11-07 1:58 ` [PATCH v1 1/6] media: imx-mipi-csis: Add VC-related register fields Laurent Pinchart
@ 2025-11-07 1:58 ` Laurent Pinchart
2025-11-07 16:29 ` Frank Li
2025-11-07 1:58 ` [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation Laurent Pinchart
` (4 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 1:58 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team,
Pengutronix Kernel Team, imx, Frank Li, Stefan Klug, Sakari Ailus
To prepare for multi-stream support, switch from the legacy .s_stream()
operation to the .enable_streams() and .disable_streams() operations.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/nxp/imx-mipi-csis.c | 146 ++++++++++++---------
1 file changed, 87 insertions(+), 59 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 42b1ec8eed96..f142e79acbcf 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -350,6 +350,7 @@ struct mipi_csis_device {
struct {
struct v4l2_subdev *sd;
const struct media_pad *pad;
+ u64 enabled_streams;
} source;
struct v4l2_mbus_config_mipi_csi2 bus;
@@ -1019,64 +1020,6 @@ static struct mipi_csis_device *sd_to_mipi_csis_device(struct v4l2_subdev *sdev)
return container_of(sdev, struct mipi_csis_device, sd);
}
-static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
-{
- struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
- const struct v4l2_mbus_framefmt *format;
- const struct csis_pix_format *csis_fmt;
- struct v4l2_subdev_state *state;
- int ret;
-
- if (!enable) {
- v4l2_subdev_disable_streams(csis->source.sd,
- csis->source.pad->index, BIT(0));
-
- mipi_csis_stop_stream(csis);
- if (csis->debug.enable)
- mipi_csis_log_counters(csis, true);
-
- pm_runtime_put(csis->dev);
-
- return 0;
- }
-
- state = v4l2_subdev_lock_and_get_active_state(sd);
-
- format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
- csis_fmt = find_csis_format(format->code);
-
- ret = mipi_csis_calculate_params(csis, csis_fmt);
- if (ret < 0)
- goto err_unlock;
-
- mipi_csis_clear_counters(csis);
-
- ret = pm_runtime_resume_and_get(csis->dev);
- if (ret < 0)
- goto err_unlock;
-
- mipi_csis_start_stream(csis, format, csis_fmt);
-
- ret = v4l2_subdev_enable_streams(csis->source.sd,
- csis->source.pad->index, BIT(0));
- if (ret < 0)
- goto err_stop;
-
- mipi_csis_log_counters(csis, true);
-
- v4l2_subdev_unlock_state(state);
-
- return 0;
-
-err_stop:
- mipi_csis_stop_stream(csis);
- pm_runtime_put(csis->dev);
-err_unlock:
- v4l2_subdev_unlock_state(state);
-
- return ret;
-}
-
static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_mbus_code_enum *code)
@@ -1211,6 +1154,89 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
return 0;
}
+static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ u32 pad, u64 streams_mask)
+{
+ struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
+ u64 sink_streams;
+ int ret;
+
+ sink_streams = v4l2_subdev_state_xlate_streams(state, CSIS_PAD_SOURCE,
+ CSIS_PAD_SINK,
+ &streams_mask);
+ if (!sink_streams || !streams_mask)
+ return -EINVAL;
+
+ /* Start the CSIS with the first stream. */
+ if (!csis->source.enabled_streams) {
+ const struct v4l2_mbus_framefmt *format;
+ const struct csis_pix_format *csis_fmt;
+
+ format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
+ csis_fmt = find_csis_format(format->code);
+
+ ret = mipi_csis_calculate_params(csis, csis_fmt);
+ if (ret)
+ return ret;
+
+ mipi_csis_clear_counters(csis);
+
+ ret = pm_runtime_resume_and_get(csis->dev);
+ if (ret < 0)
+ return ret;
+
+ mipi_csis_start_stream(csis, format, csis_fmt);
+ }
+
+ ret = v4l2_subdev_enable_streams(csis->source.sd,
+ csis->source.pad->index, sink_streams);
+ if (ret) {
+ if (!csis->source.enabled_streams) {
+ mipi_csis_stop_stream(csis);
+ pm_runtime_put(csis->dev);
+ }
+
+ return ret;
+ }
+
+ mipi_csis_log_counters(csis, true);
+
+ csis->source.enabled_streams |= streams_mask;
+
+ return 0;
+}
+
+static int mipi_csis_disable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ u32 pad, u64 streams_mask)
+{
+ struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
+ u64 sink_streams;
+
+ sink_streams = v4l2_subdev_state_xlate_streams(state, CSIS_PAD_SOURCE,
+ CSIS_PAD_SINK,
+ &streams_mask);
+ if (!sink_streams || !streams_mask)
+ return -EINVAL;
+
+ v4l2_subdev_disable_streams(csis->source.sd,
+ csis->source.pad->index, sink_streams);
+
+ csis->source.enabled_streams &= ~streams_mask;
+
+ /* Stop the CSIS with the last stream. */
+ if (!csis->source.enabled_streams) {
+ mipi_csis_stop_stream(csis);
+ pm_runtime_put(csis->dev);
+
+ if (csis->debug.enable)
+ mipi_csis_log_counters(csis, true);
+ }
+
+ return 0;
+}
+
static int mipi_csis_init_state(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state)
{
@@ -1263,7 +1289,7 @@ static const struct v4l2_subdev_core_ops mipi_csis_core_ops = {
};
static const struct v4l2_subdev_video_ops mipi_csis_video_ops = {
- .s_stream = mipi_csis_s_stream,
+ .s_stream = v4l2_subdev_s_stream_helper,
};
static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
@@ -1271,6 +1297,8 @@ static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = mipi_csis_set_fmt,
.get_frame_desc = mipi_csis_get_frame_desc,
+ .enable_streams = mipi_csis_enable_streams,
+ .disable_streams = mipi_csis_disable_streams,
};
static const struct v4l2_subdev_ops mipi_csis_subdev_ops = {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v1 2/6] media: imx-mipi-csis: Switch to .enable_streams()
2025-11-07 1:58 ` [PATCH v1 2/6] media: imx-mipi-csis: Switch to .enable_streams() Laurent Pinchart
@ 2025-11-07 16:29 ` Frank Li
2025-11-07 18:32 ` Laurent Pinchart
0 siblings, 1 reply; 26+ messages in thread
From: Frank Li @ 2025-11-07 16:29 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 03:58:09AM +0200, Laurent Pinchart wrote:
> To prepare for multi-stream support, switch from the legacy .s_stream()
> operation to the .enable_streams() and .disable_streams() operations.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx-mipi-csis.c | 146 ++++++++++++---------
> 1 file changed, 87 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 42b1ec8eed96..f142e79acbcf 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -350,6 +350,7 @@ struct mipi_csis_device {
> struct {
> struct v4l2_subdev *sd;
> const struct media_pad *pad;
> + u64 enabled_streams;
> } source;
>
> struct v4l2_mbus_config_mipi_csi2 bus;
> @@ -1019,64 +1020,6 @@ static struct mipi_csis_device *sd_to_mipi_csis_device(struct v4l2_subdev *sdev)
> return container_of(sdev, struct mipi_csis_device, sd);
> }
>
> -static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
> -{
> - struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> - const struct v4l2_mbus_framefmt *format;
> - const struct csis_pix_format *csis_fmt;
> - struct v4l2_subdev_state *state;
> - int ret;
> -
> - if (!enable) {
> - v4l2_subdev_disable_streams(csis->source.sd,
> - csis->source.pad->index, BIT(0));
> -
> - mipi_csis_stop_stream(csis);
> - if (csis->debug.enable)
> - mipi_csis_log_counters(csis, true);
> -
> - pm_runtime_put(csis->dev);
> -
> - return 0;
> - }
> -
> - state = v4l2_subdev_lock_and_get_active_state(sd);
> -
> - format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> - csis_fmt = find_csis_format(format->code);
> -
> - ret = mipi_csis_calculate_params(csis, csis_fmt);
> - if (ret < 0)
> - goto err_unlock;
> -
> - mipi_csis_clear_counters(csis);
> -
> - ret = pm_runtime_resume_and_get(csis->dev);
> - if (ret < 0)
> - goto err_unlock;
> -
> - mipi_csis_start_stream(csis, format, csis_fmt);
> -
> - ret = v4l2_subdev_enable_streams(csis->source.sd,
> - csis->source.pad->index, BIT(0));
> - if (ret < 0)
> - goto err_stop;
> -
> - mipi_csis_log_counters(csis, true);
> -
> - v4l2_subdev_unlock_state(state);
> -
> - return 0;
> -
> -err_stop:
> - mipi_csis_stop_stream(csis);
> - pm_runtime_put(csis->dev);
> -err_unlock:
> - v4l2_subdev_unlock_state(state);
> -
> - return ret;
> -}
> -
> static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_mbus_code_enum *code)
> @@ -1211,6 +1154,89 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> return 0;
> }
>
> +static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> +{
> + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> + u64 sink_streams;
> + int ret;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state, CSIS_PAD_SOURCE,
> + CSIS_PAD_SINK,
Original code use hardcode BIT(0), I think keep the same logic with original
code in this type patch to easy review.
Add bitmask enabled_streams should be following patch.
Frank
> + &streams_mask);
> + if (!sink_streams || !streams_mask)
> + return -EINVAL;
> +
> + /* Start the CSIS with the first stream. */
> + if (!csis->source.enabled_streams) {
> + const struct v4l2_mbus_framefmt *format;
> + const struct csis_pix_format *csis_fmt;
> +
> + format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> + csis_fmt = find_csis_format(format->code);
> +
> + ret = mipi_csis_calculate_params(csis, csis_fmt);
> + if (ret)
> + return ret;
> +
> + mipi_csis_clear_counters(csis);
> +
> + ret = pm_runtime_resume_and_get(csis->dev);
> + if (ret < 0)
> + return ret;
> +
> + mipi_csis_start_stream(csis, format, csis_fmt);
> + }
> +
> + ret = v4l2_subdev_enable_streams(csis->source.sd,
> + csis->source.pad->index, sink_streams);
> + if (ret) {
> + if (!csis->source.enabled_streams) {
> + mipi_csis_stop_stream(csis);
> + pm_runtime_put(csis->dev);
> + }
> +
> + return ret;
> + }
> +
> + mipi_csis_log_counters(csis, true);
> +
> + csis->source.enabled_streams |= streams_mask;
> +
> + return 0;
> +}
> +
> +static int mipi_csis_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> +{
> + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> + u64 sink_streams;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state, CSIS_PAD_SOURCE,
> + CSIS_PAD_SINK,
> + &streams_mask);
> + if (!sink_streams || !streams_mask)
> + return -EINVAL;
> +
> + v4l2_subdev_disable_streams(csis->source.sd,
> + csis->source.pad->index, sink_streams);
> +`
> + csis->source.enabled_streams &= ~streams_mask;
> +
> + /* Stop the CSIS with the last stream. */
> + if (!csis->source.enabled_streams) {
> + mipi_csis_stop_stream(csis);
> + pm_runtime_put(csis->dev);
> +
> + if (csis->debug.enable)
> + mipi_csis_log_counters(csis, true);
> + }
> +
> + return 0;
> +}
> +
> static int mipi_csis_init_state(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state)
> {
> @@ -1263,7 +1289,7 @@ static const struct v4l2_subdev_core_ops mipi_csis_core_ops = {
> };
>
> static const struct v4l2_subdev_video_ops mipi_csis_video_ops = {
> - .s_stream = mipi_csis_s_stream,
> + .s_stream = v4l2_subdev_s_stream_helper,
> };
>
> static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
> @@ -1271,6 +1297,8 @@ static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
> .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = mipi_csis_set_fmt,
> .get_frame_desc = mipi_csis_get_frame_desc,
> + .enable_streams = mipi_csis_enable_streams,
> + .disable_streams = mipi_csis_disable_streams,
> };
>
> static const struct v4l2_subdev_ops mipi_csis_subdev_ops = {
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v1 2/6] media: imx-mipi-csis: Switch to .enable_streams()
2025-11-07 16:29 ` Frank Li
@ 2025-11-07 18:32 ` Laurent Pinchart
0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 18:32 UTC (permalink / raw)
To: Frank Li
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 11:29:28AM -0500, Frank Li wrote:
> On Fri, Nov 07, 2025 at 03:58:09AM +0200, Laurent Pinchart wrote:
> > To prepare for multi-stream support, switch from the legacy .s_stream()
> > operation to the .enable_streams() and .disable_streams() operations.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > drivers/media/platform/nxp/imx-mipi-csis.c | 146 ++++++++++++---------
> > 1 file changed, 87 insertions(+), 59 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 42b1ec8eed96..f142e79acbcf 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -350,6 +350,7 @@ struct mipi_csis_device {
> > struct {
> > struct v4l2_subdev *sd;
> > const struct media_pad *pad;
> > + u64 enabled_streams;
> > } source;
> >
> > struct v4l2_mbus_config_mipi_csi2 bus;
> > @@ -1019,64 +1020,6 @@ static struct mipi_csis_device *sd_to_mipi_csis_device(struct v4l2_subdev *sdev)
> > return container_of(sdev, struct mipi_csis_device, sd);
> > }
> >
> > -static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
> > -{
> > - struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > - const struct v4l2_mbus_framefmt *format;
> > - const struct csis_pix_format *csis_fmt;
> > - struct v4l2_subdev_state *state;
> > - int ret;
> > -
> > - if (!enable) {
> > - v4l2_subdev_disable_streams(csis->source.sd,
> > - csis->source.pad->index, BIT(0));
> > -
> > - mipi_csis_stop_stream(csis);
> > - if (csis->debug.enable)
> > - mipi_csis_log_counters(csis, true);
> > -
> > - pm_runtime_put(csis->dev);
> > -
> > - return 0;
> > - }
> > -
> > - state = v4l2_subdev_lock_and_get_active_state(sd);
> > -
> > - format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> > - csis_fmt = find_csis_format(format->code);
> > -
> > - ret = mipi_csis_calculate_params(csis, csis_fmt);
> > - if (ret < 0)
> > - goto err_unlock;
> > -
> > - mipi_csis_clear_counters(csis);
> > -
> > - ret = pm_runtime_resume_and_get(csis->dev);
> > - if (ret < 0)
> > - goto err_unlock;
> > -
> > - mipi_csis_start_stream(csis, format, csis_fmt);
> > -
> > - ret = v4l2_subdev_enable_streams(csis->source.sd,
> > - csis->source.pad->index, BIT(0));
> > - if (ret < 0)
> > - goto err_stop;
> > -
> > - mipi_csis_log_counters(csis, true);
> > -
> > - v4l2_subdev_unlock_state(state);
> > -
> > - return 0;
> > -
> > -err_stop:
> > - mipi_csis_stop_stream(csis);
> > - pm_runtime_put(csis->dev);
> > -err_unlock:
> > - v4l2_subdev_unlock_state(state);
> > -
> > - return ret;
> > -}
> > -
> > static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
> > struct v4l2_subdev_state *state,
> > struct v4l2_subdev_mbus_code_enum *code)
> > @@ -1211,6 +1154,89 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> > return 0;
> > }
> >
> > +static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + u32 pad, u64 streams_mask)
> > +{
> > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > + u64 sink_streams;
> > + int ret;
> > +
> > + sink_streams = v4l2_subdev_state_xlate_streams(state, CSIS_PAD_SOURCE,
> > + CSIS_PAD_SINK,
>
> Original code use hardcode BIT(0), I think keep the same logic with original
> code in this type patch to easy review.
>
> Add bitmask enabled_streams should be following patch.
It could be moved to patch 6/6, which is where support for multiple
streams is enabled, but I've decided to instead keep it here as 6/6 is
already a bit patch. I also think it's better to handle it all here
instead of doing a partial conversion to .enable_streams(), a
self-contained feature is easier to review than a partial implementation
that then gets fixed in a separate patch.
> > + &streams_mask);
> > + if (!sink_streams || !streams_mask)
> > + return -EINVAL;
> > +
> > + /* Start the CSIS with the first stream. */
> > + if (!csis->source.enabled_streams) {
> > + const struct v4l2_mbus_framefmt *format;
> > + const struct csis_pix_format *csis_fmt;
> > +
> > + format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> > + csis_fmt = find_csis_format(format->code);
> > +
> > + ret = mipi_csis_calculate_params(csis, csis_fmt);
> > + if (ret)
> > + return ret;
> > +
> > + mipi_csis_clear_counters(csis);
> > +
> > + ret = pm_runtime_resume_and_get(csis->dev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + mipi_csis_start_stream(csis, format, csis_fmt);
> > + }
> > +
> > + ret = v4l2_subdev_enable_streams(csis->source.sd,
> > + csis->source.pad->index, sink_streams);
> > + if (ret) {
> > + if (!csis->source.enabled_streams) {
> > + mipi_csis_stop_stream(csis);
> > + pm_runtime_put(csis->dev);
> > + }
> > +
> > + return ret;
> > + }
> > +
> > + mipi_csis_log_counters(csis, true);
> > +
> > + csis->source.enabled_streams |= streams_mask;
> > +
> > + return 0;
> > +}
> > +
> > +static int mipi_csis_disable_streams(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + u32 pad, u64 streams_mask)
> > +{
> > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > + u64 sink_streams;
> > +
> > + sink_streams = v4l2_subdev_state_xlate_streams(state, CSIS_PAD_SOURCE,
> > + CSIS_PAD_SINK,
> > + &streams_mask);
> > + if (!sink_streams || !streams_mask)
> > + return -EINVAL;
> > +
> > + v4l2_subdev_disable_streams(csis->source.sd,
> > + csis->source.pad->index, sink_streams);
> > +`
> > + csis->source.enabled_streams &= ~streams_mask;
> > +
> > + /* Stop the CSIS with the last stream. */
> > + if (!csis->source.enabled_streams) {
> > + mipi_csis_stop_stream(csis);
> > + pm_runtime_put(csis->dev);
> > +
> > + if (csis->debug.enable)
> > + mipi_csis_log_counters(csis, true);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int mipi_csis_init_state(struct v4l2_subdev *sd,
> > struct v4l2_subdev_state *state)
> > {
> > @@ -1263,7 +1289,7 @@ static const struct v4l2_subdev_core_ops mipi_csis_core_ops = {
> > };
> >
> > static const struct v4l2_subdev_video_ops mipi_csis_video_ops = {
> > - .s_stream = mipi_csis_s_stream,
> > + .s_stream = v4l2_subdev_s_stream_helper,
> > };
> >
> > static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
> > @@ -1271,6 +1297,8 @@ static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
> > .get_fmt = v4l2_subdev_get_fmt,
> > .set_fmt = mipi_csis_set_fmt,
> > .get_frame_desc = mipi_csis_get_frame_desc,
> > + .enable_streams = mipi_csis_enable_streams,
> > + .disable_streams = mipi_csis_disable_streams,
> > };
> >
> > static const struct v4l2_subdev_ops mipi_csis_subdev_ops = {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation
2025-11-07 1:58 [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Laurent Pinchart
2025-11-07 1:58 ` [PATCH v1 1/6] media: imx-mipi-csis: Add VC-related register fields Laurent Pinchart
2025-11-07 1:58 ` [PATCH v1 2/6] media: imx-mipi-csis: Switch to .enable_streams() Laurent Pinchart
@ 2025-11-07 1:58 ` Laurent Pinchart
2025-11-07 16:36 ` Frank Li
2025-11-09 21:48 ` kernel test robot
2025-11-07 1:58 ` [PATCH v1 4/6] media: imx-mipi-csis: Group runtime parameters in structure Laurent Pinchart
` (3 subsequent siblings)
6 siblings, 2 replies; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 1:58 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team,
Pengutronix Kernel Team, imx, Frank Li, Stefan Klug, Sakari Ailus
To prepare for multi-stream support, implement the .set_routing()
operation. The routing table is currently hardcoded to a single route.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/nxp/imx-mipi-csis.c | 73 ++++++++++++++++++----
1 file changed, 60 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index f142e79acbcf..f4b19576a235 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -1154,6 +1154,52 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
return 0;
}
+static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ struct v4l2_subdev_krouting *routing)
+{
+ static const struct v4l2_mbus_framefmt format = {
+ .width = MIPI_CSIS_DEF_PIX_WIDTH,
+ .height = MIPI_CSIS_DEF_PIX_HEIGHT,
+ .code = mipi_csis_formats[0].code,
+ .field = V4L2_FIELD_NONE,
+ .colorspace = V4L2_XFER_FUNC_709,
+ .ycbcr_enc = V4L2_YCBCR_ENC_601,
+ .quantization = V4L2_QUANTIZATION_LIM_RANGE,
+ .xfer_func = V4L2_XFER_FUNC_SRGB,
+ };
+ int ret;
+
+ ret = v4l2_subdev_routing_validate(sd, routing,
+ V4L2_SUBDEV_ROUTING_NO_1_TO_N);
+ if (ret)
+ return ret;
+
+ /* Only a single route is supported for now. */
+ if (routing->num_routes != 1 ||
+ !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
+ return -EINVAL;
+
+ ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int mipi_csis_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ enum v4l2_subdev_format_whence which,
+ struct v4l2_subdev_krouting *routing)
+{
+ struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
+
+ if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csis->source.enabled_streams)
+ return -EBUSY;
+
+ return __mipi_csis_set_routing(sd, state, routing);
+}
+
static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
u32 pad, u64 streams_mask)
@@ -1240,22 +1286,22 @@ static int mipi_csis_disable_streams(struct v4l2_subdev *sd,
static int mipi_csis_init_state(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state)
{
- struct v4l2_subdev_format fmt = {
- .pad = CSIS_PAD_SINK,
+ struct v4l2_subdev_route routes[] = {
+ {
+ .sink_pad = CSIS_PAD_SINK,
+ .sink_stream = 0,
+ .source_pad = CSIS_PAD_SOURCE,
+ .source_stream = 0,
+ .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+ },
};
- fmt.format.code = mipi_csis_formats[0].code;
- fmt.format.width = MIPI_CSIS_DEF_PIX_WIDTH;
- fmt.format.height = MIPI_CSIS_DEF_PIX_HEIGHT;
+ struct v4l2_subdev_krouting routing = {
+ .num_routes = ARRAY_SIZE(routes),
+ .routes = routes,
+ };
- fmt.format.colorspace = V4L2_COLORSPACE_SMPTE170M;
- fmt.format.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
- fmt.format.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace);
- fmt.format.quantization =
- V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt.format.colorspace,
- fmt.format.ycbcr_enc);
-
- return mipi_csis_set_fmt(sd, state, &fmt);
+ return __mipi_csis_set_routing(sd, state, &routing);
}
static int mipi_csis_log_status(struct v4l2_subdev *sd)
@@ -1297,6 +1343,7 @@ static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = mipi_csis_set_fmt,
.get_frame_desc = mipi_csis_get_frame_desc,
+ .set_routing = mipi_csis_set_routing,
.enable_streams = mipi_csis_enable_streams,
.disable_streams = mipi_csis_disable_streams,
};
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation
2025-11-07 1:58 ` [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation Laurent Pinchart
@ 2025-11-07 16:36 ` Frank Li
2025-11-07 18:30 ` Laurent Pinchart
2025-11-09 21:48 ` kernel test robot
1 sibling, 1 reply; 26+ messages in thread
From: Frank Li @ 2025-11-07 16:36 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 03:58:10AM +0200, Laurent Pinchart wrote:
> To prepare for multi-stream support, implement the .set_routing()
> operation. The routing table is currently hardcoded to a single route.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/nxp/imx-mipi-csis.c | 73 ++++++++++++++++++----
> 1 file changed, 60 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index f142e79acbcf..f4b19576a235 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -1154,6 +1154,52 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> return 0;
> }
>
> +static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + struct v4l2_subdev_krouting *routing)
> +{
> + static const struct v4l2_mbus_framefmt format = {
> + .width = MIPI_CSIS_DEF_PIX_WIDTH,
> + .height = MIPI_CSIS_DEF_PIX_HEIGHT,
> + .code = mipi_csis_formats[0].code,
> + .field = V4L2_FIELD_NONE,
> + .colorspace = V4L2_XFER_FUNC_709,
> + .ycbcr_enc = V4L2_YCBCR_ENC_601,
Is it same as V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace)
> + .quantization = V4L2_QUANTIZATION_LIM_RANGE,
Is it same as
V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt.format.colorspace,
fmt.format.ycbcr_enc);
> + .xfer_func = V4L2_XFER_FUNC_SRGB,
Is it same as
V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
Frank
> + };
> + int ret;
> +
> + ret = v4l2_subdev_routing_validate(sd, routing,
> + V4L2_SUBDEV_ROUTING_NO_1_TO_N);
> + if (ret)
> + return ret;
> +
> + /* Only a single route is supported for now. */
> + if (routing->num_routes != 1 ||
> + !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> + return -EINVAL;
> +
> + ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int mipi_csis_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + enum v4l2_subdev_format_whence which,
> + struct v4l2_subdev_krouting *routing)
> +{
> + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> +
> + if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csis->source.enabled_streams)
> + return -EBUSY;
> +
> + return __mipi_csis_set_routing(sd, state, routing);
> +}
> +
> static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> u32 pad, u64 streams_mask)
> @@ -1240,22 +1286,22 @@ static int mipi_csis_disable_streams(struct v4l2_subdev *sd,
> static int mipi_csis_init_state(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state)
> {
> - struct v4l2_subdev_format fmt = {
> - .pad = CSIS_PAD_SINK,
> + struct v4l2_subdev_route routes[] = {
> + {
> + .sink_pad = CSIS_PAD_SINK,
> + .sink_stream = 0,
> + .source_pad = CSIS_PAD_SOURCE,
> + .source_stream = 0,
> + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> + },
> };
>
> - fmt.format.code = mipi_csis_formats[0].code;
> - fmt.format.width = MIPI_CSIS_DEF_PIX_WIDTH;
> - fmt.format.height = MIPI_CSIS_DEF_PIX_HEIGHT;
> + struct v4l2_subdev_krouting routing = {
> + .num_routes = ARRAY_SIZE(routes),
> + .routes = routes,
> + };
>
> - fmt.format.colorspace = V4L2_COLORSPACE_SMPTE170M;
> - fmt.format.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
> - fmt.format.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace);
> - fmt.format.quantization =
> - V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt.format.colorspace,
> - fmt.format.ycbcr_enc);
> -
> - return mipi_csis_set_fmt(sd, state, &fmt);
> + return __mipi_csis_set_routing(sd, state, &routing);
> }
>
> static int mipi_csis_log_status(struct v4l2_subdev *sd)
> @@ -1297,6 +1343,7 @@ static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
> .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = mipi_csis_set_fmt,
> .get_frame_desc = mipi_csis_get_frame_desc,
> + .set_routing = mipi_csis_set_routing,
> .enable_streams = mipi_csis_enable_streams,
> .disable_streams = mipi_csis_disable_streams,
> };
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation
2025-11-07 16:36 ` Frank Li
@ 2025-11-07 18:30 ` Laurent Pinchart
2025-11-07 20:38 ` Frank Li
0 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 18:30 UTC (permalink / raw)
To: Frank Li
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 11:36:18AM -0500, Frank Li wrote:
> On Fri, Nov 07, 2025 at 03:58:10AM +0200, Laurent Pinchart wrote:
> > To prepare for multi-stream support, implement the .set_routing()
> > operation. The routing table is currently hardcoded to a single route.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > drivers/media/platform/nxp/imx-mipi-csis.c | 73 ++++++++++++++++++----
> > 1 file changed, 60 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index f142e79acbcf..f4b19576a235 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -1154,6 +1154,52 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> > return 0;
> > }
> >
> > +static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + struct v4l2_subdev_krouting *routing)
> > +{
> > + static const struct v4l2_mbus_framefmt format = {
> > + .width = MIPI_CSIS_DEF_PIX_WIDTH,
> > + .height = MIPI_CSIS_DEF_PIX_HEIGHT,
> > + .code = mipi_csis_formats[0].code,
> > + .field = V4L2_FIELD_NONE,
> > + .colorspace = V4L2_XFER_FUNC_709,
> > + .ycbcr_enc = V4L2_YCBCR_ENC_601,
>
> Is it same as V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace)
>
> > + .quantization = V4L2_QUANTIZATION_LIM_RANGE,
>
> Is it same as
> V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt.format.colorspace,
> fmt.format.ycbcr_enc);
>
> > + .xfer_func = V4L2_XFER_FUNC_SRGB,
>
> Is it same as
> V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
Yes, they're the same.
> > + };
> > + int ret;
> > +
> > + ret = v4l2_subdev_routing_validate(sd, routing,
> > + V4L2_SUBDEV_ROUTING_NO_1_TO_N);
> > + if (ret)
> > + return ret;
> > +
> > + /* Only a single route is supported for now. */
> > + if (routing->num_routes != 1 ||
> > + !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > + return -EINVAL;
> > +
> > + ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int mipi_csis_set_routing(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + enum v4l2_subdev_format_whence which,
> > + struct v4l2_subdev_krouting *routing)
> > +{
> > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > +
> > + if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csis->source.enabled_streams)
> > + return -EBUSY;
> > +
> > + return __mipi_csis_set_routing(sd, state, routing);
> > +}
> > +
> > static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
> > struct v4l2_subdev_state *state,
> > u32 pad, u64 streams_mask)
> > @@ -1240,22 +1286,22 @@ static int mipi_csis_disable_streams(struct v4l2_subdev *sd,
> > static int mipi_csis_init_state(struct v4l2_subdev *sd,
> > struct v4l2_subdev_state *state)
> > {
> > - struct v4l2_subdev_format fmt = {
> > - .pad = CSIS_PAD_SINK,
> > + struct v4l2_subdev_route routes[] = {
> > + {
> > + .sink_pad = CSIS_PAD_SINK,
> > + .sink_stream = 0,
> > + .source_pad = CSIS_PAD_SOURCE,
> > + .source_stream = 0,
> > + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> > + },
> > };
> >
> > - fmt.format.code = mipi_csis_formats[0].code;
> > - fmt.format.width = MIPI_CSIS_DEF_PIX_WIDTH;
> > - fmt.format.height = MIPI_CSIS_DEF_PIX_HEIGHT;
> > + struct v4l2_subdev_krouting routing = {
> > + .num_routes = ARRAY_SIZE(routes),
> > + .routes = routes,
> > + };
> >
> > - fmt.format.colorspace = V4L2_COLORSPACE_SMPTE170M;
> > - fmt.format.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
> > - fmt.format.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace);
> > - fmt.format.quantization =
> > - V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt.format.colorspace,
> > - fmt.format.ycbcr_enc);
> > -
> > - return mipi_csis_set_fmt(sd, state, &fmt);
> > + return __mipi_csis_set_routing(sd, state, &routing);
> > }
> >
> > static int mipi_csis_log_status(struct v4l2_subdev *sd)
> > @@ -1297,6 +1343,7 @@ static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
> > .get_fmt = v4l2_subdev_get_fmt,
> > .set_fmt = mipi_csis_set_fmt,
> > .get_frame_desc = mipi_csis_get_frame_desc,
> > + .set_routing = mipi_csis_set_routing,
> > .enable_streams = mipi_csis_enable_streams,
> > .disable_streams = mipi_csis_disable_streams,
> > };
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation
2025-11-07 18:30 ` Laurent Pinchart
@ 2025-11-07 20:38 ` Frank Li
0 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-11-07 20:38 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 08:30:14PM +0200, Laurent Pinchart wrote:
> On Fri, Nov 07, 2025 at 11:36:18AM -0500, Frank Li wrote:
> > On Fri, Nov 07, 2025 at 03:58:10AM +0200, Laurent Pinchart wrote:
> > > To prepare for multi-stream support, implement the .set_routing()
> > > operation. The routing table is currently hardcoded to a single route.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > drivers/media/platform/nxp/imx-mipi-csis.c | 73 ++++++++++++++++++----
> > > 1 file changed, 60 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > index f142e79acbcf..f4b19576a235 100644
> > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > @@ -1154,6 +1154,52 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> > > return 0;
> > > }
> > >
> > > +static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> > > + struct v4l2_subdev_state *state,
> > > + struct v4l2_subdev_krouting *routing)
> > > +{
> > > + static const struct v4l2_mbus_framefmt format = {
> > > + .width = MIPI_CSIS_DEF_PIX_WIDTH,
> > > + .height = MIPI_CSIS_DEF_PIX_HEIGHT,
> > > + .code = mipi_csis_formats[0].code,
> > > + .field = V4L2_FIELD_NONE,
> > > + .colorspace = V4L2_XFER_FUNC_709,
> > > + .ycbcr_enc = V4L2_YCBCR_ENC_601,
> >
> > Is it same as V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace)
> >
> > > + .quantization = V4L2_QUANTIZATION_LIM_RANGE,
> >
> > Is it same as
> > V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt.format.colorspace,
> > fmt.format.ycbcr_enc);
> >
> > > + .xfer_func = V4L2_XFER_FUNC_SRGB,
> >
> > Is it same as
> > V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
>
> Yes, they're the same.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> > > + };
> > > + int ret;
> > > +
> > > + ret = v4l2_subdev_routing_validate(sd, routing,
> > > + V4L2_SUBDEV_ROUTING_NO_1_TO_N);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /* Only a single route is supported for now. */
> > > + if (routing->num_routes != 1 ||
> > > + !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > > + return -EINVAL;
> > > +
> > > + ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mipi_csis_set_routing(struct v4l2_subdev *sd,
> > > + struct v4l2_subdev_state *state,
> > > + enum v4l2_subdev_format_whence which,
> > > + struct v4l2_subdev_krouting *routing)
> > > +{
> > > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > > +
> > > + if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csis->source.enabled_streams)
> > > + return -EBUSY;
> > > +
> > > + return __mipi_csis_set_routing(sd, state, routing);
> > > +}
> > > +
> > > static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
> > > struct v4l2_subdev_state *state,
> > > u32 pad, u64 streams_mask)
> > > @@ -1240,22 +1286,22 @@ static int mipi_csis_disable_streams(struct v4l2_subdev *sd,
> > > static int mipi_csis_init_state(struct v4l2_subdev *sd,
> > > struct v4l2_subdev_state *state)
> > > {
> > > - struct v4l2_subdev_format fmt = {
> > > - .pad = CSIS_PAD_SINK,
> > > + struct v4l2_subdev_route routes[] = {
> > > + {
> > > + .sink_pad = CSIS_PAD_SINK,
> > > + .sink_stream = 0,
> > > + .source_pad = CSIS_PAD_SOURCE,
> > > + .source_stream = 0,
> > > + .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> > > + },
> > > };
> > >
> > > - fmt.format.code = mipi_csis_formats[0].code;
> > > - fmt.format.width = MIPI_CSIS_DEF_PIX_WIDTH;
> > > - fmt.format.height = MIPI_CSIS_DEF_PIX_HEIGHT;
> > > + struct v4l2_subdev_krouting routing = {
> > > + .num_routes = ARRAY_SIZE(routes),
> > > + .routes = routes,
> > > + };
> > >
> > > - fmt.format.colorspace = V4L2_COLORSPACE_SMPTE170M;
> > > - fmt.format.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt.format.colorspace);
> > > - fmt.format.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt.format.colorspace);
> > > - fmt.format.quantization =
> > > - V4L2_MAP_QUANTIZATION_DEFAULT(false, fmt.format.colorspace,
> > > - fmt.format.ycbcr_enc);
> > > -
> > > - return mipi_csis_set_fmt(sd, state, &fmt);
> > > + return __mipi_csis_set_routing(sd, state, &routing);
> > > }
> > >
> > > static int mipi_csis_log_status(struct v4l2_subdev *sd)
> > > @@ -1297,6 +1343,7 @@ static const struct v4l2_subdev_pad_ops mipi_csis_pad_ops = {
> > > .get_fmt = v4l2_subdev_get_fmt,
> > > .set_fmt = mipi_csis_set_fmt,
> > > .get_frame_desc = mipi_csis_get_frame_desc,
> > > + .set_routing = mipi_csis_set_routing,
> > > .enable_streams = mipi_csis_enable_streams,
> > > .disable_streams = mipi_csis_disable_streams,
> > > };
>
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation
2025-11-07 1:58 ` [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation Laurent Pinchart
2025-11-07 16:36 ` Frank Li
@ 2025-11-09 21:48 ` kernel test robot
1 sibling, 0 replies; 26+ messages in thread
From: kernel test robot @ 2025-11-09 21:48 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: llvm, oe-kbuild-all, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Frank Li,
Stefan Klug, Sakari Ailus
Hi Laurent,
kernel test robot noticed the following build errors:
[auto build test ERROR on dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa]
url: https://github.com/intel-lab-lkp/linux/commits/Laurent-Pinchart/media-imx-mipi-csis-Add-VC-related-register-fields/20251107-100134
base: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
patch link: https://lore.kernel.org/r/20251107015813.5834-4-laurent.pinchart%40ideasonboard.com
patch subject: [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation
config: arm-randconfig-003-20251110 (https://download.01.org/0day-ci/archive/20251110/202511100531.4skNcchV-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251110/202511100531.4skNcchV-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511100531.4skNcchV-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/media/platform/nxp/imx-mipi-csis.c:1164:32: error: initializer element is not a compile-time constant
.code = mipi_csis_formats[0].code,
~~~~~~~~~~~~~~~~~~~~~^~~~
1 error generated.
vim +1164 drivers/media/platform/nxp/imx-mipi-csis.c
1156
1157 static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
1158 struct v4l2_subdev_state *state,
1159 struct v4l2_subdev_krouting *routing)
1160 {
1161 static const struct v4l2_mbus_framefmt format = {
1162 .width = MIPI_CSIS_DEF_PIX_WIDTH,
1163 .height = MIPI_CSIS_DEF_PIX_HEIGHT,
> 1164 .code = mipi_csis_formats[0].code,
1165 .field = V4L2_FIELD_NONE,
1166 .colorspace = V4L2_XFER_FUNC_709,
1167 .ycbcr_enc = V4L2_YCBCR_ENC_601,
1168 .quantization = V4L2_QUANTIZATION_LIM_RANGE,
1169 .xfer_func = V4L2_XFER_FUNC_SRGB,
1170 };
1171 int ret;
1172
1173 ret = v4l2_subdev_routing_validate(sd, routing,
1174 V4L2_SUBDEV_ROUTING_NO_1_TO_N);
1175 if (ret)
1176 return ret;
1177
1178 /* Only a single route is supported for now. */
1179 if (routing->num_routes != 1 ||
1180 !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
1181 return -EINVAL;
1182
1183 ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
1184 if (ret)
1185 return ret;
1186
1187 return 0;
1188 }
1189
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v1 4/6] media: imx-mipi-csis: Group runtime parameters in structure
2025-11-07 1:58 [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Laurent Pinchart
` (2 preceding siblings ...)
2025-11-07 1:58 ` [PATCH v1 3/6] media: imx-mipi-csis: Implement the .set_routing() operation Laurent Pinchart
@ 2025-11-07 1:58 ` Laurent Pinchart
2025-11-07 16:40 ` Frank Li
2025-11-07 1:58 ` [PATCH v1 5/6] media: imx-mipi-csis: Set all per-channel registers in one function Laurent Pinchart
` (2 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 1:58 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team,
Pengutronix Kernel Team, imx, Frank Li, Stefan Klug, Sakari Ailus
The mipi_csis_set_params() functions programs the hardware with a mix of
parameters computed earlier when starting streaming and stored in the
mipi_csis_device structure, and parameters retrieved from pad formats
passed to the function through the whole call stack.
Multi-channel support will require computing and applying more
parameters. To prepare for that, refactor the code and store all
parameters in a single mipi_csis_params structure, and pass it to
functions as appropriate to replace the other arguments.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/nxp/imx-mipi-csis.c | 72 +++++++++++++---------
1 file changed, 43 insertions(+), 29 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index f4b19576a235..9dd264af181f 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -333,6 +333,19 @@ struct mipi_csis_info {
unsigned int num_clocks;
};
+struct mipi_csis_channel_params {
+ unsigned int data_type;
+ unsigned int width;
+ unsigned int height;
+};
+
+struct mipi_csis_params {
+ u32 hs_settle;
+ u32 clk_settle;
+
+ struct mipi_csis_channel_params channels[MIPI_CSIS_MAX_CHANNELS];
+};
+
struct mipi_csis_device {
struct device *dev;
void __iomem *regs;
@@ -355,8 +368,6 @@ struct mipi_csis_device {
struct v4l2_mbus_config_mipi_csi2 bus;
u32 clk_frequency;
- u32 hs_settle;
- u32 clk_settle;
spinlock_t slock; /* Protect events */
struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
@@ -587,8 +598,7 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
}
static void __mipi_csis_set_format(struct mipi_csis_device *csis,
- const struct v4l2_mbus_framefmt *format,
- const struct csis_pix_format *csis_fmt)
+ const struct mipi_csis_channel_params *params)
{
u32 val;
@@ -611,26 +621,36 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
*
* TODO: Verify which other formats require DUAL (or QUAD) modes.
*/
- if (csis_fmt->data_type == MIPI_CSI2_DT_YUV422_8B)
+ if (params->data_type == MIPI_CSI2_DT_YUV422_8B)
val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
- val |= MIPI_CSIS_ISPCFG_DATAFORMAT(csis_fmt->data_type);
+ val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
/* Pixel resolution */
mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0),
- MIPI_CSIS_ISP_RESOL_VRESOL(format->height) |
- MIPI_CSIS_ISP_RESOL_HRESOL(format->width));
+ MIPI_CSIS_ISP_RESOL_VRESOL(params->height) |
+ MIPI_CSIS_ISP_RESOL_HRESOL(params->width));
}
static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
- const struct csis_pix_format *csis_fmt)
+ const struct v4l2_subdev_state *state,
+ struct mipi_csis_params *params)
{
+ const struct v4l2_mbus_framefmt *format;
+ const struct csis_pix_format *csis_fmt;
s64 link_freq;
u32 lane_rate;
+ format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
+ csis_fmt = find_csis_format(format->code);
+
+ params->channels[0].data_type = csis_fmt->data_type;
+ params->channels[0].width = format->width;
+ params->channels[0].height = format->height;
+
/* Calculate the line rate from the pixel rate. */
link_freq = v4l2_get_link_freq(csis->source.pad, csis_fmt->width,
csis->bus.num_data_lanes * 2);
@@ -653,30 +673,29 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
* (which is documented as corresponding to CSI-2 v0.87 to v1.00) until
* we figure out how to compute it correctly.
*/
- csis->hs_settle = (lane_rate - 5000000) / 45000000;
- csis->clk_settle = 0;
+ params->hs_settle = (lane_rate - 5000000) / 45000000;
+ params->clk_settle = 0;
dev_dbg(csis->dev, "lane rate %u, Tclk_settle %u, Ths_settle %u\n",
- lane_rate, csis->clk_settle, csis->hs_settle);
+ lane_rate, params->clk_settle, params->hs_settle);
if (csis->debug.hs_settle < 0xff) {
dev_dbg(csis->dev, "overriding Ths_settle with %u\n",
csis->debug.hs_settle);
- csis->hs_settle = csis->debug.hs_settle;
+ params->hs_settle = csis->debug.hs_settle;
}
if (csis->debug.clk_settle < 4) {
dev_dbg(csis->dev, "overriding Tclk_settle with %u\n",
csis->debug.clk_settle);
- csis->clk_settle = csis->debug.clk_settle;
+ params->clk_settle = csis->debug.clk_settle;
}
return 0;
}
static void mipi_csis_set_params(struct mipi_csis_device *csis,
- const struct v4l2_mbus_framefmt *format,
- const struct csis_pix_format *csis_fmt)
+ const struct mipi_csis_params *params)
{
int lanes = csis->bus.num_data_lanes;
u32 val;
@@ -692,11 +711,11 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
- __mipi_csis_set_format(csis, format, csis_fmt);
+ __mipi_csis_set_format(csis, ¶ms->channels[0]);
mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
- MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(csis->hs_settle) |
- MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(csis->clk_settle));
+ MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params->hs_settle) |
+ MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params->clk_settle));
mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0),
MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
@@ -771,11 +790,10 @@ static int mipi_csis_clk_get(struct mipi_csis_device *csis)
}
static void mipi_csis_start_stream(struct mipi_csis_device *csis,
- const struct v4l2_mbus_framefmt *format,
- const struct csis_pix_format *csis_fmt)
+ const struct mipi_csis_params *params)
{
mipi_csis_sw_reset(csis);
- mipi_csis_set_params(csis, format, csis_fmt);
+ mipi_csis_set_params(csis, params);
mipi_csis_system_enable(csis, true);
mipi_csis_enable_interrupts(csis, true);
}
@@ -1216,13 +1234,9 @@ static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
/* Start the CSIS with the first stream. */
if (!csis->source.enabled_streams) {
- const struct v4l2_mbus_framefmt *format;
- const struct csis_pix_format *csis_fmt;
+ struct mipi_csis_params params;
- format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
- csis_fmt = find_csis_format(format->code);
-
- ret = mipi_csis_calculate_params(csis, csis_fmt);
+ ret = mipi_csis_calculate_params(csis, state, ¶ms);
if (ret)
return ret;
@@ -1232,7 +1246,7 @@ static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
if (ret < 0)
return ret;
- mipi_csis_start_stream(csis, format, csis_fmt);
+ mipi_csis_start_stream(csis, ¶ms);
}
ret = v4l2_subdev_enable_streams(csis->source.sd,
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v1 4/6] media: imx-mipi-csis: Group runtime parameters in structure
2025-11-07 1:58 ` [PATCH v1 4/6] media: imx-mipi-csis: Group runtime parameters in structure Laurent Pinchart
@ 2025-11-07 16:40 ` Frank Li
0 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-11-07 16:40 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 03:58:11AM +0200, Laurent Pinchart wrote:
> The mipi_csis_set_params() functions programs the hardware with a mix of
> parameters computed earlier when starting streaming and stored in the
> mipi_csis_device structure, and parameters retrieved from pad formats
> passed to the function through the whole call stack.
>
> Multi-channel support will require computing and applying more
> parameters. To prepare for that, refactor the code and store all
> parameters in a single mipi_csis_params structure, and pass it to
> functions as appropriate to replace the other arguments.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/media/platform/nxp/imx-mipi-csis.c | 72 +++++++++++++---------
> 1 file changed, 43 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index f4b19576a235..9dd264af181f 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -333,6 +333,19 @@ struct mipi_csis_info {
> unsigned int num_clocks;
> };
>
> +struct mipi_csis_channel_params {
> + unsigned int data_type;
> + unsigned int width;
> + unsigned int height;
> +};
> +
> +struct mipi_csis_params {
> + u32 hs_settle;
> + u32 clk_settle;
> +
> + struct mipi_csis_channel_params channels[MIPI_CSIS_MAX_CHANNELS];
> +};
> +
> struct mipi_csis_device {
> struct device *dev;
> void __iomem *regs;
> @@ -355,8 +368,6 @@ struct mipi_csis_device {
>
> struct v4l2_mbus_config_mipi_csi2 bus;
> u32 clk_frequency;
> - u32 hs_settle;
> - u32 clk_settle;
>
> spinlock_t slock; /* Protect events */
> struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
> @@ -587,8 +598,7 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
> }
>
> static void __mipi_csis_set_format(struct mipi_csis_device *csis,
> - const struct v4l2_mbus_framefmt *format,
> - const struct csis_pix_format *csis_fmt)
> + const struct mipi_csis_channel_params *params)
> {
> u32 val;
>
> @@ -611,26 +621,36 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
> *
> * TODO: Verify which other formats require DUAL (or QUAD) modes.
> */
> - if (csis_fmt->data_type == MIPI_CSI2_DT_YUV422_8B)
> + if (params->data_type == MIPI_CSI2_DT_YUV422_8B)
> val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
>
> - val |= MIPI_CSIS_ISPCFG_DATAFORMAT(csis_fmt->data_type);
> + val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
> val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
>
> mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
>
> /* Pixel resolution */
> mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0),
> - MIPI_CSIS_ISP_RESOL_VRESOL(format->height) |
> - MIPI_CSIS_ISP_RESOL_HRESOL(format->width));
> + MIPI_CSIS_ISP_RESOL_VRESOL(params->height) |
> + MIPI_CSIS_ISP_RESOL_HRESOL(params->width));
> }
>
> static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> - const struct csis_pix_format *csis_fmt)
> + const struct v4l2_subdev_state *state,
> + struct mipi_csis_params *params)
> {
> + const struct v4l2_mbus_framefmt *format;
> + const struct csis_pix_format *csis_fmt;
> s64 link_freq;
> u32 lane_rate;
>
> + format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> + csis_fmt = find_csis_format(format->code);
> +
> + params->channels[0].data_type = csis_fmt->data_type;
> + params->channels[0].width = format->width;
> + params->channels[0].height = format->height;
> +
> /* Calculate the line rate from the pixel rate. */
> link_freq = v4l2_get_link_freq(csis->source.pad, csis_fmt->width,
> csis->bus.num_data_lanes * 2);
> @@ -653,30 +673,29 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> * (which is documented as corresponding to CSI-2 v0.87 to v1.00) until
> * we figure out how to compute it correctly.
> */
> - csis->hs_settle = (lane_rate - 5000000) / 45000000;
> - csis->clk_settle = 0;
> + params->hs_settle = (lane_rate - 5000000) / 45000000;
> + params->clk_settle = 0;
>
> dev_dbg(csis->dev, "lane rate %u, Tclk_settle %u, Ths_settle %u\n",
> - lane_rate, csis->clk_settle, csis->hs_settle);
> + lane_rate, params->clk_settle, params->hs_settle);
>
> if (csis->debug.hs_settle < 0xff) {
> dev_dbg(csis->dev, "overriding Ths_settle with %u\n",
> csis->debug.hs_settle);
> - csis->hs_settle = csis->debug.hs_settle;
> + params->hs_settle = csis->debug.hs_settle;
> }
>
> if (csis->debug.clk_settle < 4) {
> dev_dbg(csis->dev, "overriding Tclk_settle with %u\n",
> csis->debug.clk_settle);
> - csis->clk_settle = csis->debug.clk_settle;
> + params->clk_settle = csis->debug.clk_settle;
> }
>
> return 0;
> }
>
> static void mipi_csis_set_params(struct mipi_csis_device *csis,
> - const struct v4l2_mbus_framefmt *format,
> - const struct csis_pix_format *csis_fmt)
> + const struct mipi_csis_params *params)
> {
> int lanes = csis->bus.num_data_lanes;
> u32 val;
> @@ -692,11 +711,11 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>
> mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
>
> - __mipi_csis_set_format(csis, format, csis_fmt);
> + __mipi_csis_set_format(csis, ¶ms->channels[0]);
>
> mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
> - MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(csis->hs_settle) |
> - MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(csis->clk_settle));
> + MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params->hs_settle) |
> + MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params->clk_settle));
>
> mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0),
> MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
> @@ -771,11 +790,10 @@ static int mipi_csis_clk_get(struct mipi_csis_device *csis)
> }
>
> static void mipi_csis_start_stream(struct mipi_csis_device *csis,
> - const struct v4l2_mbus_framefmt *format,
> - const struct csis_pix_format *csis_fmt)
> + const struct mipi_csis_params *params)
> {
> mipi_csis_sw_reset(csis);
> - mipi_csis_set_params(csis, format, csis_fmt);
> + mipi_csis_set_params(csis, params);
> mipi_csis_system_enable(csis, true);
> mipi_csis_enable_interrupts(csis, true);
> }
> @@ -1216,13 +1234,9 @@ static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
>
> /* Start the CSIS with the first stream. */
> if (!csis->source.enabled_streams) {
> - const struct v4l2_mbus_framefmt *format;
> - const struct csis_pix_format *csis_fmt;
> + struct mipi_csis_params params;
>
> - format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> - csis_fmt = find_csis_format(format->code);
> -
> - ret = mipi_csis_calculate_params(csis, csis_fmt);
> + ret = mipi_csis_calculate_params(csis, state, ¶ms);
> if (ret)
> return ret;
>
> @@ -1232,7 +1246,7 @@ static int mipi_csis_enable_streams(struct v4l2_subdev *sd,
> if (ret < 0)
> return ret;
>
> - mipi_csis_start_stream(csis, format, csis_fmt);
> + mipi_csis_start_stream(csis, ¶ms);
> }
>
> ret = v4l2_subdev_enable_streams(csis->source.sd,
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v1 5/6] media: imx-mipi-csis: Set all per-channel registers in one function
2025-11-07 1:58 [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Laurent Pinchart
` (3 preceding siblings ...)
2025-11-07 1:58 ` [PATCH v1 4/6] media: imx-mipi-csis: Group runtime parameters in structure Laurent Pinchart
@ 2025-11-07 1:58 ` Laurent Pinchart
2025-11-07 16:37 ` Frank Li
2025-11-07 1:58 ` [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support Laurent Pinchart
2025-11-07 9:31 ` [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Martin Kepplinger
6 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 1:58 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team,
Pengutronix Kernel Team, imx, Frank Li, Stefan Klug, Sakari Ailus
The __mipi_csis_set_format() function programs most of the per-channel
registers, with the exception of the MIPI_CSIS_ISP_SYNC_CH register. To
prepare for multi-channel support, move programming of that register to
the function, and rename it to mipi_csis_set_channel_params() to clearly
indicate its purpose.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/nxp/imx-mipi-csis.c | 23 +++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 9dd264af181f..d8c11223ed0a 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -597,13 +597,14 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL, val);
}
-static void __mipi_csis_set_format(struct mipi_csis_device *csis,
- const struct mipi_csis_channel_params *params)
+static void mipi_csis_set_channel_params(struct mipi_csis_device *csis,
+ unsigned int channel,
+ const struct mipi_csis_channel_params *params)
{
u32 val;
/* Color format */
- val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(0));
+ val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(channel));
val &= ~(MIPI_CSIS_ISPCFG_PARALLEL | MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK |
MIPI_CSIS_ISPCFG_DATAFORMAT_MASK |
MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL_MASK);
@@ -627,10 +628,15 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
- mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
+ mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(channel), val);
+
+ mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(channel),
+ MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
+ MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(0) |
+ MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
/* Pixel resolution */
- mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0),
+ mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(channel),
MIPI_CSIS_ISP_RESOL_VRESOL(params->height) |
MIPI_CSIS_ISP_RESOL_HRESOL(params->width));
}
@@ -711,17 +717,12 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
- __mipi_csis_set_format(csis, ¶ms->channels[0]);
+ mipi_csis_set_channel_params(csis, 0, ¶ms->channels[0]);
mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params->hs_settle) |
MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params->clk_settle));
- mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0),
- MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
- MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(0) |
- MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
-
val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v1 5/6] media: imx-mipi-csis: Set all per-channel registers in one function
2025-11-07 1:58 ` [PATCH v1 5/6] media: imx-mipi-csis: Set all per-channel registers in one function Laurent Pinchart
@ 2025-11-07 16:37 ` Frank Li
0 siblings, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-11-07 16:37 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 03:58:12AM +0200, Laurent Pinchart wrote:
> The __mipi_csis_set_format() function programs most of the per-channel
> registers, with the exception of the MIPI_CSIS_ISP_SYNC_CH register. To
> prepare for multi-channel support, move programming of that register to
> the function, and rename it to mipi_csis_set_channel_params() to clearly
> indicate its purpose.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/media/platform/nxp/imx-mipi-csis.c | 23 +++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 9dd264af181f..d8c11223ed0a 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -597,13 +597,14 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
> mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL, val);
> }
>
> -static void __mipi_csis_set_format(struct mipi_csis_device *csis,
> - const struct mipi_csis_channel_params *params)
> +static void mipi_csis_set_channel_params(struct mipi_csis_device *csis,
> + unsigned int channel,
> + const struct mipi_csis_channel_params *params)
> {
> u32 val;
>
> /* Color format */
> - val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(0));
> + val = mipi_csis_read(csis, MIPI_CSIS_ISP_CONFIG_CH(channel));
> val &= ~(MIPI_CSIS_ISPCFG_PARALLEL | MIPI_CSIS_ISPCFG_PIXEL_MODE_MASK |
> MIPI_CSIS_ISPCFG_DATAFORMAT_MASK |
> MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL_MASK);
> @@ -627,10 +628,15 @@ static void __mipi_csis_set_format(struct mipi_csis_device *csis,
> val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
> val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
>
> - mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(0), val);
> + mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(channel), val);
> +
> + mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(channel),
> + MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
> + MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(0) |
> + MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
>
> /* Pixel resolution */
> - mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(0),
> + mipi_csis_write(csis, MIPI_CSIS_ISP_RESOL_CH(channel),
> MIPI_CSIS_ISP_RESOL_VRESOL(params->height) |
> MIPI_CSIS_ISP_RESOL_HRESOL(params->width));
> }
> @@ -711,17 +717,12 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>
> mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
>
> - __mipi_csis_set_format(csis, ¶ms->channels[0]);
> + mipi_csis_set_channel_params(csis, 0, ¶ms->channels[0]);
>
> mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
> MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params->hs_settle) |
> MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params->clk_settle));
>
> - mipi_csis_write(csis, MIPI_CSIS_ISP_SYNC_CH(0),
> - MIPI_CSIS_ISP_SYNC_HSYNC_LINTV(0) |
> - MIPI_CSIS_ISP_SYNC_VSYNC_SINTV(0) |
> - MIPI_CSIS_ISP_SYNC_VSYNC_EINTV(0));
> -
> val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
> val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
2025-11-07 1:58 [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Laurent Pinchart
` (4 preceding siblings ...)
2025-11-07 1:58 ` [PATCH v1 5/6] media: imx-mipi-csis: Set all per-channel registers in one function Laurent Pinchart
@ 2025-11-07 1:58 ` Laurent Pinchart
2025-11-07 16:48 ` Frank Li
2025-11-07 9:31 ` [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Martin Kepplinger
6 siblings, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 1:58 UTC (permalink / raw)
To: linux-media
Cc: Rui Miguel Silva, Martin Kepplinger, Purism Kernel Team,
Pengutronix Kernel Team, imx, Frank Li, Stefan Klug, Sakari Ailus
CSIS output channels can be used to demultiplex CSI-2 virtual channels,
and likely data types. While this feature is not clearly documented,
tests seem to indicate that each output channel includes filters to
select which VC and DT to output, with the filtering mode being
configured globally. Four modes are supported:
- No filtering: all VCs and DTs are output on channel 0
- VC filtering: each channel filters out all but the configured VC, the
DT is ignored
- DT filtering: each channel filters out all but the configured DT, the
VC is ignored
- DT and VC filtering: each channel filters out all but the configured
VC and DT
Expose this feature to userspace through the streams API. The routing
table is expanded to support multiple routes, with the source stream ID
mapping to the output channel number. As the VC and DT values
corresponding to a stream are not known until they get queried from the
source, validation of the routes is postponed to stream enable time in
the mipi_csis_calculate_params() function that extract the configuration
of each output channel from the routing table. The validation ensures
that, when filtering is enabled, each output channel is configured to
output exactly one VC and one DT.
When multiple streams are routed to the same output channel, the output
heights is the sum of the heights of all the streams. This is
implemented when propagating formats frim sink to source pads.
Due to how the SoC glues together IP cores, multi-output operation in
the i.MX8MP is used only for the purpose of capturing multi-exposure or
multi-gain HDR streams from a camera sensor over different CSI-2 VCs and
transmitting them to the ISP. The streams are stitched together by the
ISP and can't be captured individually. This has allowed testing VC
filtering but not DT filtering. For that reason, multi-channel support
is currently limited to VC filtering only.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/platform/nxp/imx-mipi-csis.c | 264 ++++++++++++++++++---
1 file changed, 234 insertions(+), 30 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index d8c11223ed0a..b5c7ab7c541c 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -12,6 +12,7 @@
*
*/
+#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/debugfs.h>
#include <linux/delay.h>
@@ -334,7 +335,8 @@ struct mipi_csis_info {
};
struct mipi_csis_channel_params {
- unsigned int data_type;
+ u16 vc_mask;
+ u16 data_type;
unsigned int width;
unsigned int height;
};
@@ -343,6 +345,7 @@ struct mipi_csis_params {
u32 hs_settle;
u32 clk_settle;
+ bool interleave_vc;
struct mipi_csis_channel_params channels[MIPI_CSIS_MAX_CHANNELS];
};
@@ -626,7 +629,7 @@ static void mipi_csis_set_channel_params(struct mipi_csis_device *csis,
val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
- val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
+ val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(ffs(params->vc_mask) - 1);
mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(channel), val);
@@ -645,20 +648,148 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
const struct v4l2_subdev_state *state,
struct mipi_csis_params *params)
{
- const struct v4l2_mbus_framefmt *format;
- const struct csis_pix_format *csis_fmt;
+ const struct csis_pix_format *csis_fmt = NULL;
+ struct v4l2_subdev_route *route;
+ struct v4l2_mbus_frame_desc fd;
s64 link_freq;
u32 lane_rate;
+ int ret;
- format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
- csis_fmt = find_csis_format(format->code);
+ memset(params, 0, sizeof(*params));
- params->channels[0].data_type = csis_fmt->data_type;
- params->channels[0].width = format->width;
- params->channels[0].height = format->height;
+ /*
+ * Translate routing configuration to output channels parameters.
+ *
+ * The CSIS VC and DT handling is poorly documented. The device supports
+ * a global "interleave mode" parameter in the CMN_CTRL register that
+ * can be set to "VC and DT", "VC only", "DT only" or "CH0 only, no data
+ * interleave". The ISP_CONFIG registers specify DT and VC values per
+ * output channel.
+ *
+ * This can be interpreted as per-channel VC and DT filters, with the
+ * filter type being configured globally and the VC and DT configured
+ * per-channel. VC tests seem to corroborate this interpretation, but DT
+ * tests are yet to be performed.
+ */
+ ret = v4l2_subdev_call(csis->source.sd, pad, get_frame_desc,
+ csis->source.pad->index, &fd);
+ if (ret && ret != -ENOIOCTLCMD) {
+ dev_err(csis->dev, "Failed to get source frame descriptors: %d\n", ret);
+ return ret;
+ }
- /* Calculate the line rate from the pixel rate. */
- link_freq = v4l2_get_link_freq(csis->source.pad, csis_fmt->width,
+ if (ret == -ENOIOCTLCMD) {
+ const struct v4l2_mbus_framefmt *format;
+
+ /*
+ * The source doesn't report frame descriptors. Assume a single
+ * stream on VC0.
+ */
+ format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK, 0);
+ if (!format)
+ return -EPIPE;
+
+ csis_fmt = find_csis_format(format->code);
+
+ fd.type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+ fd.num_entries = 1;
+ fd.entry[0].pixelcode = format->code;
+ fd.entry[0].bus.csi2.dt = csis_fmt->data_type;
+ }
+
+ /*
+ * Translate sink streams to source streams and fill the channel
+ * configuration vc_mask and data_type fields.
+ */
+ for_each_active_route(&state->routing, route) {
+ struct mipi_csis_channel_params *channel =
+ ¶ms->channels[route->source_stream];
+ const struct v4l2_mbus_frame_desc_entry *entry = NULL;
+
+ /*
+ * Find the corresponding frame descriptor entry, to get the VC
+ * and DT for the stream. Multiple entries may match the stream,
+ * but they have to all report the same VC and DT, so we can
+ * just use the first matching entry.
+ */
+ for (unsigned int i = 0; i < fd.num_entries; ++i) {
+ if (fd.entry[i].stream == route->sink_stream) {
+ entry = &fd.entry[i];
+ break;
+ }
+ }
+
+ if (!entry) {
+ dev_dbg(csis->dev, "No frame descriptor for stream %u\n",
+ route->sink_stream);
+ return -EPIPE;
+ }
+
+ /*
+ * Routing constraint: all streams routed to the same output
+ * channel need to have the same DT.
+ */
+ if (channel->data_type &&
+ channel->data_type != entry->bus.csi2.dt) {
+ dev_dbg(csis->dev,
+ "DT mismatch on channel %u: stream %u DT 0x%02x != 0x%02x\n",
+ route->source_stream, route->sink_stream,
+ entry->bus.csi2.dt, channel->data_type);
+ return -EPIPE;
+ }
+
+ /* Record the VC and DT for the output channel. */
+ channel->vc_mask |= BIT(entry->bus.csi2.vc);
+ channel->data_type = entry->bus.csi2.dt;
+
+ /*
+ * If any output channel beside channel 0 is enabled, enable VC
+ * interleave mode.
+ */
+ if (route->source_stream > 0)
+ params->interleave_vc = true;
+ }
+
+ /*
+ * Iterate over the enabled output channels to record the width and
+ * height. Verify that the VC filtering matches the hardware
+ * constraints.
+ */
+ for (unsigned int i = 0; i < csis->num_channels; ++i) {
+ struct mipi_csis_channel_params *channel = ¶ms->channels[i];
+ const struct v4l2_mbus_framefmt *format;
+
+ if (!channel->vc_mask)
+ continue;
+
+ /*
+ * In VC interleave mode, each output channel is limited to a
+ * single VC.
+ */
+ if (params->interleave_vc && hweight8(channel->vc_mask) != 1) {
+ dev_dbg(csis->dev,
+ "Channel %u must output a single VCs\n", i);
+ return -EPIPE;
+ }
+
+ format = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE, i);
+ if (!format) {
+ dev_dbg(csis->dev, "No format for source stream %u\n", i);
+ return -EPIPE;
+ }
+
+ channel->width = format->width;
+ channel->height = format->height;
+ }
+
+ /*
+ * Calculate the line rate from the pixel rate. If the source supports
+ * the .get_frame_desc() operation it has to implement the LINK_FREQ
+ * control, as the link frequency can't be calculated from the pixel
+ * rate with multiple streams having possibly different data types.
+ */
+ link_freq = v4l2_get_link_freq(csis->source.pad,
+ csis_fmt ? csis_fmt->width : 0,
csis->bus.num_data_lanes * 2);
if (link_freq < 0) {
dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
@@ -704,6 +835,8 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
const struct mipi_csis_params *params)
{
int lanes = csis->bus.num_data_lanes;
+ u32 cmn_ctrl = 0;
+ u32 clk_ctrl = 0;
u32 val;
val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
@@ -714,19 +847,32 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
if (csis->info->version == MIPI_CSIS_V3_3)
val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
+ if (params->interleave_vc)
+ val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_VC;
mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
- mipi_csis_set_channel_params(csis, 0, ¶ms->channels[0]);
+ for (unsigned int i = 0; i < csis->num_channels; ++i) {
+ const struct mipi_csis_channel_params *channel =
+ ¶ms->channels[i];
+
+ if (!channel->vc_mask)
+ continue;
+
+ mipi_csis_set_channel_params(csis, i, channel);
+
+ cmn_ctrl |= MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(i);
+ clk_ctrl |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(i, 15)
+ | MIPI_CSIS_CLK_CTRL_WCLK_SRC(i);
+ }
mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params->hs_settle) |
MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params->clk_settle));
val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
- val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
- val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
+ val |= clk_ctrl;
mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L,
@@ -741,8 +887,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
/* Update the shadow register. */
val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
- mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
- val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
+ mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val | cmn_ctrl |
MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
}
@@ -1053,7 +1198,7 @@ static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
if (code->index > 0)
return -EINVAL;
- fmt = v4l2_subdev_state_get_format(state, code->pad);
+ fmt = v4l2_subdev_state_get_format(state, code->pad, code->stream);
code->code = fmt->code;
return 0;
}
@@ -1069,10 +1214,57 @@ static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
}
+static void mipi_csis_propagate_formats(struct mipi_csis_device *csis,
+ struct v4l2_subdev_state *state)
+{
+ const struct v4l2_mbus_framefmt *channels[MIPI_CSIS_MAX_CHANNELS] = { };
+ struct v4l2_subdev_route *route;
+
+ for_each_active_route(&state->routing, route) {
+ const struct csis_pix_format *csis_fmt;
+ struct v4l2_mbus_framefmt *sink_fmt;
+ struct v4l2_mbus_framefmt *src_fmt;
+
+ sink_fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK,
+ route->sink_stream);
+ src_fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE,
+ route->source_stream);
+
+ csis_fmt = find_csis_format(sink_fmt->code);
+
+ /*
+ * If the output channel corresponding to this source stream
+ * isn't associated with a sink stream yet, simply propagate the
+ * format from sink stream to source stream and associate the
+ * sink stream with the channel.
+ *
+ * Otherwise, the sink stream format must match the primary sink
+ * stream associated with the channel except for the height that
+ * can be different. We propagate the format from the primary to
+ * secondary sink stream, and accumulate the height in the
+ * source stream format.
+ */
+ if (!channels[route->source_stream]) {
+ *src_fmt = *sink_fmt;
+ src_fmt->code = csis_fmt->output;
+
+ channels[route->source_stream] = sink_fmt;
+ } else {
+ unsigned int height = sink_fmt->height;
+
+ *sink_fmt = *channels[route->source_stream];
+ sink_fmt->height = height;
+
+ src_fmt->height += sink_fmt->height;
+ }
+ }
+}
+
static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *sdformat)
{
+ struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
const struct csis_pix_format *csis_fmt;
struct v4l2_mbus_framefmt *fmt;
unsigned int align;
@@ -1120,7 +1312,8 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
&sdformat->format.height, 1,
CSIS_MAX_PIX_HEIGHT, 0, 0);
- fmt = v4l2_subdev_state_get_format(state, sdformat->pad);
+ fmt = v4l2_subdev_state_get_format(state, sdformat->pad,
+ sdformat->stream);
fmt->code = csis_fmt->code;
fmt->width = sdformat->format.width;
@@ -1133,12 +1326,8 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
sdformat->format = *fmt;
- /* Propagate the format from sink to source. */
- fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
- *fmt = sdformat->format;
-
- /* The format on the source pad might change due to unpacking. */
- fmt->code = csis_fmt->output;
+ /* Propagate the format. */
+ mipi_csis_propagate_formats(csis, state);
return 0;
}
@@ -1155,7 +1344,7 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
return -EINVAL;
state = v4l2_subdev_lock_and_get_active_state(sd);
- fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
+ fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE, 0);
csis_fmt = find_csis_format(fmt->code);
v4l2_subdev_unlock_state(state);
@@ -1187,6 +1376,8 @@ static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
.quantization = V4L2_QUANTIZATION_LIM_RANGE,
.xfer_func = V4L2_XFER_FUNC_SRGB,
};
+ struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
+ struct v4l2_subdev_route *route;
int ret;
ret = v4l2_subdev_routing_validate(sd, routing,
@@ -1194,15 +1385,27 @@ static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
if (ret)
return ret;
- /* Only a single route is supported for now. */
- if (routing->num_routes != 1 ||
- !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
- return -EINVAL;
+ /*
+ * The source stream identifies the output channel. Validate that it
+ * does not exceed the number of channels available in the device. The
+ * other routing constraints can't be validated now as they require
+ * querying the frame descriptor on the sink side, which can only be
+ * done when enabling streaming.
+ */
+ for_each_active_route(routing, route) {
+ if (route->source_stream >= csis->num_channels) {
+ dev_dbg(csis->dev, "Invalid source stream %u",
+ route->source_stream);
+ return -EINVAL;
+ }
+ }
ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
if (ret)
return ret;
+ mipi_csis_propagate_formats(csis, state);
+
return 0;
}
@@ -1554,7 +1757,8 @@ static int mipi_csis_subdev_init(struct mipi_csis_device *csis)
snprintf(sd->name, sizeof(sd->name), "csis-%s",
dev_name(csis->dev));
- sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
+ sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS
+ | V4L2_SUBDEV_FL_STREAMS;
sd->ctrl_handler = NULL;
sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
2025-11-07 1:58 ` [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support Laurent Pinchart
@ 2025-11-07 16:48 ` Frank Li
2025-11-07 18:43 ` Laurent Pinchart
2025-11-20 3:12 ` G.N. Zhou
0 siblings, 2 replies; 26+ messages in thread
From: Frank Li @ 2025-11-07 16:48 UTC (permalink / raw)
To: Laurent Pinchart, guoniu.zhou
Cc: linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 03:58:13AM +0200, Laurent Pinchart wrote:
> CSIS output channels can be used to demultiplex CSI-2 virtual channels,
> and likely data types. While this feature is not clearly documented,
> tests seem to indicate that each output channel includes filters to
> select which VC and DT to output, with the filtering mode being
> configured globally. Four modes are supported:
>
> - No filtering: all VCs and DTs are output on channel 0
> - VC filtering: each channel filters out all but the configured VC, the
> DT is ignored
> - DT filtering: each channel filters out all but the configured DT, the
> VC is ignored
> - DT and VC filtering: each channel filters out all but the configured
> VC and DT
>
> Expose this feature to userspace through the streams API. The routing
> table is expanded to support multiple routes, with the source stream ID
> mapping to the output channel number. As the VC and DT values
> corresponding to a stream are not known until they get queried from the
> source, validation of the routes is postponed to stream enable time in
> the mipi_csis_calculate_params() function that extract the configuration
> of each output channel from the routing table. The validation ensures
> that, when filtering is enabled, each output channel is configured to
> output exactly one VC and one DT.
>
> When multiple streams are routed to the same output channel, the output
> heights is the sum of the heights of all the streams. This is
> implemented when propagating formats frim sink to source pads.
>
> Due to how the SoC glues together IP cores, multi-output operation in
> the i.MX8MP is used only for the purpose of capturing multi-exposure or
> multi-gain HDR streams from a camera sensor over different CSI-2 VCs and
> transmitting them to the ISP. The streams are stitched together by the
> ISP and can't be captured individually. This has allowed testing VC
> filtering but not DT filtering. For that reason, multi-channel support
> is currently limited to VC filtering only.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Add guoniu.zhou@nxp.com, patch itself look good, but I am not famillar with
internal logic, Guoniu, can you help check it?
Frank
> ---
> drivers/media/platform/nxp/imx-mipi-csis.c | 264 ++++++++++++++++++---
> 1 file changed, 234 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index d8c11223ed0a..b5c7ab7c541c 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -12,6 +12,7 @@
> *
> */
>
> +#include <linux/bitops.h>
> #include <linux/clk.h>
> #include <linux/debugfs.h>
> #include <linux/delay.h>
> @@ -334,7 +335,8 @@ struct mipi_csis_info {
> };
>
> struct mipi_csis_channel_params {
> - unsigned int data_type;
> + u16 vc_mask;
> + u16 data_type;
> unsigned int width;
> unsigned int height;
> };
> @@ -343,6 +345,7 @@ struct mipi_csis_params {
> u32 hs_settle;
> u32 clk_settle;
>
> + bool interleave_vc;
> struct mipi_csis_channel_params channels[MIPI_CSIS_MAX_CHANNELS];
> };
>
> @@ -626,7 +629,7 @@ static void mipi_csis_set_channel_params(struct mipi_csis_device *csis,
> val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
>
> val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
> - val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
> + val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(ffs(params->vc_mask) - 1);
>
> mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(channel), val);
>
> @@ -645,20 +648,148 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> const struct v4l2_subdev_state *state,
> struct mipi_csis_params *params)
> {
> - const struct v4l2_mbus_framefmt *format;
> - const struct csis_pix_format *csis_fmt;
> + const struct csis_pix_format *csis_fmt = NULL;
> + struct v4l2_subdev_route *route;
> + struct v4l2_mbus_frame_desc fd;
> s64 link_freq;
> u32 lane_rate;
> + int ret;
>
> - format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> - csis_fmt = find_csis_format(format->code);
> + memset(params, 0, sizeof(*params));
>
> - params->channels[0].data_type = csis_fmt->data_type;
> - params->channels[0].width = format->width;
> - params->channels[0].height = format->height;
> + /*
> + * Translate routing configuration to output channels parameters.
> + *
> + * The CSIS VC and DT handling is poorly documented. The device supports
> + * a global "interleave mode" parameter in the CMN_CTRL register that
> + * can be set to "VC and DT", "VC only", "DT only" or "CH0 only, no data
> + * interleave". The ISP_CONFIG registers specify DT and VC values per
> + * output channel.
> + *
> + * This can be interpreted as per-channel VC and DT filters, with the
> + * filter type being configured globally and the VC and DT configured
> + * per-channel. VC tests seem to corroborate this interpretation, but DT
> + * tests are yet to be performed.
> + */
> + ret = v4l2_subdev_call(csis->source.sd, pad, get_frame_desc,
> + csis->source.pad->index, &fd);
> + if (ret && ret != -ENOIOCTLCMD) {
> + dev_err(csis->dev, "Failed to get source frame descriptors: %d\n", ret);
> + return ret;
> + }
>
> - /* Calculate the line rate from the pixel rate. */
> - link_freq = v4l2_get_link_freq(csis->source.pad, csis_fmt->width,
> + if (ret == -ENOIOCTLCMD) {
> + const struct v4l2_mbus_framefmt *format;
> +
> + /*
> + * The source doesn't report frame descriptors. Assume a single
> + * stream on VC0.
> + */
> + format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK, 0);
> + if (!format)
> + return -EPIPE;
> +
> + csis_fmt = find_csis_format(format->code);
> +
> + fd.type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> + fd.num_entries = 1;
> + fd.entry[0].pixelcode = format->code;
> + fd.entry[0].bus.csi2.dt = csis_fmt->data_type;
> + }
> +
> + /*
> + * Translate sink streams to source streams and fill the channel
> + * configuration vc_mask and data_type fields.
> + */
> + for_each_active_route(&state->routing, route) {
> + struct mipi_csis_channel_params *channel =
> + ¶ms->channels[route->source_stream];
> + const struct v4l2_mbus_frame_desc_entry *entry = NULL;
> +
> + /*
> + * Find the corresponding frame descriptor entry, to get the VC
> + * and DT for the stream. Multiple entries may match the stream,
> + * but they have to all report the same VC and DT, so we can
> + * just use the first matching entry.
> + */
> + for (unsigned int i = 0; i < fd.num_entries; ++i) {
> + if (fd.entry[i].stream == route->sink_stream) {
> + entry = &fd.entry[i];
> + break;
> + }
> + }
> +
> + if (!entry) {
> + dev_dbg(csis->dev, "No frame descriptor for stream %u\n",
> + route->sink_stream);
> + return -EPIPE;
> + }
> +
> + /*
> + * Routing constraint: all streams routed to the same output
> + * channel need to have the same DT.
> + */
> + if (channel->data_type &&
> + channel->data_type != entry->bus.csi2.dt) {
> + dev_dbg(csis->dev,
> + "DT mismatch on channel %u: stream %u DT 0x%02x != 0x%02x\n",
> + route->source_stream, route->sink_stream,
> + entry->bus.csi2.dt, channel->data_type);
> + return -EPIPE;
> + }
> +
> + /* Record the VC and DT for the output channel. */
> + channel->vc_mask |= BIT(entry->bus.csi2.vc);
> + channel->data_type = entry->bus.csi2.dt;
> +
> + /*
> + * If any output channel beside channel 0 is enabled, enable VC
> + * interleave mode.
> + */
> + if (route->source_stream > 0)
> + params->interleave_vc = true;
> + }
> +
> + /*
> + * Iterate over the enabled output channels to record the width and
> + * height. Verify that the VC filtering matches the hardware
> + * constraints.
> + */
> + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> + struct mipi_csis_channel_params *channel = ¶ms->channels[i];
> + const struct v4l2_mbus_framefmt *format;
> +
> + if (!channel->vc_mask)
> + continue;
> +
> + /*
> + * In VC interleave mode, each output channel is limited to a
> + * single VC.
> + */
> + if (params->interleave_vc && hweight8(channel->vc_mask) != 1) {
> + dev_dbg(csis->dev,
> + "Channel %u must output a single VCs\n", i);
> + return -EPIPE;
> + }
> +
> + format = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE, i);
> + if (!format) {
> + dev_dbg(csis->dev, "No format for source stream %u\n", i);
> + return -EPIPE;
> + }
> +
> + channel->width = format->width;
> + channel->height = format->height;
> + }
> +
> + /*
> + * Calculate the line rate from the pixel rate. If the source supports
> + * the .get_frame_desc() operation it has to implement the LINK_FREQ
> + * control, as the link frequency can't be calculated from the pixel
> + * rate with multiple streams having possibly different data types.
> + */
> + link_freq = v4l2_get_link_freq(csis->source.pad,
> + csis_fmt ? csis_fmt->width : 0,
> csis->bus.num_data_lanes * 2);
> if (link_freq < 0) {
> dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
> @@ -704,6 +835,8 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> const struct mipi_csis_params *params)
> {
> int lanes = csis->bus.num_data_lanes;
> + u32 cmn_ctrl = 0;
> + u32 clk_ctrl = 0;
> u32 val;
>
> val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> @@ -714,19 +847,32 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>
> if (csis->info->version == MIPI_CSIS_V3_3)
> val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> + if (params->interleave_vc)
> + val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_VC;
>
> mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
>
> - mipi_csis_set_channel_params(csis, 0, ¶ms->channels[0]);
> + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> + const struct mipi_csis_channel_params *channel =
> + ¶ms->channels[i];
> +
> + if (!channel->vc_mask)
> + continue;
> +
> + mipi_csis_set_channel_params(csis, i, channel);
> +
> + cmn_ctrl |= MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(i);
> + clk_ctrl |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(i, 15)
> + | MIPI_CSIS_CLK_CTRL_WCLK_SRC(i);
> + }
>
> mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
> MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params->hs_settle) |
> MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params->clk_settle));
>
> val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> - val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
> - val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
> val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
> + val |= clk_ctrl;
> mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
>
> mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L,
> @@ -741,8 +887,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>
> /* Update the shadow register. */
> val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> - mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> - val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
> + mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val | cmn_ctrl |
> MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
> }
>
> @@ -1053,7 +1198,7 @@ static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
> if (code->index > 0)
> return -EINVAL;
>
> - fmt = v4l2_subdev_state_get_format(state, code->pad);
> + fmt = v4l2_subdev_state_get_format(state, code->pad, code->stream);
> code->code = fmt->code;
> return 0;
> }
> @@ -1069,10 +1214,57 @@ static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
> return 0;
> }
>
> +static void mipi_csis_propagate_formats(struct mipi_csis_device *csis,
> + struct v4l2_subdev_state *state)
> +{
> + const struct v4l2_mbus_framefmt *channels[MIPI_CSIS_MAX_CHANNELS] = { };
> + struct v4l2_subdev_route *route;
> +
> + for_each_active_route(&state->routing, route) {
> + const struct csis_pix_format *csis_fmt;
> + struct v4l2_mbus_framefmt *sink_fmt;
> + struct v4l2_mbus_framefmt *src_fmt;
> +
> + sink_fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK,
> + route->sink_stream);
> + src_fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE,
> + route->source_stream);
> +
> + csis_fmt = find_csis_format(sink_fmt->code);
> +
> + /*
> + * If the output channel corresponding to this source stream
> + * isn't associated with a sink stream yet, simply propagate the
> + * format from sink stream to source stream and associate the
> + * sink stream with the channel.
> + *
> + * Otherwise, the sink stream format must match the primary sink
> + * stream associated with the channel except for the height that
> + * can be different. We propagate the format from the primary to
> + * secondary sink stream, and accumulate the height in the
> + * source stream format.
> + */
> + if (!channels[route->source_stream]) {
> + *src_fmt = *sink_fmt;
> + src_fmt->code = csis_fmt->output;
> +
> + channels[route->source_stream] = sink_fmt;
> + } else {
> + unsigned int height = sink_fmt->height;
> +
> + *sink_fmt = *channels[route->source_stream];
> + sink_fmt->height = height;
> +
> + src_fmt->height += sink_fmt->height;
> + }
> + }
> +}
> +
> static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_format *sdformat)
> {
> + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> const struct csis_pix_format *csis_fmt;
> struct v4l2_mbus_framefmt *fmt;
> unsigned int align;
> @@ -1120,7 +1312,8 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> &sdformat->format.height, 1,
> CSIS_MAX_PIX_HEIGHT, 0, 0);
>
> - fmt = v4l2_subdev_state_get_format(state, sdformat->pad);
> + fmt = v4l2_subdev_state_get_format(state, sdformat->pad,
> + sdformat->stream);
>
> fmt->code = csis_fmt->code;
> fmt->width = sdformat->format.width;
> @@ -1133,12 +1326,8 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
>
> sdformat->format = *fmt;
>
> - /* Propagate the format from sink to source. */
> - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> - *fmt = sdformat->format;
> -
> - /* The format on the source pad might change due to unpacking. */
> - fmt->code = csis_fmt->output;
> + /* Propagate the format. */
> + mipi_csis_propagate_formats(csis, state);
>
> return 0;
> }
> @@ -1155,7 +1344,7 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> return -EINVAL;
>
> state = v4l2_subdev_lock_and_get_active_state(sd);
> - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> + fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE, 0);
> csis_fmt = find_csis_format(fmt->code);
> v4l2_subdev_unlock_state(state);
>
> @@ -1187,6 +1376,8 @@ static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> .quantization = V4L2_QUANTIZATION_LIM_RANGE,
> .xfer_func = V4L2_XFER_FUNC_SRGB,
> };
> + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> + struct v4l2_subdev_route *route;
> int ret;
>
> ret = v4l2_subdev_routing_validate(sd, routing,
> @@ -1194,15 +1385,27 @@ static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> if (ret)
> return ret;
>
> - /* Only a single route is supported for now. */
> - if (routing->num_routes != 1 ||
> - !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> - return -EINVAL;
> + /*
> + * The source stream identifies the output channel. Validate that it
> + * does not exceed the number of channels available in the device. The
> + * other routing constraints can't be validated now as they require
> + * querying the frame descriptor on the sink side, which can only be
> + * done when enabling streaming.
> + */
> + for_each_active_route(routing, route) {
> + if (route->source_stream >= csis->num_channels) {
> + dev_dbg(csis->dev, "Invalid source stream %u",
> + route->source_stream);
> + return -EINVAL;
> + }
> + }
>
> ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> if (ret)
> return ret;
>
> + mipi_csis_propagate_formats(csis, state);
> +
> return 0;
> }
>
> @@ -1554,7 +1757,8 @@ static int mipi_csis_subdev_init(struct mipi_csis_device *csis)
> snprintf(sd->name, sizeof(sd->name), "csis-%s",
> dev_name(csis->dev));
>
> - sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS
> + | V4L2_SUBDEV_FL_STREAMS;
> sd->ctrl_handler = NULL;
>
> sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> --
> Regards,
>
> Laurent Pinchart
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
2025-11-07 16:48 ` Frank Li
@ 2025-11-07 18:43 ` Laurent Pinchart
2025-11-20 3:12 ` G.N. Zhou
1 sibling, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 18:43 UTC (permalink / raw)
To: Frank Li
Cc: guoniu.zhou, linux-media, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx, Stefan Klug,
Sakari Ailus
On Fri, Nov 07, 2025 at 11:48:00AM -0500, Frank Li wrote:
> On Fri, Nov 07, 2025 at 03:58:13AM +0200, Laurent Pinchart wrote:
> > CSIS output channels can be used to demultiplex CSI-2 virtual channels,
> > and likely data types. While this feature is not clearly documented,
> > tests seem to indicate that each output channel includes filters to
> > select which VC and DT to output, with the filtering mode being
> > configured globally. Four modes are supported:
> >
> > - No filtering: all VCs and DTs are output on channel 0
> > - VC filtering: each channel filters out all but the configured VC, the
> > DT is ignored
> > - DT filtering: each channel filters out all but the configured DT, the
> > VC is ignored
> > - DT and VC filtering: each channel filters out all but the configured
> > VC and DT
> >
> > Expose this feature to userspace through the streams API. The routing
> > table is expanded to support multiple routes, with the source stream ID
> > mapping to the output channel number. As the VC and DT values
> > corresponding to a stream are not known until they get queried from the
> > source, validation of the routes is postponed to stream enable time in
> > the mipi_csis_calculate_params() function that extract the configuration
> > of each output channel from the routing table. The validation ensures
> > that, when filtering is enabled, each output channel is configured to
> > output exactly one VC and one DT.
> >
> > When multiple streams are routed to the same output channel, the output
> > heights is the sum of the heights of all the streams. This is
> > implemented when propagating formats frim sink to source pads.
> >
> > Due to how the SoC glues together IP cores, multi-output operation in
> > the i.MX8MP is used only for the purpose of capturing multi-exposure or
> > multi-gain HDR streams from a camera sensor over different CSI-2 VCs and
> > transmitting them to the ISP. The streams are stitched together by the
> > ISP and can't be captured individually. This has allowed testing VC
> > filtering but not DT filtering. For that reason, multi-channel support
> > is currently limited to VC filtering only.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Add guoniu.zhou@nxp.com, patch itself look good, but I am not famillar with
> internal logic, Guoniu, can you help check it?
I've tried hard to find more information about how the CSIS operates,
but all my attempts failed :-/ If anyone can confirm (or infirm) my
understanding, that would be great.
> > ---
> > drivers/media/platform/nxp/imx-mipi-csis.c | 264 ++++++++++++++++++---
> > 1 file changed, 234 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index d8c11223ed0a..b5c7ab7c541c 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -12,6 +12,7 @@
> > *
> > */
> >
> > +#include <linux/bitops.h>
> > #include <linux/clk.h>
> > #include <linux/debugfs.h>
> > #include <linux/delay.h>
> > @@ -334,7 +335,8 @@ struct mipi_csis_info {
> > };
> >
> > struct mipi_csis_channel_params {
> > - unsigned int data_type;
> > + u16 vc_mask;
> > + u16 data_type;
> > unsigned int width;
> > unsigned int height;
> > };
> > @@ -343,6 +345,7 @@ struct mipi_csis_params {
> > u32 hs_settle;
> > u32 clk_settle;
> >
> > + bool interleave_vc;
> > struct mipi_csis_channel_params channels[MIPI_CSIS_MAX_CHANNELS];
> > };
> >
> > @@ -626,7 +629,7 @@ static void mipi_csis_set_channel_params(struct mipi_csis_device *csis,
> > val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
> >
> > val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
> > - val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
> > + val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(ffs(params->vc_mask) - 1);
> >
> > mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(channel), val);
> >
> > @@ -645,20 +648,148 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> > const struct v4l2_subdev_state *state,
> > struct mipi_csis_params *params)
> > {
> > - const struct v4l2_mbus_framefmt *format;
> > - const struct csis_pix_format *csis_fmt;
> > + const struct csis_pix_format *csis_fmt = NULL;
> > + struct v4l2_subdev_route *route;
> > + struct v4l2_mbus_frame_desc fd;
> > s64 link_freq;
> > u32 lane_rate;
> > + int ret;
> >
> > - format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> > - csis_fmt = find_csis_format(format->code);
> > + memset(params, 0, sizeof(*params));
> >
> > - params->channels[0].data_type = csis_fmt->data_type;
> > - params->channels[0].width = format->width;
> > - params->channels[0].height = format->height;
> > + /*
> > + * Translate routing configuration to output channels parameters.
> > + *
> > + * The CSIS VC and DT handling is poorly documented. The device supports
> > + * a global "interleave mode" parameter in the CMN_CTRL register that
> > + * can be set to "VC and DT", "VC only", "DT only" or "CH0 only, no data
> > + * interleave". The ISP_CONFIG registers specify DT and VC values per
> > + * output channel.
> > + *
> > + * This can be interpreted as per-channel VC and DT filters, with the
> > + * filter type being configured globally and the VC and DT configured
> > + * per-channel. VC tests seem to corroborate this interpretation, but DT
> > + * tests are yet to be performed.
> > + */
> > + ret = v4l2_subdev_call(csis->source.sd, pad, get_frame_desc,
> > + csis->source.pad->index, &fd);
> > + if (ret && ret != -ENOIOCTLCMD) {
> > + dev_err(csis->dev, "Failed to get source frame descriptors: %d\n", ret);
> > + return ret;
> > + }
> >
> > - /* Calculate the line rate from the pixel rate. */
> > - link_freq = v4l2_get_link_freq(csis->source.pad, csis_fmt->width,
> > + if (ret == -ENOIOCTLCMD) {
> > + const struct v4l2_mbus_framefmt *format;
> > +
> > + /*
> > + * The source doesn't report frame descriptors. Assume a single
> > + * stream on VC0.
> > + */
> > + format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK, 0);
> > + if (!format)
> > + return -EPIPE;
> > +
> > + csis_fmt = find_csis_format(format->code);
> > +
> > + fd.type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> > + fd.num_entries = 1;
> > + fd.entry[0].pixelcode = format->code;
> > + fd.entry[0].bus.csi2.dt = csis_fmt->data_type;
> > + }
> > +
> > + /*
> > + * Translate sink streams to source streams and fill the channel
> > + * configuration vc_mask and data_type fields.
> > + */
> > + for_each_active_route(&state->routing, route) {
> > + struct mipi_csis_channel_params *channel =
> > + ¶ms->channels[route->source_stream];
> > + const struct v4l2_mbus_frame_desc_entry *entry = NULL;
> > +
> > + /*
> > + * Find the corresponding frame descriptor entry, to get the VC
> > + * and DT for the stream. Multiple entries may match the stream,
> > + * but they have to all report the same VC and DT, so we can
> > + * just use the first matching entry.
> > + */
> > + for (unsigned int i = 0; i < fd.num_entries; ++i) {
> > + if (fd.entry[i].stream == route->sink_stream) {
> > + entry = &fd.entry[i];
> > + break;
> > + }
> > + }
> > +
> > + if (!entry) {
> > + dev_dbg(csis->dev, "No frame descriptor for stream %u\n",
> > + route->sink_stream);
> > + return -EPIPE;
> > + }
> > +
> > + /*
> > + * Routing constraint: all streams routed to the same output
> > + * channel need to have the same DT.
> > + */
> > + if (channel->data_type &&
> > + channel->data_type != entry->bus.csi2.dt) {
> > + dev_dbg(csis->dev,
> > + "DT mismatch on channel %u: stream %u DT 0x%02x != 0x%02x\n",
> > + route->source_stream, route->sink_stream,
> > + entry->bus.csi2.dt, channel->data_type);
> > + return -EPIPE;
> > + }
> > +
> > + /* Record the VC and DT for the output channel. */
> > + channel->vc_mask |= BIT(entry->bus.csi2.vc);
> > + channel->data_type = entry->bus.csi2.dt;
> > +
> > + /*
> > + * If any output channel beside channel 0 is enabled, enable VC
> > + * interleave mode.
> > + */
> > + if (route->source_stream > 0)
> > + params->interleave_vc = true;
> > + }
> > +
> > + /*
> > + * Iterate over the enabled output channels to record the width and
> > + * height. Verify that the VC filtering matches the hardware
> > + * constraints.
> > + */
> > + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> > + struct mipi_csis_channel_params *channel = ¶ms->channels[i];
> > + const struct v4l2_mbus_framefmt *format;
> > +
> > + if (!channel->vc_mask)
> > + continue;
> > +
> > + /*
> > + * In VC interleave mode, each output channel is limited to a
> > + * single VC.
> > + */
> > + if (params->interleave_vc && hweight8(channel->vc_mask) != 1) {
> > + dev_dbg(csis->dev,
> > + "Channel %u must output a single VCs\n", i);
> > + return -EPIPE;
> > + }
> > +
> > + format = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE, i);
> > + if (!format) {
> > + dev_dbg(csis->dev, "No format for source stream %u\n", i);
> > + return -EPIPE;
> > + }
> > +
> > + channel->width = format->width;
> > + channel->height = format->height;
> > + }
> > +
> > + /*
> > + * Calculate the line rate from the pixel rate. If the source supports
> > + * the .get_frame_desc() operation it has to implement the LINK_FREQ
> > + * control, as the link frequency can't be calculated from the pixel
> > + * rate with multiple streams having possibly different data types.
> > + */
> > + link_freq = v4l2_get_link_freq(csis->source.pad,
> > + csis_fmt ? csis_fmt->width : 0,
> > csis->bus.num_data_lanes * 2);
> > if (link_freq < 0) {
> > dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
> > @@ -704,6 +835,8 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> > const struct mipi_csis_params *params)
> > {
> > int lanes = csis->bus.num_data_lanes;
> > + u32 cmn_ctrl = 0;
> > + u32 clk_ctrl = 0;
> > u32 val;
> >
> > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > @@ -714,19 +847,32 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >
> > if (csis->info->version == MIPI_CSIS_V3_3)
> > val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> > + if (params->interleave_vc)
> > + val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_VC;
> >
> > mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> >
> > - mipi_csis_set_channel_params(csis, 0, ¶ms->channels[0]);
> > + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> > + const struct mipi_csis_channel_params *channel =
> > + ¶ms->channels[i];
> > +
> > + if (!channel->vc_mask)
> > + continue;
> > +
> > + mipi_csis_set_channel_params(csis, i, channel);
> > +
> > + cmn_ctrl |= MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(i);
> > + clk_ctrl |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(i, 15)
> > + | MIPI_CSIS_CLK_CTRL_WCLK_SRC(i);
> > + }
> >
> > mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
> > MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params->hs_settle) |
> > MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params->clk_settle));
> >
> > val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> > - val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
> > - val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
> > val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
> > + val |= clk_ctrl;
> > mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
> >
> > mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L,
> > @@ -741,8 +887,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >
> > /* Update the shadow register. */
> > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > - mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> > - val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
> > + mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val | cmn_ctrl |
> > MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
> > }
> >
> > @@ -1053,7 +1198,7 @@ static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
> > if (code->index > 0)
> > return -EINVAL;
> >
> > - fmt = v4l2_subdev_state_get_format(state, code->pad);
> > + fmt = v4l2_subdev_state_get_format(state, code->pad, code->stream);
> > code->code = fmt->code;
> > return 0;
> > }
> > @@ -1069,10 +1214,57 @@ static int mipi_csis_enum_mbus_code(struct v4l2_subdev *sd,
> > return 0;
> > }
> >
> > +static void mipi_csis_propagate_formats(struct mipi_csis_device *csis,
> > + struct v4l2_subdev_state *state)
> > +{
> > + const struct v4l2_mbus_framefmt *channels[MIPI_CSIS_MAX_CHANNELS] = { };
> > + struct v4l2_subdev_route *route;
> > +
> > + for_each_active_route(&state->routing, route) {
> > + const struct csis_pix_format *csis_fmt;
> > + struct v4l2_mbus_framefmt *sink_fmt;
> > + struct v4l2_mbus_framefmt *src_fmt;
> > +
> > + sink_fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK,
> > + route->sink_stream);
> > + src_fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE,
> > + route->source_stream);
> > +
> > + csis_fmt = find_csis_format(sink_fmt->code);
> > +
> > + /*
> > + * If the output channel corresponding to this source stream
> > + * isn't associated with a sink stream yet, simply propagate the
> > + * format from sink stream to source stream and associate the
> > + * sink stream with the channel.
> > + *
> > + * Otherwise, the sink stream format must match the primary sink
> > + * stream associated with the channel except for the height that
> > + * can be different. We propagate the format from the primary to
> > + * secondary sink stream, and accumulate the height in the
> > + * source stream format.
> > + */
> > + if (!channels[route->source_stream]) {
> > + *src_fmt = *sink_fmt;
> > + src_fmt->code = csis_fmt->output;
> > +
> > + channels[route->source_stream] = sink_fmt;
> > + } else {
> > + unsigned int height = sink_fmt->height;
> > +
> > + *sink_fmt = *channels[route->source_stream];
> > + sink_fmt->height = height;
> > +
> > + src_fmt->height += sink_fmt->height;
> > + }
> > + }
> > +}
> > +
> > static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> > struct v4l2_subdev_state *state,
> > struct v4l2_subdev_format *sdformat)
> > {
> > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > const struct csis_pix_format *csis_fmt;
> > struct v4l2_mbus_framefmt *fmt;
> > unsigned int align;
> > @@ -1120,7 +1312,8 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> > &sdformat->format.height, 1,
> > CSIS_MAX_PIX_HEIGHT, 0, 0);
> >
> > - fmt = v4l2_subdev_state_get_format(state, sdformat->pad);
> > + fmt = v4l2_subdev_state_get_format(state, sdformat->pad,
> > + sdformat->stream);
> >
> > fmt->code = csis_fmt->code;
> > fmt->width = sdformat->format.width;
> > @@ -1133,12 +1326,8 @@ static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> >
> > sdformat->format = *fmt;
> >
> > - /* Propagate the format from sink to source. */
> > - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> > - *fmt = sdformat->format;
> > -
> > - /* The format on the source pad might change due to unpacking. */
> > - fmt->code = csis_fmt->output;
> > + /* Propagate the format. */
> > + mipi_csis_propagate_formats(csis, state);
> >
> > return 0;
> > }
> > @@ -1155,7 +1344,7 @@ static int mipi_csis_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> > return -EINVAL;
> >
> > state = v4l2_subdev_lock_and_get_active_state(sd);
> > - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> > + fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE, 0);
> > csis_fmt = find_csis_format(fmt->code);
> > v4l2_subdev_unlock_state(state);
> >
> > @@ -1187,6 +1376,8 @@ static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> > .quantization = V4L2_QUANTIZATION_LIM_RANGE,
> > .xfer_func = V4L2_XFER_FUNC_SRGB,
> > };
> > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > + struct v4l2_subdev_route *route;
> > int ret;
> >
> > ret = v4l2_subdev_routing_validate(sd, routing,
> > @@ -1194,15 +1385,27 @@ static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> > if (ret)
> > return ret;
> >
> > - /* Only a single route is supported for now. */
> > - if (routing->num_routes != 1 ||
> > - !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > - return -EINVAL;
> > + /*
> > + * The source stream identifies the output channel. Validate that it
> > + * does not exceed the number of channels available in the device. The
> > + * other routing constraints can't be validated now as they require
> > + * querying the frame descriptor on the sink side, which can only be
> > + * done when enabling streaming.
> > + */
> > + for_each_active_route(routing, route) {
> > + if (route->source_stream >= csis->num_channels) {
> > + dev_dbg(csis->dev, "Invalid source stream %u",
> > + route->source_stream);
> > + return -EINVAL;
> > + }
> > + }
> >
> > ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> > if (ret)
> > return ret;
> >
> > + mipi_csis_propagate_formats(csis, state);
> > +
> > return 0;
> > }
> >
> > @@ -1554,7 +1757,8 @@ static int mipi_csis_subdev_init(struct mipi_csis_device *csis)
> > snprintf(sd->name, sizeof(sd->name), "csis-%s",
> > dev_name(csis->dev));
> >
> > - sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> > + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS
> > + | V4L2_SUBDEV_FL_STREAMS;
> > sd->ctrl_handler = NULL;
> >
> > sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 26+ messages in thread* RE: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
2025-11-07 16:48 ` Frank Li
2025-11-07 18:43 ` Laurent Pinchart
@ 2025-11-20 3:12 ` G.N. Zhou
2025-11-20 15:23 ` Frank Li
2025-11-20 16:22 ` Laurent Pinchart
1 sibling, 2 replies; 26+ messages in thread
From: G.N. Zhou @ 2025-11-20 3:12 UTC (permalink / raw)
To: Frank Li, Laurent Pinchart
Cc: linux-media@vger.kernel.org, Rui Miguel Silva, Martin Kepplinger,
Purism Kernel Team, Pengutronix Kernel Team, imx@lists.linux.dev,
Stefan Klug, Sakari Ailus
Hi Frank
> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: Saturday, November 8, 2025 12:48 AM
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; G.N. Zhou
> <guoniu.zhou@nxp.com>
> Cc: linux-media@vger.kernel.org; Rui Miguel Silva <rmfrfs@gmail.com>; Martin
> Kepplinger <martink@posteo.de>; Purism Kernel Team <kernel@puri.sm>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; imx@lists.linux.dev;
> Stefan Klug <stefan.klug@ideasonboard.com>; Sakari Ailus <sakari.ailus@iki.fi>
> Subject: Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
>
> On Fri, Nov 07, 2025 at 03:58:13AM +0200, Laurent Pinchart wrote:
> > CSIS output channels can be used to demultiplex CSI-2 virtual
> > channels, and likely data types. While this feature is not clearly
> > documented, tests seem to indicate that each output channel includes
> > filters to select which VC and DT to output, with the filtering mode
> > being configured globally. Four modes are supported:
> >
> > - No filtering: all VCs and DTs are output on channel 0
> > - VC filtering: each channel filters out all but the configured VC, the
> > DT is ignored
> > - DT filtering: each channel filters out all but the configured DT, the
> > VC is ignored
> > - DT and VC filtering: each channel filters out all but the configured
> > VC and DT
> >
> > Expose this feature to userspace through the streams API. The routing
> > table is expanded to support multiple routes, with the source stream
> > ID mapping to the output channel number. As the VC and DT values
> > corresponding to a stream are not known until they get queried from
> > the source, validation of the routes is postponed to stream enable
> > time in the mipi_csis_calculate_params() function that extract the
> > configuration of each output channel from the routing table. The
> > validation ensures that, when filtering is enabled, each output
> > channel is configured to output exactly one VC and one DT.
> >
> > When multiple streams are routed to the same output channel, the
> > output heights is the sum of the heights of all the streams. This is
> > implemented when propagating formats frim sink to source pads.
> >
> > Due to how the SoC glues together IP cores, multi-output operation in
> > the i.MX8MP is used only for the purpose of capturing multi-exposure
> > or multi-gain HDR streams from a camera sensor over different CSI-2
> > VCs and transmitting them to the ISP. The streams are stitched
> > together by the ISP and can't be captured individually. This has
> > allowed testing VC filtering but not DT filtering. For that reason,
> > multi-channel support is currently limited to VC filtering only.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Add guoniu.zhou@nxp.com, patch itself look good, but I am not famillar with
> internal logic, Guoniu, can you help check it?
I searched the "Samsung MIPI_CSI-2_V3.6.3.1_User_Guide" and checked all
descriptions about its interleave_mode. There is no details about the internal
logic but according to the info in the User_Guide, I think Laurent's understanding
is correct.
>
> Frank
>
> > ---
> > drivers/media/platform/nxp/imx-mipi-csis.c | 264
> > ++++++++++++++++++---
> > 1 file changed, 234 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c
> > b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index d8c11223ed0a..b5c7ab7c541c 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -12,6 +12,7 @@
> > *
> > */
> >
> > +#include <linux/bitops.h>
> > #include <linux/clk.h>
> > #include <linux/debugfs.h>
> > #include <linux/delay.h>
> > @@ -334,7 +335,8 @@ struct mipi_csis_info { };
> >
> > struct mipi_csis_channel_params {
> > - unsigned int data_type;
> > + u16 vc_mask;
> > + u16 data_type;
> > unsigned int width;
> > unsigned int height;
> > };
> > @@ -343,6 +345,7 @@ struct mipi_csis_params {
> > u32 hs_settle;
> > u32 clk_settle;
> >
> > + bool interleave_vc;
> > struct mipi_csis_channel_params
> channels[MIPI_CSIS_MAX_CHANNELS];
> > };
> >
> > @@ -626,7 +629,7 @@ static void mipi_csis_set_channel_params(struct
> mipi_csis_device *csis,
> > val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
> >
> > val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
> > - val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
> > + val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(ffs(params->vc_mask) -
> 1);
> >
> > mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(channel), val);
> >
> > @@ -645,20 +648,148 @@ static int mipi_csis_calculate_params(struct
> mipi_csis_device *csis,
> > const struct v4l2_subdev_state *state,
> > struct mipi_csis_params *params) {
> > - const struct v4l2_mbus_framefmt *format;
> > - const struct csis_pix_format *csis_fmt;
> > + const struct csis_pix_format *csis_fmt = NULL;
> > + struct v4l2_subdev_route *route;
> > + struct v4l2_mbus_frame_desc fd;
> > s64 link_freq;
> > u32 lane_rate;
> > + int ret;
> >
> > - format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> > - csis_fmt = find_csis_format(format->code);
> > + memset(params, 0, sizeof(*params));
> >
> > - params->channels[0].data_type = csis_fmt->data_type;
> > - params->channels[0].width = format->width;
> > - params->channels[0].height = format->height;
> > + /*
> > + * Translate routing configuration to output channels parameters.
> > + *
> > + * The CSIS VC and DT handling is poorly documented. The device
> supports
> > + * a global "interleave mode" parameter in the CMN_CTRL register that
> > + * can be set to "VC and DT", "VC only", "DT only" or "CH0 only, no data
> > + * interleave". The ISP_CONFIG registers specify DT and VC values per
> > + * output channel.
> > + *
> > + * This can be interpreted as per-channel VC and DT filters, with the
> > + * filter type being configured globally and the VC and DT configured
> > + * per-channel. VC tests seem to corroborate this interpretation, but DT
> > + * tests are yet to be performed.
> > + */
> > + ret = v4l2_subdev_call(csis->source.sd, pad, get_frame_desc,
> > + csis->source.pad->index, &fd);
> > + if (ret && ret != -ENOIOCTLCMD) {
> > + dev_err(csis->dev, "Failed to get source frame
> descriptors: %d\n", ret);
> > + return ret;
> > + }
> >
> > - /* Calculate the line rate from the pixel rate. */
> > - link_freq = v4l2_get_link_freq(csis->source.pad, csis_fmt->width,
> > + if (ret == -ENOIOCTLCMD) {
> > + const struct v4l2_mbus_framefmt *format;
> > +
> > + /*
> > + * The source doesn't report frame descriptors. Assume a single
> > + * stream on VC0.
> > + */
> > + format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK,
> 0);
> > + if (!format)
> > + return -EPIPE;
> > +
> > + csis_fmt = find_csis_format(format->code);
> > +
> > + fd.type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> > + fd.num_entries = 1;
> > + fd.entry[0].pixelcode = format->code;
> > + fd.entry[0].bus.csi2.dt = csis_fmt->data_type;
> > + }
> > +
> > + /*
> > + * Translate sink streams to source streams and fill the channel
> > + * configuration vc_mask and data_type fields.
> > + */
> > + for_each_active_route(&state->routing, route) {
> > + struct mipi_csis_channel_params *channel =
> > + ¶ms->channels[route->source_stream];
> > + const struct v4l2_mbus_frame_desc_entry *entry = NULL;
> > +
> > + /*
> > + * Find the corresponding frame descriptor entry, to get the VC
> > + * and DT for the stream. Multiple entries may match the
> stream,
> > + * but they have to all report the same VC and DT, so we can
> > + * just use the first matching entry.
> > + */
> > + for (unsigned int i = 0; i < fd.num_entries; ++i) {
> > + if (fd.entry[i].stream == route->sink_stream) {
> > + entry = &fd.entry[i];
> > + break;
> > + }
> > + }
> > +
> > + if (!entry) {
> > + dev_dbg(csis->dev, "No frame descriptor for
> stream %u\n",
> > + route->sink_stream);
> > + return -EPIPE;
> > + }
> > +
> > + /*
> > + * Routing constraint: all streams routed to the same output
> > + * channel need to have the same DT.
> > + */
> > + if (channel->data_type &&
> > + channel->data_type != entry->bus.csi2.dt) {
> > + dev_dbg(csis->dev,
> > + "DT mismatch on channel %u: stream %u DT
> 0x%02x != 0x%02x\n",
> > + route->source_stream, route->sink_stream,
> > + entry->bus.csi2.dt, channel->data_type);
> > + return -EPIPE;
> > + }
> > +
> > + /* Record the VC and DT for the output channel. */
> > + channel->vc_mask |= BIT(entry->bus.csi2.vc);
> > + channel->data_type = entry->bus.csi2.dt;
> > +
> > + /*
> > + * If any output channel beside channel 0 is enabled, enable VC
> > + * interleave mode.
> > + */
> > + if (route->source_stream > 0)
> > + params->interleave_vc = true;
> > + }
> > +
> > + /*
> > + * Iterate over the enabled output channels to record the width and
> > + * height. Verify that the VC filtering matches the hardware
> > + * constraints.
> > + */
> > + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> > + struct mipi_csis_channel_params *channel = ¶ms-
> >channels[i];
> > + const struct v4l2_mbus_framefmt *format;
> > +
> > + if (!channel->vc_mask)
> > + continue;
> > +
> > + /*
> > + * In VC interleave mode, each output channel is limited to a
> > + * single VC.
> > + */
> > + if (params->interleave_vc && hweight8(channel->vc_mask) !=
> 1) {
> > + dev_dbg(csis->dev,
> > + "Channel %u must output a single VCs\n", i);
> > + return -EPIPE;
> > + }
> > +
> > + format = v4l2_subdev_state_get_format(state,
> CSIS_PAD_SOURCE, i);
> > + if (!format) {
> > + dev_dbg(csis->dev, "No format for source
> stream %u\n", i);
> > + return -EPIPE;
> > + }
> > +
> > + channel->width = format->width;
> > + channel->height = format->height;
> > + }
> > +
> > + /*
> > + * Calculate the line rate from the pixel rate. If the source supports
> > + * the .get_frame_desc() operation it has to implement the LINK_FREQ
> > + * control, as the link frequency can't be calculated from the pixel
> > + * rate with multiple streams having possibly different data types.
> > + */
> > + link_freq = v4l2_get_link_freq(csis->source.pad,
> > + csis_fmt ? csis_fmt->width : 0,
> > csis->bus.num_data_lanes * 2);
> > if (link_freq < 0) {
> > dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
> @@
> > -704,6 +835,8 @@ static void mipi_csis_set_params(struct mipi_csis_device
> *csis,
> > const struct mipi_csis_params *params) {
> > int lanes = csis->bus.num_data_lanes;
> > + u32 cmn_ctrl = 0;
> > + u32 clk_ctrl = 0;
> > u32 val;
> >
> > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL); @@ -714,19 +847,32
> > @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >
> > if (csis->info->version == MIPI_CSIS_V3_3)
> > val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> > + if (params->interleave_vc)
> > + val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_VC;
> >
> > mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> >
> > - mipi_csis_set_channel_params(csis, 0, ¶ms->channels[0]);
> > + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> > + const struct mipi_csis_channel_params *channel =
> > + ¶ms->channels[i];
> > +
> > + if (!channel->vc_mask)
> > + continue;
> > +
> > + mipi_csis_set_channel_params(csis, i, channel);
> > +
> > + cmn_ctrl |= MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(i);
> > + clk_ctrl |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(i, 15)
> > + | MIPI_CSIS_CLK_CTRL_WCLK_SRC(i);
> > + }
> >
> > mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
> > MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params-
> >hs_settle) |
> > MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params-
> >clk_settle));
> >
> > val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> > - val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
> > - val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
> > val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
> > + val |= clk_ctrl;
> > mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
> >
> > mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L, @@ -741,8 +887,7
> @@
> > static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >
> > /* Update the shadow register. */
> > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > - mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> > - val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
> > + mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val | cmn_ctrl |
> > MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
> > }
> >
> > @@ -1053,7 +1198,7 @@ static int mipi_csis_enum_mbus_code(struct
> v4l2_subdev *sd,
> > if (code->index > 0)
> > return -EINVAL;
> >
> > - fmt = v4l2_subdev_state_get_format(state, code->pad);
> > + fmt = v4l2_subdev_state_get_format(state, code->pad, code-
> >stream);
> > code->code = fmt->code;
> > return 0;
> > }
> > @@ -1069,10 +1214,57 @@ static int mipi_csis_enum_mbus_code(struct
> v4l2_subdev *sd,
> > return 0;
> > }
> >
> > +static void mipi_csis_propagate_formats(struct mipi_csis_device *csis,
> > + struct v4l2_subdev_state *state) {
> > + const struct v4l2_mbus_framefmt
> *channels[MIPI_CSIS_MAX_CHANNELS] = { };
> > + struct v4l2_subdev_route *route;
> > +
> > + for_each_active_route(&state->routing, route) {
> > + const struct csis_pix_format *csis_fmt;
> > + struct v4l2_mbus_framefmt *sink_fmt;
> > + struct v4l2_mbus_framefmt *src_fmt;
> > +
> > + sink_fmt = v4l2_subdev_state_get_format(state,
> CSIS_PAD_SINK,
> > + route->sink_stream);
> > + src_fmt = v4l2_subdev_state_get_format(state,
> CSIS_PAD_SOURCE,
> > + route->source_stream);
> > +
> > + csis_fmt = find_csis_format(sink_fmt->code);
> > +
> > + /*
> > + * If the output channel corresponding to this source stream
> > + * isn't associated with a sink stream yet, simply propagate the
> > + * format from sink stream to source stream and associate the
> > + * sink stream with the channel.
> > + *
> > + * Otherwise, the sink stream format must match the primary
> sink
> > + * stream associated with the channel except for the height
> that
> > + * can be different. We propagate the format from the primary
> to
> > + * secondary sink stream, and accumulate the height in the
> > + * source stream format.
> > + */
> > + if (!channels[route->source_stream]) {
> > + *src_fmt = *sink_fmt;
> > + src_fmt->code = csis_fmt->output;
> > +
> > + channels[route->source_stream] = sink_fmt;
> > + } else {
> > + unsigned int height = sink_fmt->height;
> > +
> > + *sink_fmt = *channels[route->source_stream];
> > + sink_fmt->height = height;
> > +
> > + src_fmt->height += sink_fmt->height;
> > + }
> > + }
> > +}
> > +
> > static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> > struct v4l2_subdev_state *state,
> > struct v4l2_subdev_format *sdformat) {
> > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > const struct csis_pix_format *csis_fmt;
> > struct v4l2_mbus_framefmt *fmt;
> > unsigned int align;
> > @@ -1120,7 +1312,8 @@ static int mipi_csis_set_fmt(struct v4l2_subdev
> *sd,
> > &sdformat->format.height, 1,
> > CSIS_MAX_PIX_HEIGHT, 0, 0);
> >
> > - fmt = v4l2_subdev_state_get_format(state, sdformat->pad);
> > + fmt = v4l2_subdev_state_get_format(state, sdformat->pad,
> > + sdformat->stream);
> >
> > fmt->code = csis_fmt->code;
> > fmt->width = sdformat->format.width; @@ -1133,12 +1326,8 @@
> static
> > int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> >
> > sdformat->format = *fmt;
> >
> > - /* Propagate the format from sink to source. */
> > - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> > - *fmt = sdformat->format;
> > -
> > - /* The format on the source pad might change due to unpacking. */
> > - fmt->code = csis_fmt->output;
> > + /* Propagate the format. */
> > + mipi_csis_propagate_formats(csis, state);
> >
> > return 0;
> > }
> > @@ -1155,7 +1344,7 @@ static int mipi_csis_get_frame_desc(struct
> v4l2_subdev *sd, unsigned int pad,
> > return -EINVAL;
> >
> > state = v4l2_subdev_lock_and_get_active_state(sd);
> > - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> > + fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE, 0);
> > csis_fmt = find_csis_format(fmt->code);
> > v4l2_subdev_unlock_state(state);
> >
> > @@ -1187,6 +1376,8 @@ static int __mipi_csis_set_routing(struct
> v4l2_subdev *sd,
> > .quantization = V4L2_QUANTIZATION_LIM_RANGE,
> > .xfer_func = V4L2_XFER_FUNC_SRGB,
> > };
> > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > + struct v4l2_subdev_route *route;
> > int ret;
> >
> > ret = v4l2_subdev_routing_validate(sd, routing, @@ -1194,15
> +1385,27
> > @@ static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> > if (ret)
> > return ret;
> >
> > - /* Only a single route is supported for now. */
> > - if (routing->num_routes != 1 ||
> > - !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > - return -EINVAL;
> > + /*
> > + * The source stream identifies the output channel. Validate that it
> > + * does not exceed the number of channels available in the device. The
> > + * other routing constraints can't be validated now as they require
> > + * querying the frame descriptor on the sink side, which can only be
> > + * done when enabling streaming.
> > + */
> > + for_each_active_route(routing, route) {
> > + if (route->source_stream >= csis->num_channels) {
> > + dev_dbg(csis->dev, "Invalid source stream %u",
> > + route->source_stream);
> > + return -EINVAL;
> > + }
> > + }
> >
> > ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> > if (ret)
> > return ret;
> >
> > + mipi_csis_propagate_formats(csis, state);
> > +
> > return 0;
> > }
> >
> > @@ -1554,7 +1757,8 @@ static int mipi_csis_subdev_init(struct
> mipi_csis_device *csis)
> > snprintf(sd->name, sizeof(sd->name), "csis-%s",
> > dev_name(csis->dev));
> >
> > - sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> V4L2_SUBDEV_FL_HAS_EVENTS;
> > + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> V4L2_SUBDEV_FL_HAS_EVENTS
> > + | V4L2_SUBDEV_FL_STREAMS;
> > sd->ctrl_handler = NULL;
> >
> > sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
2025-11-20 3:12 ` G.N. Zhou
@ 2025-11-20 15:23 ` Frank Li
2025-11-20 16:22 ` Laurent Pinchart
1 sibling, 0 replies; 26+ messages in thread
From: Frank Li @ 2025-11-20 15:23 UTC (permalink / raw)
To: G.N. Zhou
Cc: Laurent Pinchart, linux-media@vger.kernel.org, Rui Miguel Silva,
Martin Kepplinger, Purism Kernel Team, Pengutronix Kernel Team,
imx@lists.linux.dev, Stefan Klug, Sakari Ailus
On Thu, Nov 20, 2025 at 03:12:56AM +0000, G.N. Zhou wrote:
> Hi Frank
>
> > -----Original Message-----
> > From: Frank Li <frank.li@nxp.com>
> > Sent: Saturday, November 8, 2025 12:48 AM
> > To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; G.N. Zhou
> > <guoniu.zhou@nxp.com>
> > Cc: linux-media@vger.kernel.org; Rui Miguel Silva <rmfrfs@gmail.com>; Martin
> > Kepplinger <martink@posteo.de>; Purism Kernel Team <kernel@puri.sm>;
> > Pengutronix Kernel Team <kernel@pengutronix.de>; imx@lists.linux.dev;
> > Stefan Klug <stefan.klug@ideasonboard.com>; Sakari Ailus <sakari.ailus@iki.fi>
> > Subject: Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
> >
> > On Fri, Nov 07, 2025 at 03:58:13AM +0200, Laurent Pinchart wrote:
...
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Add guoniu.zhou@nxp.com, patch itself look good, but I am not famillar with
> > internal logic, Guoniu, can you help check it?
>
> I searched the "Samsung MIPI_CSI-2_V3.6.3.1_User_Guide" and checked all
> descriptions about its interleave_mode. There is no details about the internal
> logic but according to the info in the User_Guide, I think Laurent's understanding
> is correct.
Thanks G.N. You can provide reviewed-by tag if you want.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Frank
>
> >
> > Frank
> >
> > > ---
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
2025-11-20 3:12 ` G.N. Zhou
2025-11-20 15:23 ` Frank Li
@ 2025-11-20 16:22 ` Laurent Pinchart
2025-12-02 0:59 ` [EXT] " G.N. Zhou
1 sibling, 1 reply; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-20 16:22 UTC (permalink / raw)
To: G.N. Zhou
Cc: Frank Li, linux-media@vger.kernel.org, Rui Miguel Silva,
Martin Kepplinger, Purism Kernel Team, Pengutronix Kernel Team,
imx@lists.linux.dev, Stefan Klug, Sakari Ailus
On Thu, Nov 20, 2025 at 03:12:56AM +0000, G.N. Zhou wrote:
> Hi Frank
>
> > -----Original Message-----
> > From: Frank Li <frank.li@nxp.com>
> > Sent: Saturday, November 8, 2025 12:48 AM
> > To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; G.N. Zhou
> > <guoniu.zhou@nxp.com>
> > Cc: linux-media@vger.kernel.org; Rui Miguel Silva <rmfrfs@gmail.com>; Martin
> > Kepplinger <martink@posteo.de>; Purism Kernel Team <kernel@puri.sm>;
> > Pengutronix Kernel Team <kernel@pengutronix.de>; imx@lists.linux.dev;
> > Stefan Klug <stefan.klug@ideasonboard.com>; Sakari Ailus <sakari.ailus@iki.fi>
> > Subject: Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
> >
> > On Fri, Nov 07, 2025 at 03:58:13AM +0200, Laurent Pinchart wrote:
> > > CSIS output channels can be used to demultiplex CSI-2 virtual
> > > channels, and likely data types. While this feature is not clearly
> > > documented, tests seem to indicate that each output channel includes
> > > filters to select which VC and DT to output, with the filtering mode
> > > being configured globally. Four modes are supported:
> > >
> > > - No filtering: all VCs and DTs are output on channel 0
> > > - VC filtering: each channel filters out all but the configured VC, the
> > > DT is ignored
> > > - DT filtering: each channel filters out all but the configured DT, the
> > > VC is ignored
> > > - DT and VC filtering: each channel filters out all but the configured
> > > VC and DT
> > >
> > > Expose this feature to userspace through the streams API. The routing
> > > table is expanded to support multiple routes, with the source stream
> > > ID mapping to the output channel number. As the VC and DT values
> > > corresponding to a stream are not known until they get queried from
> > > the source, validation of the routes is postponed to stream enable
> > > time in the mipi_csis_calculate_params() function that extract the
> > > configuration of each output channel from the routing table. The
> > > validation ensures that, when filtering is enabled, each output
> > > channel is configured to output exactly one VC and one DT.
> > >
> > > When multiple streams are routed to the same output channel, the
> > > output heights is the sum of the heights of all the streams. This is
> > > implemented when propagating formats frim sink to source pads.
> > >
> > > Due to how the SoC glues together IP cores, multi-output operation in
> > > the i.MX8MP is used only for the purpose of capturing multi-exposure
> > > or multi-gain HDR streams from a camera sensor over different CSI-2
> > > VCs and transmitting them to the ISP. The streams are stitched
> > > together by the ISP and can't be captured individually. This has
> > > allowed testing VC filtering but not DT filtering. For that reason,
> > > multi-channel support is currently limited to VC filtering only.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Add guoniu.zhou@nxp.com, patch itself look good, but I am not famillar with
> > internal logic, Guoniu, can you help check it?
>
> I searched the "Samsung MIPI_CSI-2_V3.6.3.1_User_Guide"
I wish I could access that document :-)
> and checked all
> descriptions about its interleave_mode. There is no details about the internal
> logic but according to the info in the User_Guide, I think Laurent's understanding
> is correct.
Thank you for the confirmation. I plan to send a new version of this
patch.
> > > ---
> > > drivers/media/platform/nxp/imx-mipi-csis.c | 264 ++++++++++++++++++---
> > > 1 file changed, 234 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > index d8c11223ed0a..b5c7ab7c541c 100644
> > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > @@ -12,6 +12,7 @@
> > > *
> > > */
> > >
> > > +#include <linux/bitops.h>
> > > #include <linux/clk.h>
> > > #include <linux/debugfs.h>
> > > #include <linux/delay.h>
> > > @@ -334,7 +335,8 @@ struct mipi_csis_info { };
> > >
> > > struct mipi_csis_channel_params {
> > > - unsigned int data_type;
> > > + u16 vc_mask;
> > > + u16 data_type;
> > > unsigned int width;
> > > unsigned int height;
> > > };
> > > @@ -343,6 +345,7 @@ struct mipi_csis_params {
> > > u32 hs_settle;
> > > u32 clk_settle;
> > >
> > > + bool interleave_vc;
> > > struct mipi_csis_channel_params
> > channels[MIPI_CSIS_MAX_CHANNELS];
> > > };
> > >
> > > @@ -626,7 +629,7 @@ static void mipi_csis_set_channel_params(struct
> > mipi_csis_device *csis,
> > > val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
> > >
> > > val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
> > > - val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
> > > + val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(ffs(params->vc_mask) -
> > 1);
> > >
> > > mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(channel), val);
> > >
> > > @@ -645,20 +648,148 @@ static int mipi_csis_calculate_params(struct
> > mipi_csis_device *csis,
> > > const struct v4l2_subdev_state *state,
> > > struct mipi_csis_params *params) {
> > > - const struct v4l2_mbus_framefmt *format;
> > > - const struct csis_pix_format *csis_fmt;
> > > + const struct csis_pix_format *csis_fmt = NULL;
> > > + struct v4l2_subdev_route *route;
> > > + struct v4l2_mbus_frame_desc fd;
> > > s64 link_freq;
> > > u32 lane_rate;
> > > + int ret;
> > >
> > > - format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> > > - csis_fmt = find_csis_format(format->code);
> > > + memset(params, 0, sizeof(*params));
> > >
> > > - params->channels[0].data_type = csis_fmt->data_type;
> > > - params->channels[0].width = format->width;
> > > - params->channels[0].height = format->height;
> > > + /*
> > > + * Translate routing configuration to output channels parameters.
> > > + *
> > > + * The CSIS VC and DT handling is poorly documented. The device
> > supports
> > > + * a global "interleave mode" parameter in the CMN_CTRL register that
> > > + * can be set to "VC and DT", "VC only", "DT only" or "CH0 only, no data
> > > + * interleave". The ISP_CONFIG registers specify DT and VC values per
> > > + * output channel.
> > > + *
> > > + * This can be interpreted as per-channel VC and DT filters, with the
> > > + * filter type being configured globally and the VC and DT configured
> > > + * per-channel. VC tests seem to corroborate this interpretation, but DT
> > > + * tests are yet to be performed.
> > > + */
> > > + ret = v4l2_subdev_call(csis->source.sd, pad, get_frame_desc,
> > > + csis->source.pad->index, &fd);
> > > + if (ret && ret != -ENOIOCTLCMD) {
> > > + dev_err(csis->dev, "Failed to get source frame
> > descriptors: %d\n", ret);
> > > + return ret;
> > > + }
> > >
> > > - /* Calculate the line rate from the pixel rate. */
> > > - link_freq = v4l2_get_link_freq(csis->source.pad, csis_fmt->width,
> > > + if (ret == -ENOIOCTLCMD) {
> > > + const struct v4l2_mbus_framefmt *format;
> > > +
> > > + /*
> > > + * The source doesn't report frame descriptors. Assume a single
> > > + * stream on VC0.
> > > + */
> > > + format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK,
> > 0);
> > > + if (!format)
> > > + return -EPIPE;
> > > +
> > > + csis_fmt = find_csis_format(format->code);
> > > +
> > > + fd.type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> > > + fd.num_entries = 1;
> > > + fd.entry[0].pixelcode = format->code;
> > > + fd.entry[0].bus.csi2.dt = csis_fmt->data_type;
> > > + }
> > > +
> > > + /*
> > > + * Translate sink streams to source streams and fill the channel
> > > + * configuration vc_mask and data_type fields.
> > > + */
> > > + for_each_active_route(&state->routing, route) {
> > > + struct mipi_csis_channel_params *channel =
> > > + ¶ms->channels[route->source_stream];
> > > + const struct v4l2_mbus_frame_desc_entry *entry = NULL;
> > > +
> > > + /*
> > > + * Find the corresponding frame descriptor entry, to get the VC
> > > + * and DT for the stream. Multiple entries may match the
> > stream,
> > > + * but they have to all report the same VC and DT, so we can
> > > + * just use the first matching entry.
> > > + */
> > > + for (unsigned int i = 0; i < fd.num_entries; ++i) {
> > > + if (fd.entry[i].stream == route->sink_stream) {
> > > + entry = &fd.entry[i];
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (!entry) {
> > > + dev_dbg(csis->dev, "No frame descriptor for
> > stream %u\n",
> > > + route->sink_stream);
> > > + return -EPIPE;
> > > + }
> > > +
> > > + /*
> > > + * Routing constraint: all streams routed to the same output
> > > + * channel need to have the same DT.
> > > + */
> > > + if (channel->data_type &&
> > > + channel->data_type != entry->bus.csi2.dt) {
> > > + dev_dbg(csis->dev,
> > > + "DT mismatch on channel %u: stream %u DT
> > 0x%02x != 0x%02x\n",
> > > + route->source_stream, route->sink_stream,
> > > + entry->bus.csi2.dt, channel->data_type);
> > > + return -EPIPE;
> > > + }
> > > +
> > > + /* Record the VC and DT for the output channel. */
> > > + channel->vc_mask |= BIT(entry->bus.csi2.vc);
> > > + channel->data_type = entry->bus.csi2.dt;
> > > +
> > > + /*
> > > + * If any output channel beside channel 0 is enabled, enable VC
> > > + * interleave mode.
> > > + */
> > > + if (route->source_stream > 0)
> > > + params->interleave_vc = true;
> > > + }
> > > +
> > > + /*
> > > + * Iterate over the enabled output channels to record the width and
> > > + * height. Verify that the VC filtering matches the hardware
> > > + * constraints.
> > > + */
> > > + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> > > + struct mipi_csis_channel_params *channel = ¶ms-
> > >channels[i];
> > > + const struct v4l2_mbus_framefmt *format;
> > > +
> > > + if (!channel->vc_mask)
> > > + continue;
> > > +
> > > + /*
> > > + * In VC interleave mode, each output channel is limited to a
> > > + * single VC.
> > > + */
> > > + if (params->interleave_vc && hweight8(channel->vc_mask) !=
> > 1) {
> > > + dev_dbg(csis->dev,
> > > + "Channel %u must output a single VCs\n", i);
> > > + return -EPIPE;
> > > + }
> > > +
> > > + format = v4l2_subdev_state_get_format(state,
> > CSIS_PAD_SOURCE, i);
> > > + if (!format) {
> > > + dev_dbg(csis->dev, "No format for source
> > stream %u\n", i);
> > > + return -EPIPE;
> > > + }
> > > +
> > > + channel->width = format->width;
> > > + channel->height = format->height;
> > > + }
> > > +
> > > + /*
> > > + * Calculate the line rate from the pixel rate. If the source supports
> > > + * the .get_frame_desc() operation it has to implement the LINK_FREQ
> > > + * control, as the link frequency can't be calculated from the pixel
> > > + * rate with multiple streams having possibly different data types.
> > > + */
> > > + link_freq = v4l2_get_link_freq(csis->source.pad,
> > > + csis_fmt ? csis_fmt->width : 0,
> > > csis->bus.num_data_lanes * 2);
> > > if (link_freq < 0) {
> > > dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
> > @@
> > > -704,6 +835,8 @@ static void mipi_csis_set_params(struct mipi_csis_device
> > *csis,
> > > const struct mipi_csis_params *params) {
> > > int lanes = csis->bus.num_data_lanes;
> > > + u32 cmn_ctrl = 0;
> > > + u32 clk_ctrl = 0;
> > > u32 val;
> > >
> > > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL); @@ -714,19 +847,32
> > > @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> > >
> > > if (csis->info->version == MIPI_CSIS_V3_3)
> > > val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> > > + if (params->interleave_vc)
> > > + val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_VC;
> > >
> > > mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> > >
> > > - mipi_csis_set_channel_params(csis, 0, ¶ms->channels[0]);
> > > + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> > > + const struct mipi_csis_channel_params *channel =
> > > + ¶ms->channels[i];
> > > +
> > > + if (!channel->vc_mask)
> > > + continue;
> > > +
> > > + mipi_csis_set_channel_params(csis, i, channel);
> > > +
> > > + cmn_ctrl |= MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(i);
> > > + clk_ctrl |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(i, 15)
> > > + | MIPI_CSIS_CLK_CTRL_WCLK_SRC(i);
> > > + }
> > >
> > > mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
> > > MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params-
> > >hs_settle) |
> > > MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params-
> > >clk_settle));
> > >
> > > val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> > > - val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
> > > - val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
> > > val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
> > > + val |= clk_ctrl;
> > > mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
> > >
> > > mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L, @@ -741,8 +887,7
> > @@
> > > static void mipi_csis_set_params(struct mipi_csis_device *csis,
> > >
> > > /* Update the shadow register. */
> > > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > > - mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> > > - val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
> > > + mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val | cmn_ctrl |
> > > MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
> > > }
> > >
> > > @@ -1053,7 +1198,7 @@ static int mipi_csis_enum_mbus_code(struct
> > v4l2_subdev *sd,
> > > if (code->index > 0)
> > > return -EINVAL;
> > >
> > > - fmt = v4l2_subdev_state_get_format(state, code->pad);
> > > + fmt = v4l2_subdev_state_get_format(state, code->pad, code-
> > >stream);
> > > code->code = fmt->code;
> > > return 0;
> > > }
> > > @@ -1069,10 +1214,57 @@ static int mipi_csis_enum_mbus_code(struct
> > v4l2_subdev *sd,
> > > return 0;
> > > }
> > >
> > > +static void mipi_csis_propagate_formats(struct mipi_csis_device *csis,
> > > + struct v4l2_subdev_state *state) {
> > > + const struct v4l2_mbus_framefmt
> > *channels[MIPI_CSIS_MAX_CHANNELS] = { };
> > > + struct v4l2_subdev_route *route;
> > > +
> > > + for_each_active_route(&state->routing, route) {
> > > + const struct csis_pix_format *csis_fmt;
> > > + struct v4l2_mbus_framefmt *sink_fmt;
> > > + struct v4l2_mbus_framefmt *src_fmt;
> > > +
> > > + sink_fmt = v4l2_subdev_state_get_format(state,
> > CSIS_PAD_SINK,
> > > + route->sink_stream);
> > > + src_fmt = v4l2_subdev_state_get_format(state,
> > CSIS_PAD_SOURCE,
> > > + route->source_stream);
> > > +
> > > + csis_fmt = find_csis_format(sink_fmt->code);
> > > +
> > > + /*
> > > + * If the output channel corresponding to this source stream
> > > + * isn't associated with a sink stream yet, simply propagate the
> > > + * format from sink stream to source stream and associate the
> > > + * sink stream with the channel.
> > > + *
> > > + * Otherwise, the sink stream format must match the primary
> > sink
> > > + * stream associated with the channel except for the height
> > that
> > > + * can be different. We propagate the format from the primary
> > to
> > > + * secondary sink stream, and accumulate the height in the
> > > + * source stream format.
> > > + */
> > > + if (!channels[route->source_stream]) {
> > > + *src_fmt = *sink_fmt;
> > > + src_fmt->code = csis_fmt->output;
> > > +
> > > + channels[route->source_stream] = sink_fmt;
> > > + } else {
> > > + unsigned int height = sink_fmt->height;
> > > +
> > > + *sink_fmt = *channels[route->source_stream];
> > > + sink_fmt->height = height;
> > > +
> > > + src_fmt->height += sink_fmt->height;
> > > + }
> > > + }
> > > +}
> > > +
> > > static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> > > struct v4l2_subdev_state *state,
> > > struct v4l2_subdev_format *sdformat) {
> > > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > > const struct csis_pix_format *csis_fmt;
> > > struct v4l2_mbus_framefmt *fmt;
> > > unsigned int align;
> > > @@ -1120,7 +1312,8 @@ static int mipi_csis_set_fmt(struct v4l2_subdev
> > *sd,
> > > &sdformat->format.height, 1,
> > > CSIS_MAX_PIX_HEIGHT, 0, 0);
> > >
> > > - fmt = v4l2_subdev_state_get_format(state, sdformat->pad);
> > > + fmt = v4l2_subdev_state_get_format(state, sdformat->pad,
> > > + sdformat->stream);
> > >
> > > fmt->code = csis_fmt->code;
> > > fmt->width = sdformat->format.width; @@ -1133,12 +1326,8 @@
> > static
> > > int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> > >
> > > sdformat->format = *fmt;
> > >
> > > - /* Propagate the format from sink to source. */
> > > - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> > > - *fmt = sdformat->format;
> > > -
> > > - /* The format on the source pad might change due to unpacking. */
> > > - fmt->code = csis_fmt->output;
> > > + /* Propagate the format. */
> > > + mipi_csis_propagate_formats(csis, state);
> > >
> > > return 0;
> > > }
> > > @@ -1155,7 +1344,7 @@ static int mipi_csis_get_frame_desc(struct
> > v4l2_subdev *sd, unsigned int pad,
> > > return -EINVAL;
> > >
> > > state = v4l2_subdev_lock_and_get_active_state(sd);
> > > - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> > > + fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE, 0);
> > > csis_fmt = find_csis_format(fmt->code);
> > > v4l2_subdev_unlock_state(state);
> > >
> > > @@ -1187,6 +1376,8 @@ static int __mipi_csis_set_routing(struct
> > v4l2_subdev *sd,
> > > .quantization = V4L2_QUANTIZATION_LIM_RANGE,
> > > .xfer_func = V4L2_XFER_FUNC_SRGB,
> > > };
> > > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > > + struct v4l2_subdev_route *route;
> > > int ret;
> > >
> > > ret = v4l2_subdev_routing_validate(sd, routing, @@ -1194,15
> > +1385,27
> > > @@ static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> > > if (ret)
> > > return ret;
> > >
> > > - /* Only a single route is supported for now. */
> > > - if (routing->num_routes != 1 ||
> > > - !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > > - return -EINVAL;
> > > + /*
> > > + * The source stream identifies the output channel. Validate that it
> > > + * does not exceed the number of channels available in the device. The
> > > + * other routing constraints can't be validated now as they require
> > > + * querying the frame descriptor on the sink side, which can only be
> > > + * done when enabling streaming.
> > > + */
> > > + for_each_active_route(routing, route) {
> > > + if (route->source_stream >= csis->num_channels) {
> > > + dev_dbg(csis->dev, "Invalid source stream %u",
> > > + route->source_stream);
> > > + return -EINVAL;
> > > + }
> > > + }
> > >
> > > ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> > > if (ret)
> > > return ret;
> > >
> > > + mipi_csis_propagate_formats(csis, state);
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -1554,7 +1757,8 @@ static int mipi_csis_subdev_init(struct
> > mipi_csis_device *csis)
> > > snprintf(sd->name, sizeof(sd->name), "csis-%s",
> > > dev_name(csis->dev));
> > >
> > > - sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > V4L2_SUBDEV_FL_HAS_EVENTS;
> > > + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > V4L2_SUBDEV_FL_HAS_EVENTS
> > > + | V4L2_SUBDEV_FL_STREAMS;
> > > sd->ctrl_handler = NULL;
> > >
> > > sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 26+ messages in thread* RE: [EXT] Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support
2025-11-20 16:22 ` Laurent Pinchart
@ 2025-12-02 0:59 ` G.N. Zhou
0 siblings, 0 replies; 26+ messages in thread
From: G.N. Zhou @ 2025-12-02 0:59 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Frank Li, linux-media@vger.kernel.org, Rui Miguel Silva,
Martin Kepplinger, Purism Kernel Team, Pengutronix Kernel Team,
imx@lists.linux.dev, Stefan Klug, Sakari Ailus
Hi Laurent,
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Friday, November 21, 2025 12:22 AM
> To: G.N. Zhou <guoniu.zhou@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; linux-media@vger.kernel.org; Rui Miguel Silva
> <rmfrfs@gmail.com>; Martin Kepplinger <martink@posteo.de>; Purism Kernel
> Team <kernel@puri.sm>; Pengutronix Kernel Team <kernel@pengutronix.de>;
> imx@lists.linux.dev; Stefan Klug <stefan.klug@ideasonboard.com>; Sakari Ailus
> <sakari.ailus@iki.fi>
> Subject: [EXT] Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel
> support
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
>
>
> On Thu, Nov 20, 2025 at 03:12:56AM +0000, G.N. Zhou wrote:
> > Hi Frank
> >
> > > -----Original Message-----
> > > From: Frank Li <frank.li@nxp.com>
> > > Sent: Saturday, November 8, 2025 12:48 AM
> > > To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>; G.N. Zhou
> > > <guoniu.zhou@nxp.com>
> > > Cc: linux-media@vger.kernel.org; Rui Miguel Silva
> > > <rmfrfs@gmail.com>; Martin Kepplinger <martink@posteo.de>; Purism
> > > Kernel Team <kernel@puri.sm>; Pengutronix Kernel Team
> > > <kernel@pengutronix.de>; imx@lists.linux.dev; Stefan Klug
> > > <stefan.klug@ideasonboard.com>; Sakari Ailus <sakari.ailus@iki.fi>
> > > Subject: Re: [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel
> > > support
> > >
> > > On Fri, Nov 07, 2025 at 03:58:13AM +0200, Laurent Pinchart wrote:
> > > > CSIS output channels can be used to demultiplex CSI-2 virtual
> > > > channels, and likely data types. While this feature is not clearly
> > > > documented, tests seem to indicate that each output channel
> > > > includes filters to select which VC and DT to output, with the
> > > > filtering mode being configured globally. Four modes are supported:
> > > >
> > > > - No filtering: all VCs and DTs are output on channel 0
> > > > - VC filtering: each channel filters out all but the configured VC, the
> > > > DT is ignored
> > > > - DT filtering: each channel filters out all but the configured DT, the
> > > > VC is ignored
> > > > - DT and VC filtering: each channel filters out all but the configured
> > > > VC and DT
> > > >
> > > > Expose this feature to userspace through the streams API. The
> > > > routing table is expanded to support multiple routes, with the
> > > > source stream ID mapping to the output channel number. As the VC
> > > > and DT values corresponding to a stream are not known until they
> > > > get queried from the source, validation of the routes is postponed
> > > > to stream enable time in the mipi_csis_calculate_params() function
> > > > that extract the configuration of each output channel from the
> > > > routing table. The validation ensures that, when filtering is
> > > > enabled, each output channel is configured to output exactly one VC and
> one DT.
> > > >
> > > > When multiple streams are routed to the same output channel, the
> > > > output heights is the sum of the heights of all the streams. This
> > > > is implemented when propagating formats frim sink to source pads.
> > > >
> > > > Due to how the SoC glues together IP cores, multi-output operation
> > > > in the i.MX8MP is used only for the purpose of capturing
> > > > multi-exposure or multi-gain HDR streams from a camera sensor over
> > > > different CSI-2 VCs and transmitting them to the ISP. The streams
> > > > are stitched together by the ISP and can't be captured
> > > > individually. This has allowed testing VC filtering but not DT
> > > > filtering. For that reason, multi-channel support is currently limited to VC
> filtering only.
> > > >
> > > > Signed-off-by: Laurent Pinchart
> > > > <laurent.pinchart@ideasonboard.com>
> > >
> > > Add guoniu.zhou@nxp.com, patch itself look good, but I am not
> > > famillar with internal logic, Guoniu, can you help check it?
> >
> > I searched the "Samsung MIPI_CSI-2_V3.6.3.1_User_Guide"
>
> I wish I could access that document :-)
Sorry, the document marked as "internal user only". So I'm afraid I can't share it.
>
> > and checked all
> > descriptions about its interleave_mode. There is no details about the
> > internal logic but according to the info in the User_Guide, I think
> > Laurent's understanding is correct.
>
> Thank you for the confirmation. I plan to send a new version of this patch.
>
> > > > ---
> > > > drivers/media/platform/nxp/imx-mipi-csis.c | 264
> > > > ++++++++++++++++++---
> > > > 1 file changed, 234 insertions(+), 30 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > > b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > > index d8c11223ed0a..b5c7ab7c541c 100644
> > > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > > @@ -12,6 +12,7 @@
> > > > *
> > > > */
> > > >
> > > > +#include <linux/bitops.h>
> > > > #include <linux/clk.h>
> > > > #include <linux/debugfs.h>
> > > > #include <linux/delay.h>
> > > > @@ -334,7 +335,8 @@ struct mipi_csis_info { };
> > > >
> > > > struct mipi_csis_channel_params {
> > > > - unsigned int data_type;
> > > > + u16 vc_mask;
> > > > + u16 data_type;
> > > > unsigned int width;
> > > > unsigned int height;
> > > > };
> > > > @@ -343,6 +345,7 @@ struct mipi_csis_params {
> > > > u32 hs_settle;
> > > > u32 clk_settle;
> > > >
> > > > + bool interleave_vc;
> > > > struct mipi_csis_channel_params
> > > channels[MIPI_CSIS_MAX_CHANNELS];
> > > > };
> > > >
> > > > @@ -626,7 +629,7 @@ static void
> > > > mipi_csis_set_channel_params(struct
> > > mipi_csis_device *csis,
> > > > val |= MIPI_CSIS_ISPCFG_PIXEL_MODE_DUAL;
> > > >
> > > > val |= MIPI_CSIS_ISPCFG_DATAFORMAT(params->data_type);
> > > > - val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(0);
> > > > + val |= MIPI_CSIS_ISPCFG_VIRTUAL_CHANNEL(ffs(params->vc_mask) -
> > > 1);
> > > >
> > > > mipi_csis_write(csis, MIPI_CSIS_ISP_CONFIG_CH(channel), val);
> > > >
> > > > @@ -645,20 +648,148 @@ static int
> > > > mipi_csis_calculate_params(struct
> > > mipi_csis_device *csis,
> > > > const struct v4l2_subdev_state *state,
> > > > struct mipi_csis_params *params)
> > > > {
> > > > - const struct v4l2_mbus_framefmt *format;
> > > > - const struct csis_pix_format *csis_fmt;
> > > > + const struct csis_pix_format *csis_fmt = NULL; struct
> > > > + v4l2_subdev_route *route; struct v4l2_mbus_frame_desc fd;
> > > > s64 link_freq;
> > > > u32 lane_rate;
> > > > + int ret;
> > > >
> > > > - format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> > > > - csis_fmt = find_csis_format(format->code);
> > > > + memset(params, 0, sizeof(*params));
> > > >
> > > > - params->channels[0].data_type = csis_fmt->data_type;
> > > > - params->channels[0].width = format->width;
> > > > - params->channels[0].height = format->height;
> > > > + /*
> > > > + * Translate routing configuration to output channels parameters.
> > > > + *
> > > > + * The CSIS VC and DT handling is poorly documented. The device
> > > supports
> > > > + * a global "interleave mode" parameter in the CMN_CTRL register
> > > > + that
> > > > + * can be set to "VC and DT", "VC only", "DT only" or "CH0 only,
> > > > + no data
> > > > + * interleave". The ISP_CONFIG registers specify DT and VC
> > > > + values per
> > > > + * output channel.
> > > > + *
> > > > + * This can be interpreted as per-channel VC and DT filters,
> > > > + with the
> > > > + * filter type being configured globally and the VC and DT
> > > > + configured
> > > > + * per-channel. VC tests seem to corroborate this
> > > > + interpretation, but DT
> > > > + * tests are yet to be performed.
> > > > + */
> > > > + ret = v4l2_subdev_call(csis->source.sd, pad, get_frame_desc,
> > > > + csis->source.pad->index, &fd); if (ret &&
> > > > + ret != -ENOIOCTLCMD) {
> > > > + dev_err(csis->dev, "Failed to get source frame
> > > descriptors: %d\n", ret);
> > > > + return ret;
> > > > + }
> > > >
> > > > - /* Calculate the line rate from the pixel rate. */
> > > > - link_freq = v4l2_get_link_freq(csis->source.pad,
> > > > csis_fmt->width,
> > > > + if (ret == -ENOIOCTLCMD) {
> > > > + const struct v4l2_mbus_framefmt *format;
> > > > +
> > > > + /*
> > > > + * The source doesn't report frame descriptors. Assume a single
> > > > + * stream on VC0.
> > > > + */
> > > > + format = v4l2_subdev_state_get_format(state,
> > > > + CSIS_PAD_SINK,
> > > 0);
> > > > + if (!format)
> > > > + return -EPIPE;
> > > > +
> > > > + csis_fmt = find_csis_format(format->code);
> > > > +
> > > > + fd.type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> > > > + fd.num_entries = 1;
> > > > + fd.entry[0].pixelcode = format->code;
> > > > + fd.entry[0].bus.csi2.dt = csis_fmt->data_type; }
> > > > +
> > > > + /*
> > > > + * Translate sink streams to source streams and fill the channel
> > > > + * configuration vc_mask and data_type fields.
> > > > + */
> > > > + for_each_active_route(&state->routing, route) {
> > > > + struct mipi_csis_channel_params *channel =
> > > > + ¶ms->channels[route->source_stream];
> > > > + const struct v4l2_mbus_frame_desc_entry *entry = NULL;
> > > > +
> > > > + /*
> > > > + * Find the corresponding frame descriptor entry, to get the VC
> > > > + * and DT for the stream. Multiple entries may match the
> > > stream,
> > > > + * but they have to all report the same VC and DT, so we can
> > > > + * just use the first matching entry.
> > > > + */
> > > > + for (unsigned int i = 0; i < fd.num_entries; ++i) {
> > > > + if (fd.entry[i].stream == route->sink_stream) {
> > > > + entry = &fd.entry[i];
> > > > + break;
> > > > + }
> > > > + }
> > > > +
> > > > + if (!entry) {
> > > > + dev_dbg(csis->dev, "No frame descriptor for
> > > stream %u\n",
> > > > + route->sink_stream);
> > > > + return -EPIPE;
> > > > + }
> > > > +
> > > > + /*
> > > > + * Routing constraint: all streams routed to the same output
> > > > + * channel need to have the same DT.
> > > > + */
> > > > + if (channel->data_type &&
> > > > + channel->data_type != entry->bus.csi2.dt) {
> > > > + dev_dbg(csis->dev,
> > > > + "DT mismatch on channel %u: stream %u DT
> > > 0x%02x != 0x%02x\n",
> > > > + route->source_stream, route->sink_stream,
> > > > + entry->bus.csi2.dt, channel->data_type);
> > > > + return -EPIPE;
> > > > + }
> > > > +
> > > > + /* Record the VC and DT for the output channel. */
> > > > + channel->vc_mask |= BIT(entry->bus.csi2.vc);
> > > > + channel->data_type = entry->bus.csi2.dt;
> > > > +
> > > > + /*
> > > > + * If any output channel beside channel 0 is enabled, enable VC
> > > > + * interleave mode.
> > > > + */
> > > > + if (route->source_stream > 0)
> > > > + params->interleave_vc = true; }
> > > > +
> > > > + /*
> > > > + * Iterate over the enabled output channels to record the width
> > > > + and
> > > > + * height. Verify that the VC filtering matches the hardware
> > > > + * constraints.
> > > > + */
> > > > + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> > > > + struct mipi_csis_channel_params *channel = ¶ms-
> > > >channels[i];
> > > > + const struct v4l2_mbus_framefmt *format;
> > > > +
> > > > + if (!channel->vc_mask)
> > > > + continue;
> > > > +
> > > > + /*
> > > > + * In VC interleave mode, each output channel is limited to a
> > > > + * single VC.
> > > > + */
> > > > + if (params->interleave_vc && hweight8(channel->vc_mask)
> > > > + !=
> > > 1) {
> > > > + dev_dbg(csis->dev,
> > > > + "Channel %u must output a single VCs\n", i);
> > > > + return -EPIPE;
> > > > + }
> > > > +
> > > > + format = v4l2_subdev_state_get_format(state,
> > > CSIS_PAD_SOURCE, i);
> > > > + if (!format) {
> > > > + dev_dbg(csis->dev, "No format for source
> > > stream %u\n", i);
> > > > + return -EPIPE;
> > > > + }
> > > > +
> > > > + channel->width = format->width;
> > > > + channel->height = format->height; }
> > > > +
> > > > + /*
> > > > + * Calculate the line rate from the pixel rate. If the source
> > > > + supports
> > > > + * the .get_frame_desc() operation it has to implement the
> > > > + LINK_FREQ
> > > > + * control, as the link frequency can't be calculated from the
> > > > + pixel
> > > > + * rate with multiple streams having possibly different data types.
> > > > + */
> > > > + link_freq = v4l2_get_link_freq(csis->source.pad,
> > > > + csis_fmt ? csis_fmt->width : 0,
> > > > csis->bus.num_data_lanes * 2);
> > > > if (link_freq < 0) {
> > > > dev_err(csis->dev, "Unable to obtain link frequency:
> > > > %d\n",
> > > @@
> > > > -704,6 +835,8 @@ static void mipi_csis_set_params(struct
> > > > mipi_csis_device
> > > *csis,
> > > > const struct mipi_csis_params *params) {
> > > > int lanes = csis->bus.num_data_lanes;
> > > > + u32 cmn_ctrl = 0;
> > > > + u32 clk_ctrl = 0;
> > > > u32 val;
> > > >
> > > > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL); @@ -714,19
> > > > +847,32 @@ static void mipi_csis_set_params(struct
> > > > mipi_csis_device *csis,
> > > >
> > > > if (csis->info->version == MIPI_CSIS_V3_3)
> > > > val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_DT;
> > > > + if (params->interleave_vc)
> > > > + val |= MIPI_CSIS_CMN_CTRL_INTERLEAVE_MODE_VC;
> > > >
> > > > mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val);
> > > >
> > > > - mipi_csis_set_channel_params(csis, 0, ¶ms->channels[0]);
> > > > + for (unsigned int i = 0; i < csis->num_channels; ++i) {
> > > > + const struct mipi_csis_channel_params *channel =
> > > > + ¶ms->channels[i];
> > > > +
> > > > + if (!channel->vc_mask)
> > > > + continue;
> > > > +
> > > > + mipi_csis_set_channel_params(csis, i, channel);
> > > > +
> > > > + cmn_ctrl |= MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(i);
> > > > + clk_ctrl |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(i, 15)
> > > > + | MIPI_CSIS_CLK_CTRL_WCLK_SRC(i); }
> > > >
> > > > mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL,
> > > > MIPI_CSIS_DPHY_CMN_CTRL_HSSETTLE(params-
> > > >hs_settle) |
> > > > MIPI_CSIS_DPHY_CMN_CTRL_CLKSETTLE(params-
> > > >clk_settle));
> > > >
> > > > val = mipi_csis_read(csis, MIPI_CSIS_CLK_CTRL);
> > > > - val |= MIPI_CSIS_CLK_CTRL_WCLK_SRC(0);
> > > > - val |= MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL(0, 15);
> > > > val &= ~MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MASK;
> > > > + val |= clk_ctrl;
> > > > mipi_csis_write(csis, MIPI_CSIS_CLK_CTRL, val);
> > > >
> > > > mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_L, @@ -741,8 +887,7
> > > @@
> > > > static void mipi_csis_set_params(struct mipi_csis_device *csis,
> > > >
> > > > /* Update the shadow register. */
> > > > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > > > - mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL,
> > > > - val | MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW(0) |
> > > > + mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, val | cmn_ctrl |
> > > > MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
> > > > }
> > > >
> > > > @@ -1053,7 +1198,7 @@ static int mipi_csis_enum_mbus_code(struct
> > > v4l2_subdev *sd,
> > > > if (code->index > 0)
> > > > return -EINVAL;
> > > >
> > > > - fmt = v4l2_subdev_state_get_format(state, code->pad);
> > > > + fmt = v4l2_subdev_state_get_format(state, code->pad,
> > > > + code-
> > > >stream);
> > > > code->code = fmt->code;
> > > > return 0;
> > > > }
> > > > @@ -1069,10 +1214,57 @@ static int mipi_csis_enum_mbus_code(struct
> > > v4l2_subdev *sd,
> > > > return 0;
> > > > }
> > > >
> > > > +static void mipi_csis_propagate_formats(struct mipi_csis_device *csis,
> > > > + struct v4l2_subdev_state *state)
> > > > +{ const struct v4l2_mbus_framefmt
> > > *channels[MIPI_CSIS_MAX_CHANNELS] = { };
> > > > + struct v4l2_subdev_route *route;
> > > > +
> > > > + for_each_active_route(&state->routing, route) {
> > > > + const struct csis_pix_format *csis_fmt;
> > > > + struct v4l2_mbus_framefmt *sink_fmt;
> > > > + struct v4l2_mbus_framefmt *src_fmt;
> > > > +
> > > > + sink_fmt = v4l2_subdev_state_get_format(state,
> > > CSIS_PAD_SINK,
> > > > + route->sink_stream);
> > > > + src_fmt = v4l2_subdev_state_get_format(state,
> > > CSIS_PAD_SOURCE,
> > > > +
> > > > + route->source_stream);
> > > > +
> > > > + csis_fmt = find_csis_format(sink_fmt->code);
> > > > +
> > > > + /*
> > > > + * If the output channel corresponding to this source stream
> > > > + * isn't associated with a sink stream yet, simply propagate the
> > > > + * format from sink stream to source stream and associate the
> > > > + * sink stream with the channel.
> > > > + *
> > > > + * Otherwise, the sink stream format must match the
> > > > + primary
> > > sink
> > > > + * stream associated with the channel except for the
> > > > + height
> > > that
> > > > + * can be different. We propagate the format from the
> > > > + primary
> > > to
> > > > + * secondary sink stream, and accumulate the height in the
> > > > + * source stream format.
> > > > + */
> > > > + if (!channels[route->source_stream]) {
> > > > + *src_fmt = *sink_fmt;
> > > > + src_fmt->code = csis_fmt->output;
> > > > +
> > > > + channels[route->source_stream] = sink_fmt;
> > > > + } else {
> > > > + unsigned int height = sink_fmt->height;
> > > > +
> > > > + *sink_fmt = *channels[route->source_stream];
> > > > + sink_fmt->height = height;
> > > > +
> > > > + src_fmt->height += sink_fmt->height;
> > > > + }
> > > > + }
> > > > +}
> > > > +
> > > > static int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> > > > struct v4l2_subdev_state *state,
> > > > struct v4l2_subdev_format *sdformat) {
> > > > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > > > const struct csis_pix_format *csis_fmt;
> > > > struct v4l2_mbus_framefmt *fmt;
> > > > unsigned int align;
> > > > @@ -1120,7 +1312,8 @@ static int mipi_csis_set_fmt(struct
> > > > v4l2_subdev
> > > *sd,
> > > > &sdformat->format.height, 1,
> > > > CSIS_MAX_PIX_HEIGHT, 0, 0);
> > > >
> > > > - fmt = v4l2_subdev_state_get_format(state, sdformat->pad);
> > > > + fmt = v4l2_subdev_state_get_format(state, sdformat->pad,
> > > > + sdformat->stream);
> > > >
> > > > fmt->code = csis_fmt->code;
> > > > fmt->width = sdformat->format.width; @@ -1133,12 +1326,8 @@
> > > static
> > > > int mipi_csis_set_fmt(struct v4l2_subdev *sd,
> > > >
> > > > sdformat->format = *fmt;
> > > >
> > > > - /* Propagate the format from sink to source. */
> > > > - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> > > > - *fmt = sdformat->format;
> > > > -
> > > > - /* The format on the source pad might change due to unpacking.
> > > > */
> > > > - fmt->code = csis_fmt->output;
> > > > + /* Propagate the format. */
> > > > + mipi_csis_propagate_formats(csis, state);
> > > >
> > > > return 0;
> > > > }
> > > > @@ -1155,7 +1344,7 @@ static int mipi_csis_get_frame_desc(struct
> > > v4l2_subdev *sd, unsigned int pad,
> > > > return -EINVAL;
> > > >
> > > > state = v4l2_subdev_lock_and_get_active_state(sd);
> > > > - fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE);
> > > > + fmt = v4l2_subdev_state_get_format(state, CSIS_PAD_SOURCE, 0);
> > > > csis_fmt = find_csis_format(fmt->code);
> > > > v4l2_subdev_unlock_state(state);
> > > >
> > > > @@ -1187,6 +1376,8 @@ static int __mipi_csis_set_routing(struct
> > > v4l2_subdev *sd,
> > > > .quantization = V4L2_QUANTIZATION_LIM_RANGE,
> > > > .xfer_func = V4L2_XFER_FUNC_SRGB,
> > > > };
> > > > + struct mipi_csis_device *csis = sd_to_mipi_csis_device(sd);
> > > > + struct v4l2_subdev_route *route;
> > > > int ret;
> > > >
> > > > ret = v4l2_subdev_routing_validate(sd, routing, @@ -1194,15
> > > +1385,27
> > > > @@ static int __mipi_csis_set_routing(struct v4l2_subdev *sd,
> > > > if (ret)
> > > > return ret;
> > > >
> > > > - /* Only a single route is supported for now. */
> > > > - if (routing->num_routes != 1 ||
> > > > - !(routing->routes[0].flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> > > > - return -EINVAL;
> > > > + /*
> > > > + * The source stream identifies the output channel. Validate
> > > > + that it
> > > > + * does not exceed the number of channels available in the
> > > > + device. The
> > > > + * other routing constraints can't be validated now as they
> > > > + require
> > > > + * querying the frame descriptor on the sink side, which can
> > > > + only be
> > > > + * done when enabling streaming.
> > > > + */
> > > > + for_each_active_route(routing, route) {
> > > > + if (route->source_stream >= csis->num_channels) {
> > > > + dev_dbg(csis->dev, "Invalid source stream %u",
> > > > + route->source_stream);
> > > > + return -EINVAL;
> > > > + }
> > > > + }
> > > >
> > > > ret = v4l2_subdev_set_routing_with_fmt(sd, state, routing, &format);
> > > > if (ret)
> > > > return ret;
> > > >
> > > > + mipi_csis_propagate_formats(csis, state);
> > > > +
> > > > return 0;
> > > > }
> > > >
> > > > @@ -1554,7 +1757,8 @@ static int mipi_csis_subdev_init(struct
> > > mipi_csis_device *csis)
> > > > snprintf(sd->name, sizeof(sd->name), "csis-%s",
> > > > dev_name(csis->dev));
> > > >
> > > > - sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > > V4L2_SUBDEV_FL_HAS_EVENTS;
> > > > + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > > V4L2_SUBDEV_FL_HAS_EVENTS
> > > > + | V4L2_SUBDEV_FL_STREAMS;
> > > > sd->ctrl_handler = NULL;
> > > >
> > > > sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
>
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v1 0/6] media: imx-mipi-csis: Add streams support
2025-11-07 1:58 [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Laurent Pinchart
` (5 preceding siblings ...)
2025-11-07 1:58 ` [PATCH v1 6/6] media: imx-mipi-csis: Add multi-channel support Laurent Pinchart
@ 2025-11-07 9:31 ` Martin Kepplinger
2025-11-07 18:28 ` Laurent Pinchart
6 siblings, 1 reply; 26+ messages in thread
From: Martin Kepplinger @ 2025-11-07 9:31 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: Rui Miguel Silva, Purism Kernel Team, Pengutronix Kernel Team,
imx, Frank Li, Stefan Klug, Sakari Ailus
Am Freitag, dem 07.11.2025 um 03:58 +0200 schrieb Laurent Pinchart:
> Hello,
>
> The reference manual for the NXP i.MX7D in which the CSIS is
> integrated
> lists four outputs named "channels" in various diagrams and tables,
> but
> only documents registers for channel 0. Reference manuals for i.MX8
> SoCs
> that integrate the CSIS gradually removed mentions of channels 1 to 3
> as
> newer revisions of the documents got released, which was interpreted
> as
> an indication that the CSIS has only a single output.
>
> That interpretation turned out not to be correct. In the i.MX8MP, the
> CSIS features multiple output channels. Channel 0 is connected to the
> ISI, and channels 0, 1 and 2 are connected to the ISP. Multi-channel
> support towards the ISP is meant to support HDR, where camera sensors
> transmit multiple exposures or gains over different virtual channels.
> The glue logic between the CSIS and ISP hardcodes this use case and
> combines the channels in a way suitable for the ISP's single input
> port.
Hi Laurent,
This is going to be a dumb question, I know this, but I'm equally
confused as interested:
What you implement here is *not* muxing CSIS to (one of the 2) ISI-
pipes, right? This specifically is something I still need to do on
mainline (nxp-tree has a way to "mux" this, with rather cryptic
devicetree properties) where a board physically has the camera
connected to the *second* CSI interface but I need data to run through
the *first* ISI pipe. By default the second ISI pipe is used for the
second CSI interface but only the first ISI pipe can do 4k. Is this
"muxing" implemented in mainline right now? (ISI driver most
probably?).
Also, your patch is not about the connection CSIS->ISP, right? Because
that is something I tested using libcamera a while ago and have a dtso
lying around for.
Can you put this new functionality into perspective for lazy people
like me? :) For the ISI-case this doesn't seem to have any implication
right?
anyways, fascinating :)
martin
>
> With a time machine, we would probably redesign the CSIS DT bindings
> to
> model each output channel with a port, and the V4L2 subdev would use
> multiple source pads. Without a time machine, we need to preserve
> backward compatibility with userspace. Not only would the addition of
> new ports and pads make this tricky, they would also require
> modelling
> the glue logic between the CSIS and ISP with a subdev. This would
> conflict with the need to preserve backward compatibility.
>
> Fortunately for us, the CSIS is an old IP. The NXP i.MX9 series uses
> a
> different CSI-2 receiver, and the probability of needing to support
> an
> SoC that connects different output channels to different devices is
> extremely low. We can therefore implement a solution tailored to the
> needs of the i.MX8MP, and model the output channels as separate
> streams
> instead of separate pads. Should the need arise in the future to
> support
> connecting output channels to different devices in a new SoC, we will
> still be able to model this with extra ports in the device tree
> without
> breaking backward compatibility. The driver could then create extra
> subdev pads accordingly.
>
> The series starts with 5 preparatory patches to ease review. Patch
> 1/6
> adds missing register definitions. Patch 2/6 then switches from
> .s_stream() to .enable_streams() and .disable_streams(), a pre-
> requisite
> for multi-stream support. Patch 3/6 follows by implementing the
> .s_routing() operation, supporting a single route exactly as done
> implicitly so far. Patches 4/6 and 5/6 refactor the code to group
> parameters in structures and clean up how they are written to
> registers.
> Finally, patch 6/6 adds multi-channel support.
>
> The series has been tested on an i.MX8MP with libcamera, both without
> changes to verify that backward compatibility is preserved, and with
> HDR
> support (work still in progress) to verify that VC filtering works as
> expected.
>
> Laurent Pinchart (6):
> media: imx-mipi-csis: Add VC-related register fields
> media: imx-mipi-csis: Switch to .enable_streams()
> media: imx-mipi-csis: Implement the .set_routing() operation
> media: imx-mipi-csis: Group runtime parameters in structure
> media: imx-mipi-csis: Set all per-channel registers in one function
> media: imx-mipi-csis: Add multi-channel support
>
> drivers/media/platform/nxp/imx-mipi-csis.c | 551 ++++++++++++++++---
> --
> 1 file changed, 429 insertions(+), 122 deletions(-)
>
>
> base-commit: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v1 0/6] media: imx-mipi-csis: Add streams support
2025-11-07 9:31 ` [PATCH v1 0/6] media: imx-mipi-csis: Add streams support Martin Kepplinger
@ 2025-11-07 18:28 ` Laurent Pinchart
0 siblings, 0 replies; 26+ messages in thread
From: Laurent Pinchart @ 2025-11-07 18:28 UTC (permalink / raw)
To: Martin Kepplinger
Cc: linux-media, Rui Miguel Silva, Purism Kernel Team,
Pengutronix Kernel Team, imx, Frank Li, Stefan Klug, Sakari Ailus
Hi Martin,
On Fri, Nov 07, 2025 at 09:31:39AM +0000, Martin Kepplinger wrote:
> Am Freitag, dem 07.11.2025 um 03:58 +0200 schrieb Laurent Pinchart:
> > Hello,
> >
> > The reference manual for the NXP i.MX7D in which the CSIS is integrated
> > lists four outputs named "channels" in various diagrams and tables, but
> > only documents registers for channel 0. Reference manuals for i.MX8 SoCs
> > that integrate the CSIS gradually removed mentions of channels 1 to 3 as
> > newer revisions of the documents got released, which was interpreted as
> > an indication that the CSIS has only a single output.
> >
> > That interpretation turned out not to be correct. In the i.MX8MP, the
> > CSIS features multiple output channels. Channel 0 is connected to the
> > ISI, and channels 0, 1 and 2 are connected to the ISP. Multi-channel
> > support towards the ISP is meant to support HDR, where camera sensors
> > transmit multiple exposures or gains over different virtual channels.
> > The glue logic between the CSIS and ISP hardcodes this use case and
> > combines the channels in a way suitable for the ISP's single input
> > port.
>
> Hi Laurent,
>
> This is going to be a dumb question, I know this, but I'm equally
> confused as interested:
>
> What you implement here is *not* muxing CSIS to (one of the 2) ISI-
> pipes, right? This specifically is something I still need to do on
> mainline (nxp-tree has a way to "mux" this, with rather cryptic
> devicetree properties) where a board physically has the camera
> connected to the *second* CSI interface but I need data to run through
> the *first* ISI pipe. By default the second ISI pipe is used for the
> second CSI interface but only the first ISI pipe can do 4k. Is this
> "muxing" implemented in mainline right now? (ISI driver most
> probably?).
You can configure routing of inputs to channels within the ISI crossbar
subdev using the set routing ioctl. This should work in mainline today.
> Also, your patch is not about the connection CSIS->ISP, right? Because
> that is something I tested using libcamera a while ago and have a dtso
> lying around for.
Connecting the CSIS to the ISP also works today with a change in DT,
that's correct. I would like to make this more dynamic by declaring both
connections in DT and dynamically configuring the routing from
userspace, but that's an entirely different issue (and will require lots
of work).
> Can you put this new functionality into perspective for lazy people
> like me? :) For the ISI-case this doesn't seem to have any implication
> right?
What this series is about is how to demultiplex CSI-2 virtual channels
(not to be confused with the CSIS output channels, despite being both
called channels they are entirely different things) inside the CSIS and
route them to different CSIS outputs. This is not a feature useful for
the ISI as only CSIS output channel 0 is connected to the ISI.
> anyways, fascinating :)
>
> > With a time machine, we would probably redesign the CSIS DT bindings to
> > model each output channel with a port, and the V4L2 subdev would use
> > multiple source pads. Without a time machine, we need to preserve
> > backward compatibility with userspace. Not only would the addition of
> > new ports and pads make this tricky, they would also require modelling
> > the glue logic between the CSIS and ISP with a subdev. This would
> > conflict with the need to preserve backward compatibility.
> >
> > Fortunately for us, the CSIS is an old IP. The NXP i.MX9 series uses a
> > different CSI-2 receiver, and the probability of needing to support an
> > SoC that connects different output channels to different devices is
> > extremely low. We can therefore implement a solution tailored to the
> > needs of the i.MX8MP, and model the output channels as separate streams
> > instead of separate pads. Should the need arise in the future to support
> > connecting output channels to different devices in a new SoC, we will
> > still be able to model this with extra ports in the device tree without
> > breaking backward compatibility. The driver could then create extra
> > subdev pads accordingly.
> >
> > The series starts with 5 preparatory patches to ease review. Patch 1/6
> > adds missing register definitions. Patch 2/6 then switches from
> > .s_stream() to .enable_streams() and .disable_streams(), a pre-requisite
> > for multi-stream support. Patch 3/6 follows by implementing the
> > .s_routing() operation, supporting a single route exactly as done
> > implicitly so far. Patches 4/6 and 5/6 refactor the code to group
> > parameters in structures and clean up how they are written to registers.
> > Finally, patch 6/6 adds multi-channel support.
> >
> > The series has been tested on an i.MX8MP with libcamera, both without
> > changes to verify that backward compatibility is preserved, and with HDR
> > support (work still in progress) to verify that VC filtering works as
> > expected.
> >
> > Laurent Pinchart (6):
> > media: imx-mipi-csis: Add VC-related register fields
> > media: imx-mipi-csis: Switch to .enable_streams()
> > media: imx-mipi-csis: Implement the .set_routing() operation
> > media: imx-mipi-csis: Group runtime parameters in structure
> > media: imx-mipi-csis: Set all per-channel registers in one function
> > media: imx-mipi-csis: Add multi-channel support
> >
> > drivers/media/platform/nxp/imx-mipi-csis.c | 551 ++++++++++++++++---
> > --
> > 1 file changed, 429 insertions(+), 122 deletions(-)
> >
> >
> > base-commit: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 26+ messages in thread