| Message ID | 20240222-multistream-v1-2-1837ed916eeb@ti.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: cadence,ti: CSI2RX Multistream Support | expand |
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); > >
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
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 --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);