[PATCH v2 0/4] dt-bindings: display: ti,tfp410.txt: convert to yaml

[PATCH v2 0/4] dt-bindings: display: ti,tfp410.txt: convert to yaml

[Ricardo Cañuelo]

This series converts the DT binding documentation for the TI TFP410
DPI-to-DVI encoder to json-schema.

Some minor changes were made to two DTs in order to make them compliant
with the binding. These changes shouldn't have any functional effect.

This also fixes a minor bug in the ti-tfp410 driver that would cause a
wrong calculation of the setup and hold times when the de-skew feature
is enabled. The retrieval of the de-skew value from the DT has also been
updated to reflect the binding changes.

Changes in v2:

  - ti,tfp410.yaml
    - Set license as GPL-2.0-only (Sam Ravnborg)
    - Complete pclk-sample and bus-width comments (Sam)
    - Remove quotes from compatible value (Sam)
    - Remove the allOf keyword from the ti,deskew definition (Sam)
    - Set endpoint as optional in port definitions
    - Set a range for ti,deskew

  DTs fixes (added):
    - dove-sbc-a510.dts: s/powerdown-gpio/powerdown-gpios (Sam)
    - dove-sbc-a510.dts: Add dummy ports node
    - imx53-cx9020.dts: Group the ports in a ports node

  ti-tfp410.c (added):
    - Fix setup time and hold time calculation based on the deskew value
      (Laurent Pinchart)

Ricardo Cañuelo (4):
  ARM: dts: dove: Make the DT compliant with the ti,tfp410 binding
  ARM: dts: ims53: Group port definitions for the dvi-converter
  dt-bindings: display: ti,tfp410.txt: convert to yaml
  drm/bridge: tfp410: Fix setup and hold time calculation

 .../bindings/display/bridge/ti,tfp410.txt     |  66 ----------
 .../bindings/display/bridge/ti,tfp410.yaml    | 124 ++++++++++++++++++
 arch/arm/boot/dts/dove-sbc-a510.dts           |  13 +-
 arch/arm/boot/dts/imx53-cx9020.dts            |  25 ++--
 drivers/gpu/drm/bridge/ti-tfp410.c            |  10 +-
 5 files changed, 155 insertions(+), 83 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml

-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/4] ARM: dts: dove: Make the DT compliant with the ti, tfp410 binding

[Ricardo Cañuelo]

Define a 'ports' node for 'dvi: video@39' and use the proper naming for
the powerdown-gpios property to make it compliant with the ti,tfp410
binding.

This fills the minimum requirements to meet the binding requirements,
port endpoints are not defined.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 arch/arm/boot/dts/dove-sbc-a510.dts | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/dove-sbc-a510.dts b/arch/arm/boot/dts/dove-sbc-a510.dts
index 2bb85a9b7614..32804c981625 100644
--- a/arch/arm/boot/dts/dove-sbc-a510.dts
+++ b/arch/arm/boot/dts/dove-sbc-a510.dts
@@ -132,7 +132,18 @@
 	dvi: video@39 {
 		compatible = "ti,tfp410";
 		reg = <0x39>;
-		powerdown-gpio = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
+		powerdown-gpios = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			port@0 {
+				reg = <0>;
+			};
+			port@1 {
+				reg = <1>;
+			};
+		};
 	};
 };
 
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/4] ARM: dts: ims53: Group port definitions for the dvi-converter

[Ricardo Cañuelo]

Group the port definitions of the dvi-converter in a 'ports' node to
make it compliant with the ti,tfp410 binding.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 arch/arm/boot/dts/imx53-cx9020.dts | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/imx53-cx9020.dts b/arch/arm/boot/dts/imx53-cx9020.dts
index 0a475c234054..cfb18849a92b 100644
--- a/arch/arm/boot/dts/imx53-cx9020.dts
+++ b/arch/arm/boot/dts/imx53-cx9020.dts
@@ -59,23 +59,26 @@
 	};
 
 	dvi-converter {
-		#address-cells = <1>;
-		#size-cells = <0>;
 		compatible = "ti,tfp410";
 
-		port@0 {
-			reg = <0>;
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
 
-			tfp410_in: endpoint {
-				remote-endpoint = <&display0_out>;
+				tfp410_in: endpoint {
+					remote-endpoint = <&display0_out>;
+				};
 			};
-		};
 
-		port@1 {
-			reg = <1>;
+			port@1 {
+				reg = <1>;
 
-			tfp410_out: endpoint {
-				remote-endpoint = <&dvi_connector_in>;
+				tfp410_out: endpoint {
+					remote-endpoint = <&dvi_connector_in>;
+				};
 			};
 		};
 	};
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/4] dt-bindings: display: ti,tfp410.txt: convert to yaml

[Ricardo Cañuelo]

Convert the DT binding documentation for the TI TFP410 DPI-to-DVI
encoder to json-schema.

