diff mbox series

[v2,6/6] media: rcar-csi2: Negotiate data lanes number

Message ID 20200415105004.2497356-7-jacopo+renesas@jmondi.org (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series v4l2-subdev: Introduce get_mbus_format pad op | expand

Commit Message

Jacopo Mondi April 15, 2020, 10:50 a.m. UTC 
Use the newly introduced get_mbus_config() subdevice pad operation to
retrieve the remote subdevice MIPI CSI-2 bus configuration and configure
the number of active data lanes accordingly.

In order to be able to call the remote subdevice operation cache the
index of the remote pad connected to the single CSI-2 input port.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---

Niklas I've not been able to fully address comments as -ENOIOCTL command
has to be handled separately as reported by email.

---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 53 +++++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)

--
2.26.0

Comments

Laurent Pinchart April 20, 2020, 2:08 a.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Wed, Apr 15, 2020 at 12:50:03PM +0200, Jacopo Mondi wrote:
> Use the newly introduced get_mbus_config() subdevice pad operation to
> retrieve the remote subdevice MIPI CSI-2 bus configuration and configure
> the number of active data lanes accordingly.
> 
> In order to be able to call the remote subdevice operation cache the
> index of the remote pad connected to the single CSI-2 input port.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> 
> Niklas I've not been able to fully address comments as -ENOIOCTL command
> has to be handled separately as reported by email.
> 
> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 53 +++++++++++++++++++--
>  1 file changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index faa9fb23a2e9..0061d5ff37e3 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -363,6 +363,7 @@ struct rcar_csi2 {
>  	struct v4l2_async_notifier notifier;
>  	struct v4l2_async_subdev asd;
>  	struct v4l2_subdev *remote;
> +	unsigned short remote_pad;

You can make this an unsigned int, it will be more efficient and won't
consume more memory.

> 
>  	struct v4l2_mbus_framefmt mf;
> 
> @@ -371,6 +372,7 @@ struct rcar_csi2 {
> 
>  	unsigned short lanes;
>  	unsigned char lane_swap[4];
> +	unsigned short active_lanes;
>  };
> 
>  static inline struct rcar_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
> @@ -414,7 +416,7 @@ static int rcsi2_wait_phy_start(struct rcar_csi2 *priv)
> 
>  	/* Wait for the clock and data lanes to enter LP-11 state. */
>  	for (timeout = 0; timeout <= 20; timeout++) {
> -		const u32 lane_mask = (1 << priv->lanes) - 1;
> +		const u32 lane_mask = (1 << priv->active_lanes) - 1;
> 
>  		if ((rcsi2_read(priv, PHCLM_REG) & PHCLM_STOPSTATECKL)  &&
>  		    (rcsi2_read(priv, PHDLM_REG) & lane_mask) == lane_mask)
> @@ -471,11 +473,49 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp)
>  	 * bps = link_freq * 2
>  	 */
>  	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
> -	do_div(mbps, priv->lanes * 1000000);
> +	do_div(mbps, priv->active_lanes * 1000000);
> 
>  	return mbps;
>  }
> 
> +static int rcsi2_config_active_lanes(struct rcar_csi2 *priv)
> +{
> +	struct v4l2_mbus_pad_config mbus_config;

 = { 0 };

here and you can remove the memset().

> +	int ret;
> +
> +	priv->active_lanes = priv->lanes;
> +
> +	memset(&mbus_config, 0, sizeof(mbus_config));
> +	ret = v4l2_subdev_call(priv->remote, pad, get_mbus_config,
> +			       priv->remote_pad, &mbus_config);
> +	if (ret == -ENOIOCTLCMD) {
> +		dev_dbg(priv->dev, "No remote mbus configuration available\n");
> +		return 0;
> +	}
> +
> +	if (ret) {
> +		dev_err(priv->dev, "Failed to get remote mbus configuration\n");

In the unlikely case that the source implements the operation but only
supports it on other pads (the adv748x driver returns -EINVAL when
called on the sink pad for instance), is this really an error ? I think
it's related to the question I've asked in another patch, regarding what
error values are allowed for this operation and under what conditions.

> +		return ret;
> +	}
> +
> +	if (mbus_config.type != V4L2_MBUS_CSI2_DPHY) {
> +		dev_err(priv->dev, "Unsupported mbus type %u\n",
> +			mbus_config.type);
> +		return -EINVAL;
> +	}
> +
> +	if (mbus_config.csi2_dphy.data_lanes > priv->lanes) {
> +		dev_err(priv->dev,
> +			"Unsupported mbus config: too many data lanes %u\n",
> +			mbus_config.csi2_dphy.data_lanes);
> +		return -EINVAL;
> +	}
> +
> +	priv->active_lanes = mbus_config.csi2_dphy.data_lanes;
> +
> +	return 0;
> +}
> +
>  static int rcsi2_start_receiver(struct rcar_csi2 *priv)
>  {
>  	const struct rcar_csi2_format *format;
> @@ -490,6 +530,11 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
>  	/* Code is validated in set_fmt. */
>  	format = rcsi2_code_to_fmt(priv->mf.code);
> 
> +	/* Get the remote mbus config to get the number of enabled lanes. */
> +	ret = rcsi2_config_active_lanes(priv);
> +	if (ret)
> +		return ret;
> +
>  	/*
>  	 * Enable all supported CSI-2 channels with virtual channel and
>  	 * data type matching.
> @@ -522,7 +567,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
>  	}
> 
>  	phycnt = PHYCNT_ENABLECLK;
> -	phycnt |= (1 << priv->lanes) - 1;
> +	phycnt |= (1 << priv->active_lanes) - 1;
> 
>  	mbps = rcsi2_calc_mbps(priv, format->bpp);
>  	if (mbps < 0)
> @@ -748,6 +793,7 @@ static int rcsi2_notify_bound(struct v4l2_async_notifier *notifier,
>  	}
> 
>  	priv->remote = subdev;
> +	priv->remote_pad = pad;
> 
>  	dev_dbg(priv->dev, "Bound %s pad: %d\n", subdev->name, pad);
> 
> @@ -793,6 +839,7 @@ static int rcsi2_parse_v4l2(struct rcar_csi2 *priv,
>  			priv->lanes);
>  		return -EINVAL;
>  	}
> +	priv->active_lanes = priv->lanes;
> 
>  	for (i = 0; i < ARRAY_SIZE(priv->lane_swap); i++) {
>  		priv->lane_swap[i] = i < priv->lanes ?
diff mbox series

Patch

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index faa9fb23a2e9..0061d5ff37e3 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -363,6 +363,7 @@  struct rcar_csi2 {
 	struct v4l2_async_notifier notifier;
 	struct v4l2_async_subdev asd;
 	struct v4l2_subdev *remote;
+	unsigned short remote_pad;

 	struct v4l2_mbus_framefmt mf;

@@ -371,6 +372,7 @@  struct rcar_csi2 {

 	unsigned short lanes;
 	unsigned char lane_swap[4];
+	unsigned short active_lanes;
 };

 static inline struct rcar_csi2 *sd_to_csi2(struct v4l2_subdev *sd)
@@ -414,7 +416,7 @@  static int rcsi2_wait_phy_start(struct rcar_csi2 *priv)

 	/* Wait for the clock and data lanes to enter LP-11 state. */
 	for (timeout = 0; timeout <= 20; timeout++) {
-		const u32 lane_mask = (1 << priv->lanes) - 1;
+		const u32 lane_mask = (1 << priv->active_lanes) - 1;

 		if ((rcsi2_read(priv, PHCLM_REG) & PHCLM_STOPSTATECKL)  &&
 		    (rcsi2_read(priv, PHDLM_REG) & lane_mask) == lane_mask)
@@ -471,11 +473,49 @@  static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp)
 	 * bps = link_freq * 2
 	 */
 	mbps = v4l2_ctrl_g_ctrl_int64(ctrl) * bpp;
-	do_div(mbps, priv->lanes * 1000000);
+	do_div(mbps, priv->active_lanes * 1000000);

 	return mbps;
 }

