public inbox for imx@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH v3 0/3] media: imx-mipi-csis: Get the number of active lanes from mbus_config
@ 2025-09-15 13:18 Isaac Scott
  2025-09-15 13:18 ` [PATCH v3 1/3] media: v4l: Add helper to get number of active lanes via a pad Isaac Scott
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Isaac Scott @ 2025-09-15 13:18 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rui Miguel Silva, Laurent Pinchart,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: linux-media, linux-kernel, imx, linux-arm-kernel, Frank Li,
	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-rc6.

---------

Changes v2 -> v3:

- Rename dt_lanes to max_data_lanes
- Remove check for < 0 on unsigned int max_data_lanes in
  v4l2_get_active_data_lanes()
- Added comment to explain that mbus_config is expected to be zeroed at
  init in drivers implementing get_mbus_config subdev pad op
- Wrapped signature in header file and source for
  v4l2_get_active_data_lanes()
- Added kernel-doc documentation for v4l2_get_active_data_lanes()
- Added debug message to indicate an invalid number of active lanes
- Changed csis->max_data_lanes to csis->num_data_lanes
- Changed uses of csis->bus.num_data_lanes to csis->num_data_lanes where
  appropriate to make csis->bus immutable after probe

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

---
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 | 17 +++++++++++-----
 drivers/media/v4l2-core/v4l2-common.c      | 32 ++++++++++++++++++++++++++++++
 include/media/v4l2-common.h                | 21 ++++++++++++++++++++
 3 files changed, 65 insertions(+), 5 deletions(-)
---
base-commit: f83ec76bf285bea5727f478a68b894f5543ca76e
change-id: 20250915-mbus-config-active-lanes-14ad58c7a186

Best regards,
-- 
Isaac Scott <isaac.scott@ideasonboard.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/3] media: v4l: Add helper to get number of active lanes via a pad
  2025-09-15 13:18 [PATCH v3 0/3] media: imx-mipi-csis: Get the number of active lanes from mbus_config Isaac Scott
@ 2025-09-15 13:18 ` Isaac Scott
  2025-09-22  6:48   ` Sakari Ailus
  2025-09-15 13:18 ` [PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt Isaac Scott
  2025-09-15 13:18 ` [PATCH v3 3/3] media: imx-mipi-csis: Get number of active lanes via mbus_config Isaac Scott
  2 siblings, 1 reply; 9+ messages in thread
From: Isaac Scott @ 2025-09-15 13:18 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rui Miguel Silva, Laurent Pinchart,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: linux-media, linux-kernel, imx, linux-arm-kernel, Frank Li,
	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.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 32 ++++++++++++++++++++++++++++++++
 include/media/v4l2-common.h           | 21 +++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 6e585bc76367..2ce8407f1397 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -571,6 +571,38 @@ 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 max_data_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;
+
+	/*
+	 * This relies on the mbus_config being zeroed at init time.
+	 */
+	if (!mbus_config.bus.mipi_csi2.num_data_lanes)
+		return max_data_lanes;
+
+	lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
+
+	if (lanes > max_data_lanes) {
+		dev_dbg(sd->dev, "Active data lanes (%u) exceeds max (%u)\n",
+			lanes, max_data_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..6af0695460ab 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -584,6 +584,27 @@ 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);
+
+/**
+ * v4l2_get_active_data_lanes - Get number of active data lanes from driver
+ *
+ * @pad: The transmitter's media pad.
+ * @max_data_lanes: The maximum number of active data lanes supported by
+ * 		    the MIPI CSI link in hardware. This can be configured
+ * 		    in device tree.
+ *
+ * This function is intended for obtaining the number of data lanes that are
+ * actively being used by the driver for a MIPI CSI-2 device on a given media pad.
+ * This information is derived from a mbus_config fetched from a device driver
+ * using the get_mbus_config v4l2_subdev pad op.
+ *
+ * Return:
+ * * >0: Number of active data lanes
+ * * %-EINVAL: Number of active data lanes is invalid, as it exceeds the maximum
+ *	       supported data lanes listed in device tree.
+ */
+unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad,
+					unsigned int max_data_lanes);
 #else
 #define v4l2_get_link_freq(handler, mul, div)		\
 	__v4l2_get_link_freq_ctrl(handler, mul, div)

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt
  2025-09-15 13:18 [PATCH v3 0/3] media: imx-mipi-csis: Get the number of active lanes from mbus_config Isaac Scott
  2025-09-15 13:18 ` [PATCH v3 1/3] media: v4l: Add helper to get number of active lanes via a pad Isaac Scott
@ 2025-09-15 13:18 ` Isaac Scott
  2025-09-22  6:49   ` Sakari Ailus
  2025-09-15 13:18 ` [PATCH v3 3/3] media: imx-mipi-csis: Get number of active lanes via mbus_config Isaac Scott
  2 siblings, 1 reply; 9+ messages in thread
From: Isaac Scott @ 2025-09-15 13:18 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rui Miguel Silva, Laurent Pinchart,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: linux-media, linux-kernel, imx, linux-arm-kernel, 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 | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 2beb5f43c2c0..6afbedfe131e 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 num_data_lanes;
+
 	spinlock_t slock;	/* Protect events */
 	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
 	struct dentry *debugfs_root;
@@ -535,7 +537,7 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
 	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
 	val &= ~MIPI_CSIS_DPHY_CMN_CTRL_ENABLE;
 	if (on) {
-		mask = (1 << (csis->bus.num_data_lanes + 1)) - 1;
+		mask = (1 << (csis->num_data_lanes + 1)) - 1;
 		val |= (mask & MIPI_CSIS_DPHY_CMN_CTRL_ENABLE);
 	}
 	mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL, val);
@@ -586,7 +588,7 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
 
 	/* Calculate the line rate from the pixel rate. */
 	link_freq = v4l2_get_link_freq(src_pad, csis_fmt->width,
-				       csis->bus.num_data_lanes * 2);
+				       csis->num_data_lanes * 2);
 	if (link_freq < 0) {
 		dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
 			(int)link_freq);
@@ -631,7 +633,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
 				 const struct v4l2_mbus_framefmt *format,
 				 const struct csis_pix_format *csis_fmt)
 {
-	int lanes = csis->bus.num_data_lanes;
+	int lanes = csis->num_data_lanes;
 	u32 val;
 
 	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
@@ -1299,8 +1301,10 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
 	}
 
 	csis->bus = vep.bus.mipi_csi2;
+	csis->bus.num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
+	csis->num_data_lanes = csis->bus.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->num_data_lanes);
 	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
 
 	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
@@ -1498,7 +1502,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
 	}
 
 	dev_info(dev, "lanes: %d, freq: %u\n",
-		 csis->bus.num_data_lanes, csis->clk_frequency);
+		 csis->num_data_lanes, csis->clk_frequency);
 
 	return 0;
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 3/3] media: imx-mipi-csis: Get number of active lanes via mbus_config
  2025-09-15 13:18 [PATCH v3 0/3] media: imx-mipi-csis: Get the number of active lanes from mbus_config Isaac Scott
  2025-09-15 13:18 ` [PATCH v3 1/3] media: v4l: Add helper to get number of active lanes via a pad Isaac Scott
  2025-09-15 13:18 ` [PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt Isaac Scott
@ 2025-09-15 13:18 ` Isaac Scott
  2025-09-22  8:09   ` Laurent Pinchart
  2 siblings, 1 reply; 9+ messages in thread
From: Isaac Scott @ 2025-09-15 13:18 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rui Miguel Silva, Laurent Pinchart,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam
  Cc: linux-media, linux-kernel, imx, linux-arm-kernel, 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, 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 6afbedfe131e..d3424ad54b4e 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->num_data_lanes = v4l2_get_active_data_lanes(csis->source.pad,
+							  csis->bus.num_data_lanes);
+
 	ret = mipi_csis_calculate_params(csis, csis_fmt);
 	if (ret < 0)
 		goto err_unlock;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/3] media: v4l: Add helper to get number of active lanes via a pad
  2025-09-15 13:18 ` [PATCH v3 1/3] media: v4l: Add helper to get number of active lanes via a pad Isaac Scott
@ 2025-09-22  6:48   ` Sakari Ailus
  2025-09-22  7:16     ` Laurent Pinchart
  0 siblings, 1 reply; 9+ messages in thread
From: Sakari Ailus @ 2025-09-22  6:48 UTC (permalink / raw)
  To: Isaac Scott
  Cc: Mauro Carvalho Chehab, Rui Miguel Silva, Laurent Pinchart,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
	imx, linux-arm-kernel, Frank Li

Hi Isaac,

Thanks for the update.

On Mon, Sep 15, 2025 at 02:18:33PM +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.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 32 ++++++++++++++++++++++++++++++++
>  include/media/v4l2-common.h           | 21 +++++++++++++++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 6e585bc76367..2ce8407f1397 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -571,6 +571,38 @@ 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 max_data_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;
> +
> +	/*
> +	 * This relies on the mbus_config being zeroed at init time.
> +	 */
> +	if (!mbus_config.bus.mipi_csi2.num_data_lanes)

I'd either use the local variable for this (lanes) either all the time, or
not at all.

> +		return max_data_lanes;
> +
> +	lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> +
> +	if (lanes > max_data_lanes) {
> +		dev_dbg(sd->dev, "Active data lanes (%u) exceeds max (%u)\n",
> +			lanes, max_data_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..6af0695460ab 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -584,6 +584,27 @@ 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);

Is your tree up to date?

> +
> +/**
> + * v4l2_get_active_data_lanes - Get number of active data lanes from driver
> + *
> + * @pad: The transmitter's media pad.
> + * @max_data_lanes: The maximum number of active data lanes supported by
> + * 		    the MIPI CSI link in hardware. This can be configured
> + * 		    in device tree.

I'd remove the latter sentence. Alternatively, it needs to be improved:
there are other sources for this information than DT.

> + *
> + * This function is intended for obtaining the number of data lanes that are
> + * actively being used by the driver for a MIPI CSI-2 device on a given media pad.
> + * This information is derived from a mbus_config fetched from a device driver
> + * using the get_mbus_config v4l2_subdev pad op.
> + *
> + * Return:
> + * * >0: Number of active data lanes
> + * * %-EINVAL: Number of active data lanes is invalid, as it exceeds the maximum
> + *	       supported data lanes listed in device tree.
> + */
> +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad,
> +					unsigned int max_data_lanes);
>  #else
>  #define v4l2_get_link_freq(handler, mul, div)		\
>  	__v4l2_get_link_freq_ctrl(handler, mul, div)
> 

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt
  2025-09-15 13:18 ` [PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt Isaac Scott
@ 2025-09-22  6:49   ` Sakari Ailus
  2025-09-22  8:07     ` Laurent Pinchart
  0 siblings, 1 reply; 9+ messages in thread
From: Sakari Ailus @ 2025-09-22  6:49 UTC (permalink / raw)
  To: Isaac Scott
  Cc: Mauro Carvalho Chehab, Rui Miguel Silva, Laurent Pinchart,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
	imx, linux-arm-kernel

Hi Isaac,

On Mon, Sep 15, 2025 at 02:18:34PM +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 | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 2beb5f43c2c0..6afbedfe131e 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 num_data_lanes;
> +
>  	spinlock_t slock;	/* Protect events */
>  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
>  	struct dentry *debugfs_root;
> @@ -535,7 +537,7 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
>  	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
>  	val &= ~MIPI_CSIS_DPHY_CMN_CTRL_ENABLE;
>  	if (on) {
> -		mask = (1 << (csis->bus.num_data_lanes + 1)) - 1;
> +		mask = (1 << (csis->num_data_lanes + 1)) - 1;

Please use 1U or BIT() for bit-shifted values.

>  		val |= (mask & MIPI_CSIS_DPHY_CMN_CTRL_ENABLE);
>  	}
>  	mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL, val);
> @@ -586,7 +588,7 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
>  
>  	/* Calculate the line rate from the pixel rate. */
>  	link_freq = v4l2_get_link_freq(src_pad, csis_fmt->width,
> -				       csis->bus.num_data_lanes * 2);
> +				       csis->num_data_lanes * 2);
>  	if (link_freq < 0) {
>  		dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
>  			(int)link_freq);
> @@ -631,7 +633,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
>  				 const struct v4l2_mbus_framefmt *format,
>  				 const struct csis_pix_format *csis_fmt)
>  {
> -	int lanes = csis->bus.num_data_lanes;
> +	int lanes = csis->num_data_lanes;
>  	u32 val;
>  
>  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> @@ -1299,8 +1301,10 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
>  	}
>  
>  	csis->bus = vep.bus.mipi_csi2;
> +	csis->bus.num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;
> +	csis->num_data_lanes = csis->bus.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->num_data_lanes);
>  	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
>  
>  	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
> @@ -1498,7 +1502,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
>  	}
>  
>  	dev_info(dev, "lanes: %d, freq: %u\n",
> -		 csis->bus.num_data_lanes, csis->clk_frequency);
> +		 csis->num_data_lanes, csis->clk_frequency);
>  
>  	return 0;
>  
> 

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/3] media: v4l: Add helper to get number of active lanes via a pad
  2025-09-22  6:48   ` Sakari Ailus
@ 2025-09-22  7:16     ` Laurent Pinchart
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-09-22  7:16 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Isaac Scott, Mauro Carvalho Chehab, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
	imx, linux-arm-kernel, Frank Li

On Mon, Sep 22, 2025 at 09:48:39AM +0300, Sakari Ailus wrote:
> On Mon, Sep 15, 2025 at 02:18:33PM +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.
> > 
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-common.c | 32 ++++++++++++++++++++++++++++++++
> >  include/media/v4l2-common.h           | 21 +++++++++++++++++++++
> >  2 files changed, 53 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> > index 6e585bc76367..2ce8407f1397 100644
> > --- a/drivers/media/v4l2-core/v4l2-common.c
> > +++ b/drivers/media/v4l2-core/v4l2-common.c
> > @@ -571,6 +571,38 @@ 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 max_data_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;
> > +
> > +	/*
> > +	 * This relies on the mbus_config being zeroed at init time.
> > +	 */

This holds on a single line.

> > +	if (!mbus_config.bus.mipi_csi2.num_data_lanes)
> 
> I'd either use the local variable for this (lanes) either all the time, or
> not at all.
> 
> > +		return max_data_lanes;
> > +
> > +	lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
> > +
> > +	if (lanes > max_data_lanes) {
> > +		dev_dbg(sd->dev, "Active data lanes (%u) exceeds max (%u)\n",
> > +			lanes, max_data_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..6af0695460ab 100644
> > --- a/include/media/v4l2-common.h
> > +++ b/include/media/v4l2-common.h
> > @@ -584,6 +584,27 @@ 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);
> 
> Is your tree up to date?

We seem to be moving faster than we used to :-)

> > +
> > +/**
> > + * v4l2_get_active_data_lanes - Get number of active data lanes from driver
> > + *
> > + * @pad: The transmitter's media pad.
> > + * @max_data_lanes: The maximum number of active data lanes supported by
> > + * 		    the MIPI CSI link in hardware. This can be configured
> > + * 		    in device tree.
> 
> I'd remove the latter sentence. Alternatively, it needs to be improved:
> there are other sources for this information than DT.
> 
> > + *
> > + * This function is intended for obtaining the number of data lanes that are
> > + * actively being used by the driver for a MIPI CSI-2 device on a given media pad.
> > + * This information is derived from a mbus_config fetched from a device driver
> > + * using the get_mbus_config v4l2_subdev pad op.
> > + *
> > + * Return:
> > + * * >0: Number of active data lanes
> > + * * %-EINVAL: Number of active data lanes is invalid, as it exceeds the maximum
> > + *	       supported data lanes listed in device tree.

Drop "listed in the device tree" here too.

With those small issues fixed,

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

> > + */
> > +unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad,
> > +					unsigned int max_data_lanes);
> >  #else
> >  #define v4l2_get_link_freq(handler, mul, div)		\
> >  	__v4l2_get_link_freq_ctrl(handler, mul, div)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt
  2025-09-22  6:49   ` Sakari Ailus
@ 2025-09-22  8:07     ` Laurent Pinchart
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-09-22  8:07 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Isaac Scott, Mauro Carvalho Chehab, Rui Miguel Silva,
	Martin Kepplinger, Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
	imx, linux-arm-kernel

On Mon, Sep 22, 2025 at 09:49:27AM +0300, Sakari Ailus wrote:
> On Mon, Sep 15, 2025 at 02:18:34PM +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 | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 2beb5f43c2c0..6afbedfe131e 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 num_data_lanes;
> > +
> >  	spinlock_t slock;	/* Protect events */
> >  	struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
> >  	struct dentry *debugfs_root;
> > @@ -535,7 +537,7 @@ static void mipi_csis_system_enable(struct mipi_csis_device *csis, int on)
> >  	val = mipi_csis_read(csis, MIPI_CSIS_DPHY_CMN_CTRL);
> >  	val &= ~MIPI_CSIS_DPHY_CMN_CTRL_ENABLE;
> >  	if (on) {
> > -		mask = (1 << (csis->bus.num_data_lanes + 1)) - 1;
> > +		mask = (1 << (csis->num_data_lanes + 1)) - 1;
> 
> Please use 1U or BIT() for bit-shifted values.

BIT() isn't appropriate here. GENMASK(csis->num_data_lanes, 0) could be
fine, but this patch just changes the variable, I wouldn't insist in
fixing separate issues.

> >  		val |= (mask & MIPI_CSIS_DPHY_CMN_CTRL_ENABLE);
> >  	}
> >  	mipi_csis_write(csis, MIPI_CSIS_DPHY_CMN_CTRL, val);
> > @@ -586,7 +588,7 @@ static int mipi_csis_calculate_params(struct mipi_csis_device *csis,
> >  
> >  	/* Calculate the line rate from the pixel rate. */
> >  	link_freq = v4l2_get_link_freq(src_pad, csis_fmt->width,
> > -				       csis->bus.num_data_lanes * 2);
> > +				       csis->num_data_lanes * 2);
> >  	if (link_freq < 0) {
> >  		dev_err(csis->dev, "Unable to obtain link frequency: %d\n",
> >  			(int)link_freq);
> > @@ -631,7 +633,7 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis,
> >  				 const struct v4l2_mbus_framefmt *format,
> >  				 const struct csis_pix_format *csis_fmt)
> >  {
> > -	int lanes = csis->bus.num_data_lanes;
> > +	int lanes = csis->num_data_lanes;
> >  	u32 val;
> >  
> >  	val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL);
> > @@ -1299,8 +1301,10 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
> >  	}
> >  
> >  	csis->bus = vep.bus.mipi_csi2;
> > +	csis->bus.num_data_lanes = vep.bus.mipi_csi2.num_data_lanes;

That doesn't seem to be needed.

> > +	csis->num_data_lanes = csis->bus.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->num_data_lanes);

Neither is this change. What you print here is the number of connected
data lanes, not the number of effectively used lanes. You could change
it to 

	dev_dbg(csis->dev, "max data lanes: %u\n", csis->bus.num_data_lanes);

You'll want to also update the commit message, which I think needs some
improvement regardless.

> >  	dev_dbg(csis->dev, "flags: 0x%08x\n", csis->bus.flags);
> >  
> >  	asd = v4l2_async_nf_add_fwnode_remote(&csis->notifier, ep,
> > @@ -1498,7 +1502,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	dev_info(dev, "lanes: %d, freq: %u\n",
> > -		 csis->bus.num_data_lanes, csis->clk_frequency);
> > +		 csis->num_data_lanes, csis->clk_frequency);

Drop this change too. This message is actually redundant, the number of
lanes is printed in mipi_csis_async_register(). You could submit a
separate patch to remove this, possibly replacing it with a dev_dbg() in
mipi_csis_parse_dt() to print clk_frequency.

> >  
> >  	return 0;
> >  

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/3] media: imx-mipi-csis: Get number of active lanes via mbus_config
  2025-09-15 13:18 ` [PATCH v3 3/3] media: imx-mipi-csis: Get number of active lanes via mbus_config Isaac Scott
@ 2025-09-22  8:09   ` Laurent Pinchart
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2025-09-22  8:09 UTC (permalink / raw)
  To: Isaac Scott
  Cc: Mauro Carvalho Chehab, Rui Miguel Silva, Martin Kepplinger,
	Purism Kernel Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, linux-media, linux-kernel,
	imx, linux-arm-kernel

Hi Isaac,

Thank you for the patch.

On Mon, Sep 15, 2025 at 02:18:35PM +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
> differing number of lanes to device tree, and use that number of active
> lanes.

It would be useful to explain there that the difference would be caused
by the source using less data lanes than the number of connected lanes
on the board.

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

Is this still true ?

> 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 6afbedfe131e..d3424ad54b4e 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->num_data_lanes = v4l2_get_active_data_lanes(csis->source.pad,
> +							  csis->bus.num_data_lanes);
> +

If the function returns an error you'll have a problem.

>  	ret = mipi_csis_calculate_params(csis, csis_fmt);
>  	if (ret < 0)
>  		goto err_unlock;

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-09-22  8:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-15 13:18 [PATCH v3 0/3] media: imx-mipi-csis: Get the number of active lanes from mbus_config Isaac Scott
2025-09-15 13:18 ` [PATCH v3 1/3] media: v4l: Add helper to get number of active lanes via a pad Isaac Scott
2025-09-22  6:48   ` Sakari Ailus
2025-09-22  7:16     ` Laurent Pinchart
2025-09-15 13:18 ` [PATCH v3 2/3] media: imx-mipi-csis: Store the number of data_lanes configured in dt Isaac Scott
2025-09-22  6:49   ` Sakari Ailus
2025-09-22  8:07     ` Laurent Pinchart
2025-09-15 13:18 ` [PATCH v3 3/3] media: imx-mipi-csis: Get number of active lanes via mbus_config Isaac Scott
2025-09-22  8:09   ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox