linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/4] media: imx-mipi-csis: Get the number of active lanes from mbus_config
@ 2025-10-22 10:22 Isaac Scott
  2025-10-22 10:22 ` [PATCH v5 1/4] media: v4l: Add helper to get number of active lanes via a pad Isaac Scott
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Isaac Scott @ 2025-10-22 10:22 UTC (permalink / raw)
  To: mchehab
  Cc: rmfrfs, laurent.pinchart, martink, kernel, shawnguo, s.hauer,
	kernel, festevam, sakari.ailus, 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.17.2, compile tested on media/next.

---------

Changes v4 -> v5:

- Collected tag for patch 2/4
- Updated commit messages on 3/4 and 4/4

Changes v3 -> v4:

- Updated base tree to media/next
- Used local 'lanes' variable consistently in
  v4l2_get_active_data_lanes()
- Removed device tree references in documentation
- Made comment a single line
- Collected tag for patch 1/4
- Removed unnecessary num_data_lanes assignments in async_register in
  imx-mipi-csis
- Removed some debug print changes
- Checked return value of v4l2_get_active_data_lanes() before assignment
  to csis->num_data_lanes
- Added patch to move redundant debug print in mipi_csis_probe()

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

Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
---
Isaac Scott (4):
  media: v4l: Add helper to get number of active lanes via a pad
  media: imx-mipi-csis: Move redundant debug print in probe
  media: imx-mipi-csis: Add num_data_lanes to mipi_csis_device
  media: imx-mipi-csis: Support active data lanes differing from maximum

 drivers/media/platform/nxp/imx-mipi-csis.c | 19 ++++++++------
 drivers/media/v4l2-core/v4l2-common.c      | 29 ++++++++++++++++++++++
 include/media/v4l2-common.h                | 20 +++++++++++++++
 3 files changed, 61 insertions(+), 7 deletions(-)

-- 
2.43.0



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

* [PATCH v5 1/4] media: v4l: Add helper to get number of active lanes via a pad
  2025-10-22 10:22 [PATCH v5 0/4] media: imx-mipi-csis: Get the number of active lanes from mbus_config Isaac Scott
@ 2025-10-22 10:22 ` Isaac Scott
  2025-10-22 10:22 ` [PATCH v5 2/4] media: imx-mipi-csis: Move redundant debug print in probe Isaac Scott
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Isaac Scott @ 2025-10-22 10:22 UTC (permalink / raw)
  To: mchehab
  Cc: rmfrfs, laurent.pinchart, martink, kernel, shawnguo, s.hauer,
	kernel, festevam, sakari.ailus, 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 maximum
number of lanes available.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 29 +++++++++++++++++++++++++++
 include/media/v4l2-common.h           | 20 ++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index b367d479d6b3..2b4cec25e751 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -573,6 +573,35 @@ s64 v4l2_get_link_freq(const 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);
+
+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 */
+	lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
+	if (!lanes)
+		return max_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
 
 /*
diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
index 5c0a7f6b5bb6..950df46cb27a 100644
--- a/include/media/v4l2-common.h
+++ b/include/media/v4l2-common.h
@@ -581,6 +581,26 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
 #ifdef CONFIG_MEDIA_CONTROLLER
 s64 v4l2_get_link_freq(const 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 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.
+ */
+unsigned int v4l2_get_active_data_lanes(const struct media_pad *pad,
+					unsigned int max_data_lanes);
 #endif
 
 void v4l2_simplify_fraction(u32 *numerator, u32 *denominator,
-- 
2.43.0



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

* [PATCH v5 2/4] media: imx-mipi-csis: Move redundant debug print in probe
  2025-10-22 10:22 [PATCH v5 0/4] media: imx-mipi-csis: Get the number of active lanes from mbus_config Isaac Scott
  2025-10-22 10:22 ` [PATCH v5 1/4] media: v4l: Add helper to get number of active lanes via a pad Isaac Scott
