[PATCH v2 0/3] media: imx-mipi-csis: Get the number of active lanes from mbus_config

[PATCH v2 0/3] media: imx-mipi-csis: Get the number of active lanes from mbus_config

[Isaac Scott]

It is possible that the number of desired active MIPI CSI2 data lanes
does not match the maximum listed in device tree. Add a helper function
to v4l2_common that calls the get_mbus_config op to get the number of
actively used data lanes in drivers that support it.

Compare it to the number of lanes configured in device tree, and if its
invalid, use the number present in device tree.

This series also uses the helper in imx-mipi-csis driver to set the
currently configured num_data_lanes, while keeping track of the number
of data lanes set in device tree to ensure we can still use all possible
lanes if we need to, and the upstream subdev driver requests them.

Tested on v6.15, compile tested on v6.17-rc4.

---------

Changes v1 -> v2:

- Added helper function to get active data lanes in v4l2-common
- Store the maximum data lanes possible, as configured in device tree
- Added media: prefix to commit titles

Isaac Scott (3):
  media: v4l: Add helper to get number of active lanes via a pad
  media: imx-mipi-csis: Store the number of data_lanes configured in dt
  media: imx-mipi-csis: Get number of active lanes via mbus_config

 drivers/media/platform/nxp/imx-mipi-csis.c |  8 ++++++-
 drivers/media/v4l2-core/v4l2-common.c      | 25 ++++++++++++++++++++++
 include/media/v4l2-common.h                |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH v2 1/3] media: v4l: Add helper to get number of active lanes via a pad

[Isaac Scott]

Sometimes, users will not use all of the MIPI CSI 2 lanes available when
connecting to the MIPI CSI receiver of their device. Add a helper
function that checks the mbus_config for the device driver to allow
users to define the number of active data lanes through the
get_mbus_config op.

If the driver does not implement this op, fall back to using the number
of data lanes specified in device tree.

Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 25 +++++++++++++++++++++++++
 include/media/v4l2-common.h           |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 6e585bc76367..8683107b3704 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -571,6 +571,31 @@ s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
 	return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div);
 }
 EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad);
+
+unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes)
+{
+	struct v4l2_mbus_config mbus_config = {};
+	struct v4l2_subdev *sd;
+	unsigned int lanes;
+	int ret;
+
+	sd = media_entity_to_v4l2_subdev(pad->entity);
+	ret = v4l2_subdev_call(sd, pad, get_mbus_config, pad->index,
+			       &mbus_config);
+	if (ret < 0 && ret != -ENOIOCTLCMD)
+		return ret;
+
+	if (!mbus_config.bus.mipi_csi2.num_data_lanes)
+		return dt_lanes;
+
+	lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
+
+	if (lanes < 0 || lanes > dt_lanes)
+		return -EINVAL;
+
+	return lanes;
+}
+EXPORT_SYMBOL_GPL(v4l2_get_active_data_lanes);
 #endif /* CONFIG_MEDIA_CONTROLLER */
 
 /*
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 0a43f56578bc..3f8937260c76 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -584,6 +584,7 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
 	(pad, mul, div)
 s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
 			     unsigned int div);
+unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes);
 #else
 #define v4l2_get_link_freq(handler, mul, div)		\
 	__v4l2_get_link_freq_ctrl(handler, mul, div)
-- 
2.43.0

[PATCH v2 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt

[Isaac Scott]

The number of lanes actively used by a MIPI CSI transmitter may differ
from that which is defined in device tree. To allow us to be able to set
the number of configured lanes without changing the maximum lane count,
store the number of lanes configured in device tree, and adjust the
debug print to reflect the device tree value.

Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 2beb5f43c2c0..fc89325f2f94 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -313,6 +313,8 @@ struct mipi_csis_device {
 	u32 hs_settle;
 	u32 clk_settle;
 
+	unsigned int max_data_lanes;
+
 	spinlock_t slock;	/* Protect events */
 	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
 	struct dentry *debugfs_root;
