diff mbox series

[v2,19/19] media: ti-vpe: cal: fix stop state timeout

Message ID 20200319075023.22151-20-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show
Series CAL fixes and improvements | expand

Commit Message

Tomi Valkeinen March 19, 2020, 7:50 a.m. UTC 
The stop-state timeout needs to be over 100us as per CSI spec. With the
CAL fclk of 266 MHZ on DRA76, with the current value the driver uses,
the timeout is 24us. Too small timeout will cause failure to enable the
streaming.

Also, the fclk can be different on other SoCs, as is the case with AM65x
where the fclk is 250 MHz.

This patch fixes the timeout by calculating it correctly based on the
fclk rate.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/ti-vpe/cal.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Comments

Benoit Parrot March 19, 2020, 10:53 p.m. UTC | #1 
Tomi,

Thanks for the patch.

On 3/19/20 2:50 AM, Tomi Valkeinen wrote:
> The stop-state timeout needs to be over 100us as per CSI spec. With the
> CAL fclk of 266 MHZ on DRA76, with the current value the driver uses,
> the timeout is 24us. Too small timeout will cause failure to enable the
> streaming.
> 
> Also, the fclk can be different on other SoCs, as is the case with AM65x
> where the fclk is 250 MHz.
> 
> This patch fixes the timeout by calculating it correctly based on the
> fclk rate.
> 

Isn't this in relation to the clock sourcing the PHY module which is fixed
at 96Mhz (LVDSRX_96M_GFCLK)?