The 'ti,deskew' is now an unsigned value from 0 to 7 instead of a signed
value from -4 to 3. The rest of the binding is a direct translation from
the old one.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 .../bindings/display/bridge/ti,tfp410.txt     |  66 ----------
 .../bindings/display/bridge/ti,tfp410.yaml    | 124 ++++++++++++++++++
 2 files changed, 124 insertions(+), 66 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
deleted file mode 100644
index 5ff4f64ef8e8..000000000000
--- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
+++ /dev/null
@@ -1,66 +0,0 @@
-TFP410 DPI to DVI encoder
-=========================
-
-Required properties:
-- compatible: "ti,tfp410"
-
-Optional properties:
-- powerdown-gpios: power-down gpio
-- reg: I2C address. If and only if present the device node should be placed
-  into the I2C controller node where the TFP410 I2C is connected to.
-- ti,deskew: data de-skew in 350ps increments, from -4 to +3, as configured
-  through th DK[3:1] pins. This property shall be present only if the TFP410
-  is not connected through I2C.
-
-Required nodes:
-
-This device has two video ports. Their connections are modeled using the OF
-graph bindings specified in [1]. Each port node shall have a single endpoint.
-
-- Port 0 is the DPI input port. Its endpoint subnode shall contain a
-  pclk-sample and bus-width property and a remote-endpoint property as specified
-  in [1].
-  - If pclk-sample is not defined, pclk-sample = 0 should be assumed for
-    backward compatibility.
-  - If bus-width is not defined then bus-width = 24 should be assumed for
-    backward compatibility.
-    bus-width = 24: 24 data lines are connected and single-edge mode
-    bus-width = 12: 12 data lines are connected and dual-edge mode
-
-- Port 1 is the DVI output port. Its endpoint subnode shall contain a
-  remote-endpoint property is specified in [1].
-
-[1] Documentation/devicetree/bindings/media/video-interfaces.txt
-
-
-Example
--------
-
-tfp410: encoder@0 {
-	compatible = "ti,tfp410";
-	powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>;
-	ti,deskew = <4>;
-
-	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port@0 {
-			reg = <0>;
-
-			tfp410_in: endpoint@0 {
-				pclk-sample = <1>;
-				bus-width = <24>;
-				remote-endpoint = <&dpi_out>;
-			};
-		};
-
-		port@1 {
-			reg = <1>;
-
-			tfp410_out: endpoint@0 {
-				remote-endpoint = <&dvi_connector_in>;
-			};
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
new file mode 100644
index 000000000000..a9f4fd8ea621
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
@@ -0,0 +1,124 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/ti,tfp410.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TFP410 DPI to DVI encoder
+
+maintainers:
+  - Tomi Valkeinen <tomi.valkeinen@ti.com>
+  - Jyri Sarha <jsarha@ti.com>
+
+properties:
+  compatible:
+    const: ti,tfp410
+
+  reg:
+    description: I2C address of the device.
+    maxItems: 1
+
+  powerdown-gpios:
+    maxItems: 1
+
+  ti,deskew:
+    description:
+      Data de-skew value in 350ps increments, from 0 to 7, as configured
+      through the DK[3:1] pins. The de-skew multiplier is computed as
+      (DK[3:1] - 4), so it ranges from -4 to 3. This property shall be
+      present only if the TFP410 is not connected through I2C.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 7
+
+  ports:
+    description:
+      A node containing input and output port nodes with endpoint
+      definitions as documented in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+    type: object
+
+    properties:
+      port@0:
+        description: DPI input port.
+        type: object
+
+        properties:
+          reg:
+            const: 0
+
+          endpoint:
+            type: object
+
+            properties:
+              pclk-sample:
+                description:
+                  Endpoint sampling edge. If not defined, pclk-sample =
+                  0 should be assumed for backwards compatibility.
+                enum:
+                  - 0  # Falling edge
+                  - 1  # Rising edge
+                default: 0
+
+              bus-width:
+                description:
+                  Endpoint bus width. If not defined, bus-width = 24
+                  should be assumed for backwards compatibility.
+                enum:
+                  - 12  # 12 data lines connected and dual-edge mode
+                  - 24  # 24 data lines connected and single-edge mode
+                default: 24
+
+      port@1:
+        description: DVI output port.
+        type: object
+
+        properties:
+          reg:
+            const: 1
+
+          endpoint:
+            type: object
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    tfp410: encoder {
+        compatible = "ti,tfp410";
+        powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>;
+        ti,deskew = <3>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                tfp410_in: endpoint {
+                    pclk-sample = <1>;
+                    bus-width = <24>;
+                    remote-endpoint = <&dpi_out>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                tfp410_out: endpoint {
+                    remote-endpoint = <&dvi_connector_in>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/4] drm/bridge: tfp410: Fix setup and hold time calculation

[Ricardo Cañuelo]

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(-)

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;
 }
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/4] ARM: dts: ims53: Group port definitions for the dvi-converter

[Shawn Guo]

On Thu, May 14, 2020 at 04:36:10PM +0200, Ricardo Cañuelo wrote:
> Group the port definitions of the dvi-converter in a 'ports' node to
> make it compliant with the ti,tfp410 binding.
> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>

s/ims53/imx53-cx9020 in subject.

I fixed it up and applied the patch.

Shawn
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] ARM: dts: dove: Make the DT compliant with the ti,tfp410 binding

[Rob Herring]

On Thu, May 14, 2020 at 04:36:09PM +0200, Ricardo Cañuelo wrote:
> Define a 'ports' node for 'dvi: video@39' and use the proper naming for
> the powerdown-gpios property to make it compliant with the ti,tfp410
> binding.
> 
> This fills the minimum requirements to meet the binding requirements,
> port endpoints are not defined.

Just make 'ports' optional. This isn't really any better unless you add 
endpoints too.

> 
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  arch/arm/boot/dts/dove-sbc-a510.dts | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/dove-sbc-a510.dts b/arch/arm/boot/dts/dove-sbc-a510.dts
> index 2bb85a9b7614..32804c981625 100644
> --- a/arch/arm/boot/dts/dove-sbc-a510.dts
> +++ b/arch/arm/boot/dts/dove-sbc-a510.dts
> @@ -132,7 +132,18 @@
>  	dvi: video@39 {
>  		compatible = "ti,tfp410";
>  		reg = <0x39>;
> -		powerdown-gpio = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
> +		powerdown-gpios = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			port@0 {
> +				reg = <0>;
> +			};
> +			port@1 {
> +				reg = <1>;
> +			};
> +		};
>  	};
>  };
>  
> -- 
> 2.18.0
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] ARM: dts: dove: Make the DT compliant with the ti,tfp410 binding