@@ -1299,8 +1301,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
 	}
 
 	csis->bus = vep.bus.mipi_csi2;
+	csis->max_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
 
-	dev_dbg(csis->dev, "data lanes: %d\n", csis->bus.num_data_lanes);
+	dev_dbg(csis->dev, "data lanes: %d\n", csis->max_data_lanes);
 	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
 
 	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
-- 
2.43.0

[PATCH v2 3/3] media: imx-mipi-csis: Get number of active lanes via mbus_config

[Isaac Scott]

The number of lanes actively used by a MIPI CSI transmitter may differ
from that which is defined in device tree. As such, call on
v4l2_get_active_data_lanes to check if the driver reports a
differing number of lanes to device tree, and use that number of active
lanes.

If the number of active data lanes is invalid, or the op is not
supported, it will use the number of lanes defined in device tree.

Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
---
 drivers/media/platform/nxp/imx-mipi-csis.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index fc89325f2f94..985be511bcd0 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -967,6 +967,9 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
 	format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
 	csis_fmt = find_csis_format(format->code);
 
+	csis->bus.num_data_lanes = v4l2_get_active_data_lanes(csis->source.pad,
+							      csis->max_data_lanes);
+
 	ret = mipi_csis_calculate_params(csis, csis_fmt);
 	if (ret < 0)
 		goto err_unlock;
-- 
2.43.0

Re: [PATCH v2 1/3] media: v4l: Add helper to get number of active lanes via a pad

[Frank Li]

On Wed, Sep 03, 2025 at 11:22:40AM +0100, Isaac Scott wrote:
> Sometimes, users will not use all of the MIPI CSI 2 lanes available when
> connecting to the MIPI CSI receiver of their device. Add a helper
> function that checks the mbus_config for the device driver to allow
> users to define the number of active data lanes through the
> get_mbus_config op.
>
> If the driver does not implement this op, fall back to using the number
> of data lanes specified in device tree.
>
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 25 +++++++++++++++++++++++++
>  include/media/v4l2-common.h           |  1 +
>  2 files changed, 26 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 6e585bc76367..8683107b3704 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -571,6 +571,31 @@ s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
>  	return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div);
>  }
>  EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad);
> +
> +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes)
> +{
> +	struct v4l2_mbus_config mbus_config = {};
> +	struct v4l2_subdev *sd;
> +	unsigned int lanes;
> +	int ret;
> +
> +	sd = media_entity_to_v4l2_subdev(pad->entity);
> +	ret = v4l2_subdev_call(sd, pad, get_mbus_config, pad->index,
> +			       &mbus_config);
> +	if (ret < 0 && ret != -ENOIOCTLCMD)
> +		return ret;
> +
> +	if (!mbus_config.bus.mipi_csi2.num_data_lanes)
> +		return dt_lanes;
> +
> +	lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> +
> +	if (lanes < 0 || lanes > dt_lanes)
> +		return -EINVAL;
> +
> +	return lanes;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_get_active_data_lanes);
>  #endif /* CONFIG_MEDIA_CONTROLLER */
>
>  /*
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 0a43f56578bc..3f8937260c76 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -584,6 +584,7 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
>  	(pad, mul, div)
>  s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
>  			     unsigned int div);
> +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes);
>  #else
>  #define v4l2_get_link_freq(handler, mul, div)		\
>  	__v4l2_get_link_freq_ctrl(handler, mul, div)
> --
> 2.43.0
>

Re: [PATCH v2 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt

[Frank Li]

On Wed, Sep 03, 2025 at 11:22:41AM +0100, Isaac Scott wrote:
> The number of lanes actively used by a MIPI CSI transmitter may differ
> from that which is defined in device tree. To allow us to be able to set
> the number of configured lanes without changing the maximum lane count,
> store the number of lanes configured in device tree, and adjust the
> debug print to reflect the device tree value.
>
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 2beb5f43c2c0..fc89325f2f94 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -313,6 +313,8 @@ struct mipi_csis_device {
>  	u32 hs_settle;
>  	u32 clk_settle;
>
> +	unsigned int max_data_lanes;
> +

is num_data_lanes better? you get from vep.bus.mipi_csi2.num_data_lanes

Frank

>  	spinlock_t slock;	/* Protect events */
>  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
>  	struct dentry *debugfs_root;
> @@ -1299,8 +1301,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
>  	}
>
>  	csis->bus = vep.bus.mipi_csi2;
> +	csis->max_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
>
> -	dev_dbg(csis->dev, "data lanes: %d\n", csis->bus.num_data_lanes);
> +	dev_dbg(csis->dev, "data lanes: %d\n", csis->max_data_lanes);
>  	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
>
>  	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
> --
> 2.43.0
>

Re: [PATCH v2 3/3] media: imx-mipi-csis: Get number of active lanes via mbus_config

[Frank Li]

On Wed, Sep 03, 2025 at 11:22:42AM +0100, Isaac Scott wrote:
> The number of lanes actively used by a MIPI CSI transmitter may differ
> from that which is defined in device tree. As such, call on
> v4l2_get_active_data_lanes to check if the driver reports a

function need (), v4l2_get_active_data_lanes()

> differing number of lanes to device tree, and use that number of active
> lanes.
>
> If the number of active data lanes is invalid, or the op is not
> supported, it will use the number of lanes defined in device tree.

remove "it will" to keep simple.

Frank
>
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index fc89325f2f94..985be511bcd0 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -967,6 +967,9 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
>  	format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
>  	csis_fmt = find_csis_format(format->code);
>
> +	csis->bus.num_data_lanes = v4l2_get_active_data_lanes(csis->source.pad,
> +							      csis->max_data_lanes);
> +
>  	ret = mipi_csis_calculate_params(csis, csis_fmt);
>  	if (ret < 0)
>  		goto err_unlock;
> --
> 2.43.0
>

Re: [PATCH v2 1/3] media: v4l: Add helper to get number of active lanes via a pad

[Niklas Söderlund]

Hi Issac,

Thanks for your work.

On 2025-09-03 11:22:40 +0100, Isaac Scott wrote:
> Sometimes, users will not use all of the MIPI CSI 2 lanes available when
> connecting to the MIPI CSI receiver of their device. Add a helper
> function that checks the mbus_config for the device driver to allow
> users to define the number of active data lanes through the
> get_mbus_config op.
> 
> If the driver does not implement this op, fall back to using the number
> of data lanes specified in device tree.
> 
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 25 +++++++++++++++++++++++++
>  include/media/v4l2-common.h           |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 6e585bc76367..8683107b3704 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -571,6 +571,31 @@ s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
>  	return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div);
>  }
>  EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad);
> +
> +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes)
> +{
> +	struct v4l2_mbus_config mbus_config = {};
> +	struct v4l2_subdev *sd;
> +	unsigned int lanes;
> +	int ret;
> +
> +	sd = media_entity_to_v4l2_subdev(pad->entity);
> +	ret = v4l2_subdev_call(sd, pad, get_mbus_config, pad->index,
> +			       &mbus_config);
> +	if (ret < 0 && ret != -ENOIOCTLCMD)
> +		return ret;

The function prototype is 'unsigned int' and here you return a signed 
value here.

> +

Maybe add a comment here that this check depends on mbus_config being 
zeroed at init if the call above pass with -ENOIOCTLCMD? It's not 
immediately clear what is going on here even tho the code is clever ;-)

> +	if (!mbus_config.bus.mipi_csi2.num_data_lanes)
> +		return dt_lanes;
> +
> +	lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> +
> +	if (lanes < 0 || lanes > dt_lanes)
> +		return -EINVAL;
> +
> +	return lanes;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_get_active_data_lanes);
>  #endif /* CONFIG_MEDIA_CONTROLLER */
>  
>  /*
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 0a43f56578bc..3f8937260c76 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -584,6 +584,7 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
>  	(pad, mul, div)
>  s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
>  			     unsigned int div);
> +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes);
>  #else
>  #define v4l2_get_link_freq(handler, mul, div)		\
>  	__v4l2_get_link_freq_ctrl(handler, mul, div)
> -- 
> 2.43.0
> 

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v2 1/3] media: v4l: Add helper to get number of active lanes via a pad

[Sakari Ailus]

Hi Isaac,

Thanks for the update.

On Wed, Sep 03, 2025 at 11:22:40AM +0100, Isaac Scott wrote:
> Sometimes, users will not use all of the MIPI CSI 2 lanes available when
> connecting to the MIPI CSI receiver of their device. Add a helper
> function that checks the mbus_config for the device driver to allow
> users to define the number of active data lanes through the
> get_mbus_config op.
> 
> If the driver does not implement this op, fall back to using the number
> of data lanes specified in device tree.
> 
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 25 +++++++++++++++++++++++++
>  include/media/v4l2-common.h           |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 6e585bc76367..8683107b3704 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -571,6 +571,31 @@ s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
>  	return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div);
>  }
>  EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad);
> +
> +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes)

This line would benefit from being wrapped.

> +{
> +	struct v4l2_mbus_config mbus_config = {};
> +	struct v4l2_subdev *sd;
> +	unsigned int lanes;
> +	int ret;
> +
> +	sd = media_entity_to_v4l2_subdev(pad->entity);
> +	ret = v4l2_subdev_call(sd, pad, get_mbus_config, pad->index,
> +			       &mbus_config);
> +	if (ret < 0 && ret != -ENOIOCTLCMD)
> +		return ret;
> +
> +	if (!mbus_config.bus.mipi_csi2.num_data_lanes)
> +		return dt_lanes;
> +
> +	lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> +
> +	if (lanes < 0 || lanes > dt_lanes)

lanes is unsigned int so no need to check for less than 0.

I might just not use a local variable for this, up to you.

> +		return -EINVAL;
> +
> +	return lanes;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_get_active_data_lanes);
>  #endif /* CONFIG_MEDIA_CONTROLLER */
>  
>  /*
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 0a43f56578bc..3f8937260c76 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -584,6 +584,7 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
>  	(pad, mul, div)
>  s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
>  			     unsigned int div);

Some kernel-doc documentation would be nice.

> +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes);

Please wrap this, too.

>  #else
>  #define v4l2_get_link_freq(handler, mul, div)		\
>  	__v4l2_get_link_freq_ctrl(handler, mul, div)

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v2 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt

[Laurent Pinchart]

On Wed, Sep 03, 2025 at 11:33:15AM -0400, Frank Li wrote:
> On Wed, Sep 03, 2025 at 11:22:41AM +0100, Isaac Scott wrote:
> > The number of lanes actively used by a MIPI CSI transmitter may differ
> > from that which is defined in device tree. To allow us to be able to set
> > the number of configured lanes without changing the maximum lane count,
> > store the number of lanes configured in device tree, and adjust the
> > debug print to reflect the device tree value.
> >
> > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 2beb5f43c2c0..fc89325f2f94 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -313,6 +313,8 @@ struct mipi_csis_device {
> >  	u32 hs_settle;
> >  	u32 clk_settle;
> >
> > +	unsigned int max_data_lanes;
> > +
> 
> is num_data_lanes better? you get from vep.bus.mipi_csi2.num_data_lanes

That's a good point, but I think I prefer max_data_lanes here as it
conveys better the fact that the field stores the maximum number of data
lanes that can be used, not the number of data lanes being used at a
given point of time.

This being said, why do we need this ? The maximum number of data lanes
can be accessed through csis->bus.num_data_lanes. I've looked at patch
3/3 to answer this question, it there csis->bus.num_data_lanes is
modified to store the number of data lanes used at runtime. I think it
would be better to consider csis->bus as immutable after probe, and
store the number of used data lanes in csis->num_data_lanes.

Isaac, could you replace this patch by another one that adds
csis->num_data_lanes, sets it to csis->bus.num_data_lanes in
mipi_csis_async_register(), and replace usage of
csis->bus.num_data_lanes with csis->num_data_lanes through the driver ?
Patch 3/3 should then modify csis->num_data_lanes, not
csis->bus.num_data_lanes.

> >  	spinlock_t slock;	/* Protect events */
> >  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
> >  	struct dentry *debugfs_root;
> > @@ -1299,8 +1301,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
> >  	}
> >
> >  	csis->bus = vep.bus.mipi_csi2;
> > +	csis->max_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
> >
> > -	dev_dbg(csis->dev, "data lanes: %d\n", csis->bus.num_data_lanes);
> > +	dev_dbg(csis->dev, "data lanes: %d\n", csis->max_data_lanes);
> >  	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
> >
> >  	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/3] media: v4l: Add helper to get number of active lanes via a pad

