[PATCH v3 1/2] media: tc358743: fix connected/active CSI-2 lane reporting

[PATCH v3 1/2] media: tc358743: fix connected/active CSI-2 lane reporting

[Philipp Zabel]

g_mbus_config was supposed to indicate all supported lane numbers, not
only the number of those currently in active use. Since the TC358743
can dynamically reduce the number of active lanes if the required
bandwidth allows for it, report all lane numbers up to the connected
number of lanes as supported in pdata mode.
In device tree mode, do not report lane count and clock mode at all, as
the receiver driver can determine these from the device tree.

To allow communicating the number of currently active lanes, add a new
bitfield to the v4l2_mbus_config flags. This is a temporary fix, to be
used only until a better solution is found.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
Changes since v2 [1]:
 - Rebased onto media/master

[1] https://patchwork.kernel.org/patch/9964141/
---
 drivers/media/i2c/tc358743.c  | 30 ++++++++++++++++--------------
 include/media/v4l2-mediabus.h |  9 +++++++++
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 00dc930e049f..b1e1ed4d9e0c 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1606,28 +1606,29 @@ static int tc358743_g_mbus_config(struct v4l2_subdev *sd,
 			     struct v4l2_mbus_config *cfg)
 {
 	struct tc358743_state *state = to_state(sd);
+	const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
+
+	if (state->csi_lanes_in_use > state->bus.num_data_lanes)
+		return -EINVAL;
 
 	cfg->type = V4L2_MBUS_CSI2_DPHY;
+	cfg->flags = (state->csi_lanes_in_use << __ffs(mask)) & mask;
 
-	/* Support for non-continuous CSI-2 clock is missing in the driver */
-	cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+	/* In DT mode, only report the number of active lanes */
+	if (sd->dev->of_node)
+		return 0;
 
-	switch (state->csi_lanes_in_use) {
-	case 1:
+	/* Support for non-continuous CSI-2 clock is missing in pdata mode */
+	cfg->flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+
+	if (state->bus.num_data_lanes > 0)
 		cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
-		break;
-	case 2:
+	if (state->bus.num_data_lanes > 1)
 		cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
-		break;
-	case 3:
+	if (state->bus.num_data_lanes > 2)
 		cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
-		break;
-	case 4:
+	if (state->bus.num_data_lanes > 3)
 		cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
-		break;
-	default:
-		return -EINVAL;
-	}
 
 	return 0;
 }
@@ -2053,6 +2054,7 @@ static int tc358743_probe(struct i2c_client *client,
 	if (pdata) {
 		state->pdata = *pdata;
 		state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+		state->bus.num_data_lanes = 4;
 	} else {
 		err = tc358743_probe_of(state);
 		if (err == -ENODEV)
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 66cb746ceeb5..e127e3d1740e 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -71,6 +71,15 @@
 					 V4L2_MBUS_CSI2_CHANNEL_2 | \
 					 V4L2_MBUS_CSI2_CHANNEL_3)
 
+/*
+ * Number of lanes in use, 0 == use all available lanes (default)
+ *
+ * This is a temporary fix for devices that need to reduce the number of active
+ * lanes for certain modes, until g_mbus_config() can be replaced with a better
+ * solution.
+ */
+#define V4L2_MBUS_CSI2_LANE_MASK                (0xf << 10)
+
 /**
  * enum v4l2_mbus_type - media bus type
  * @V4L2_MBUS_UNKNOWN:	unknown bus type, no V4L2 mediabus configuration
-- 
2.19.1

[PATCH v3 2/2] media: imx: ask source subdevice for number of active data lanes

[Philipp Zabel]

Temporarily use g_mbus_config() to determine the number of active data
lanes used by the transmitter. If g_mbus_config is not supported or
does not return the number of active lines, default to using all
connected data lines.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
Changes since v2 [1]:
 - Rebased onto media/master

[1] https://patchwork.kernel.org/patch/9964151/
---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 6a1cee55a49b..ae91f0d138f3 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -135,10 +135,8 @@ static void csi2_enable(struct csi2_dev *csi2, bool enable)
 	}
 }
 
-static void csi2_set_lanes(struct csi2_dev *csi2)
+static void csi2_set_lanes(struct csi2_dev *csi2, int lanes)
 {
-	int lanes = csi2->bus.num_data_lanes;
-
 	writel(lanes - 1, csi2->base + CSI2_N_LANES);
 }
 
@@ -301,6 +299,9 @@ static void csi2ipu_gasket_init(struct csi2_dev *csi2)
 
 static int csi2_start(struct csi2_dev *csi2)
 {
+	const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
+	struct v4l2_mbus_config cfg;
+	int lanes = 0;
 	int ret;
 
 	ret = clk_prepare_enable(csi2->pix_clk);
@@ -316,7 +317,10 @@ static int csi2_start(struct csi2_dev *csi2)
 		goto err_disable_clk;
 
 	/* Step 4 */
-	csi2_set_lanes(csi2);
+	ret = v4l2_subdev_call(csi2->src_sd, video, g_mbus_config, &cfg);
+	if (ret == 0)
+		lanes = (cfg.flags & mask) >> __ffs(mask);
+	csi2_set_lanes(csi2, lanes ?: csi2->bus.num_data_lanes);
 	csi2_enable(csi2, true);
 
 	/* Step 5 */
-- 
2.19.1

Re: [PATCH v3 1/2] media: tc358743: fix connected/active CSI-2 lane reporting

[Hans Verkuil]

On 12/5/18 5:14 PM, Philipp Zabel wrote:
> g_mbus_config was supposed to indicate all supported lane numbers, not
> only the number of those currently in active use. Since the TC358743
> can dynamically reduce the number of active lanes if the required
> bandwidth allows for it, report all lane numbers up to the connected
> number of lanes as supported in pdata mode.
> In device tree mode, do not report lane count and clock mode at all, as
> the receiver driver can determine these from the device tree.
> 
> To allow communicating the number of currently active lanes, add a new
> bitfield to the v4l2_mbus_config flags. This is a temporary fix, to be
> used only until a better solution is found.

I don't like this hack at all.

Just introduce a new g_csi_active_lanes() op.

I really want to get rid of g/s_mbus_config, and I rather not introduce a call
to g_mbus_config in imx.

Regards,

	Hans

> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
> ---
> Changes since v2 [1]:
>  - Rebased onto media/master
> 
> [1] https://patchwork.kernel.org/patch/9964141/
> ---
>  drivers/media/i2c/tc358743.c  | 30 ++++++++++++++++--------------
>  include/media/v4l2-mediabus.h |  9 +++++++++
>  2 files changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 00dc930e049f..b1e1ed4d9e0c 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -1606,28 +1606,29 @@ static int tc358743_g_mbus_config(struct v4l2_subdev *sd,
>  			     struct v4l2_mbus_config *cfg)
>  {
>  	struct tc358743_state *state = to_state(sd);
> +	const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
> +
> +	if (state->csi_lanes_in_use > state->bus.num_data_lanes)
> +		return -EINVAL;
>  
>  	cfg->type = V4L2_MBUS_CSI2_DPHY;
> +	cfg->flags = (state->csi_lanes_in_use << __ffs(mask)) & mask;
>  
> -	/* Support for non-continuous CSI-2 clock is missing in the driver */
> -	cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> +	/* In DT mode, only report the number of active lanes */
> +	if (sd->dev->of_node)
> +		return 0;
>  
> -	switch (state->csi_lanes_in_use) {
> -	case 1:
> +	/* Support for non-continuous CSI-2 clock is missing in pdata mode */
> +	cfg->flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> +
> +	if (state->bus.num_data_lanes > 0)
>  		cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
> -		break;
> -	case 2:
> +	if (state->bus.num_data_lanes > 1)
>  		cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
> -		break;
> -	case 3:
> +	if (state->bus.num_data_lanes > 2)
>  		cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
> -		break;
> -	case 4:
> +	if (state->bus.num_data_lanes > 3)
>  		cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
>  
>  	return 0;
>  }
> @@ -2053,6 +2054,7 @@ static int tc358743_probe(struct i2c_client *client,
>  	if (pdata) {
>  		state->pdata = *pdata;
>  		state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> +		state->bus.num_data_lanes = 4;
>  	} else {
>  		err = tc358743_probe_of(state);
>  		if (err == -ENODEV)
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index 66cb746ceeb5..e127e3d1740e 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -71,6 +71,15 @@
>  					 V4L2_MBUS_CSI2_CHANNEL_2 | \
>  					 V4L2_MBUS_CSI2_CHANNEL_3)
>  
> +/*
> + * Number of lanes in use, 0 == use all available lanes (default)
> + *
> + * This is a temporary fix for devices that need to reduce the number of active
> + * lanes for certain modes, until g_mbus_config() can be replaced with a better
> + * solution.
> + */
> +#define V4L2_MBUS_CSI2_LANE_MASK                (0xf << 10)
> +
>  /**
>   * enum v4l2_mbus_type - media bus type
>   * @V4L2_MBUS_UNKNOWN:	unknown bus type, no V4L2 mediabus configuration
>