@ 2025-10-22 10:22 ` Isaac Scott
  2025-10-26 21:05   ` Laurent Pinchart
  2025-10-22 10:22 ` [PATCH v5 3/4] media: imx-mipi-csis: Add num_data_lanes to mipi_csis_device Isaac Scott
  2025-10-22 10:22 ` [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum Isaac Scott
  3 siblings, 1 reply; 12+ messages in thread
From: Isaac Scott @ 2025-10-22 10:22 UTC (permalink / raw)
  To: mchehab
  Cc: rmfrfs, laurent.pinchart, martink, kernel, shawnguo, s.hauer,
	kernel, festevam, sakari.ailus, linux-media, linux-kernel, imx,
	linux-arm-kernel, Frank.Li, Isaac Scott

The number of data lanes is already printed as part of
mipi_csis_async_register(), making the first part of this print
redundant. Remove the redundant print, and move the debug print for
clock frequency to mipi_csis_parse_dt().

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

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index d5de7854f579..7c2a679dca2e 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -1481,6 +1481,7 @@ static int mipi_csis_parse_dt(struct mipi_csis_device *csis)
 	struct device_node *node = csis->dev->of_node;
 
 	of_property_read_u32(node, "clock-frequency", &csis->clk_frequency);
+	dev_dbg(csis->dev, "clock frequency: %u\n", csis->clk_frequency);
 
 	csis->num_channels = 1;
 	of_property_read_u32(node, "fsl,num-channels", &csis->num_channels);
@@ -1566,9 +1567,6 @@ static int mipi_csis_probe(struct platform_device *pdev)
 			goto err_unregister_all;
 	}
 
-	dev_info(dev, "lanes: %d, freq: %u\n",
-		 csis->bus.num_data_lanes, csis->clk_frequency);
-
 	return 0;
 
 err_unregister_all:
-- 
2.43.0



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

* [PATCH v5 3/4] media: imx-mipi-csis: Add num_data_lanes to mipi_csis_device
  2025-10-22 10:22 [PATCH v5 0/4] media: imx-mipi-csis: Get the number of active lanes from mbus_config Isaac Scott
  2025-10-22 10:22 ` [PATCH v5 1/4] media: v4l: Add helper to get number of active lanes via a pad Isaac Scott
  2025-10-22 10:22 ` [PATCH v5 2/4] media: imx-mipi-csis: Move redundant debug print in probe Isaac Scott
@ 2025-10-22 10:22 ` Isaac Scott
  2025-10-22 14:59   ` Frank Li
  2025-10-26 21:07   ` Laurent Pinchart
  2025-10-22 10:22 ` [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum Isaac Scott
  3 siblings, 2 replies; 12+ messages in thread
From: Isaac Scott @ 2025-10-22 10:22 UTC (permalink / raw)
  To: mchehab
  Cc: rmfrfs, laurent.pinchart, martink, kernel, shawnguo, s.hauer,
	kernel, festevam, sakari.ailus, linux-media, linux-kernel, imx,
	linux-arm-kernel, Frank.Li, Isaac Scott

Add the num_data_lanes field to the mipi_csis_device struct, and set it
equal to csis->bus.num_data_lanes. This is in preparation to support
cases when the data lanes actively used differs from the maximum
supported data lanes.

No functional changes intended by this commit.

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

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 7c2a679dca2e..838a1ad123b5 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -351,6 +351,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;
@@ -573,7 +575,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);
@@ -623,7 +625,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(csis->source.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);
@@ -668,7 +670,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);
@@ -1366,8 +1368,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
 	}
 
 	csis->bus = vep.bus.mipi_csi2;
+	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, "max data lanes: %d\n", csis->bus.num_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



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

* [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum
  2025-10-22 10:22 [PATCH v5 0/4] media: imx-mipi-csis: Get the number of active lanes from mbus_config Isaac Scott
                   ` (2 preceding siblings ...)
  2025-10-22 10:22 ` [PATCH v5 3/4] media: imx-mipi-csis: Add num_data_lanes to mipi_csis_device Isaac Scott
@ 2025-10-22 10:22 ` Isaac Scott
  2025-10-22 11:13   ` Sakari Ailus
                     ` (2 more replies)
  3 siblings, 3 replies; 12+ messages in thread
From: Isaac Scott @ 2025-10-22 10:22 UTC (permalink / raw)
  To: mchehab
  Cc: rmfrfs, laurent.pinchart, martink, kernel, shawnguo, s.hauer,
	kernel, festevam, sakari.ailus, linux-media, linux-kernel, imx,
	linux-arm-kernel, Frank.Li, Isaac Scott

Call on v4l2_get_active_data_lanes() to check if the driver reports that
the number of lanes actively used by the MIPI CSI transmitter differs to
the maximum defined in device tree.

If the number of active data lanes reported by the driver is invalid, or
the operation is not supported, fall back to the number of allowed data
lanes.

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

diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
index 838a1ad123b5..637ef6e614fa 100644
--- a/drivers/media/platform/nxp/imx-mipi-csis.c
+++ b/drivers/media/platform/nxp/imx-mipi-csis.c
@@ -1034,6 +1034,10 @@ 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);
 
+	ret = v4l2_get_active_data_lanes(csis->source.pad,
+					 csis->bus.num_data_lanes);
+	csis->num_data_lanes = ret < 0 ? csis->bus.num_data_lanes : ret;
+
 	ret = mipi_csis_calculate_params(csis, csis_fmt);
 	if (ret < 0)
 		goto err_unlock;
-- 
2.43.0



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

* Re: [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum
  2025-10-22 10:22 ` [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum Isaac Scott
@ 2025-10-22 11:13   ` Sakari Ailus
  2025-10-26 21:11     ` Laurent Pinchart
  2025-10-22 15:00   ` Frank Li
  2025-10-22 16:21   ` Bryan O'Donoghue
  2 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2025-10-22 11:13 UTC (permalink / raw)
  To: Isaac Scott
  Cc: mchehab, rmfrfs, laurent.pinchart, martink, kernel, shawnguo,
	s.hauer, kernel, festevam, linux-media, linux-kernel, imx,
	linux-arm-kernel, Frank.Li

Hi Isaac,

On Wed, Oct 22, 2025 at 11:22:28AM +0100, Isaac Scott wrote:
> Call on v4l2_get_active_data_lanes() to check if the driver reports that
> the number of lanes actively used by the MIPI CSI transmitter differs to
> the maximum defined in device tree.
> 
> If the number of active data lanes reported by the driver is invalid, or
> the operation is not supported, fall back to the number of allowed data
> lanes.
> 
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 838a1ad123b5..637ef6e614fa 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -1034,6 +1034,10 @@ 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);
>  
> +	ret = v4l2_get_active_data_lanes(csis->source.pad,
> +					 csis->bus.num_data_lanes);
> +	csis->num_data_lanes = ret < 0 ? csis->bus.num_data_lanes : ret;

I guess this works but should we return an error here instead?

Alternatively, the function could always return some number of lanes as
well (with a printed warning on error). I think I'd do the former though.

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

-- 
Regards,

Sakari Ailus


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

* Re: [PATCH v5 3/4] media: imx-mipi-csis: Add num_data_lanes to mipi_csis_device
  2025-10-22 10:22 ` [PATCH v5 3/4] media: imx-mipi-csis: Add num_data_lanes to mipi_csis_device Isaac Scott
@ 2025-10-22 14:59   ` Frank Li
  2025-10-26 21:07   ` Laurent Pinchart
  1 sibling, 0 replies; 12+ messages in thread
From: Frank Li @ 2025-10-22 14:59 UTC (permalink / raw)
  To: Isaac Scott
  Cc: mchehab, rmfrfs, laurent.pinchart, martink, kernel, shawnguo,
	s.hauer, kernel, festevam, sakari.ailus, linux-media,
	linux-kernel, imx, linux-arm-kernel

On Wed, Oct 22, 2025 at 11:22:27AM +0100, Isaac Scott wrote:
> Add the num_data_lanes field to the mipi_csis_device struct, and set it
> equal to csis->bus.num_data_lanes. This is in preparation to support
> cases when the data lanes actively used differs from the maximum
> supported data lanes.
>
> No functional changes intended by this commit.
>
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 7c2a679dca2e..838a1ad123b5 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -351,6 +351,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;
> @@ -573,7 +575,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);
> @@ -623,7 +625,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(csis->source.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);
> @@ -668,7 +670,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);
> @@ -1366,8 +1368,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
>  	}
>
>  	csis->bus = vep.bus.mipi_csi2;
> +	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, "max data lanes: %d\n", csis->bus.num_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
>


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

* Re: [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum
  2025-10-22 10:22 ` [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum Isaac Scott
  2025-10-22 11:13   ` Sakari Ailus
@ 2025-10-22 15:00   ` Frank Li
  2025-10-22 16:21   ` Bryan O'Donoghue
  2 siblings, 0 replies; 12+ messages in thread
From: Frank Li @ 2025-10-22 15:00 UTC (permalink / raw)
  To: Isaac Scott
  Cc: mchehab, rmfrfs, laurent.pinchart, martink, kernel, shawnguo,
	s.hauer, kernel, festevam, sakari.ailus, linux-media,
	linux-kernel, imx, linux-arm-kernel

On Wed, Oct 22, 2025 at 11:22:28AM +0100, Isaac Scott wrote:
> Call on v4l2_get_active_data_lanes() to check if the driver reports that
> the number of lanes actively used by the MIPI CSI transmitter differs to
> the maximum defined in device tree.
>
> If the number of active data lanes reported by the driver is invalid, or
> the operation is not supported, fall back to the number of allowed data
> lanes.
>
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/media/platform/nxp/imx-mipi-csis.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 838a1ad123b5..637ef6e614fa 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -1034,6 +1034,10 @@ 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);
>
> +	ret = v4l2_get_active_data_lanes(csis->source.pad,
> +					 csis->bus.num_data_lanes);
> +	csis->num_data_lanes = ret < 0 ? csis->bus.num_data_lanes : ret;
> +
>  	ret = mipi_csis_calculate_params(csis, csis_fmt);
>  	if (ret < 0)
>  		goto err_unlock;
> --
> 2.43.0
>


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

* Re: [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum
  2025-10-22 10:22 ` [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum Isaac Scott
  2025-10-22 11:13   ` Sakari Ailus
  2025-10-22 15:00   ` Frank Li
@ 2025-10-22 16:21   ` Bryan O'Donoghue
  2 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2025-10-22 16:21 UTC (permalink / raw)
  To: Isaac Scott, mchehab
  Cc: rmfrfs, laurent.pinchart, martink, kernel, shawnguo, s.hauer,
	kernel, festevam, sakari.ailus, linux-media, linux-kernel, imx,
	linux-arm-kernel, Frank.Li

On 22/10/2025 11:22, Isaac Scott wrote:
> Call on v4l2_get_active_data_lanes() to check if the driver reports that
> the number of lanes actively used by the MIPI CSI transmitter differs to
> the maximum defined in device tree.
> 
> If the number of active data lanes reported by the driver is invalid, or
> the operation is not supported, fall back to the number of allowed data
> lanes.
> 
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>   drivers/media/platform/nxp/imx-mipi-csis.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 838a1ad123b5..637ef6e614fa 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -1034,6 +1034,10 @@ 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);
> 
> +	ret = v4l2_get_active_data_lanes(csis->source.pad,
> +					 csis->bus.num_data_lanes);
> +	csis->num_data_lanes = ret < 0 ? csis->bus.num_data_lanes : ret;

The function you've added can return -EINVAL;

Isn't that a fundamental error at this point ? It would make sense to 
trap -EINVAL and refuse to go further.

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



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

* Re: [PATCH v5 2/4] media: imx-mipi-csis: Move redundant debug print in probe
  2025-10-22 10:22 ` [PATCH v5 2/4] media: imx-mipi-csis: Move redundant debug print in probe Isaac Scott
@ 2025-10-26 21:05   ` Laurent Pinchart
  0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2025-10-26 21:05 UTC (permalink / raw)
  To: Isaac Scott
  Cc: mchehab, rmfrfs, martink, kernel, shawnguo, s.hauer, kernel,
	festevam, sakari.ailus, linux-media, linux-kernel, imx,
	linux-arm-kernel, Frank.Li

Hi Isaac,

Thank you for the patch.

On Wed, Oct 22, 2025 at 11:22:26AM +0100, Isaac Scott wrote:
> The number of data lanes is already printed as part of
> mipi_csis_async_register(), making the first part of this print
> redundant. Remove the redundant print, and move the debug print for
> clock frequency to mipi_csis_parse_dt().

There's a bit of room for improvement in the commit message. If you read
it in isolation, without looking at the code change first, you'll see
things like "this print" being quite unclear. No need to change it, just
keep it in mind for your next patch series :-)

> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>

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

> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index d5de7854f579..7c2a679dca2e 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -1481,6 +1481,7 @@ static int mipi_csis_parse_dt(struct mipi_csis_device *csis)
>  	struct device_node *node = csis->dev->of_node;
>  
>  	of_property_read_u32(node, "clock-frequency", &csis->clk_frequency);
> +	dev_dbg(csis->dev, "clock frequency: %u\n", csis->clk_frequency);
>  
>  	csis->num_channels = 1;
>  	of_property_read_u32(node, "fsl,num-channels", &csis->num_channels);
> @@ -1566,9 +1567,6 @@ static int mipi_csis_probe(struct platform_device *pdev)
>  			goto err_unregister_all;
>  	}
>  
> -	dev_info(dev, "lanes: %d, freq: %u\n",
> -		 csis->bus.num_data_lanes, csis->clk_frequency);
> -
>  	return 0;
>  
>  err_unregister_all:

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v5 3/4] media: imx-mipi-csis: Add num_data_lanes to mipi_csis_device
  2025-10-22 10:22 ` [PATCH v5 3/4] media: imx-mipi-csis: Add num_data_lanes to mipi_csis_device Isaac Scott
  2025-10-22 14:59   ` Frank Li
@ 2025-10-26 21:07   ` Laurent Pinchart
  1 sibling, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2025-10-26 21:07 UTC (permalink / raw)
  To: Isaac Scott
  Cc: mchehab, rmfrfs, martink, kernel, shawnguo, s.hauer, kernel,
	festevam, sakari.ailus, linux-media, linux-kernel, imx,
	linux-arm-kernel, Frank.Li

Hi Isaac,

Thank you for the patch.

On Wed, Oct 22, 2025 at 11:22:27AM +0100, Isaac Scott wrote:
> Add the num_data_lanes field to the mipi_csis_device struct, and set it
> equal to csis->bus.num_data_lanes. This is in preparation to support
> cases when the data lanes actively used differs from the maximum
> supported data lanes.
> 
> No functional changes intended by this commit.
> 
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>

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

> ---
>  drivers/media/platform/nxp/imx-mipi-csis.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> index 7c2a679dca2e..838a1ad123b5 100644
> --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> @@ -351,6 +351,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;
> @@ -573,7 +575,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);
> @@ -623,7 +625,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(csis->source.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);
> @@ -668,7 +670,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);
> @@ -1366,8 +1368,9 @@ static int mipi_csis_async_register(struct mipi_csis_device *csis)
>  	}
>  
>  	csis->bus = vep.bus.mipi_csi2;
> +	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, "max data lanes: %d\n", csis->bus.num_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


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