[Laurent Pinchart]

On Thu, Sep 04, 2025 at 11:44:49AM +0300, Sakari Ailus wrote:
> On Wed, Sep 03, 2025 at 11:22:40AM +0100, Isaac Scott wrote:
> > Sometimes, users will not use all of the MIPI CSI 2 lanes available when
> > connecting to the MIPI CSI receiver of their device. Add a helper
> > function that checks the mbus_config for the device driver to allow
> > users to define the number of active data lanes through the
> > get_mbus_config op.
> > 
> > If the driver does not implement this op, fall back to using the number
> > of data lanes specified in device tree.
> > 
> > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-common.c | 25 +++++++++++++++++++++++++
> >  include/media/v4l2-common.h           |  1 +
> >  2 files changed, 26 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > index 6e585bc76367..8683107b3704 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -571,6 +571,31 @@ s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
> >  	return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div);
> >  }
> >  EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad);
> > +
> > +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes)
> 
> This line would benefit from being wrapped.

I'd also rename the dt_lanes parameter to max_data_lanes. "dt" sounds
like data type.

> > +{
> > +	struct v4l2_mbus_config mbus_config = {};
> > +	struct v4l2_subdev *sd;
> > +	unsigned int lanes;
> > +	int ret;
> > +
> > +	sd = media_entity_to_v4l2_subdev(pad->entity);
> > +	ret = v4l2_subdev_call(sd, pad, get_mbus_config, pad->index,
> > +			       &mbus_config);
> > +	if (ret < 0 && ret != -ENOIOCTLCMD)
> > +		return ret;
> > +
> > +	if (!mbus_config.bus.mipi_csi2.num_data_lanes)
> > +		return dt_lanes;
> > +
> > +	lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> > +
> > +	if (lanes < 0 || lanes > dt_lanes)
> 
> lanes is unsigned int so no need to check for less than 0.
> 
> I might just not use a local variable for this, up to you.
> 

A debug message would also be nice.

> > +		return -EINVAL;
> > +
> > +	return lanes;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_get_active_data_lanes);
> >  #endif /* CONFIG_MEDIA_CONTROLLER */
> >  
> >  /*
> > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> > index 0a43f56578bc..3f8937260c76 100644
> > --- a/include/media/v4l2-common.h
> > +++ b/include/media/v4l2-common.h
> > @@ -584,6 +584,7 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
> >  	(pad, mul, div)
> >  s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
> >  			     unsigned int div);
> 
> Some kernel-doc documentation would be nice.
> 
> > +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes);
> 
> Please wrap this, too.
> 
> >  #else
> >  #define v4l2_get_link_freq(handler, mul, div)		\
> >  	__v4l2_get_link_freq_ctrl(handler, mul, div)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt

[Isaac Scott]

Hi all,

Thank you very much for the reviews!

Quoting Laurent Pinchart (2025-09-04 16:01:53)
> On Wed, Sep 03, 2025 at 11:33:15AM -0400, Frank Li wrote:
> > On Wed, Sep 03, 2025 at 11:22:41AM +0100, Isaac Scott wrote:
> > > The number of lanes actively used by a MIPI CSI transmitter may differ
> > > from that which is defined in device tree. To allow us to be able to set
> > > the number of configured lanes without changing the maximum lane count,
> > > store the number of lanes configured in device tree, and adjust the
> > > debug print to reflect the device tree value.
> > >
> > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > > ---
> > >  drivers/media/platform/nxp/imx-mipi-csis.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > index 2beb5f43c2c0..fc89325f2f94 100644
> > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > > @@ -313,6 +313,8 @@ struct mipi_csis_device {
> > >     u32 hs_settle;
> > >     u32 clk_settle;
> > >
> > > +   unsigned int max_data_lanes;
> > > +
> > 
> > is num_data_lanes better? you get from vep.bus.mipi_csi2.num_data_lanes
> 
> That's a good point, but I think I prefer max_data_lanes here as it
> conveys better the fact that the field stores the maximum number of data
> lanes that can be used, not the number of data lanes being used at a
> given point of time.
> 
> This being said, why do we need this ? The maximum number of data lanes
> can be accessed through csis->bus.num_data_lanes. I've looked at patch
> 3/3 to answer this question, it there csis->bus.num_data_lanes is
> modified to store the number of data lanes used at runtime. I think it
> would be better to consider csis->bus as immutable after probe, and
> store the number of used data lanes in csis->num_data_lanes.

Yes, this makes a lot of sense. I only used max_data_lanes because I
wanted to avoid potential confusion between csis->bus.num_data_lanes and
csis->num_data_lanes; if that's not acutally an issue, I'll change it to
num_data_lanes.

> 
> Isaac, could you replace this patch by another one that adds
> csis->num_data_lanes, sets it to csis->bus.num_data_lanes in
> mipi_csis_async_register(), and replace usage of
> csis->bus.num_data_lanes with csis->num_data_lanes through the driver ?
> Patch 3/3 should then modify csis->num_data_lanes, not
> csis->bus.num_data_lanes.
> 

Good idea! Thanks, I'll take a look at submitting a v3.

Best wishes,

Isaac

> > >     spinlock_t slock;       /* Protect events */
> > >     struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
> > >     struct dentry *debugfs_root;
> > > @@ -1299,8 +1301,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
> > >     }
> > >
> > >     csis->bus = vep.bus.mipi_csi2;
> > > +   csis->max_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
> > >
> > > -   dev_dbg(csis->dev, "data lanes: %d\n", csis->bus.num_data_lanes);
> > > +   dev_dbg(csis->dev, "data lanes: %d\n", csis->max_data_lanes);
> > >     dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
> > >
> > >     asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Re: [PATCH v2 3/3] media: imx-mipi-csis: Get number of active lanes via mbus_config

[Isaac Scott]

Hi Frank,

Thank you for the review!

Quoting Frank Li (2025-09-03 16:35:16)
> On Wed, Sep 03, 2025 at 11:22:42AM +0100, Isaac Scott wrote:
> > The number of lanes actively used by a MIPI CSI transmitter may differ
> > from that which is defined in device tree. As such, call on
> > v4l2_get_active_data_lanes to check if the driver reports a
> 
> function need (), v4l2_get_active_data_lanes()
> 
> > differing number of lanes to device tree, and use that number of active
> > lanes.
> >
> > If the number of active data lanes is invalid, or the op is not
> > supported, it will use the number of lanes defined in device tree.
> 
> remove "it will" to keep simple.

Thanks, I'll change this in v3.

> 
> Frank

Best wishes,

Isaac

> >
> > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index fc89325f2f94..985be511bcd0 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -967,6 +967,9 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
> >       format = v4l2_subdev_state_get_format(state, CSIS_PAD_SINK);
> >       csis_fmt = find_csis_format(format->code);
> >
> > +     csis->bus.num_data_lanes = v4l2_get_active_data_lanes(csis->source.pad,
> > +                                                           csis->max_data_lanes);
> > +
> >       ret = mipi_csis_calculate_params(csis, csis_fmt);
> >       if (ret < 0)
> >               goto err_unlock;
> > --
> > 2.43.0
> >