Benoit

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Tested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/ti-vpe/cal.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index 0f90078ee8c2..d935c628597b 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -6,6 +6,7 @@
>   * Benoit Parrot, <bparrot@ti.com>
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/ioctl.h>
> @@ -340,6 +341,7 @@ static const struct cal_data am654_cal_data = {
>   * all instances.
>   */
>  struct cal_dev {
> +	struct clk		*fclk;
>  	int			irq;
>  	void __iomem		*base;
>  	struct resource		*res;
> @@ -767,6 +769,7 @@ static void csi2_phy_config(struct cal_ctx *ctx);
>  static void csi2_phy_init(struct cal_ctx *ctx)
>  {
>  	u32 val;
> +	u32 sscounter;
>  
>  	/* Steps
>  	 *  1. Configure D-PHY mode and enable required lanes
> @@ -803,10 +806,20 @@ static void csi2_phy_init(struct cal_ctx *ctx)
>  	csi2_phy_config(ctx);
>  
>  	/* 3.B. Program Stop States */
> +	/*
> +	 * The stop-state-counter is based on fclk cycles, and we always use
> +	 * the x16 and x4 settings, so stop-state-timeout =
> +	 * fclk-cycle * 16 * 4 * counter.
> +	 *
> +	 * Stop-state-timeout must be more than 100us as per CSI2 spec, so we
> +	 * calculate a timeout that's 100us (rounding up).
> +	 */
> +	sscounter = DIV_ROUND_UP(clk_get_rate(ctx->dev->fclk), 10000 *  16 * 4);
> +
>  	val = reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port));
>  	set_field(&val, 1, CAL_CSI2_TIMING_STOP_STATE_X16_IO1_MASK);
> -	set_field(&val, 0, CAL_CSI2_TIMING_STOP_STATE_X4_IO1_MASK);
> -	set_field(&val, 407, CAL_CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK);
> +	set_field(&val, 1, CAL_CSI2_TIMING_STOP_STATE_X4_IO1_MASK);
> +	set_field(&val, sscounter, CAL_CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK);
>  	reg_write(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port), val);
>  	ctx_dbg(3, ctx, "CAL_CSI2_TIMING(%d) = 0x%08x Stop States\n",
>  		ctx->csi2_port,
> @@ -2257,6 +2270,12 @@ static int cal_probe(struct platform_device *pdev)
>  	/* save pdev pointer */
>  	dev->pdev = pdev;
>  
> +	dev->fclk = devm_clk_get(&pdev->dev, "fck");
> +	if (IS_ERR(dev->fclk)) {
> +		dev_err(&pdev->dev, "cannot get CAL fclk\n");
> +		return PTR_ERR(dev->fclk);
> +	}
> +
>  	syscon_camerrx = syscon_regmap_lookup_by_phandle(parent,
>  							 "ti,camerrx-control");
>  	ret = of_property_read_u32_index(parent, "ti,camerrx-control", 1,
>
Tomi Valkeinen March 20, 2020, 9:33 a.m. UTC | #2 
On 20/03/2020 00:53, Benoit Parrot wrote:
> Tomi,
> 
> Thanks for the patch.
> 
> On 3/19/20 2:50 AM, Tomi Valkeinen wrote:
>> The stop-state timeout needs to be over 100us as per CSI spec. With the
>> CAL fclk of 266 MHZ on DRA76, with the current value the driver uses,
>> the timeout is 24us. Too small timeout will cause failure to enable the
>> streaming.
>>
>> Also, the fclk can be different on other SoCs, as is the case with AM65x
>> where the fclk is 250 MHz.
>>
>> This patch fixes the timeout by calculating it correctly based on the
>> fclk rate.
>>
> 
> Isn't this in relation to the clock sourcing the PHY module which is fixed
> at 96Mhz (LVDSRX_96M_GFCLK)?

It's not clearly said in the docs. In register descs, it's "L3 cycles", in the content it's 
"functional clock cycles" (without specifying which func clock).

As far as I see, this timeout is part of the CAL, not the PHY, so I think it's the CAL functional clock.

  Tomi
diff mbox series

Patch

diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 0f90078ee8c2..d935c628597b 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -6,6 +6,7 @@ 
  * Benoit Parrot, <bparrot@ti.com>
  */
 
+#include <linux/clk.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/ioctl.h>
@@ -340,6 +341,7 @@  static const struct cal_data am654_cal_data = {
  * all instances.
  */
 struct cal_dev {
+	struct clk		*fclk;
 	int			irq;
 	void __iomem		*base;
 	struct resource		*res;
@@ -767,6 +769,7 @@  static void csi2_phy_config(struct cal_ctx *ctx);
 static void csi2_phy_init(struct cal_ctx *ctx)
 {
 	u32 val;
+	u32 sscounter;
 
 	/* Steps
 	 *  1. Configure D-PHY mode and enable required lanes
@@ -803,10 +806,20 @@  static void csi2_phy_init(struct cal_ctx *ctx)
 	csi2_phy_config(ctx);
 
 	/* 3.B. Program Stop States */
+	/*
+	 * The stop-state-counter is based on fclk cycles, and we always use
+	 * the x16 and x4 settings, so stop-state-timeout =
+	 * fclk-cycle * 16 * 4 * counter.
+	 *
+	 * Stop-state-timeout must be more than 100us as per CSI2 spec, so we
+	 * calculate a timeout that's 100us (rounding up).
+	 */
+	sscounter = DIV_ROUND_UP(clk_get_rate(ctx->dev->fclk), 10000 *  16 * 4);
+
 	val = reg_read(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port));
 	set_field(&val, 1, CAL_CSI2_TIMING_STOP_STATE_X16_IO1_MASK);
-	set_field(&val, 0, CAL_CSI2_TIMING_STOP_STATE_X4_IO1_MASK);
-	set_field(&val, 407, CAL_CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK);
+	set_field(&val, 1, CAL_CSI2_TIMING_STOP_STATE_X4_IO1_MASK);
+	set_field(&val, sscounter, CAL_CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK);
 	reg_write(ctx->dev, CAL_CSI2_TIMING(ctx->csi2_port), val);
 	ctx_dbg(3, ctx, "CAL_CSI2_TIMING(%d) = 0x%08x Stop States\n",
 		ctx->csi2_port,
@@ -2257,6 +2270,12 @@  static int cal_probe(struct platform_device *pdev)
 	/* save pdev pointer */
 	dev->pdev = pdev;
 
+	dev->fclk = devm_clk_get(&pdev->dev, "fck");
+	if (IS_ERR(dev->fclk)) {
+		dev_err(&pdev->dev, "cannot get CAL fclk\n");
+		return PTR_ERR(dev->fclk);
+	}
+
 	syscon_camerrx = syscon_regmap_lookup_by_phandle(parent,
 							 "ti,camerrx-control");
 	ret = of_property_read_u32_index(parent, "ti,camerrx-control", 1,