[Laurent Pinchart]

Hi Rob,

On Thu, May 28, 2020 at 11:37:55AM -0600, Rob Herring wrote:
> On Thu, May 14, 2020 at 04:36:09PM +0200, Ricardo Cañuelo wrote:
> > Define a 'ports' node for 'dvi: video@39' and use the proper naming for
> > the powerdown-gpios property to make it compliant with the ti,tfp410
> > binding.
> > 
> > This fills the minimum requirements to meet the binding requirements,
> > port endpoints are not defined.
> 
> Just make 'ports' optional. This isn't really any better unless you add 
> endpoints too.

I wonder how this is supposed to work though. The ti-tfp410 driver will
fail to probe if there's no endpoint. I'd rather already add a node for
the DVI connector and connect it to port@1 of the TFP410.

> > Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> > ---
> >  arch/arm/boot/dts/dove-sbc-a510.dts | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/dove-sbc-a510.dts b/arch/arm/boot/dts/dove-sbc-a510.dts
> > index 2bb85a9b7614..32804c981625 100644
> > --- a/arch/arm/boot/dts/dove-sbc-a510.dts
> > +++ b/arch/arm/boot/dts/dove-sbc-a510.dts
> > @@ -132,7 +132,18 @@
> >  	dvi: video@39 {
> >  		compatible = "ti,tfp410";
> >  		reg = <0x39>;
> > -		powerdown-gpio = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
> > +		powerdown-gpios = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +			port@0 {
> > +				reg = <0>;
> > +			};
> > +			port@1 {
> > +				reg = <1>;
> > +			};
> > +		};
> >  	};
> >  };
> >  

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/4] dt-bindings: display: ti,tfp410.txt: convert to yaml

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Thu, May 14, 2020 at 04:36:11PM +0200, Ricardo Cañuelo wrote:
> Convert the DT binding documentation for the TI TFP410 DPI-to-DVI
> encoder to json-schema.
> 
> The 'ti,deskew' is now an unsigned value from 0 to 7 instead of a signed
> value from -4 to 3. The rest of the binding is a direct translation from
> the old one.

I would have modified this in a separate patch.

> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  .../bindings/display/bridge/ti,tfp410.txt     |  66 ----------
>  .../bindings/display/bridge/ti,tfp410.yaml    | 124 ++++++++++++++++++
>  2 files changed, 124 insertions(+), 66 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> deleted file mode 100644
> index 5ff4f64ef8e8..000000000000
> --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> +++ /dev/null
> @@ -1,66 +0,0 @@
> -TFP410 DPI to DVI encoder
> -=========================
> -
> -Required properties:
> -- compatible: "ti,tfp410"
> -
> -Optional properties:
> -- powerdown-gpios: power-down gpio
> -- reg: I2C address. If and only if present the device node should be placed
> -  into the I2C controller node where the TFP410 I2C is connected to.
> -- ti,deskew: data de-skew in 350ps increments, from -4 to +3, as configured
> -  through th DK[3:1] pins. This property shall be present only if the TFP410
> -  is not connected through I2C.
> -
> -Required nodes:
> -
> -This device has two video ports. Their connections are modeled using the OF
> -graph bindings specified in [1]. Each port node shall have a single endpoint.
> -
> -- Port 0 is the DPI input port. Its endpoint subnode shall contain a
> -  pclk-sample and bus-width property and a remote-endpoint property as specified
> -  in [1].
> -  - If pclk-sample is not defined, pclk-sample = 0 should be assumed for
> -    backward compatibility.
> -  - If bus-width is not defined then bus-width = 24 should be assumed for
> -    backward compatibility.
> -    bus-width = 24: 24 data lines are connected and single-edge mode
> -    bus-width = 12: 12 data lines are connected and dual-edge mode
> -
> -- Port 1 is the DVI output port. Its endpoint subnode shall contain a
> -  remote-endpoint property is specified in [1].
> -
> -[1] Documentation/devicetree/bindings/media/video-interfaces.txt
> -
> -
> -Example
> --------
> -
> -tfp410: encoder@0 {
> -	compatible = "ti,tfp410";
> -	powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>;
> -	ti,deskew = <4>;
> -
> -	ports {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		port@0 {
> -			reg = <0>;
> -
> -			tfp410_in: endpoint@0 {
> -				pclk-sample = <1>;
> -				bus-width = <24>;
> -				remote-endpoint = <&dpi_out>;
> -			};
> -		};
> -
> -		port@1 {
> -			reg = <1>;
> -
> -			tfp410_out: endpoint@0 {
> -				remote-endpoint = <&dvi_connector_in>;
> -			};
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> new file mode 100644
> index 000000000000..a9f4fd8ea621
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.yaml
> @@ -0,0 +1,124 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/ti,tfp410.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TFP410 DPI to DVI encoder
> +
> +maintainers:
> +  - Tomi Valkeinen <tomi.valkeinen@ti.com>
> +  - Jyri Sarha <jsarha@ti.com>
> +
> +properties:
> +  compatible:
> +    const: ti,tfp410
> +
> +  reg:
> +    description: I2C address of the device.
> +    maxItems: 1
> +
> +  powerdown-gpios:
> +    maxItems: 1
> +
> +  ti,deskew:
> +    description:
> +      Data de-skew value in 350ps increments, from 0 to 7, as configured
> +      through the DK[3:1] pins. The de-skew multiplier is computed as
> +      (DK[3:1] - 4), so it ranges from -4 to 3. This property shall be
> +      present only if the TFP410 is not connected through I2C.

I'd replace the last sentence with

if:
  required:
    - reg
then:
  properties:
    ti,deskew: false
else:
  required:
    - ti,deskew

(between the required: and additionalProperties: objects below)

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 7
> +
> +  ports:
> +    description:
> +      A node containing input and output port nodes with endpoint
> +      definitions as documented in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt
> +    type: object
> +
> +    properties:
> +      port@0:
> +        description: DPI input port.
> +        type: object
> +
> +        properties:
> +          reg:
> +            const: 0
> +
> +          endpoint:
> +            type: object
> +
> +            properties:
> +              pclk-sample:
> +                description:
> +                  Endpoint sampling edge. If not defined, pclk-sample =
> +                  0 should be assumed for backwards compatibility.

Should the second sentence be dropped, as it's expressed with default: 0
?

> +                enum:
> +                  - 0  # Falling edge
> +                  - 1  # Rising edge
> +                default: 0
> +
> +              bus-width:
> +                description:
> +                  Endpoint bus width. If not defined, bus-width = 24
> +                  should be assumed for backwards compatibility.

Same here for the second sentence.

With these small issues fixed,

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

> +                enum:
> +                  - 12  # 12 data lines connected and dual-edge mode
> +                  - 24  # 24 data lines connected and single-edge mode
> +                default: 24
> +
> +      port@1:
> +        description: DVI output port.
> +        type: object
> +
> +        properties:
> +          reg:
> +            const: 1
> +
> +          endpoint:
> +            type: object
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    tfp410: encoder {
> +        compatible = "ti,tfp410";
> +        powerdown-gpios = <&twl_gpio 2 GPIO_ACTIVE_LOW>;
> +        ti,deskew = <3>;
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                tfp410_in: endpoint {
> +                    pclk-sample = <1>;
> +                    bus-width = <24>;
> +                    remote-endpoint = <&dpi_out>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                tfp410_out: endpoint {
> +                    remote-endpoint = <&dvi_connector_in>;
> +                };
> +            };
> +        };
> +    };
> +
> +...

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/4] drm/bridge: tfp410: Fix setup and hold time calculation

[Laurent Pinchart]

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;
>  }

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel