diff mbox series

[RFC,02/21] media: cadence: csi2rx: configure DPHY before starting source stream

Message ID 20240222-multistream-v1-2-1837ed916eeb@ti.com
State New
Headers
Series media: cadence,ti: CSI2RX Multistream Support |

Commit Message

Jai Luthra Feb. 22, 2024, 11:31 a.m. UTC 
  From: Pratyush Yadav <p.yadav@ti.com>

When the source device is operating above 1.5 Gbps per lane, it needs to
send the Skew Calibration Sequence before sending any HS data. If the
DPHY is initialized after the source stream is started, then it might
miss the sequence and not be able to receive data properly. Move the
start of source subdev to the end of the sequence to make sure
everything is ready to receive data before the source starts streaming.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
Signed-off-by: Jai Luthra <j-luthra@ti.com>
---
 drivers/media/platform/cadence/cdns-csi2rx.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

Comments

Julien Massot Feb. 22, 2024, 2:29 p.m. UTC | #1 
Hi Jai,

On 2/22/24 12:31, Jai Luthra wrote:
> From: Pratyush Yadav <p.yadav@ti.com>
> 
> When the source device is operating above 1.5 Gbps per lane, it needs to
> send the Skew Calibration Sequence before sending any HS data. If the
> DPHY is initialized after the source stream is started, then it might
> miss the sequence and not be able to receive data properly. Move the
> start of source subdev to the end of the sequence to make sure
> everything is ready to receive data before the source starts streaming.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> Signed-off-by: Jai Luthra <j-luthra@ti.com>

Thanks for your patch!
This patch is useful even at lower data rate than 1.5 Gbps.
I'm having the issue at 800Mbps with max96714F deserializer.

Should this patch deserve a Fixes tag?
Fixes: 3295cf1241d3 ("media: cadence: Add support for external dphy")

Reviewed-by: Julien Massot <julien.massot@collabora.com>
Tested-by: Julien Massot <julien.massot@collabora.com>