+static int rcsi2_config_active_lanes(struct rcar_csi2 *priv)
+{
+	struct v4l2_mbus_pad_config mbus_config;
+	int ret;
+
+	priv->active_lanes = priv->lanes;
+
+	memset(&mbus_config, 0, sizeof(mbus_config));
+	ret = v4l2_subdev_call(priv->remote, pad, get_mbus_config,
+			       priv->remote_pad, &mbus_config);
+	if (ret == -ENOIOCTLCMD) {
+		dev_dbg(priv->dev, "No remote mbus configuration available\n");
+		return 0;
+	}
+
+	if (ret) {
+		dev_err(priv->dev, "Failed to get remote mbus configuration\n");
+		return ret;
+	}
+
+	if (mbus_config.type != V4L2_MBUS_CSI2_DPHY) {
+		dev_err(priv->dev, "Unsupported mbus type %u\n",
+			mbus_config.type);
+		return -EINVAL;
+	}
+
+	if (mbus_config.csi2_dphy.data_lanes > priv->lanes) {
+		dev_err(priv->dev,
+			"Unsupported mbus config: too many data lanes %u\n",
+			mbus_config.csi2_dphy.data_lanes);
+		return -EINVAL;
+	}
+
+	priv->active_lanes = mbus_config.csi2_dphy.data_lanes;
+
+	return 0;
+}
+
 static int rcsi2_start_receiver(struct rcar_csi2 *priv)
 {
 	const struct rcar_csi2_format *format;
@@ -490,6 +530,11 @@  static int rcsi2_start_receiver(struct rcar_csi2 *priv)
 	/* Code is validated in set_fmt. */
 	format = rcsi2_code_to_fmt(priv->mf.code);

+	/* Get the remote mbus config to get the number of enabled lanes. */
+	ret = rcsi2_config_active_lanes(priv);
+	if (ret)
+		return ret;
+
 	/*
 	 * Enable all supported CSI-2 channels with virtual channel and
 	 * data type matching.
@@ -522,7 +567,7 @@  static int rcsi2_start_receiver(struct rcar_csi2 *priv)
 	}

 	phycnt = PHYCNT_ENABLECLK;
-	phycnt |= (1 << priv->lanes) - 1;
+	phycnt |= (1 << priv->active_lanes) - 1;

 	mbps = rcsi2_calc_mbps(priv, format->bpp);
 	if (mbps < 0)
@@ -748,6 +793,7 @@  static int rcsi2_notify_bound(struct v4l2_async_notifier *notifier,
 	}

 	priv->remote = subdev;
+	priv->remote_pad = pad;

 	dev_dbg(priv->dev, "Bound %s pad: %d\n", subdev->name, pad);

@@ -793,6 +839,7 @@  static int rcsi2_parse_v4l2(struct rcar_csi2 *priv,
 			priv->lanes);
 		return -EINVAL;
 	}
+	priv->active_lanes = priv->lanes;

 	for (i = 0; i < ARRAY_SIZE(priv->lane_swap); i++) {
 		priv->lane_swap[i] = i < priv->lanes ?