[PATCH v3 0/8] Add dual-LVDS panel support to EK874

[PATCH v3 0/8] Add dual-LVDS panel support to EK874

[Fabrizio Castro]

Dear All,

this series adds support for dual-LVDS panel IDK-2121WR
from Advantech:
https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm

V3 approaches the problem in a completely different way, we now
have two new properties to mark the ports in the DT as receiving
even pixels and odd pixels: dual-lvds-even-pixels and dual-lvds-odd-pixels,
which means device drivers should not use bridge specific or panel
specific dual_link flags. Also, in this case the DT describes the
connection fully.

In order for the solution to be generic, I have exported a new helper
(drm_of_lvds_get_dual_link_configuration) to walk the device tree,
and figure out if the connection is dual-LVDS. The same helper gives
information about the configuration of the connection. If Px is connected
to a port expecting even pixels and Py is connected to a port expecting
odd pixels, then the helper returns DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS
(like in the example below), otherwise it returns
DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS.


 --------            dual-lvds-even-pixels  --------
|        |----                         ----|        |
|        | Px |---------------------->| Pn |        |
|        |----                         ----|        |
| SOURCE |           dual-lvds-odd-pixels  |  SINK  |
|        |----                         ----|        |
|        | Py |---------------------->| Pm |        |
|        |----                         ----|        |
 --------                                   --------

The device driver for the encoder then will work out if with the current
wiring the pixels need swapping or not.

The same solution works for both panels and bridges.

Since the DT describes the connection fully, driver
drivers/gpu/drm/panel/panel-lvds.c works out-of-the-box, no changes
required, however, this implementation opens up a problem with the
dt-bindings.
Driver drivers/gpu/drm/panel/panel-lvds.c can still be pleased by
a port node, but also by a ports node.
I have created Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
with the idea of including it from panels and bridges dt-bindings
supporting dual-LVDS (and of course the dt-bindings for the specific
devices should say which port should be marked as what), but file
Documentation/devicetree/bindings/display/panel/lvds.yaml formally
requires property "port", while with this implementation it should require
OneOf "port" and "ports", and unfortunately I can't seem to find a neat way
aroud that, other than creating a new compatible string
(e.g. "panel-dual-lvds"), a new dt-binding document for it, and of course adding
support for the new compatible string to drivers/gpu/drm/panel/panel-lvds.c.
As a result, this series is missing (at least) a patch necessary to fully
document the new implementation within
Documentation/devicetree/bindings/display/panel/lvds.yaml

Rob, do you have any suggestions? Do you think this idea works ok from a
documentation point of view? By the way, I don't really know what I am doing
with the yaml dt-bindings, I hope you won't be horrified by this series :-P

I hope I was able to deliver the concept clearly, if not please just ask.

Comments are very much appreciated.

Thanks,
Fab

Fabrizio Castro (8):
  dt-bindings: display: Add bindings for LVDS bus-timings
  dt-bindings: display: Add idk-2121wr binding
  drm: Add bus timings helper
  drm: rcar-du: lvds: Add dual-LVDS panels support
  drm: bridge: thc63: Do not report input bus mode through bridge
    timings
  arm64: dts: renesas: Add EK874 board with idk-2121wr display support
  [HACK] arm64: dts: renesas: draak: Enable LVDS
  [HACK] arm64: dts: renesas: draak: Enable LVDS dual-link operation

 .../bindings/display/bus-timings/lvds.yaml         |  38 +++++++
 .../display/panel/advantech,idk-2121wr.yaml        |  90 ++++++++++++++++
 arch/arm64/boot/dts/renesas/Makefile               |   3 +-
 .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 116 +++++++++++++++++++++
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  21 +++-
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts     |  26 +++--
 drivers/gpu/drm/Makefile                           |   3 +-
 drivers/gpu/drm/bridge/thc63lvd1024.c              |   9 +-
 drivers/gpu/drm/drm_bus_timings.c                  |  97 +++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_lvds.c                | 110 +++++++++++--------
 include/drm/drm_bridge.h                           |   8 --
 include/drm/drm_bus_timings.h                      |  21 ++++
 12 files changed, 473 insertions(+), 69 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
 create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
 create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
 create mode 100644 drivers/gpu/drm/drm_bus_timings.c
 create mode 100644 include/drm/drm_bus_timings.h

-- 
2.7.4

[PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings

[Fabrizio Castro]

Dual-LVDS connections need markers in the DT, this patch adds
some common documentation to be referenced by both panels and
bridges.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v2->v3:
* new patch
---
 .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml

diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
new file mode 100644
index 0000000..f35b55a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Properties for bus timings of LVDS interfaces
+
+maintainers:
+  - Thierry Reding <thierry.reding@gmail.com>
+  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
+
+description: |
+  This document defines device tree properties common to LVDS and dual-LVDS
+  interfaces, where a dual-LVDS interface is a dual-link connection with even
+  pixels traveling on one connection, and with odd pixels traveling on the other
+  connection.
+  This document doesn't constitue a device tree binding specification by itself
+  but is meant to be referenced by device tree bindings.
+  When referenced from panel or bridge device tree bindings, the properties
+  defined in this document are defined as follows. The panel and bridge device
+  tree bindings are responsible for defining whether each property is required
+  or optional.
+
+properties:
+  dual-lvds-even-pixels:
+    type: boolean
+    description:
+      This property is specific to an input port of a sink device. When
+      specified, it marks the port as recipient of even-pixels.
+
+  dual-lvds-odd-pixels:
+    type: boolean
+    description:
+      This property is specific to an input port of a sink device. When
+      specified, it marks the port as recipient of odd-pixels.
+
+...
-- 
2.7.4

[PATCH v3 2/8] dt-bindings: display: Add idk-2121wr binding

[Fabrizio Castro]

Add binding for the idk-2121wr LVDS panel from Advantech.

Some panel-specific documentation can be found here:
https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v2->v3:
* new patch
---
 .../display/panel/advantech,idk-2121wr.yaml        | 90 ++++++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
new file mode 100644
index 0000000..b2ccdc8
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/advantech,idk-2121wr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Advantech IDK-2121WR 21.5" Full-HD dual-LVDS panel
+
+maintainers:
+  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
+  - Thierry Reding <thierry.reding@gmail.com>
+
+description: |
+  The IDK-2121WR from Advantech is a Full-HD dual-LVDS panel.
+
+  The panels expects odd pixels from the first port, and even pixels from
+  the second port, therefore the ports must be marked accordingly.
+
+allOf:
+  - $ref: lvds.yaml#
+  - $ref: ../bus-timings/lvds.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: advantech,idk-2121wr
+      - {} # panel-lvds, but not listed here to avoid false select
+
+  data-mapping:
+    const: vesa-24
+
+  width-mm:
+    const: 476
+
+  height-mm:
+    const: 268
+
+  panel-timing: true
+  ports: true
+
+additionalProperties: false
+
+required:
+  - compatible
+
+examples:
+  - |+
+    panel-lvds {
+      compatible = "advantech,idk-2121wr", "panel-lvds";
+
+      width-mm = <476>;
+      height-mm = <268>;
+
+      data-mapping = "vesa-24";
+
+      panel-timing {
+        clock-frequency = <148500000>;
+        hactive = <1920>;
+        vactive = <1080>;
+        hsync-len = <44>;
+        hfront-porch = <88>;
+        hback-porch = <148>;
+        vfront-porch = <4>;
+        vback-porch = <36>;
+        vsync-len = <5>;
+      };
+
+      ports {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        port@0 {
+          reg = <0>;
+          dual-lvds-odd-pixels;
+          panel_in0: endpoint {
+            remote-endpoint = <&lvds0_out>;
+          };
+        };
+
+        port@1 {
+          reg = <1>;
+          dual-lvds-even-pixels;
+          panel_in1: endpoint {
+            remote-endpoint = <&lvds1_out>;
+          };
+        };
+      };
+    };
+
+...
-- 
2.7.4

[PATCH v3 6/8] arm64: dts: renesas: Add EK874 board with idk-2121wr display support

[Fabrizio Castro]

The EK874 is advertised as compatible with panel IDK-2121WR from
Advantech, however the panel isn't sold alongside the board.
A new dts, adding everything that's required to get the panel to
to work with the EK874, is the most convenient way to support the
EK874 when it's connected to the IDK-2121WR.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

---
v1->v2:
* Added comment for lvds-connector-en-gpio
* Renamed &lvds0_panel_in to panel_in0
* Renamed &lvds1_panel_in to panel_in1

v2->v3:
* removed renesas,swap-data property
* added dual-lvds-odd-pixels and dual-lvds-even-pixels properties

Geert,

no need to review this patch unless they like the idea behind this
series.

Thanks,
Fab

---
 arch/arm64/boot/dts/renesas/Makefile               |   3 +-
 .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 116 +++++++++++++++++++++
 2 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts

diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
index 42b74c2..ce48478 100644
--- a/arch/arm64/boot/dts/renesas/Makefile
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -1,7 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m.dtb
 dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-ex.dtb
-dtb-$(CONFIG_ARCH_R8A774C0) += r8a774c0-cat874.dtb r8a774c0-ek874.dtb
+dtb-$(CONFIG_ARCH_R8A774C0) += r8a774c0-cat874.dtb r8a774c0-ek874.dtb \
+			       r8a774c0-ek874-idk-2121wr.dtb
 dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-x.dtb r8a7795-h3ulcb.dtb
 dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-h3ulcb-kf.dtb
 dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-xs.dtb
diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
new file mode 100644
index 0000000..a7b27d0
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the Silicon Linux RZ/G2E evaluation kit (EK874),
+ * connected to an Advantech IDK-2121WR 21.5" LVDS panel
+ *
+ * Copyright (C) 2019 Renesas Electronics Corp.
+ */
+
+#include "r8a774c0-ek874.dts"
+
+/ {
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm5 0 50000>;
+
+		brightness-levels = <0 4 8 16 32 64 128 255>;
+		default-brightness-level = <6>;
+
+		power-supply = <&reg_12p0v>;
+		enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
+	};
+
+	panel-lvds {
+		compatible = "advantech,idk-2121wr", "panel-lvds";
+
+		width-mm = <476>;
+		height-mm = <268>;
+
+		data-mapping = "vesa-24";
+
+		panel-timing {
+			clock-frequency = <148500000>;
+			hactive = <1920>;
+			vactive = <1080>;
+			hsync-len = <44>;
+			hfront-porch = <88>;
+			hback-porch = <148>;
+			vfront-porch = <4>;
+			vback-porch = <36>;
+			vsync-len = <5>;
+		};
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				dual-lvds-odd-pixels;
+				panel_in0: endpoint {
+					remote-endpoint = <&lvds0_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				dual-lvds-even-pixels;
+				panel_in1: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
+		};
+	};
+};
+
+&gpio0 {
+	/*
+	 * When GP0_17 is low LVDS[01] are connected to the LVDS connector
+	 * When GP0_17 is high LVDS[01] are connected to the LT8918L
+	 */
+	lvds-connector-en-gpio{
+		gpio-hog;
+		gpios = <17 GPIO_ACTIVE_HIGH>;
+		output-low;
+		line-name = "lvds-connector-en-gpio";
+	};
+};
+
+&lvds0 {
+	ports {
+		port@1 {
+			lvds0_out: endpoint {
+				remote-endpoint = <&panel_in0>;
+			};
+		};
+	};
+};
+
+&lvds1 {
+	status = "okay";
+
+	clocks = <&cpg CPG_MOD 727>, <&x13_clk>, <&extal_clk>;
+	clock-names = "fck", "dclkin.0", "extal";
+
+	ports {
+		port@1 {
+			lvds1_out: endpoint {
+				remote-endpoint = <&panel_in1>;
+			};
+		};
+	};
+};
+
+&pfc {
+	pwm5_pins: pwm5 {
+		groups = "pwm5_a";
+		function = "pwm5";
+	};
+};
+
+&pwm5 {
+	pinctrl-0 = <&pwm5_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
-- 
2.7.4

[PATCH v3 7/8] [HACK] arm64: dts: renesas: draak: Enable LVDS

[Fabrizio Castro]

Enable and connect the second LVDS encoder to the second LVDS input of
the THC63LVD1024 for dual-link LVDS operation. This requires changing
the default settings of SW45 and SW47 to OFF and ON respectively.

This patch is based on Laurent's dual-LVDS work:
https://patchwork.kernel.org/patch/10965045/

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
v2->v3:
* new patch

Geert,

no need to review this patch unless they like the idea behind this
series.

Thanks,
Fab

---
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index b38f9d4..38b9c5a 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -87,11 +87,20 @@
 
 			port@0 {
 				reg = <0>;
-				thc63lvd1024_in: endpoint {
+				dual-lvds-even-pixels;
+				thc63lvd1024_in0: endpoint {
 					remote-endpoint = <&lvds0_out>;
 				};
 			};
 
+			port@1 {
+				reg = <1>;
+				dual-lvds-odd-pixels;
+				thc63lvd1024_in1: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
+
 			port@2 {
 				reg = <2>;
 				thc63lvd1024_out: endpoint {
@@ -489,7 +498,7 @@
 	ports {
 		port@1 {
 			lvds0_out: endpoint {
-				remote-endpoint = <&thc63lvd1024_in>;
+				remote-endpoint = <&thc63lvd1024_in0>;
 			};
 		};
 	};
@@ -507,6 +516,14 @@
 		 <&x13_clk>,
 		 <&extal_clk>;
 	clock-names = "fck", "dclkin.0", "extal";
+
+	ports {
+		port@1 {
+			lvds1_out: endpoint {
+				remote-endpoint = <&thc63lvd1024_in1>;
+			};
+		};
+	};
 };
 
 &ohci0 {
-- 
2.7.4

[PATCH v3 8/8] [HACK] arm64: dts: renesas: draak: Enable LVDS dual-link operation

[Fabrizio Castro]

Enable and connect the second LVDS encoder to the second LVDS input of
the THC63LVD1024 for dual-link LVDS operation. This requires changing
the default settings of SW45 and SW47 to OFF and ON respectively.

This patch is based on Laurent's dual-LVDS work:
https://patchwork.kernel.org/patch/10965045/

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
v2->v3:
* new patch

Geert,

no need to review this patch unless they like the idea behind this
series.

Thanks,
Fab

---
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 67634cb..b4b8cde 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -77,11 +77,20 @@
 
 			port@0 {
 				reg = <0>;
-				thc63lvd1024_in: endpoint {
+				dual-lvds-even-pixels;
+				thc63lvd1024_in0: endpoint {
 					remote-endpoint = <&lvds0_out>;
 				};
 			};
 
+			port@1 {
+				reg = <1>;
+				dual-lvds-odd-pixels;
+				thc63lvd1024_in1: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
+
 			port@2 {
 				reg = <2>;
 				thc63lvd1024_out: endpoint {
@@ -368,24 +377,27 @@
 	ports {
 		port@1 {
 			lvds0_out: endpoint {
-				remote-endpoint = <&thc63lvd1024_in>;
+				remote-endpoint = <&thc63lvd1024_in0>;
 			};
 		};
 	};
 };
 
 &lvds1 {
-	/*
-	 * Even though the LVDS1 output is not connected, the encoder must be
-	 * enabled to supply a pixel clock to the DU for the DPAD output when
-	 * LVDS0 is in use.
-	 */
 	status = "okay";
 
 	clocks = <&cpg CPG_MOD 727>,
 		 <&x12_clk>,
 		 <&extal_clk>;
 	clock-names = "fck", "dclkin.0", "extal";
+
+	ports {
+		port@1 {
+			lvds1_out: endpoint {
+				remote-endpoint = <&thc63lvd1024_in1>;
+			};
+		};
+	};
 };
 
 &ohci0 {
-- 
2.7.4

Re: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings

[Geert Uytterhoeven]

Hi Fabrizio,

On Wed, Aug 28, 2019 at 8:36 PM Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
> Dual-LVDS connections need markers in the DT, this patch adds
> some common documentation to be referenced by both panels and
> bridges.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common Properties for bus timings of LVDS interfaces
> +
> +maintainers:
> +  - Thierry Reding <thierry.reding@gmail.com>
> +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> +
> +description: |
> +  This document defines device tree properties common to LVDS and dual-LVDS
> +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> +  pixels traveling on one connection, and with odd pixels traveling on the other
> +  connection.
> +  This document doesn't constitue a device tree binding specification by itself
> +  but is meant to be referenced by device tree bindings.
> +  When referenced from panel or bridge device tree bindings, the properties
> +  defined in this document are defined as follows. The panel and bridge device
> +  tree bindings are responsible for defining whether each property is required
> +  or optional.
> +
> +properties:
> +  dual-lvds-even-pixels:
> +    type: boolean
> +    description:
> +      This property is specific to an input port of a sink device. When
> +      specified, it marks the port as recipient of even-pixels.
> +
> +  dual-lvds-odd-pixels:
> +    type: boolean
> +    description:
> +      This property is specific to an input port of a sink device. When
> +      specified, it marks the port as recipient of odd-pixels.

Do you need the "dual-" prefix? Isn't that implied by even/odd?
Or is it better to keep it, for readability?

I'm also thinking about a possible future extension to triple or quad LVDS.
As I'm not aware of English word equivalents of even/odd for triple/quad,
perhaps this should be specified using a numerical value instead?

If I go too far, please just say so ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings

[Fabrizio Castro]

Hi Geert,

Thank you for your feedback!

> From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Geert Uytterhoeven
> Sent: 29 August 2019 08:58
> Subject: Re: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings
> 
> Hi Fabrizio,
> 
> On Wed, Aug 28, 2019 at 8:36 PM Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> wrote:
> > Dual-LVDS connections need markers in the DT, this patch adds
> > some common documentation to be referenced by both panels and
> > bridges.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Common Properties for bus timings of LVDS interfaces
> > +
> > +maintainers:
> > +  - Thierry Reding <thierry.reding@gmail.com>
> > +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > +
> > +description: |
> > +  This document defines device tree properties common to LVDS and dual-LVDS
> > +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> > +  pixels traveling on one connection, and with odd pixels traveling on the other
> > +  connection.
> > +  This document doesn't constitue a device tree binding specification by itself
> > +  but is meant to be referenced by device tree bindings.
> > +  When referenced from panel or bridge device tree bindings, the properties
> > +  defined in this document are defined as follows. The panel and bridge device
> > +  tree bindings are responsible for defining whether each property is required
> > +  or optional.
> > +
> > +properties:
> > +  dual-lvds-even-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> > +      specified, it marks the port as recipient of even-pixels.
> > +
> > +  dual-lvds-odd-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> > +      specified, it marks the port as recipient of odd-pixels.
> 
> Do you need the "dual-" prefix? Isn't that implied by even/odd?
> Or is it better to keep it, for readability?

I decided to go with "dual-lvds-even-pixels" and "dual-lvds-odd-pixels"
because the "dual-lvds" prefix uniquely identifies the type of bus, and I
decided to go with the "pixels" suffix because "dual-lvds-odd" just doesn't
sound right. I guess "dual-lvds-even-pixels" and "dual-lvds-odd-pixels"
are the most readable and future proof labels I could think of, but maybe
there is something better we can do? Laurent?

> 
> I'm also thinking about a possible future extension to triple or quad LVDS.
> As I'm not aware of English word equivalents of even/odd for triple/quad,
> perhaps this should be specified using a numerical value instead?

I would have to see a use case for other LVDS configurations for providing
a proper answer to this question, but perhaps we could accept that other
configurations for LVDS connections may come with labels that are tailored
to help uniquely identifying the ports while being readable? Perhaps numerical
values would work better in other cases? Laurent?

> 
> If I go too far, please just say so ;-)

Definitely worth discussing!

Thanks,
Fab

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings

[Rob Herring]

On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
>
> Dual-LVDS connections need markers in the DT, this patch adds
> some common documentation to be referenced by both panels and
> bridges.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
>
> ---
> v2->v3:
> * new patch
> ---
>  .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> new file mode 100644
> index 0000000..f35b55a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: GPL-2.0

(GPL-2.0-only OR BSD-2-Clause) is preferred for new bindings.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common Properties for bus timings of LVDS interfaces
> +
> +maintainers:
> +  - Thierry Reding <thierry.reding@gmail.com>
> +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> +
> +description: |
> +  This document defines device tree properties common to LVDS and dual-LVDS
> +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> +  pixels traveling on one connection, and with odd pixels traveling on the other
> +  connection.
> +  This document doesn't constitue a device tree binding specification by itself

typo: constitute

> +  but is meant to be referenced by device tree bindings.
> +  When referenced from panel or bridge device tree bindings, the properties
> +  defined in this document are defined as follows. The panel and bridge device
> +  tree bindings are responsible for defining whether each property is required
> +  or optional.
> +
> +properties:
> +  dual-lvds-even-pixels:
> +    type: boolean
> +    description:
> +      This property is specific to an input port of a sink device. When

The schema should define what nodes these go in. The description seems
to indicate in 'port' nodes (or endpoint?), but your use in the panel
binding puts them in the parent.

> +      specified, it marks the port as recipient of even-pixels.
> +
> +  dual-lvds-odd-pixels:
> +    type: boolean
> +    description:
> +      This property is specific to an input port of a sink device. When
> +      specified, it marks the port as recipient of odd-pixels.

However, I don't think you even need these. A panel's port numbers are
fixed can imply even or odd. For example port@0 can be even and port@1
can be odd. The port numbering is typically panel specific, but we may
be able to define the numbering generically if we don't already have
panels with multiple ports.

Also, aren't there dual link DSI panels?

Rob

RE: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings

[Fabrizio Castro]

Hi Rob,

Thank you for your feedback!

> From: Rob Herring <robh+dt@kernel.org>
> Sent: 29 August 2019 15:03
> Subject: Re: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings
> 
> On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> wrote:
> >
> > Dual-LVDS connections need markers in the DT, this patch adds
> > some common documentation to be referenced by both panels and
> > bridges.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v2->v3:
> > * new patch
> > ---
> >  .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml b/Documentation/devicetree/bindings/display/bus-
> timings/lvds.yaml
> > new file mode 100644
> > index 0000000..f35b55a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> (GPL-2.0-only OR BSD-2-Clause) is preferred for new bindings.
> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Common Properties for bus timings of LVDS interfaces
> > +
> > +maintainers:
> > +  - Thierry Reding <thierry.reding@gmail.com>
> > +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > +
> > +description: |
> > +  This document defines device tree properties common to LVDS and dual-LVDS
> > +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> > +  pixels traveling on one connection, and with odd pixels traveling on the other
> > +  connection.
> > +  This document doesn't constitue a device tree binding specification by itself
> 
> typo: constitute

Well spotted!

> 
> > +  but is meant to be referenced by device tree bindings.
> > +  When referenced from panel or bridge device tree bindings, the properties
> > +  defined in this document are defined as follows. The panel and bridge device
> > +  tree bindings are responsible for defining whether each property is required
> > +  or optional.
> > +
> > +properties:
> > +  dual-lvds-even-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> 
> The schema should define what nodes these go in. The description seems
> to indicate in 'port' nodes (or endpoint?), but your use in the panel
> binding puts them in the parent.

Did you manage to read this?
https://patchwork.kernel.org/cover/11119607/

Could you please advice on how to do this properly?

> 
> > +      specified, it marks the port as recipient of even-pixels.
> > +
> > +  dual-lvds-odd-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> > +      specified, it marks the port as recipient of odd-pixels.
> 
> However, I don't think you even need these. A panel's port numbers are
> fixed can imply even or odd. For example port@0 can be even and port@1
> can be odd. The port numbering is typically panel specific, but we may
> be able to define the numbering generically if we don't already have
> panels with multiple ports.
> 
> Also, aren't there dual link DSI panels?

This is the result of a discussion on here:
https://patchwork.kernel.org/patch/11095547/

Have you come across it?

Thanks!
Fab

> 
> Rob

Re: [PATCH v3 0/8] Add dual-LVDS panel support to EK874

[Rob Herring]

On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
>
> Dear All,
>
> this series adds support for dual-LVDS panel IDK-2121WR
> from Advantech:
> https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
>
> V3 approaches the problem in a completely different way, we now
> have two new properties to mark the ports in the DT as receiving
> even pixels and odd pixels: dual-lvds-even-pixels and dual-lvds-odd-pixels,
> which means device drivers should not use bridge specific or panel
> specific dual_link flags. Also, in this case the DT describes the
> connection fully.
>
> In order for the solution to be generic, I have exported a new helper
> (drm_of_lvds_get_dual_link_configuration) to walk the device tree,
> and figure out if the connection is dual-LVDS. The same helper gives
> information about the configuration of the connection. If Px is connected
> to a port expecting even pixels and Py is connected to a port expecting
> odd pixels, then the helper returns DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS
> (like in the example below), otherwise it returns
> DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS.
>
>
>  --------            dual-lvds-even-pixels  --------
> |        |----                         ----|        |
> |        | Px |---------------------->| Pn |        |
> |        |----                         ----|        |
> | SOURCE |           dual-lvds-odd-pixels  |  SINK  |
> |        |----                         ----|        |
> |        | Py |---------------------->| Pm |        |
> |        |----                         ----|        |
>  --------                                   --------
>
> The device driver for the encoder then will work out if with the current
> wiring the pixels need swapping or not.
>
> The same solution works for both panels and bridges.
>
> Since the DT describes the connection fully, driver
> drivers/gpu/drm/panel/panel-lvds.c works out-of-the-box, no changes
> required, however, this implementation opens up a problem with the
> dt-bindings.
> Driver drivers/gpu/drm/panel/panel-lvds.c can still be pleased by
> a port node, but also by a ports node.
> I have created Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> with the idea of including it from panels and bridges dt-bindings
> supporting dual-LVDS (and of course the dt-bindings for the specific
> devices should say which port should be marked as what), but file
> Documentation/devicetree/bindings/display/panel/lvds.yaml formally
> requires property "port", while with this implementation it should require
> OneOf "port" and "ports", and unfortunately I can't seem to find a neat way
> aroud that, other than creating a new compatible string

Just add 'ports' and drop 'port' from being required in the common
binding. Then it is up to the panel specific bindings to define which
one is required. Or we just leave it to allow either form which the
graph code can handle.

We could have this in the common binding:

oneOf:
 - required: [ports]
 - required: [port]

Rob

RE: [PATCH v3 0/8] Add dual-LVDS panel support to EK874

[Fabrizio Castro]

Hi Rob,

Thank you for your feedback!

> From: Rob Herring <robh+dt@kernel.org>
> Sent: 29 August 2019 16:27
> To: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Subject: Re: [PATCH v3 0/8] Add dual-LVDS panel support to EK874
> 
> On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> wrote:
> >
> > Dear All,
> >
> > this series adds support for dual-LVDS panel IDK-2121WR
> > from Advantech:
> > https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> >
> > V3 approaches the problem in a completely different way, we now
> > have two new properties to mark the ports in the DT as receiving
> > even pixels and odd pixels: dual-lvds-even-pixels and dual-lvds-odd-pixels,
> > which means device drivers should not use bridge specific or panel
> > specific dual_link flags. Also, in this case the DT describes the
> > connection fully.
> >
> > In order for the solution to be generic, I have exported a new helper
> > (drm_of_lvds_get_dual_link_configuration) to walk the device tree,
> > and figure out if the connection is dual-LVDS. The same helper gives
> > information about the configuration of the connection. If Px is connected
> > to a port expecting even pixels and Py is connected to a port expecting
> > odd pixels, then the helper returns DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS
> > (like in the example below), otherwise it returns
> > DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS.
> >
> >
> >  --------            dual-lvds-even-pixels  --------
> > |        |----                         ----|        |
> > |        | Px |---------------------->| Pn |        |
> > |        |----                         ----|        |
> > | SOURCE |           dual-lvds-odd-pixels  |  SINK  |
> > |        |----                         ----|        |
> > |        | Py |---------------------->| Pm |        |
> > |        |----                         ----|        |
> >  --------                                   --------
> >
> > The device driver for the encoder then will work out if with the current
> > wiring the pixels need swapping or not.
> >
> > The same solution works for both panels and bridges.
> >
> > Since the DT describes the connection fully, driver
> > drivers/gpu/drm/panel/panel-lvds.c works out-of-the-box, no changes
> > required, however, this implementation opens up a problem with the
> > dt-bindings.
> > Driver drivers/gpu/drm/panel/panel-lvds.c can still be pleased by
> > a port node, but also by a ports node.
> > I have created Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> > with the idea of including it from panels and bridges dt-bindings
> > supporting dual-LVDS (and of course the dt-bindings for the specific
> > devices should say which port should be marked as what), but file
> > Documentation/devicetree/bindings/display/panel/lvds.yaml formally
> > requires property "port", while with this implementation it should require
> > OneOf "port" and "ports", and unfortunately I can't seem to find a neat way
> > aroud that, other than creating a new compatible string
> 
> Just add 'ports' and drop 'port' from being required in the common
> binding. Then it is up to the panel specific bindings to define which
> one is required. Or we just leave it to allow either form which the
> graph code can handle.
> 
> We could have this in the common binding:
> 
> oneOf:
>  - required: [ports]
>  - required: [port]


Thank you for Rob for looking into this. I will wait for a feedback from Laurent
on the code before sending out v4.

Thanks,
Fab

> 
> Rob

RE: [PATCH v3 0/8] Add dual-LVDS panel support to EK874

[Fabrizio Castro]

Hi Laurent,

Did you have any time to look into this series?

Thanks,
Fab

> From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Sent: 28 August 2019 19:37
> Subject: [PATCH v3 0/8] Add dual-LVDS panel support to EK874
> 
> Dear All,
> 
> this series adds support for dual-LVDS panel IDK-2121WR
> from Advantech:
> https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> 
> V3 approaches the problem in a completely different way, we now
> have two new properties to mark the ports in the DT as receiving
> even pixels and odd pixels: dual-lvds-even-pixels and dual-lvds-odd-pixels,
> which means device drivers should not use bridge specific or panel
> specific dual_link flags. Also, in this case the DT describes the
> connection fully.
> 
> In order for the solution to be generic, I have exported a new helper
> (drm_of_lvds_get_dual_link_configuration) to walk the device tree,
> and figure out if the connection is dual-LVDS. The same helper gives
> information about the configuration of the connection. If Px is connected
> to a port expecting even pixels and Py is connected to a port expecting
> odd pixels, then the helper returns DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS
> (like in the example below), otherwise it returns
> DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS.
> 
> 
>  --------            dual-lvds-even-pixels  --------
> |        |----                         ----|        |
> |        | Px |---------------------->| Pn |        |
> |        |----                         ----|        |
> | SOURCE |           dual-lvds-odd-pixels  |  SINK  |
> |        |----                         ----|        |
> |        | Py |---------------------->| Pm |        |
> |        |----                         ----|        |
>  --------                                   --------
> 
> The device driver for the encoder then will work out if with the current
> wiring the pixels need swapping or not.
> 
> The same solution works for both panels and bridges.
> 
> Since the DT describes the connection fully, driver
> drivers/gpu/drm/panel/panel-lvds.c works out-of-the-box, no changes
> required, however, this implementation opens up a problem with the
> dt-bindings.
> Driver drivers/gpu/drm/panel/panel-lvds.c can still be pleased by
> a port node, but also by a ports node.
> I have created Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> with the idea of including it from panels and bridges dt-bindings
> supporting dual-LVDS (and of course the dt-bindings for the specific
> devices should say which port should be marked as what), but file
> Documentation/devicetree/bindings/display/panel/lvds.yaml formally
> requires property "port", while with this implementation it should require
> OneOf "port" and "ports", and unfortunately I can't seem to find a neat way
> aroud that, other than creating a new compatible string
> (e.g. "panel-dual-lvds"), a new dt-binding document for it, and of course adding
> support for the new compatible string to drivers/gpu/drm/panel/panel-lvds.c.
> As a result, this series is missing (at least) a patch necessary to fully
> document the new implementation within
> Documentation/devicetree/bindings/display/panel/lvds.yaml
> 
> Rob, do you have any suggestions? Do you think this idea works ok from a
> documentation point of view? By the way, I don't really know what I am doing
> with the yaml dt-bindings, I hope you won't be horrified by this series :-P
> 
> I hope I was able to deliver the concept clearly, if not please just ask.
> 
> Comments are very much appreciated.
> 
> Thanks,
> Fab
> 
> Fabrizio Castro (8):
>   dt-bindings: display: Add bindings for LVDS bus-timings
>   dt-bindings: display: Add idk-2121wr binding
>   drm: Add bus timings helper
>   drm: rcar-du: lvds: Add dual-LVDS panels support
>   drm: bridge: thc63: Do not report input bus mode through bridge
>     timings
>   arm64: dts: renesas: Add EK874 board with idk-2121wr display support
>   [HACK] arm64: dts: renesas: draak: Enable LVDS
>   [HACK] arm64: dts: renesas: draak: Enable LVDS dual-link operation
> 
>  .../bindings/display/bus-timings/lvds.yaml         |  38 +++++++
>  .../display/panel/advantech,idk-2121wr.yaml        |  90 ++++++++++++++++
>  arch/arm64/boot/dts/renesas/Makefile               |   3 +-
>  .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 116 +++++++++++++++++++++
>  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  21 +++-
>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts     |  26 +++--
>  drivers/gpu/drm/Makefile                           |   3 +-
>  drivers/gpu/drm/bridge/thc63lvd1024.c              |   9 +-
>  drivers/gpu/drm/drm_bus_timings.c                  |  97 +++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_lvds.c                | 110 +++++++++++--------
>  include/drm/drm_bridge.h                           |   8 --
>  include/drm/drm_bus_timings.h                      |  21 ++++
>  12 files changed, 473 insertions(+), 69 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
>  create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
>  create mode 100644 drivers/gpu/drm/drm_bus_timings.c
>  create mode 100644 include/drm/drm_bus_timings.h
> 
> --
> 2.7.4

Re: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings

[Laurent Pinchart]

Hello Fabrizio,

On Thu, Aug 29, 2019 at 02:38:06PM +0000, Fabrizio Castro wrote:
> On 29 August 2019 15:03 Rob Herring wrote:
> > On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro wrote:
> >>
> >> Dual-LVDS connections need markers in the DT, this patch adds
> >> some common documentation to be referenced by both panels and
> >> bridges.
> >>
> >> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >>
> >> ---
> >> v2->v3:
> >> * new patch
> >> ---
> >>  .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
> >>  1 file changed, 38 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> >> new file mode 100644
> >> index 0000000..f35b55a
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> >> @@ -0,0 +1,38 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> > 
> > (GPL-2.0-only OR BSD-2-Clause) is preferred for new bindings.
> > 
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Common Properties for bus timings of LVDS interfaces
> >> +
> >> +maintainers:
> >> +  - Thierry Reding <thierry.reding@gmail.com>
> >> +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >> +
> >> +description: |
> >> +  This document defines device tree properties common to LVDS and dual-LVDS
> >> +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> >> +  pixels traveling on one connection, and with odd pixels traveling on the other
> >> +  connection.

As you define a dual-LVDS interface as "a dual-link connection", should
this be "even pixels traveling on one link, and with odd pixels
traveling on the other link" ?

> >> +  This document doesn't constitue a device tree binding specification by itself
> > 
> > typo: constitute
> 
> Well spotted!
> 
> >> +  but is meant to be referenced by device tree bindings.
> >> +  When referenced from panel or bridge device tree bindings, the properties
> >> +  defined in this document are defined as follows. The panel and bridge device
> >> +  tree bindings are responsible for defining whether each property is required
> >> +  or optional.
> >> +
> >> +properties:
> >> +  dual-lvds-even-pixels:
> >> +    type: boolean
> >> +    description:
> >> +      This property is specific to an input port of a sink device. When
> > 
> > The schema should define what nodes these go in. The description seems
> > to indicate in 'port' nodes (or endpoint?), but your use in the panel
> > binding puts them in the parent.
> 
> Did you manage to read this?
> https://patchwork.kernel.org/cover/11119607/
> 
> Could you please advice on how to do this properly?
> 
> >> +      specified, it marks the port as recipient of even-pixels.
> >> +
> >> +  dual-lvds-odd-pixels:
> >> +    type: boolean
> >> +    description:
> >> +      This property is specific to an input port of a sink device. When
> >> +      specified, it marks the port as recipient of odd-pixels.
> > 
> > However, I don't think you even need these. A panel's port numbers are
> > fixed can imply even or odd. For example port@0 can be even and port@1
> > can be odd. The port numbering is typically panel specific, but we may
> > be able to define the numbering generically if we don't already have
> > panels with multiple ports.
> > 
> > Also, aren't there dual link DSI panels?
> 
> This is the result of a discussion on here:
> https://patchwork.kernel.org/patch/11095547/
> 
> Have you come across it?

Let me repeat my comments from that thread for Rob in order to
centralize the discussion here.

> Taking one step back to look at the big picture, what we need is for the
> source to know what pixel (odd or even) to send on each LVDS output. For
> that it needs to know what pixel is expected by the sink the the inputs
> connected to the source's outputs. From each source output we can easily
> locate the DT nodes corresponding to the connected sink's input ports,
> but that doesn't give us enough information. From there, we could
> 
> - Infer the odd/even pixel expected by the source based on the port
>   numbers. This would require common DT bindings for all dual-link LVDS
>   sinks that specify the port ordering (for instance the bindings could
>   mandate that lowest numbered port correspond to even pixels).
> 
> - Read the odd/even pixel expected by the source from an explicit DT
>   property, as proposed above. This would also require common DT
>   bindings for all dual-link LVDS sinks that define these new
>   properties. This would I think be better than implicitly infering it
>   from DT port numbers.
> 
> - Retrieve the information from the sink drm_bridge at runtime. This
>   would require a way to query the bridge for the mapping from port
>   number to odd/even pixel. The DRM_LINK_DUAL_LVDS_ODD_EVEN flag could
>   be used for that, and would then be defined as "the lowest numbered
>   port corresponds to odd pixels and the highest numbered port
>   corresponds to even pixels".
> 
> The second and third options would both work I think. The third one is
> roughly what you've implemented, except that I think the flag
> description should be clarified.

Rob, what's your opinion ? We could certainly, in the context of a
panel, decide of a fixed mapping of port numbers to odd/even pixels, but
the issue is that sources need to know which pixels to send on which
link (assuming of course that this can be configured on the source
side). We thus need a way for the source to answer, at runtime, the
question "which of ports A and B of the sink correspond to even and odd
pixels ?". This can't be inferred by the source from the sink port
numbers, as the mapping between port number and odd/even pixels is
specific to each sink. We thus need to either store that property in the
DT node of the sink (option 2) or query it at runtime from the sink
(option 3).

I like option 2 as it's more explicit, but option 3 minimizes the
required properties in DT, which is always good. Patch 3/8 introduces a
helper that abstracts this from a sink point of view (which I think is a
very good idea), so once we decide which option to use, only 3/8 may
need to be adapted, the other patches should hopefully not require
rework.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/8] dt-bindings: display: Add idk-2121wr binding

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Wed, Aug 28, 2019 at 07:36:36PM +0100, Fabrizio Castro wrote:
> Add binding for the idk-2121wr LVDS panel from Advantech.
> 
> Some panel-specific documentation can be found here:
> https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> ---
> v2->v3:
> * new patch
> ---
>  .../display/panel/advantech,idk-2121wr.yaml        | 90 ++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> new file mode 100644
> index 0000000..b2ccdc8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/advantech,idk-2121wr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Advantech IDK-2121WR 21.5" Full-HD dual-LVDS panel
> +
> +maintainers:
> +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> +  - Thierry Reding <thierry.reding@gmail.com>
> +
> +description: |
> +  The IDK-2121WR from Advantech is a Full-HD dual-LVDS panel.
> +
> +  The panels expects odd pixels from the first port, and even pixels from

s/panels/panel/
Maybe s/from the/on the/g ?

> +  the second port, therefore the ports must be marked accordingly.
> +
> +allOf:
> +  - $ref: lvds.yaml#
> +  - $ref: ../bus-timings/lvds.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: advantech,idk-2121wr
> +      - {} # panel-lvds, but not listed here to avoid false select
> +
> +  data-mapping:
> +    const: vesa-24
> +
> +  width-mm:
> +    const: 476
> +
> +  height-mm:
> +    const: 268
> +
> +  panel-timing: true
> +  ports: true
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible

Shouldn't data-mapping, width-mm, height-mm and ports be required too ?

As you mentioned in the cover letter, validating ports, port and the new
dual-lvds-*-pixels properties would be nice. I'm not YAML schema
specialist, so I'm fine with a best effort approach here, but as far as
I understand Rob proposed a way forward, could you try it ?

Apart from that, the bindings look sne to me, so

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

once the above issues get addressed.

> +
> +examples:
> +  - |+
> +    panel-lvds {
> +      compatible = "advantech,idk-2121wr", "panel-lvds";
> +
> +      width-mm = <476>;
> +      height-mm = <268>;
> +
> +      data-mapping = "vesa-24";
> +
> +      panel-timing {
> +        clock-frequency = <148500000>;
> +        hactive = <1920>;
> +        vactive = <1080>;
> +        hsync-len = <44>;
> +        hfront-porch = <88>;
> +        hback-porch = <148>;
> +        vfront-porch = <4>;
> +        vback-porch = <36>;
> +        vsync-len = <5>;
> +      };
> +
> +      ports {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        port@0 {
> +          reg = <0>;
> +          dual-lvds-odd-pixels;
> +          panel_in0: endpoint {
> +            remote-endpoint = <&lvds0_out>;
> +          };
> +        };
> +
> +        port@1 {
> +          reg = <1>;
> +          dual-lvds-even-pixels;
> +          panel_in1: endpoint {
> +            remote-endpoint = <&lvds1_out>;
> +          };
> +        };
> +      };
> +    };
> +
> +...

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 6/8] arm64: dts: renesas: Add EK874 board with idk-2121wr display support

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Wed, Aug 28, 2019 at 07:36:40PM +0100, Fabrizio Castro wrote:
> The EK874 is advertised as compatible with panel IDK-2121WR from
> Advantech, however the panel isn't sold alongside the board.
> A new dts, adding everything that's required to get the panel to
> to work with the EK874, is the most convenient way to support the
> EK874 when it's connected to the IDK-2121WR.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> ---
> v1->v2:
> * Added comment for lvds-connector-en-gpio
> * Renamed &lvds0_panel_in to panel_in0
> * Renamed &lvds1_panel_in to panel_in1
> 
> v2->v3:
> * removed renesas,swap-data property
> * added dual-lvds-odd-pixels and dual-lvds-even-pixels properties
> 
> Geert,
> 
> no need to review this patch unless they like the idea behind this
> series.

I like the idea :-) We still have a few issues to solve, but it should
make it in.

The patch looks sane to me, but I haven't checked if it matches the
hardware, so

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

(even if this really calls for DT overlays)

> ---
>  arch/arm64/boot/dts/renesas/Makefile               |   3 +-
>  .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 116 +++++++++++++++++++++
>  2 files changed, 118 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> 
> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index 42b74c2..ce48478 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -1,7 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0
>  dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m.dtb
>  dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-ex.dtb
> -dtb-$(CONFIG_ARCH_R8A774C0) += r8a774c0-cat874.dtb r8a774c0-ek874.dtb
> +dtb-$(CONFIG_ARCH_R8A774C0) += r8a774c0-cat874.dtb r8a774c0-ek874.dtb \
> +			       r8a774c0-ek874-idk-2121wr.dtb
>  dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-x.dtb r8a7795-h3ulcb.dtb
>  dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-h3ulcb-kf.dtb
>  dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-xs.dtb
> diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> new file mode 100644
> index 0000000..a7b27d0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the Silicon Linux RZ/G2E evaluation kit (EK874),
> + * connected to an Advantech IDK-2121WR 21.5" LVDS panel
> + *
> + * Copyright (C) 2019 Renesas Electronics Corp.
> + */
> +
> +#include "r8a774c0-ek874.dts"
> +
> +/ {
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pwms = <&pwm5 0 50000>;
> +
> +		brightness-levels = <0 4 8 16 32 64 128 255>;
> +		default-brightness-level = <6>;
> +
> +		power-supply = <&reg_12p0v>;
> +		enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	panel-lvds {
> +		compatible = "advantech,idk-2121wr", "panel-lvds";
> +
> +		width-mm = <476>;
> +		height-mm = <268>;
> +
> +		data-mapping = "vesa-24";
> +
> +		panel-timing {
> +			clock-frequency = <148500000>;
> +			hactive = <1920>;
> +			vactive = <1080>;
> +			hsync-len = <44>;
> +			hfront-porch = <88>;
> +			hback-porch = <148>;
> +			vfront-porch = <4>;
> +			vback-porch = <36>;
> +			vsync-len = <5>;
> +		};
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				dual-lvds-odd-pixels;
> +				panel_in0: endpoint {
> +					remote-endpoint = <&lvds0_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				dual-lvds-even-pixels;
> +				panel_in1: endpoint {
> +					remote-endpoint = <&lvds1_out>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +&gpio0 {
> +	/*
> +	 * When GP0_17 is low LVDS[01] are connected to the LVDS connector
> +	 * When GP0_17 is high LVDS[01] are connected to the LT8918L
> +	 */
> +	lvds-connector-en-gpio{
> +		gpio-hog;
> +		gpios = <17 GPIO_ACTIVE_HIGH>;
> +		output-low;
> +		line-name = "lvds-connector-en-gpio";
> +	};
> +};
> +
> +&lvds0 {
> +	ports {
> +		port@1 {
> +			lvds0_out: endpoint {
> +				remote-endpoint = <&panel_in0>;
> +			};
> +		};
> +	};
> +};
> +
> +&lvds1 {
> +	status = "okay";
> +
> +	clocks = <&cpg CPG_MOD 727>, <&x13_clk>, <&extal_clk>;
> +	clock-names = "fck", "dclkin.0", "extal";
> +
> +	ports {
> +		port@1 {
> +			lvds1_out: endpoint {
> +				remote-endpoint = <&panel_in1>;
> +			};
> +		};
> +	};
> +};
> +
> +&pfc {
> +	pwm5_pins: pwm5 {
> +		groups = "pwm5_a";
> +		function = "pwm5";
> +	};
> +};
> +
> +&pwm5 {
> +	pinctrl-0 = <&pwm5_pins>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 7/8] [HACK] arm64: dts: renesas: draak: Enable LVDS

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

The subject is wrong, it should be

[HACK] arm64: dts: renesas: ebisu: Enable LVDS dual-link operation

On Wed, Aug 28, 2019 at 07:36:41PM +0100, Fabrizio Castro wrote:
> Enable and connect the second LVDS encoder to the second LVDS input of
> the THC63LVD1024 for dual-link LVDS operation. This requires changing
> the default settings of SW45 and SW47 to OFF and ON respectively.
> 
> This patch is based on Laurent's dual-LVDS work:
> https://patchwork.kernel.org/patch/10965045/
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
> v2->v3:
> * new patch
> 
> Geert,
> 
> no need to review this patch unless they like the idea behind this
> series.
> 
> Thanks,
> Fab
> 
> ---
>  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> index b38f9d4..38b9c5a 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> @@ -87,11 +87,20 @@
>  
>  			port@0 {
>  				reg = <0>;
> -				thc63lvd1024_in: endpoint {
> +				dual-lvds-even-pixels;
> +				thc63lvd1024_in0: endpoint {
>  					remote-endpoint = <&lvds0_out>;
>  				};
>  			};
>  
> +			port@1 {
> +				reg = <1>;
> +				dual-lvds-odd-pixels;
> +				thc63lvd1024_in1: endpoint {
> +					remote-endpoint = <&lvds1_out>;
> +				};
> +			};
> +
>  			port@2 {
>  				reg = <2>;
>  				thc63lvd1024_out: endpoint {
> @@ -489,7 +498,7 @@
>  	ports {
>  		port@1 {
>  			lvds0_out: endpoint {
> -				remote-endpoint = <&thc63lvd1024_in>;
> +				remote-endpoint = <&thc63lvd1024_in0>;
>  			};
>  		};
>  	};
> @@ -507,6 +516,14 @@
>  		 <&x13_clk>,
>  		 <&extal_clk>;
>  	clock-names = "fck", "dclkin.0", "extal";
> +
> +	ports {
> +		port@1 {
> +			lvds1_out: endpoint {
> +				remote-endpoint = <&thc63lvd1024_in1>;
> +			};
> +		};
> +	};
>  };
>  
>  &ohci0 {
> -- 
> 2.7.4
> 

-- 
Regards,

Laurent Pinchart

RE: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings

[Fabrizio Castro]

Hi Rob,

> From: Rob Herring <robh+dt@kernel.org>
> Sent: 29 August 2019 15:03
> Subject: Re: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings
> 
> On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> wrote:
> >
> > Dual-LVDS connections need markers in the DT, this patch adds
> > some common documentation to be referenced by both panels and
> > bridges.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v2->v3:
> > * new patch
> > ---
> >  .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml b/Documentation/devicetree/bindings/display/bus-
> timings/lvds.yaml
> > new file mode 100644
> > index 0000000..f35b55a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> (GPL-2.0-only OR BSD-2-Clause) is preferred for new bindings.
> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Common Properties for bus timings of LVDS interfaces
> > +
> > +maintainers:
> > +  - Thierry Reding <thierry.reding@gmail.com>
> > +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > +
> > +description: |
> > +  This document defines device tree properties common to LVDS and dual-LVDS
> > +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> > +  pixels traveling on one connection, and with odd pixels traveling on the other
> > +  connection.
> > +  This document doesn't constitue a device tree binding specification by itself
> 
> typo: constitute
> 
> > +  but is meant to be referenced by device tree bindings.
> > +  When referenced from panel or bridge device tree bindings, the properties
> > +  defined in this document are defined as follows. The panel and bridge device
> > +  tree bindings are responsible for defining whether each property is required
> > +  or optional.
> > +
> > +properties:
> > +  dual-lvds-even-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> 
> The schema should define what nodes these go in. The description seems
> to indicate in 'port' nodes (or endpoint?), but your use in the panel
> binding puts them in the parent.
> 
> > +      specified, it marks the port as recipient of even-pixels.
> > +
> > +  dual-lvds-odd-pixels:
> > +    type: boolean
> > +    description:
> > +      This property is specific to an input port of a sink device. When
> > +      specified, it marks the port as recipient of odd-pixels.
> 
> However, I don't think you even need these. A panel's port numbers are
> fixed can imply even or odd. For example port@0 can be even and port@1
> can be odd. The port numbering is typically panel specific, but we may
> be able to define the numbering generically if we don't already have
> panels with multiple ports.

The ports on the receiving end of the link can be identified by any number,
not necessarily 0 and 1. Since at this point in time we only have 1 use case
for this I'll merge the generic and panel specific dt-schemas together, for
simplicity.
Perhaps we can split them back once we have a second use case?
I'll send a new patch shortly.

Thank you very much for your help and your patience!

Fab

> 
> Also, aren't there dual link DSI panels?
> 
> Rob

RE: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: devicetree-owner@vger.kernel.org <devicetree-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> Sent: 07 November 2019 18:01
> Subject: Re: [PATCH v3 1/8] dt-bindings: display: Add bindings for LVDS bus-timings
> 
> Hello Fabrizio,
> 
> On Thu, Aug 29, 2019 at 02:38:06PM +0000, Fabrizio Castro wrote:
> > On 29 August 2019 15:03 Rob Herring wrote:
> > > On Wed, Aug 28, 2019 at 1:36 PM Fabrizio Castro wrote:
> > >>
> > >> Dual-LVDS connections need markers in the DT, this patch adds
> > >> some common documentation to be referenced by both panels and
> > >> bridges.
> > >>
> > >> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > >>
> > >> ---
> > >> v2->v3:
> > >> * new patch
> > >> ---
> > >>  .../bindings/display/bus-timings/lvds.yaml         | 38 ++++++++++++++++++++++
> > >>  1 file changed, 38 insertions(+)
> > >>  create mode 100644 Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> > >> new file mode 100644
> > >> index 0000000..f35b55a
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/display/bus-timings/lvds.yaml
> > >> @@ -0,0 +1,38 @@
> > >> +# SPDX-License-Identifier: GPL-2.0
> > >
> > > (GPL-2.0-only OR BSD-2-Clause) is preferred for new bindings.
> > >
> > >> +%YAML 1.2
> > >> +---
> > >> +$id: http://devicetree.org/schemas/display/bus-timings/lvds.yaml#
> > >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > >> +
> > >> +title: Common Properties for bus timings of LVDS interfaces
> > >> +
> > >> +maintainers:
> > >> +  - Thierry Reding <thierry.reding@gmail.com>
> > >> +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > >> +
> > >> +description: |
> > >> +  This document defines device tree properties common to LVDS and dual-LVDS
> > >> +  interfaces, where a dual-LVDS interface is a dual-link connection with even
> > >> +  pixels traveling on one connection, and with odd pixels traveling on the other
> > >> +  connection.
> 
> As you define a dual-LVDS interface as "a dual-link connection", should
> this be "even pixels traveling on one link, and with odd pixels
> traveling on the other link" ?

Will change.

Thanks,
Fab

> 
> > >> +  This document doesn't constitue a device tree binding specification by itself
> > >
> > > typo: constitute
> >
> > Well spotted!
> >
> > >> +  but is meant to be referenced by device tree bindings.
> > >> +  When referenced from panel or bridge device tree bindings, the properties
> > >> +  defined in this document are defined as follows. The panel and bridge device
> > >> +  tree bindings are responsible for defining whether each property is required
> > >> +  or optional.
> > >> +
> > >> +properties:
> > >> +  dual-lvds-even-pixels:
> > >> +    type: boolean
> > >> +    description:
> > >> +      This property is specific to an input port of a sink device. When
> > >
> > > The schema should define what nodes these go in. The description seems
> > > to indicate in 'port' nodes (or endpoint?), but your use in the panel
> > > binding puts them in the parent.
> >
> > Did you manage to read this?
> > https://patchwork.kernel.org/cover/11119607/
> >
> > Could you please advice on how to do this properly?
> >
> > >> +      specified, it marks the port as recipient of even-pixels.
> > >> +
> > >> +  dual-lvds-odd-pixels:
> > >> +    type: boolean
> > >> +    description:
> > >> +      This property is specific to an input port of a sink device. When
> > >> +      specified, it marks the port as recipient of odd-pixels.
> > >
> > > However, I don't think you even need these. A panel's port numbers are
> > > fixed can imply even or odd. For example port@0 can be even and port@1
> > > can be odd. The port numbering is typically panel specific, but we may
> > > be able to define the numbering generically if we don't already have
> > > panels with multiple ports.
> > >
> > > Also, aren't there dual link DSI panels?
> >
> > This is the result of a discussion on here:
> > https://patchwork.kernel.org/patch/11095547/
> >
> > Have you come across it?
> 
> Let me repeat my comments from that thread for Rob in order to
> centralize the discussion here.
> 
> > Taking one step back to look at the big picture, what we need is for the
> > source to know what pixel (odd or even) to send on each LVDS output. For
> > that it needs to know what pixel is expected by the sink the the inputs
> > connected to the source's outputs. From each source output we can easily
> > locate the DT nodes corresponding to the connected sink's input ports,
> > but that doesn't give us enough information. From there, we could
> >
> > - Infer the odd/even pixel expected by the source based on the port
> >   numbers. This would require common DT bindings for all dual-link LVDS
> >   sinks that specify the port ordering (for instance the bindings could
> >   mandate that lowest numbered port correspond to even pixels).
> >
> > - Read the odd/even pixel expected by the source from an explicit DT
> >   property, as proposed above. This would also require common DT
> >   bindings for all dual-link LVDS sinks that define these new
> >   properties. This would I think be better than implicitly infering it
> >   from DT port numbers.
> >
> > - Retrieve the information from the sink drm_bridge at runtime. This
> >   would require a way to query the bridge for the mapping from port
> >   number to odd/even pixel. The DRM_LINK_DUAL_LVDS_ODD_EVEN flag could
> >   be used for that, and would then be defined as "the lowest numbered
> >   port corresponds to odd pixels and the highest numbered port
> >   corresponds to even pixels".
> >
> > The second and third options would both work I think. The third one is
> > roughly what you've implemented, except that I think the flag
> > description should be clarified.
> 
> Rob, what's your opinion ? We could certainly, in the context of a
> panel, decide of a fixed mapping of port numbers to odd/even pixels, but
> the issue is that sources need to know which pixels to send on which
> link (assuming of course that this can be configured on the source
> side). We thus need a way for the source to answer, at runtime, the
> question "which of ports A and B of the sink correspond to even and odd
> pixels ?". This can't be inferred by the source from the sink port
> numbers, as the mapping between port number and odd/even pixels is
> specific to each sink. We thus need to either store that property in the
> DT node of the sink (option 2) or query it at runtime from the sink
> (option 3).
> 
> I like option 2 as it's more explicit, but option 3 minimizes the
> required properties in DT, which is always good. Patch 3/8 introduces a
> helper that abstracts this from a sink point of view (which I think is a
> very good idea), so once we decide which option to use, only 3/8 may
> need to be adapted, the other patches should hopefully not require
> rework.
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH v3 2/8] dt-bindings: display: Add idk-2121wr binding

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 07 November 2019 18:12
> Subject: Re: [PATCH v3 2/8] dt-bindings: display: Add idk-2121wr binding
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Wed, Aug 28, 2019 at 07:36:36PM +0100, Fabrizio Castro wrote:
> > Add binding for the idk-2121wr LVDS panel from Advantech.
> >
> > Some panel-specific documentation can be found here:
> > https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > ---
> > v2->v3:
> > * new patch
> > ---
> >  .../display/panel/advantech,idk-2121wr.yaml        | 90 ++++++++++++++++++++++
> >  1 file changed, 90 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> > new file mode 100644
> > index 0000000..b2ccdc8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml
> > @@ -0,0 +1,90 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/advantech,idk-2121wr.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Advantech IDK-2121WR 21.5" Full-HD dual-LVDS panel
> > +
> > +maintainers:
> > +  - Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > +  - Thierry Reding <thierry.reding@gmail.com>
> > +
> > +description: |
> > +  The IDK-2121WR from Advantech is a Full-HD dual-LVDS panel.
> > +
> > +  The panels expects odd pixels from the first port, and even pixels from
> 
> s/panels/panel/
> Maybe s/from the/on the/g ?

Will fix

> 
> > +  the second port, therefore the ports must be marked accordingly.
> > +
> > +allOf:
> > +  - $ref: lvds.yaml#
> > +  - $ref: ../bus-timings/lvds.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: advantech,idk-2121wr
> > +      - {} # panel-lvds, but not listed here to avoid false select
> > +
> > +  data-mapping:
> > +    const: vesa-24
> > +
> > +  width-mm:
> > +    const: 476
> > +
> > +  height-mm:
> > +    const: 268
> > +
> > +  panel-timing: true
> > +  ports: true
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> 
> Shouldn't data-mapping, width-mm, height-mm and ports be required too ?

Those are required by lvds.yaml, and this file has a reference to it.
Anyhow, I think the best course of action is to merge this with the
generic bus-timings/lvds.yaml for the time being, and maybe split
them back later on once we have another use case.

Thanks,
Fab

> 
> As you mentioned in the cover letter, validating ports, port and the new
> dual-lvds-*-pixels properties would be nice. I'm not YAML schema
> specialist, so I'm fine with a best effort approach here, but as far as
> I understand Rob proposed a way forward, could you try it ?
> 
> Apart from that, the bindings look sne to me, so
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> once the above issues get addressed.
> 
> > +
> > +examples:
> > +  - |+
> > +    panel-lvds {
> > +      compatible = "advantech,idk-2121wr", "panel-lvds";
> > +
> > +      width-mm = <476>;
> > +      height-mm = <268>;
> > +
> > +      data-mapping = "vesa-24";
> > +
> > +      panel-timing {
> > +        clock-frequency = <148500000>;
> > +        hactive = <1920>;
> > +        vactive = <1080>;
> > +        hsync-len = <44>;
> > +        hfront-porch = <88>;
> > +        hback-porch = <148>;
> > +        vfront-porch = <4>;
> > +        vback-porch = <36>;
> > +        vsync-len = <5>;
> > +      };
> > +
> > +      ports {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        port@0 {
> > +          reg = <0>;
> > +          dual-lvds-odd-pixels;
> > +          panel_in0: endpoint {
> > +            remote-endpoint = <&lvds0_out>;
> > +          };
> > +        };
> > +
> > +        port@1 {
> > +          reg = <1>;
> > +          dual-lvds-even-pixels;
> > +          panel_in1: endpoint {
> > +            remote-endpoint = <&lvds1_out>;
> > +          };
> > +        };
> > +      };
> > +    };
> > +
> > +...
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH v3 7/8] [HACK] arm64: dts: renesas: draak: Enable LVDS

[Fabrizio Castro]

Hello Laurent,

Thank you for the feedback!

> From: linux-renesas-soc-owner@vger.kernel.org <linux-renesas-soc-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> Sent: 07 November 2019 19:58
> Subject: Re: [PATCH v3 7/8] [HACK] arm64: dts: renesas: draak: Enable LVDS
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> The subject is wrong, it should be
> 
> [HACK] arm64: dts: renesas: ebisu: Enable LVDS dual-link operation

This sounds like a copy and paste gone really wrong!
Sorry about that. I'll be dropping this patch in v4.

Thanks,
Fab

> 
> On Wed, Aug 28, 2019 at 07:36:41PM +0100, Fabrizio Castro wrote:
> > Enable and connect the second LVDS encoder to the second LVDS input of
> > the THC63LVD1024 for dual-link LVDS operation. This requires changing
> > the default settings of SW45 and SW47 to OFF and ON respectively.
> >
> > This patch is based on Laurent's dual-LVDS work:
> > https://patchwork.kernel.org/patch/10965045/
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> > v2->v3:
> > * new patch
> >
> > Geert,
> >
> > no need to review this patch unless they like the idea behind this
> > series.
> >
> > Thanks,
> > Fab
> >
> > ---
> >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > index b38f9d4..38b9c5a 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > @@ -87,11 +87,20 @@
> >
> >  			port@0 {
> >  				reg = <0>;
> > -				thc63lvd1024_in: endpoint {
> > +				dual-lvds-even-pixels;
> > +				thc63lvd1024_in0: endpoint {
> >  					remote-endpoint = <&lvds0_out>;
> >  				};
> >  			};
> >
> > +			port@1 {
> > +				reg = <1>;
> > +				dual-lvds-odd-pixels;
> > +				thc63lvd1024_in1: endpoint {
> > +					remote-endpoint = <&lvds1_out>;
> > +				};
> > +			};
> > +
> >  			port@2 {
> >  				reg = <2>;
> >  				thc63lvd1024_out: endpoint {
> > @@ -489,7 +498,7 @@
> >  	ports {
> >  		port@1 {
> >  			lvds0_out: endpoint {
> > -				remote-endpoint = <&thc63lvd1024_in>;
> > +				remote-endpoint = <&thc63lvd1024_in0>;
> >  			};
> >  		};
> >  	};
> > @@ -507,6 +516,14 @@
> >  		 <&x13_clk>,
> >  		 <&extal_clk>;
> >  	clock-names = "fck", "dclkin.0", "extal";
> > +
> > +	ports {
> > +		port@1 {
> > +			lvds1_out: endpoint {
> > +				remote-endpoint = <&thc63lvd1024_in1>;
> > +			};
> > +		};
> > +	};
> >  };
> >
> >  &ohci0 {
> > --
> > 2.7.4
> >
> 
> --
> Regards,
> 
> Laurent Pinchart