Re: [PATCH v2 1/3] media: v4l: Add helper to get number of active lanes via a pad

[Isaac Scott]

Hi Sakari,

Thank you for the review!

Quoting Sakari Ailus (2025-09-04 09:44:49)
> Hi Isaac,
> 
> Thanks for the update.
> 
> On Wed, Sep 03, 2025 at 11:22:40AM +0100, Isaac Scott wrote:
> > Sometimes, users will not use all of the MIPI CSI 2 lanes available when
> > connecting to the MIPI CSI receiver of their device. Add a helper
> > function that checks the mbus_config for the device driver to allow
> > users to define the number of active data lanes through the
> > get_mbus_config op.
> > 
> > If the driver does not implement this op, fall back to using the number
> > of data lanes specified in device tree.
> > 
> > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-common.c | 25 +++++++++++++++++++++++++
> >  include/media/v4l2-common.h           |  1 +
> >  2 files changed, 26 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > index 6e585bc76367..8683107b3704 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -571,6 +571,31 @@ s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
> >       return __v4l2_get_link_freq_ctrl(sd->ctrl_handler, mul, div);
> >  }
> >  EXPORT_SYMBOL_GPL(__v4l2_get_link_freq_pad);
> > +
> > +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes)
> 
> This line would benefit from being wrapped.

OK, I'll wrap it in v3.

> 
> > +{
> > +     struct v4l2_mbus_config mbus_config = {};
> > +     struct v4l2_subdev *sd;
> > +     unsigned int lanes;
> > +     int ret;
> > +
> > +     sd = media_entity_to_v4l2_subdev(pad->entity);
> > +     ret = v4l2_subdev_call(sd, pad, get_mbus_config, pad->index,
> > +                            &mbus_config);
> > +     if (ret < 0 && ret != -ENOIOCTLCMD)
> > +             return ret;
> > +
> > +     if (!mbus_config.bus.mipi_csi2.num_data_lanes)
> > +             return dt_lanes;
> > +
> > +     lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> > +
> > +     if (lanes < 0 || lanes > dt_lanes)
> 
> lanes is unsigned int so no need to check for less than 0.
> 
> I might just not use a local variable for this, up to you.
> 
> > +             return -EINVAL;
> > +
> > +     return lanes;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_get_active_data_lanes);
> >  #endif /* CONFIG_MEDIA_CONTROLLER */
> >  
> >  /*
> > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> > index 0a43f56578bc..3f8937260c76 100644
> > --- a/include/media/v4l2-common.h
> > +++ b/include/media/v4l2-common.h
> > @@ -584,6 +584,7 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
> >       (pad, mul, div)
> >  s64 __v4l2_get_link_freq_pad(struct media_pad *pad, unsigned int mul,
> >                            unsigned int div);
> 
> Some kernel-doc documentation would be nice.

Ah, of course, I'll include it in v3.

> 
> > +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad, unsigned int dt_lanes);
> 
> Please wrap this, too.

Will do!

> 
> >  #else
> >  #define v4l2_get_link_freq(handler, mul, div)                \
> >       __v4l2_get_link_freq_ctrl(handler, mul, div)
> 
> -- 
> Kind regards,
> 
> Sakari Ailus

Best wishes,

Isaac