* Re: [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum
  2025-10-22 11:13   ` Sakari Ailus
@ 2025-10-26 21:11     ` Laurent Pinchart
  0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2025-10-26 21:11 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Isaac Scott, mchehab, rmfrfs, martink, kernel, shawnguo, s.hauer,
	kernel, festevam, linux-media, linux-kernel, imx,
	linux-arm-kernel, Frank.Li

On Wed, Oct 22, 2025 at 02:13:09PM +0300, Sakari Ailus wrote:
> On Wed, Oct 22, 2025 at 11:22:28AM +0100, Isaac Scott wrote:
> > Call on v4l2_get_active_data_lanes() to check if the driver reports that
> > the number of lanes actively used by the MIPI CSI transmitter differs to
> > the maximum defined in device tree.
> > 
> > If the number of active data lanes reported by the driver is invalid, or
> > the operation is not supported, fall back to the number of allowed data
> > lanes.
> > 
> > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > ---
> >  drivers/media/platform/nxp/imx-mipi-csis.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c
> > index 838a1ad123b5..637ef6e614fa 100644
> > --- a/drivers/media/platform/nxp/imx-mipi-csis.c
> > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c
> > @@ -1034,6 +1034,10 @@ 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);
> >  
> > +	ret = v4l2_get_active_data_lanes(csis->source.pad,
> > +					 csis->bus.num_data_lanes);
> > +	csis->num_data_lanes = ret < 0 ? csis->bus.num_data_lanes : ret;
> 
> I guess this works but should we return an error here instead?
> 
> Alternatively, the function could always return some number of lanes as
> well (with a printed warning on error). I think I'd do the former though.

Agreed, I would return an error.

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

-- 
Regards,

Laurent Pinchart


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

end of thread, other threads:[~2025-10-26 21:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-22 10:22 [PATCH v5 0/4] media: imx-mipi-csis: Get the number of active lanes from mbus_config Isaac Scott
2025-10-22 10:22 ` [PATCH v5 1/4] media: v4l: Add helper to get number of active lanes via a pad Isaac Scott
2025-10-22 10:22 ` [PATCH v5 2/4] media: imx-mipi-csis: Move redundant debug print in probe Isaac Scott
2025-10-26 21:05   ` Laurent Pinchart
2025-10-22 10:22 ` [PATCH v5 3/4] media: imx-mipi-csis: Add num_data_lanes to mipi_csis_device Isaac Scott
2025-10-22 14:59   ` Frank Li
2025-10-26 21:07   ` Laurent Pinchart
2025-10-22 10:22 ` [PATCH v5 4/4] media: imx-mipi-csis: Support active data lanes differing from maximum Isaac Scott
2025-10-22 11:13   ` Sakari Ailus
2025-10-26 21:11     ` Laurent Pinchart
2025-10-22 15:00   ` Frank Li
2025-10-22 16:21   ` Bryan O'Donoghue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).