> ---
>   drivers/media/platform/cadence/cdns-csi2rx.c | 26 ++++++++++++++------------
>   1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
> index 70b7f8a9e4f2..75e602c1d762 100644
> --- a/drivers/media/platform/cadence/cdns-csi2rx.c
> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
> @@ -243,10 +243,6 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>   
>   	writel(reg, csi2rx->base + CSI2RX_STATIC_CFG_REG);
>   
> -	ret = v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, true);
> -	if (ret)
> -		goto err_disable_pclk;
> -
>   	/* Enable DPHY clk and data lanes. */
>   	if (csi2rx->dphy) {
>   		reg = CSI2RX_DPHY_CL_EN | CSI2RX_DPHY_CL_RST;
> @@ -256,6 +252,13 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>   		}
>   
>   		writel(reg, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG);
> +
> +		ret = csi2rx_configure_ext_dphy(csi2rx);
> +		if (ret) {
> +			dev_err(csi2rx->dev,
> +				"Failed to configure external DPHY: %d\n", ret);
> +			goto err_disable_pclk;
> +		}
>   	}
>   
>   	/*
> @@ -295,14 +298,9 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>   
>   	reset_control_deassert(csi2rx->sys_rst);
>   
> -	if (csi2rx->dphy) {
> -		ret = csi2rx_configure_ext_dphy(csi2rx);
> -		if (ret) {
> -			dev_err(csi2rx->dev,
> -				"Failed to configure external DPHY: %d\n", ret);
> -			goto err_disable_sysclk;
> -		}
> -	}
> +	ret = v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, true);
> +	if (ret)
> +		goto err_disable_sysclk;
>   
>   	clk_disable_unprepare(csi2rx->p_clk);
>   
> @@ -316,6 +314,10 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
>   		clk_disable_unprepare(csi2rx->pixel_clk[i - 1]);
>   	}
>   
> +	if (csi2rx->dphy) {
> +		writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG);
> +		phy_power_off(csi2rx->dphy);
> +	}
>   err_disable_pclk:
>   	clk_disable_unprepare(csi2rx->p_clk);
>   
>
Changhuang Liang Feb. 23, 2024, 2:46 a.m. UTC | #2 
Hi, Jai

> [PATCH RFC 02/21] media: cadence: csi2rx: configure DPHY before
> starting source stream
> 
> From: Pratyush Yadav <p.yadav@ti.com>
> 
> When the source device is operating above 1.5 Gbps per lane, it needs to send
> the Skew Calibration Sequence before sending any HS data. If the DPHY is
> initialized after the source stream is started, then it might miss the sequence
> and not be able to receive data properly. Move the start of source subdev to
> the end of the sequence to make sure everything is ready to receive data
> before the source starts streaming.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> Signed-off-by: Jai Luthra <j-luthra@ti.com>
> ---

I also tested this patch on StarFive JH7100 CAMSS and it worked fine.

Reviewed-by: Changhuang Liang <Changhuang.liang@starfivetech.com>
Tested-by: Changhuang Liang <Changhuang.liang@starfivetech.com>

regards
Changhuang
Jai Luthra Feb. 23, 2024, 7:23 a.m. UTC | #3 
Hi Julien,

On Feb 22, 2024 at 15:29:54 +0100, Julien Massot wrote:
> Hi Jai,
> 
> On 2/22/24 12:31, Jai Luthra wrote:
> > From: Pratyush Yadav <p.yadav@ti.com>
> > 
> > When the source device is operating above 1.5 Gbps per lane, it needs to
> > send the Skew Calibration Sequence before sending any HS data. If the
> > DPHY is initialized after the source stream is started, then it might
> > miss the sequence and not be able to receive data properly. Move the
> > start of source subdev to the end of the sequence to make sure
> > everything is ready to receive data before the source starts streaming.
> > 
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > Signed-off-by: Jai Luthra <j-luthra@ti.com>
> 
> Thanks for your patch!
> This patch is useful even at lower data rate than 1.5 Gbps.
> I'm having the issue at 800Mbps with max96714F deserializer.
> 
> Should this patch deserve a Fixes tag?

Thanks, yes in that case I will add the fixes tag and repost this fix 
separately so it can be picked sooner.

> Fixes: 3295cf1241d3 ("media: cadence: Add support for external dphy")
> 
> Reviewed-by: Julien Massot <julien.massot@collabora.com>
> Tested-by: Julien Massot <julien.massot@collabora.com>
> 
> > ---
> >   drivers/media/platform/cadence/cdns-csi2rx.c | 26 ++++++++++++++------------
> >   1 file changed, 14 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
> > index 70b7f8a9e4f2..75e602c1d762 100644
> > --- a/drivers/media/platform/cadence/cdns-csi2rx.c
> > +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
> > @@ -243,10 +243,6 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
> >   	writel(reg, csi2rx->base + CSI2RX_STATIC_CFG_REG);
> > -	ret = v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, true);
> > -	if (ret)
> > -		goto err_disable_pclk;
> > -
> >   	/* Enable DPHY clk and data lanes. */
> >   	if (csi2rx->dphy) {
> >   		reg = CSI2RX_DPHY_CL_EN | CSI2RX_DPHY_CL_RST;
> > @@ -256,6 +252,13 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
> >   		}
> >   		writel(reg, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG);
> > +
> > +		ret = csi2rx_configure_ext_dphy(csi2rx);
> > +		if (ret) {
> > +			dev_err(csi2rx->dev,
> > +				"Failed to configure external DPHY: %d\n", ret);
> > +			goto err_disable_pclk;
> > +		}
> >   	}
> >   	/*
> > @@ -295,14 +298,9 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
> >   	reset_control_deassert(csi2rx->sys_rst);
> > -	if (csi2rx->dphy) {
> > -		ret = csi2rx_configure_ext_dphy(csi2rx);
> > -		if (ret) {
> > -			dev_err(csi2rx->dev,
> > -				"Failed to configure external DPHY: %d\n", ret);
> > -			goto err_disable_sysclk;
> > -		}
> > -	}
> > +	ret = v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, true);
> > +	if (ret)
> > +		goto err_disable_sysclk;
> >   	clk_disable_unprepare(csi2rx->p_clk);
> > @@ -316,6 +314,10 @@ static int csi2rx_start(struct csi2rx_priv *csi2rx)
> >   		clk_disable_unprepare(csi2rx->pixel_clk[i - 1]);
> >   	}
> > +	if (csi2rx->dphy) {
> > +		writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG);
> > +		phy_power_off(csi2rx->dphy);
> > +	}
> >   err_disable_pclk:
> >   	clk_disable_unprepare(csi2rx->p_clk);
> > 
> 
> -- 
> Julien Massot
> Senior Software Engineer
> Collabora Ltd.
> Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
> Registered in England & Wales, no. 5513718
diff mbox series

Patch

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 70b7f8a9e4f2..75e602c1d762 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -243,10 +243,6 @@  static int csi2rx_start(struct csi2rx_priv *csi2rx)
 
 	writel(reg, csi2rx->base + CSI2RX_STATIC_CFG_REG);
 
-	ret = v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, true);
-	if (ret)
-		goto err_disable_pclk;
-
 	/* Enable DPHY clk and data lanes. */
 	if (csi2rx->dphy) {
 		reg = CSI2RX_DPHY_CL_EN | CSI2RX_DPHY_CL_RST;
@@ -256,6 +252,13 @@  static int csi2rx_start(struct csi2rx_priv *csi2rx)
 		}
 
 		writel(reg, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG);
+
+		ret = csi2rx_configure_ext_dphy(csi2rx);
+		if (ret) {
+			dev_err(csi2rx->dev,
+				"Failed to configure external DPHY: %d\n", ret);
+			goto err_disable_pclk;
+		}
 	}
 
 	/*
@@ -295,14 +298,9 @@  static int csi2rx_start(struct csi2rx_priv *csi2rx)
 
 	reset_control_deassert(csi2rx->sys_rst);
 
-	if (csi2rx->dphy) {
-		ret = csi2rx_configure_ext_dphy(csi2rx);
-		if (ret) {
-			dev_err(csi2rx->dev,
-				"Failed to configure external DPHY: %d\n", ret);
-			goto err_disable_sysclk;
-		}
-	}
+	ret = v4l2_subdev_call(csi2rx->source_subdev, video, s_stream, true);
+	if (ret)
+		goto err_disable_sysclk;
 
 	clk_disable_unprepare(csi2rx->p_clk);
 
@@ -316,6 +314,10 @@  static int csi2rx_start(struct csi2rx_priv *csi2rx)
 		clk_disable_unprepare(csi2rx->pixel_clk[i - 1]);
 	}
 
+	if (csi2rx->dphy) {
+		writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG);
+		phy_power_off(csi2rx->dphy);
+	}
 err_disable_pclk:
 	clk_disable_unprepare(csi2rx->p_clk);