[v2,4/4] drm/bridge: tfp410: Fix setup and hold time calculation
diff mbox series

Message ID 20200514143612.2094-5-ricardo.canuelo@collabora.com
State New
Headers show
Series
  • dt-bindings: display: ti,tfp410.txt: convert to yaml
Related show

Commit Message

Ricardo Cañuelo May 14, 2020, 2:36 p.m. UTC
The tfp410 has a data de-skew feature that allows the user to compensate
the skew between IDCK and the pixel data and control signals.

In the driver, the setup and hold times are calculated from the de-skew
value. This retrieves the deskew value from the DT using the proper
datatype and range check as described by the binding (u32 from 0 to 7)
and fixes the calculation of the setup and hold times.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 drivers/gpu/drm/bridge/ti-tfp410.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Laurent Pinchart May 28, 2020, 9:46 p.m. UTC | #1
Hi Ricardo,

Thank you for the patch.

On Thu, May 14, 2020 at 04:36:12PM +0200, Ricardo Cañuelo wrote:
> The tfp410 has a data de-skew feature that allows the user to compensate
> the skew between IDCK and the pixel data and control signals.
> 
> In the driver, the setup and hold times are calculated from the de-skew
> value. This retrieves the deskew value from the DT using the proper
> datatype and range check as described by the binding (u32 from 0 to 7)
> and fixes the calculation of the setup and hold times.

How about mentioning that the fix results from a change in the DT
bindings ? Otherwise it sounds it was a bug in the driver.

I would also mention that the patch fixes the min() calculation, which
was clearly wrong. I think I would have split this in two patches.

With this addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  drivers/gpu/drm/bridge/ti-tfp410.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> index e3eb6364c0f7..21d99b4ea0c9 100644
> --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> @@ -220,7 +220,7 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
>  	struct device_node *ep;
>  	u32 pclk_sample = 0;
>  	u32 bus_width = 24;
> -	s32 deskew = 0;
> +	u32 deskew = 0;
>  
>  	/* Start with defaults. */
>  	*timings = tfp410_default_timings;
> @@ -274,12 +274,12 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
>  	}
>  
>  	/* Get the setup and hold time from vendor-specific properties. */
> -	of_property_read_u32(dvi->dev->of_node, "ti,deskew", (u32 *)&deskew);
> -	if (deskew < -4 || deskew > 3)
> +	of_property_read_u32(dvi->dev->of_node, "ti,deskew", &deskew);
> +	if (deskew > 7)
>  		return -EINVAL;
>  
> -	timings->setup_time_ps = min(0, 1200 - 350 * deskew);
> -	timings->hold_time_ps = min(0, 1300 + 350 * deskew);
> +	timings->setup_time_ps = 1200 - 350 * ((s32)deskew - 4);
> +	timings->hold_time_ps = max(0, 1300 + 350 * ((s32)deskew - 4));
>  
>  	return 0;
>  }

Patch
diff mbox series

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index e3eb6364c0f7..21d99b4ea0c9 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -220,7 +220,7 @@  static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
 	struct device_node *ep;
 	u32 pclk_sample = 0;
 	u32 bus_width = 24;
-	s32 deskew = 0;
+	u32 deskew = 0;
 
 	/* Start with defaults. */
 	*timings = tfp410_default_timings;
@@ -274,12 +274,12 @@  static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
 	}
 
 	/* Get the setup and hold time from vendor-specific properties. */
-	of_property_read_u32(dvi->dev->of_node, "ti,deskew", (u32 *)&deskew);
-	if (deskew < -4 || deskew > 3)
+	of_property_read_u32(dvi->dev->of_node, "ti,deskew", &deskew);
+	if (deskew > 7)
 		return -EINVAL;
 
-	timings->setup_time_ps = min(0, 1200 - 350 * deskew);
-	timings->hold_time_ps = min(0, 1300 + 350 * deskew);
+	timings->setup_time_ps = 1200 - 350 * ((s32)deskew - 4);
+	timings->hold_time_ps = max(0, 1300 + 350 * ((s32)deskew - 4));
 
 	return 0;
 }