[PATCH/RFC 00/12] Add dual-LVDS panel support to EK874

[PATCH/RFC 00/12] 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
Dual link support is very recent for R-Car Gen3, and I couldn't
find much on dual link panels in the kernel either, therefore
comments are very welcome to get this right.

The panel doesn't come with the EK874 kit, but it's advertised as
supported, therefore this series adds a new dts file to support
the configuration of the EK874 + IDK-2121WR.

Finally, this series depends on a fix that's still pending:
https://patchwork.kernel.org/patch/11054755/

Thanks,
Fab

Fabrizio Castro (12):
  dt-bindings: display: renesas: lvds: RZ/G2E needs renesas,companion
    too
  dt-bindings: display: renesas: lvds: Document renesas,swap-data
  dt-bindings: panel: lvds: Add dual-link LVDS display support
  dt-bindings: display: Add bindings for Advantech IDK-2121WR
  drm: rcar-du: lvds: Add data swap support
  drm: rcar-du: lvds: Do not look at ports for identifying bridges
  drm: rcar-du: lvds: Add support for dual link panels
  drm: rcar-du: lvds: Fix bridge_to_rcar_lvds
  drm: rcar-du: lvds: Fix companion's mode
  arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1
  arm64: dts: renesas: cat874: Add definition for 12V regulator
  arm64: dts: renesas: Add EK874 board with idk-2121wr display support

 .../bindings/display/bridge/renesas,lvds.txt       |  11 +-
 .../display/panel/advantech,idk-2121wr.txt         |  62 ++++++++++++
 .../bindings/display/panel/panel-lvds.txt          |  91 ++++++++++++-----
 arch/arm64/boot/dts/renesas/Makefile               |   3 +-
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts    |   9 ++
 .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 112 +++++++++++++++++++++
 arch/arm64/boot/dts/renesas/r8a774c0.dtsi          |   2 +
 drivers/gpu/drm/rcar-du/rcar_lvds.c                |  65 ++++++------
 8 files changed, 291 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
 create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts

-- 
2.7.4

[PATCH/RFC 01/12] dt-bindings: display: renesas: lvds: RZ/G2E needs renesas,companion too

[Fabrizio Castro]

Document RZ/G2E support for property renesas,companion.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index c6a196d..dece79e 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -49,9 +49,9 @@ Each port shall have a single endpoint.
 Optional properties:
 
 - renesas,companion : phandle to the companion LVDS encoder. This property is
-  mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to
-  the second encoder to be used as a companion in dual-link mode. It shall not
-  be set for any other LVDS encoder.
+  mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
+  and shall point to the second encoder to be used as a companion in dual-link
+  mode. It shall not be set for any other LVDS encoder.
 
 
 Example:
-- 
2.7.4

[PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data

[Fabrizio Castro]

R-Car D3, R-Car E3, and RZ/G2E support dual-link mode.
In such a mode, the first LVDS encoder emits even data, and the
second LVDS encoder emits odd data. This patch documents property
renesas,swap-data, used to swap even and odd data around.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index dece79e..8980179 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -52,6 +52,11 @@ Optional properties:
   mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
   and shall point to the second encoder to be used as a companion in dual-link
   mode. It shall not be set for any other LVDS encoder.
+- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
+  emits even data, and the second LVDS encoder emits odd data. When property
+  renesas,swap-data is specified, the data emitted by the two encoders will be
+  swapped around. This property can only be used in conjunction with property
+  renesas,companion.
 
 
 Example:
-- 
2.7.4

[PATCH/RFC 03/12] dt-bindings: panel: lvds: Add dual-link LVDS display support

[Fabrizio Castro]

Dual-link LVDS displays have two ports, therefore document this
with the bindings.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 .../bindings/display/panel/panel-lvds.txt          | 91 ++++++++++++++++------
 1 file changed, 67 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
index 250850a..07795441 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
+++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
@@ -41,7 +41,8 @@ Required nodes:
 
 - panel-timing: See panel-common.txt.
 - ports: See panel-common.txt. These bindings require a single port subnode
-  corresponding to the panel LVDS input.
+  (for a single link display) or two port subnodes (for a dual link display)
+  corresponding to the panel LVDS input(s).
 
 
 LVDS data mappings are defined as follows.
@@ -92,30 +93,72 @@ CTL3: 0
 Example
 -------
 
-panel {
-	compatible = "mitsubishi,aa121td01", "panel-lvds";
-
-	width-mm = <261>;
-	height-mm = <163>;
-
-	data-mapping = "jeida-24";
-
-	panel-timing {
-		/* 1280x800 @60Hz */
-		clock-frequency = <71000000>;
-		hactive = <1280>;
-		vactive = <800>;
-		hsync-len = <70>;
-		hfront-porch = <20>;
-		hback-porch = <70>;
-		vsync-len = <5>;
-		vfront-porch = <3>;
-		vback-porch = <15>;
+Single port:
+	panel {
+		compatible = "mitsubishi,aa121td01", "panel-lvds";
+
+		width-mm = <261>;
+		height-mm = <163>;
+
+		data-mapping = "jeida-24";
+
+		panel-timing {
+			/* 1280x800 @60Hz */
+			clock-frequency = <71000000>;
+			hactive = <1280>;
+			vactive = <800>;
+			hsync-len = <70>;
+			hfront-porch = <20>;
+			hback-porch = <70>;
+			vsync-len = <5>;
+			vfront-porch = <3>;
+			vback-porch = <15>;
+		};
+
+		port {
+			panel_in: endpoint {
+				remote-endpoint = <&lvds_encoder>;
+			};
+		};
 	};
 
-	port {
-		panel_in: endpoint {
-			remote-endpoint = <&lvds_encoder>;
+Two ports:
+	panel {
+		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>;
+				lvds0_panel_in: endpoint {
+					remote-endpoint = <&lvds0_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				lvds1_panel_in: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
 		};
 	};
-};
-- 
2.7.4

[PATCH/RFC 04/12] dt-bindings: display: Add bindings for Advantech IDK-2121WR

[Fabrizio Castro]

This panel is handled through the generic lvds-panel bindings,
so only needs its additional compatible specified.

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>
---
 .../display/panel/advantech,idk-2121wr.txt         | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt

diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
new file mode 100644
index 0000000..70b15b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
@@ -0,0 +1,62 @@
+Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel
+===============================================
+
+Required properties:
+- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
+
+This binding is compatible with the lvds-panel binding, which is specified
+in panel-lvds.txt in this directory.
+
+Example
+-------
+
+	panel {
+		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>;
+				lvds0_panel_in: endpoint {
+					remote-endpoint = <&lvds0_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				lvds1_panel_in: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
+		};
+	};
+
+	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>;
+	};
-- 
2.7.4

[PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support

[Fabrizio Castro]

When in vertical stripe mode of operation, there is the option
of swapping even data and odd data on the two LVDS interfaces
used to drive the video output.
Add data swap support by exposing a new DT property named
"renesas,swap-data".

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 3aeaf9e..c306fab 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -69,6 +69,7 @@ struct rcar_lvds {
 
 	struct drm_bridge *companion;
 	bool dual_link;
+	bool stripe_swap_data;
 };
 
 #define bridge_to_rcar_lvds(bridge) \
@@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
 	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
 
 	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
-		/*
-		 * Configure vertical stripe based on the mode of operation of
-		 * the connected device.
-		 */
-		rcar_lvds_write(lvds, LVDSTRIPE,
-				lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
+		u32 lvdstripe = 0;
+
+		if (lvds->dual_link)
+			/*
+			 * Configure vertical stripe based on the mode of
+			 * operation of the connected device.
+			 */
+			lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?
+						       LVDSTRIPE_ST_SWAP : 0);
+		rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
 	}
 
 	/*
@@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 		}
 	}
 
-	if (lvds->dual_link)
+	if (lvds->dual_link) {
+		lvds->stripe_swap_data = of_property_read_bool(
+						lvds->dev->of_node,
+						"renesas,swap-data");
 		ret = rcar_lvds_parse_dt_companion(lvds);
+	}
 
 done:
 	of_node_put(local_output);
-- 
2.7.4

[PATCH/RFC 06/12] drm: rcar-du: lvds: Do not look at ports for identifying bridges

[Fabrizio Castro]

We may be connected to a dual LVDS display, therefore checking
if node != remote_input for identifying bridges is not going to
work anymore.
We could try to match the ports on the remote end to the LVDS
encoders, however the companion LVDS encoder instance doesn't
hold a reference to the primary LVDS encoder instance.
We know we could be connected to either a bridge, or a panel,
therefore look through the registered bridges and panels, until
we have a match.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 29 +++--------------------------
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index c306fab..2d54ae5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -711,10 +711,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
 static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 {
 	struct device_node *local_output = NULL;
-	struct device_node *remote_input = NULL;
 	struct device_node *remote = NULL;
-	struct device_node *node;
-	bool is_bridge = false;
 	int ret = 0;
 
 	local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
@@ -742,27 +739,8 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 		goto done;
 	}
 
-	remote_input = of_graph_get_remote_endpoint(local_output);
-
-	for_each_endpoint_of_node(remote, node) {
-		if (node != remote_input) {
-			/*
-			 * We've found one endpoint other than the input, this
-			 * must be a bridge.
-			 */
-			is_bridge = true;
-			of_node_put(node);
-			break;
-		}
-	}
-
-	if (is_bridge) {
-		lvds->next_bridge = of_drm_find_bridge(remote);
-		if (!lvds->next_bridge) {
-			ret = -EPROBE_DEFER;
-			goto done;
-		}
-
+	lvds->next_bridge = of_drm_find_bridge(remote);
+	if (lvds->next_bridge) {
 		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
 			lvds->dual_link = lvds->next_bridge->timings
 					? lvds->next_bridge->timings->dual_link
@@ -770,7 +748,7 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 	} else {
 		lvds->panel = of_drm_find_panel(remote);
 		if (IS_ERR(lvds->panel)) {
-			ret = PTR_ERR(lvds->panel);
+			ret = -EPROBE_DEFER;
 			goto done;
 		}
 	}
@@ -784,7 +762,6 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 
 done:
 	of_node_put(local_output);
-	of_node_put(remote_input);
 	of_node_put(remote);
 
 	switch (ret) {
-- 
2.7.4

[PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels

[Fabrizio Castro]

If the display comes with two ports, assume it supports dual
link.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 2d54ae5..97c51c2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 			ret = -EPROBE_DEFER;
 			goto done;
 		}
+		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
+			lvds->dual_link = of_graph_get_endpoint_count(remote)
+					== 2;
 	}
 
 	if (lvds->dual_link) {
-- 
2.7.4

[PATCH/RFC 08/12] drm: rcar-du: lvds: Fix bridge_to_rcar_lvds

[Fabrizio Castro]

Using name "bridge" for macro bridge_to_rcar_lvds argument doesn't
work when the pointer name used by the caller is not "bridge".
Rename the argument to "bridge_ptr" to allow for any pointer
name.

Fixes: c6a27fa41fab ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 97c51c2..edd63f5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -72,8 +72,8 @@ struct rcar_lvds {
 	bool stripe_swap_data;
 };
 
-#define bridge_to_rcar_lvds(bridge) \
-	container_of(bridge, struct rcar_lvds, bridge)
+#define bridge_to_rcar_lvds(bridge_ptr) \
+	container_of(bridge_ptr, struct rcar_lvds, bridge)
 
 #define connector_to_rcar_lvds(connector) \
 	container_of(connector, struct rcar_lvds, connector)
-- 
2.7.4

[PATCH/RFC 09/12] drm: rcar-du: lvds: Fix companion's mode

[Fabrizio Castro]

The companion encoder needs to be told to use the same
mode as the primary encoder.

Fixes: e9e8798ab7b8 ("drm: rcar-du: lvds: Add support for dual-link mode")
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index edd63f5..7944ae9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -415,8 +415,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
 		return;
 
 	/* Enable the companion LVDS encoder in dual-link mode. */
-	if (lvds->dual_link && lvds->companion)
+	if (lvds->dual_link && lvds->companion) {
+		struct rcar_lvds *companion_lvds = bridge_to_rcar_lvds(
+							lvds->companion);
+		companion_lvds->mode = lvds->mode;
 		lvds->companion->funcs->enable(lvds->companion);
+	}
 
 	/*
 	 * Hardcode the channels and control signals routing for now.
-- 
2.7.4

[PATCH/RFC 10/12] arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1

[Fabrizio Castro]

Add the new renesas,companion property to the LVDS0 node to point to the
companion LVDS encoder LVDS1.
Based on similar work from Laurent Pinchart for the r8a7799[05].

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a774c0.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi
index e7b5bf2..b36d3b08 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi
@@ -1844,6 +1844,8 @@
 			resets = <&cpg 727>;
 			status = "disabled";
 
+			renesas,companion = <&lvds1>;
+
 			ports {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.7.4

[PATCH/RFC 11/12] arm64: dts: renesas: cat874: Add definition for 12V regulator

[Fabrizio Castro]

Power rail "D12.0V" comes straight from the power barrel connector,
and it's used in both main board and sub board.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 46a77ee..651383c 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -65,6 +65,15 @@
 		reg = <0x0 0x48000000 0x0 0x78000000>;
 	};
 
+	reg_12p0v: regulator-12p0v {
+		compatible = "regulator-fixed";
+		regulator-name = "D12.0V";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
 	sound: sound {
 		compatible = "simple-audio-card";
 
-- 
2.7.4

[PATCH/RFC 12/12] 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>
---
 arch/arm64/boot/dts/renesas/Makefile               |   3 +-
 .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 112 +++++++++++++++++++++
 2 files changed, 114 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..d989998
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
@@ -0,0 +1,112 @@
+// 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>;
+				lvds0_panel_in: endpoint {
+					remote-endpoint = <&lvds0_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				lvds1_panel_in: endpoint {
+					remote-endpoint = <&lvds1_out>;
+				};
+			};
+		};
+	};
+};
+
+&gpio0 {
+	lvds-connector-en-gpio{
+		gpio-hog;
+		gpios = <17 GPIO_ACTIVE_HIGH>;
+		output-low;
+		line-name = "lvds-connector-en-gpio";
+	};
+};
+
+&lvds0 {
+	renesas,swap-data;
+
+	ports {
+		port@1 {
+			lvds0_out: endpoint {
+				remote-endpoint = <&lvds0_panel_in>;
+			};
+		};
+	};
+};
+
+&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 = <&lvds1_panel_in>;
+			};
+		};
+	};
+};
+
+&pfc {
+	pwm5_pins: pwm5 {
+		groups = "pwm5_a";
+		function = "pwm5";
+	};
+};
+
+&pwm5 {
+	pinctrl-0 = <&pwm5_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
-- 
2.7.4

Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
> R-Car D3, R-Car E3, and RZ/G2E support dual-link mode.
> In such a mode, the first LVDS encoder emits even data, and the
> second LVDS encoder emits odd data. This patch documents property
> renesas,swap-data, used to swap even and odd data around.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> index dece79e..8980179 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> @@ -52,6 +52,11 @@ Optional properties:
>    mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
>    and shall point to the second encoder to be used as a companion in dual-link
>    mode. It shall not be set for any other LVDS encoder.
> +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
> +  emits even data, and the second LVDS encoder emits odd data. When property
> +  renesas,swap-data is specified, the data emitted by the two encoders will be
> +  swapped around. This property can only be used in conjunction with property
> +  renesas,companion.

From an LVDS encoder point of view this is more a configuration option
than a description of the hardware. Wouldn't it be better for the LVDS
sink to report which of the odd or even pixels it expects on each of its
endpoints ? The LVDS encoder driver could then query that at runtime and
configure itself accordingly. Ideally this should be queried through the
drm_bridge_timings structure (or through a similar mean), not through
DT. An LVDS sink that has a fixed mapping of odd/even pixels to
endpoints wouldn't need the information to be specified in DT at all.

>  
>  
>  Example:

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 01/12] dt-bindings: display: renesas: lvds: RZ/G2E needs renesas,companion too

[Laurent Pinchart]

Hello Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:33:58AM +0100, Fabrizio Castro wrote:
> Document RZ/G2E support for property renesas,companion.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

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

and taken in my tree.

> ---
>  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> index c6a196d..dece79e 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> @@ -49,9 +49,9 @@ Each port shall have a single endpoint.
>  Optional properties:
>  
>  - renesas,companion : phandle to the companion LVDS encoder. This property is
> -  mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to
> -  the second encoder to be used as a companion in dual-link mode. It shall not
> -  be set for any other LVDS encoder.
> +  mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
> +  and shall point to the second encoder to be used as a companion in dual-link
> +  mode. It shall not be set for any other LVDS encoder.
>  
>  
>  Example:

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 03/12] dt-bindings: panel: lvds: Add dual-link LVDS display support

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:00AM +0100, Fabrizio Castro wrote:
> Dual-link LVDS displays have two ports, therefore document this
> with the bindings.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
>  .../bindings/display/panel/panel-lvds.txt          | 91 ++++++++++++++++------
>  1 file changed, 67 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> index 250850a..07795441 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> @@ -41,7 +41,8 @@ Required nodes:
>  
>  - panel-timing: See panel-common.txt.
>  - ports: See panel-common.txt. These bindings require a single port subnode
> -  corresponding to the panel LVDS input.
> +  (for a single link display) or two port subnodes (for a dual link display)
> +  corresponding to the panel LVDS input(s).

I think you should expand this a bit to explain what the ports
correspond to in the dual link mode.

>  LVDS data mappings are defined as follows.
> @@ -92,30 +93,72 @@ CTL3: 0
>  Example
>  -------
>  
> -panel {
> -	compatible = "mitsubishi,aa121td01", "panel-lvds";
> -
> -	width-mm = <261>;
> -	height-mm = <163>;
> -
> -	data-mapping = "jeida-24";
> -
> -	panel-timing {
> -		/* 1280x800 @60Hz */
> -		clock-frequency = <71000000>;
> -		hactive = <1280>;
> -		vactive = <800>;
> -		hsync-len = <70>;
> -		hfront-porch = <20>;
> -		hback-porch = <70>;
> -		vsync-len = <5>;
> -		vfront-porch = <3>;
> -		vback-porch = <15>;
> +Single port:
> +	panel {
> +		compatible = "mitsubishi,aa121td01", "panel-lvds";
> +
> +		width-mm = <261>;
> +		height-mm = <163>;
> +
> +		data-mapping = "jeida-24";
> +
> +		panel-timing {
> +			/* 1280x800 @60Hz */
> +			clock-frequency = <71000000>;
> +			hactive = <1280>;
> +			vactive = <800>;
> +			hsync-len = <70>;
> +			hfront-porch = <20>;
> +			hback-porch = <70>;
> +			vsync-len = <5>;
> +			vfront-porch = <3>;
> +			vback-porch = <15>;
> +		};
> +
> +		port {
> +			panel_in: endpoint {
> +				remote-endpoint = <&lvds_encoder>;
> +			};
> +		};
>  	};
>  
> -	port {
> -		panel_in: endpoint {
> -			remote-endpoint = <&lvds_encoder>;
> +Two ports:
> +	panel {
> +		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>;
> +				lvds0_panel_in: endpoint {

I would name the label panel_in0 and panel_in1 below to have a common
prefix showing that both refer to the same panel.

> +					remote-endpoint = <&lvds0_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				lvds1_panel_in: endpoint {
> +					remote-endpoint = <&lvds1_out>;
> +				};
> +			};
>  		};
>  	};
> -};

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 04/12] dt-bindings: display: Add bindings for Advantech IDK-2121WR

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:01AM +0100, Fabrizio Castro wrote:
> This panel is handled through the generic lvds-panel bindings,
> so only needs its additional compatible specified.
> 
> Some panel specific documentation can be found here:

s/panel specific/panel-specific/

> 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>
> ---
>  .../display/panel/advantech,idk-2121wr.txt         | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> new file mode 100644
> index 0000000..70b15b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> @@ -0,0 +1,62 @@
> +Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel
> +===============================================
> +
> +Required properties:
> +- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
> +
> +This binding is compatible with the lvds-panel binding, which is specified
> +in panel-lvds.txt in this directory.

How about adding "The panel operates in dual-link mode and thus requires
two port nodes." ?

> +
> +Example
> +-------
> +
> +	panel {
> +		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>;
> +				lvds0_panel_in: endpoint {
> +					remote-endpoint = <&lvds0_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				lvds1_panel_in: endpoint {
> +					remote-endpoint = <&lvds1_out>;
> +				};
> +			};
> +		};
> +	};
> +
> +	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>;
> +	};

I think you can drop the backlight here, it's a bit out of scope.

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
> When in vertical stripe mode of operation, there is the option
> of swapping even data and odd data on the two LVDS interfaces
> used to drive the video output.
> Add data swap support by exposing a new DT property named
> "renesas,swap-data".
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 3aeaf9e..c306fab 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -69,6 +69,7 @@ struct rcar_lvds {
>  
>  	struct drm_bridge *companion;
>  	bool dual_link;
> +	bool stripe_swap_data;
>  };
>  
>  #define bridge_to_rcar_lvds(bridge) \
> @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>  	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
>  
>  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> -		/*
> -		 * Configure vertical stripe based on the mode of operation of
> -		 * the connected device.
> -		 */
> -		rcar_lvds_write(lvds, LVDSTRIPE,
> -				lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> +		u32 lvdstripe = 0;
> +
> +		if (lvds->dual_link)
> +			/*
> +			 * Configure vertical stripe based on the mode of
> +			 * operation of the connected device.
> +			 */
> +			lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?
> +						       LVDSTRIPE_ST_SWAP : 0);

Would the following be simpler ?

		lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0)
			  | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);

> +		rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
>  	}
>  
>  	/*
> @@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  		}
>  	}
>  
> -	if (lvds->dual_link)
> +	if (lvds->dual_link) {
> +		lvds->stripe_swap_data = of_property_read_bool(
> +						lvds->dev->of_node,
> +						"renesas,swap-data");
>  		ret = rcar_lvds_parse_dt_companion(lvds);
> +	}

As explained in the review of the corresponding DT bindings, I think
this should be queried from the remote device rather than specified in
DT.

>  
>  done:
>  	of_node_put(local_output);

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 06/12] drm: rcar-du: lvds: Do not look at ports for identifying bridges

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:03AM +0100, Fabrizio Castro wrote:
> We may be connected to a dual LVDS display, therefore checking
> if node != remote_input for identifying bridges is not going to
> work anymore.
> We could try to match the ports on the remote end to the LVDS
> encoders, however the companion LVDS encoder instance doesn't
> hold a reference to the primary LVDS encoder instance.
> We know we could be connected to either a bridge, or a panel,
> therefore look through the registered bridges and panels, until
> we have a match.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 29 +++--------------------------
>  1 file changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index c306fab..2d54ae5 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -711,10 +711,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
>  static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  {
>  	struct device_node *local_output = NULL;
> -	struct device_node *remote_input = NULL;
>  	struct device_node *remote = NULL;
> -	struct device_node *node;
> -	bool is_bridge = false;
>  	int ret = 0;
>  
>  	local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
> @@ -742,27 +739,8 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  		goto done;
>  	}
>  
> -	remote_input = of_graph_get_remote_endpoint(local_output);
> -
> -	for_each_endpoint_of_node(remote, node) {
> -		if (node != remote_input) {
> -			/*
> -			 * We've found one endpoint other than the input, this
> -			 * must be a bridge.
> -			 */
> -			is_bridge = true;
> -			of_node_put(node);
> -			break;
> -		}
> -	}
> -
> -	if (is_bridge) {
> -		lvds->next_bridge = of_drm_find_bridge(remote);
> -		if (!lvds->next_bridge) {
> -			ret = -EPROBE_DEFER;
> -			goto done;
> -		}
> -
> +	lvds->next_bridge = of_drm_find_bridge(remote);

How about using drm_of_find_panel_or_bridge() ?

> +	if (lvds->next_bridge) {
>  		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
>  			lvds->dual_link = lvds->next_bridge->timings
>  					? lvds->next_bridge->timings->dual_link
> @@ -770,7 +748,7 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  	} else {
>  		lvds->panel = of_drm_find_panel(remote);
>  		if (IS_ERR(lvds->panel)) {
> -			ret = PTR_ERR(lvds->panel);
> +			ret = -EPROBE_DEFER;
>  			goto done;
>  		}
>  	}
> @@ -784,7 +762,6 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  
>  done:
>  	of_node_put(local_output);
> -	of_node_put(remote_input);
>  	of_node_put(remote);
>  
>  	switch (ret) {

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
> If the display comes with two ports, assume it supports dual
> link.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 2d54ae5..97c51c2 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  			ret = -EPROBE_DEFER;
>  			goto done;
>  		}
> +		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> +			lvds->dual_link = of_graph_get_endpoint_count(remote)
> +					== 2;

This is a bit of a hack, as I think the information should be queried
from the panel, like we do for bridges. I'd say we can live with this
for now, but as the data swap flag should come from the panel as well,
we will need infrastructure for that, and we can as well through the
dual link flag there at the same time.

I think we should use the drm_bridge_timings structure for this purpose,
as it would make life more difficult for users of drm_bridge and
drm_panel to have two different structures (especially when wrapping a
drm_panel with drm_panel_bridge_add()). The structure could be renamed
if desired.

>  	}
>  
>  	if (lvds->dual_link) {

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 08/12] drm: rcar-du: lvds: Fix bridge_to_rcar_lvds

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:05AM +0100, Fabrizio Castro wrote:
> Using name "bridge" for macro bridge_to_rcar_lvds argument doesn't
> work when the pointer name used by the caller is not "bridge".
> Rename the argument to "bridge_ptr" to allow for any pointer
> name.
> 
> Fixes: c6a27fa41fab ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 97c51c2..edd63f5 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -72,8 +72,8 @@ struct rcar_lvds {
>  	bool stripe_swap_data;
>  };
>  
> -#define bridge_to_rcar_lvds(bridge) \
> -	container_of(bridge, struct rcar_lvds, bridge)
> +#define bridge_to_rcar_lvds(bridge_ptr) \
> +	container_of(bridge_ptr, struct rcar_lvds, bridge)

How about just 'b' instead of 'bridge_ptr' ? If that's fine with you
I'll take the modified patch in my tree, no need to resubmit.

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

>  
>  #define connector_to_rcar_lvds(connector) \
>  	container_of(connector, struct rcar_lvds, connector)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 09/12] drm: rcar-du: lvds: Fix companion's mode

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:06AM +0100, Fabrizio Castro wrote:
> The companion encoder needs to be told to use the same
> mode as the primary encoder.
> 
> Fixes: e9e8798ab7b8 ("drm: rcar-du: lvds: Add support for dual-link mode")
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index edd63f5..7944ae9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -415,8 +415,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>  		return;
>  
>  	/* Enable the companion LVDS encoder in dual-link mode. */
> -	if (lvds->dual_link && lvds->companion)
> +	if (lvds->dual_link && lvds->companion) {
> +		struct rcar_lvds *companion_lvds = bridge_to_rcar_lvds(
> +							lvds->companion);
> +		companion_lvds->mode = lvds->mode;
>  		lvds->companion->funcs->enable(lvds->companion);
> +	}

Would it make sense to do this in rcar_lvds_mode_set() instead, to keep
the mode set code grouped in a single place ?

>  
>  	/*
>  	 * Hardcode the channels and control signals routing for now.

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 10/12] arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:07AM +0100, Fabrizio Castro wrote:
> Add the new renesas,companion property to the LVDS0 node to point to the
> companion LVDS encoder LVDS1.
> Based on similar work from Laurent Pinchart for the r8a7799[05].
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

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

and taken in my tree.

> ---
>  arch/arm64/boot/dts/renesas/r8a774c0.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi
> index e7b5bf2..b36d3b08 100644
> --- a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi
> @@ -1844,6 +1844,8 @@
>  			resets = <&cpg 727>;
>  			status = "disabled";
>  
> +			renesas,companion = <&lvds1>;
> +
>  			ports {
>  				#address-cells = <1>;
>  				#size-cells = <0>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 11/12] arm64: dts: renesas: cat874: Add definition for 12V regulator

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:08AM +0100, Fabrizio Castro wrote:
> Power rail "D12.0V" comes straight from the power barrel connector,
> and it's used in both main board and sub board.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

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

I don't plan to take this in my tree without patch 12/12, so if you
think the rest of the series won't be ready in time for v5.4, feel free
to get this patch merged through Simon or Geert already.

> ---
>  arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> index 46a77ee..651383c 100644
> --- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> @@ -65,6 +65,15 @@
>  		reg = <0x0 0x48000000 0x0 0x78000000>;
>  	};
>  
> +	reg_12p0v: regulator-12p0v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "D12.0V";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
>  	sound: sound {
>  		compatible = "simple-audio-card";
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:09AM +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>
> ---
>  arch/arm64/boot/dts/renesas/Makefile               |   3 +-
>  .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 112 +++++++++++++++++++++
>  2 files changed, 114 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..d989998
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> @@ -0,0 +1,112 @@
> +// 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>;
> +				lvds0_panel_in: endpoint {
> +					remote-endpoint = <&lvds0_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				lvds1_panel_in: endpoint {
> +					remote-endpoint = <&lvds1_out>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +&gpio0 {
> +	lvds-connector-en-gpio{
> +		gpio-hog;
> +		gpios = <17 GPIO_ACTIVE_HIGH>;
> +		output-low;
> +		line-name = "lvds-connector-en-gpio";
> +	};

Any chance to specify this as the panel's enable signal in the panel DT
node ?

> +};
> +
> +&lvds0 {
> +	renesas,swap-data;

Let's discuss this property in reply to the DT bindings patch.

> +
> +	ports {
> +		port@1 {
> +			lvds0_out: endpoint {
> +				remote-endpoint = <&lvds0_panel_in>;
> +			};
> +		};
> +	};
> +};
> +
> +&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 = <&lvds1_panel_in>;
> +			};
> +		};
> +	};
> +};
> +
> +&pfc {
> +	pwm5_pins: pwm5 {
> +		groups = "pwm5_a";
> +		function = "pwm5";
> +	};
> +};
> +
> +&pwm5 {
> +	pinctrl-0 = <&pwm5_pins>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +};

I haven't reviewed pinouts in detail, but the patch otherwise looks sane
to me. Another candidate for DT overlays though ;-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support

[Geert Uytterhoeven]

Hi Laurent,

On Fri, Aug 2, 2019 at 10:06 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
> > When in vertical stripe mode of operation, there is the option
> > of swapping even data and odd data on the two LVDS interfaces
> > used to drive the video output.
> > Add data swap support by exposing a new DT property named
> > "renesas,swap-data".
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c

> > @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >       rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> >
> >       if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> > -             /*
> > -              * Configure vertical stripe based on the mode of operation of
> > -              * the connected device.
> > -              */
> > -             rcar_lvds_write(lvds, LVDSTRIPE,
> > -                             lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> > +             u32 lvdstripe = 0;
> > +
> > +             if (lvds->dual_link)
> > +                     /*
> > +                      * Configure vertical stripe based on the mode of
> > +                      * operation of the connected device.
> > +                      */
> > +                     lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?
> > +                                                    LVDSTRIPE_ST_SWAP : 0);
>
> Would the following be simpler ?
>
>                 lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0)
>                           | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);

From the point of view of "wc -l": yes.
From the point of view of readability, I'd go for:

    if (lvds->dual_link)
            lvdstripe |= LVDSTRIPE_ST_ON;
    if (lvds->stripe_swap_data)
            lvdstripe |= LVDSTRIPE_ST_SWAP;

> > +             rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
> >       }
> >
> >       /*
> > @@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)

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/RFC 10/12] arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1

[Geert Uytterhoeven]

Hi Laurent,

On Fri, Aug 2, 2019 at 10:27 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Aug 02, 2019 at 08:34:07AM +0100, Fabrizio Castro wrote:
> > Add the new renesas,companion property to the LVDS0 node to point to the
> > companion LVDS encoder LVDS1.
> > Based on similar work from Laurent Pinchart for the r8a7799[05].
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> and taken in my tree.

Shouldn't this go through renesas-devel and arm-soc?

> > ---
> >  arch/arm64/boot/dts/renesas/r8a774c0.dtsi | 2 ++

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/RFC 10/12] arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1

[Laurent Pinchart]

Hi Geert,

On Fri, Aug 02, 2019 at 11:03:54AM +0200, Geert Uytterhoeven wrote:
> On Fri, Aug 2, 2019 at 10:27 AM Laurent Pinchart wrote:
> > On Fri, Aug 02, 2019 at 08:34:07AM +0100, Fabrizio Castro wrote:
> > > Add the new renesas,companion property to the LVDS0 node to point to the
> > > companion LVDS encoder LVDS1.
> > > Based on similar work from Laurent Pinchart for the r8a7799[05].
> > >
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > and taken in my tree.
> 
> Shouldn't this go through renesas-devel and arm-soc?

I'm collecting multimedia-related DT patches for v5.4, but if you or
Simon want to take this patch, it will save me from sending a pull
request, so please go ahead :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support

[Geert Uytterhoeven]

Hi Fabrizio,

On Fri, Aug 2, 2019 at 9:35 AM Fabrizio Castro
<fabrizio.castro@bp.renesas.com> 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>

Thanks for your patch!

> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> @@ -0,0 +1,112 @@

[...]

> +       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>;
> +                               lvds0_panel_in: endpoint {
> +                                       remote-endpoint = <&lvds0_out>;
> +                               };
> +                       };
> +
> +                       port@1 {
> +                               reg = <1>;
> +                               lvds1_panel_in: endpoint {
> +                                       remote-endpoint = <&lvds1_out>;
> +                               };
> +                       };
> +               };
> +       };
> +};

[...]

> +&lvds0 {
> +       renesas,swap-data;
> +
> +       ports {
> +               port@1 {
> +                       lvds0_out: endpoint {
> +                               remote-endpoint = <&lvds0_panel_in>;
> +                       };
> +               };
> +       };
> +};
> +
> +&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 = <&lvds1_panel_in>;
> +                       };
> +               };
> +       };
> +};

Shouldn't the actual panel definition, and the lvds remote-endpoint setup,
be extracted into a separate .dtsi, to be included here?

Cfr. arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi and
arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi.

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/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 08:44
> Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
> > R-Car D3, R-Car E3, and RZ/G2E support dual-link mode.
> > In such a mode, the first LVDS encoder emits even data, and the
> > second LVDS encoder emits odd data. This patch documents property
> > renesas,swap-data, used to swap even and odd data around.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> >  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > index dece79e..8980179 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > @@ -52,6 +52,11 @@ Optional properties:
> >    mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
> >    and shall point to the second encoder to be used as a companion in dual-link
> >    mode. It shall not be set for any other LVDS encoder.
> > +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
> > +  emits even data, and the second LVDS encoder emits odd data. When property
> > +  renesas,swap-data is specified, the data emitted by the two encoders will be
> > +  swapped around. This property can only be used in conjunction with property
> > +  renesas,companion.
> 
> From an LVDS encoder point of view this is more a configuration option
> than a description of the hardware. Wouldn't it be better for the LVDS
> sink to report which of the odd or even pixels it expects on each of its
> endpoints ?

Yes, that would be my preference too, and it would be better, I am just not entirely
what's the best place for this information though

> The LVDS encoder driver could then query that at runtime and
> configure itself accordingly. Ideally this should be queried through the
> drm_bridge_timings structure (or through a similar mean), not through
> DT. An LVDS sink that has a fixed mapping of odd/even pixels to
> endpoints wouldn't need the information to be specified in DT at all.

Isn't drm_bridge_timings specific for bridges?

Thanks!
Fab

> 
> >
> >
> >  Example:
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 03/12] dt-bindings: panel: lvds: Add dual-link LVDS display support

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:00
> Subject: Re: [PATCH/RFC 03/12] dt-bindings: panel: lvds: Add dual-link LVDS display support
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:00AM +0100, Fabrizio Castro wrote:
> > Dual-link LVDS displays have two ports, therefore document this
> > with the bindings.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> >  .../bindings/display/panel/panel-lvds.txt          | 91 ++++++++++++++++------
> >  1 file changed, 67 insertions(+), 24 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > index 250850a..07795441 100644
> > --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> > @@ -41,7 +41,8 @@ Required nodes:
> >
> >  - panel-timing: See panel-common.txt.
> >  - ports: See panel-common.txt. These bindings require a single port subnode
> > -  corresponding to the panel LVDS input.
> > +  (for a single link display) or two port subnodes (for a dual link display)
> > +  corresponding to the panel LVDS input(s).
> 
> I think you should expand this a bit to explain what the ports
> correspond to in the dual link mode.

Will change.

> 
> >  LVDS data mappings are defined as follows.
> > @@ -92,30 +93,72 @@ CTL3: 0
> >  Example
> >  -------
> >
> > -panel {
> > -	compatible = "mitsubishi,aa121td01", "panel-lvds";
> > -
> > -	width-mm = <261>;
> > -	height-mm = <163>;
> > -
> > -	data-mapping = "jeida-24";
> > -
> > -	panel-timing {
> > -		/* 1280x800 @60Hz */
> > -		clock-frequency = <71000000>;
> > -		hactive = <1280>;
> > -		vactive = <800>;
> > -		hsync-len = <70>;
> > -		hfront-porch = <20>;
> > -		hback-porch = <70>;
> > -		vsync-len = <5>;
> > -		vfront-porch = <3>;
> > -		vback-porch = <15>;
> > +Single port:
> > +	panel {
> > +		compatible = "mitsubishi,aa121td01", "panel-lvds";
> > +
> > +		width-mm = <261>;
> > +		height-mm = <163>;
> > +
> > +		data-mapping = "jeida-24";
> > +
> > +		panel-timing {
> > +			/* 1280x800 @60Hz */
> > +			clock-frequency = <71000000>;
> > +			hactive = <1280>;
> > +			vactive = <800>;
> > +			hsync-len = <70>;
> > +			hfront-porch = <20>;
> > +			hback-porch = <70>;
> > +			vsync-len = <5>;
> > +			vfront-porch = <3>;
> > +			vback-porch = <15>;
> > +		};
> > +
> > +		port {
> > +			panel_in: endpoint {
> > +				remote-endpoint = <&lvds_encoder>;
> > +			};
> > +		};
> >  	};
> >
> > -	port {
> > -		panel_in: endpoint {
> > -			remote-endpoint = <&lvds_encoder>;
> > +Two ports:
> > +	panel {
> > +		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>;
> > +				lvds0_panel_in: endpoint {
> 
> I would name the label panel_in0 and panel_in1 below to have a common
> prefix showing that both refer to the same panel.

I agree, will change, thank you for pointing this out.

> 
> > +					remote-endpoint = <&lvds0_out>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				reg = <1>;
> > +				lvds1_panel_in: endpoint {
> > +					remote-endpoint = <&lvds1_out>;
> > +				};
> > +			};
> >  		};
> >  	};
> > -};

Thanks,
Fab

> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 04/12] dt-bindings: display: Add bindings for Advantech IDK-2121WR

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:04
> Subject: Re: [PATCH/RFC 04/12] dt-bindings: display: Add bindings for Advantech IDK-2121WR
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:01AM +0100, Fabrizio Castro wrote:
> > This panel is handled through the generic lvds-panel bindings,
> > so only needs its additional compatible specified.
> >
> > Some panel specific documentation can be found here:
> 
> s/panel specific/panel-specific/

Will change

> 
> > 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>
> > ---
> >  .../display/panel/advantech,idk-2121wr.txt         | 62 ++++++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> > new file mode 100644
> > index 0000000..70b15b6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> > @@ -0,0 +1,62 @@
> > +Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel
> > +===============================================
> > +
> > +Required properties:
> > +- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
> > +
> > +This binding is compatible with the lvds-panel binding, which is specified
> > +in panel-lvds.txt in this directory.
> 
> How about adding "The panel operates in dual-link mode and thus requires
> two port nodes." ?

You are right, will add.

> 
> > +
> > +Example
> > +-------
> > +
> > +	panel {
> > +		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>;
> > +				lvds0_panel_in: endpoint {
> > +					remote-endpoint = <&lvds0_out>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				reg = <1>;
> > +				lvds1_panel_in: endpoint {
> > +					remote-endpoint = <&lvds1_out>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +
> > +	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>;
> > +	};
> 
> I think you can drop the backlight here, it's a bit out of scope.

Agreed.

Thanks,
Fab

> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 06/12] drm: rcar-du: lvds: Do not look at ports for identifying bridges

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:08
> Subject: Re: [PATCH/RFC 06/12] drm: rcar-du: lvds: Do not look at ports for identifying bridges
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:03AM +0100, Fabrizio Castro wrote:
> > We may be connected to a dual LVDS display, therefore checking
> > if node != remote_input for identifying bridges is not going to
> > work anymore.
> > We could try to match the ports on the remote end to the LVDS
> > encoders, however the companion LVDS encoder instance doesn't
> > hold a reference to the primary LVDS encoder instance.
> > We know we could be connected to either a bridge, or a panel,
> > therefore look through the registered bridges and panels, until
> > we have a match.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 29 +++--------------------------
> >  1 file changed, 3 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index c306fab..2d54ae5 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -711,10 +711,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
> >  static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >  {
> >  	struct device_node *local_output = NULL;
> > -	struct device_node *remote_input = NULL;
> >  	struct device_node *remote = NULL;
> > -	struct device_node *node;
> > -	bool is_bridge = false;
> >  	int ret = 0;
> >
> >  	local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
> > @@ -742,27 +739,8 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >  		goto done;
> >  	}
> >
> > -	remote_input = of_graph_get_remote_endpoint(local_output);
> > -
> > -	for_each_endpoint_of_node(remote, node) {
> > -		if (node != remote_input) {
> > -			/*
> > -			 * We've found one endpoint other than the input, this
> > -			 * must be a bridge.
> > -			 */
> > -			is_bridge = true;
> > -			of_node_put(node);
> > -			break;
> > -		}
> > -	}
> > -
> > -	if (is_bridge) {
> > -		lvds->next_bridge = of_drm_find_bridge(remote);
> > -		if (!lvds->next_bridge) {
> > -			ret = -EPROBE_DEFER;
> > -			goto done;
> > -		}
> > -
> > +	lvds->next_bridge = of_drm_find_bridge(remote);
> 
> How about using drm_of_find_panel_or_bridge() ?

It sounds like drm_of_find_panel_or_bridge is exactly what we need here, I'll give it a try,
thank you for the pointer!

Thanks,
Fab

> 
> > +	if (lvds->next_bridge) {
> >  		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> >  			lvds->dual_link = lvds->next_bridge->timings
> >  					? lvds->next_bridge->timings->dual_link
> > @@ -770,7 +748,7 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >  	} else {
> >  		lvds->panel = of_drm_find_panel(remote);
> >  		if (IS_ERR(lvds->panel)) {
> > -			ret = PTR_ERR(lvds->panel);
> > +			ret = -EPROBE_DEFER;
> >  			goto done;
> >  		}
> >  	}
> > @@ -784,7 +762,6 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >
> >  done:
> >  	of_node_put(local_output);
> > -	of_node_put(remote_input);
> >  	of_node_put(remote);
> >
> >  	switch (ret) {
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:20
> Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
> > If the display comes with two ports, assume it supports dual
> > link.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 2d54ae5..97c51c2 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >  			ret = -EPROBE_DEFER;
> >  			goto done;
> >  		}
> > +		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> > +			lvds->dual_link = of_graph_get_endpoint_count(remote)
> > +					== 2;
> 
> This is a bit of a hack, as I think the information should be queried
> from the panel, like we do for bridges. I'd say we can live with this
> for now, but as the data swap flag should come from the panel as well,
> we will need infrastructure for that, and we can as well through the
> dual link flag there at the same time.

I totally agree, this is a nasty hack, my series is missing the infrastructure
for describing this information

> 
> I think we should use the drm_bridge_timings structure for this purpose,
> as it would make life more difficult for users of drm_bridge and
> drm_panel to have two different structures (especially when wrapping a
> drm_panel with drm_panel_bridge_add()). The structure could be renamed
> if desired.

I am not too sure using drm_bridge_timings for panels would make everybody
happy? Is there any alternative? Perhaps this calls for a new struct we could
embed in both drm_bridge_timings and some drm_panel_<whatever> data
structure?

Thanks,
Fab
 
> 
> >  	}
> >
> >  	if (lvds->dual_link) {
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 08/12] drm: rcar-du: lvds: Fix bridge_to_rcar_lvds

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:22
> Subject: Re: [PATCH/RFC 08/12] drm: rcar-du: lvds: Fix bridge_to_rcar_lvds
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:05AM +0100, Fabrizio Castro wrote:
> > Using name "bridge" for macro bridge_to_rcar_lvds argument doesn't
> > work when the pointer name used by the caller is not "bridge".
> > Rename the argument to "bridge_ptr" to allow for any pointer
> > name.
> >
> > Fixes: c6a27fa41fab ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 97c51c2..edd63f5 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -72,8 +72,8 @@ struct rcar_lvds {
> >  	bool stripe_swap_data;
> >  };
> >
> > -#define bridge_to_rcar_lvds(bridge) \
> > -	container_of(bridge, struct rcar_lvds, bridge)
> > +#define bridge_to_rcar_lvds(bridge_ptr) \
> > +	container_of(bridge_ptr, struct rcar_lvds, bridge)
> 
> How about just 'b' instead of 'bridge_ptr' ? If that's fine with you
> I'll take the modified patch in my tree, no need to resubmit.

That's fine by me, thank your fixing.

Thanks,
Fab

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> >
> >  #define connector_to_rcar_lvds(connector) \
> >  	container_of(connector, struct rcar_lvds, connector)
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 09/12] drm: rcar-du: lvds: Fix companion's mode

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:26
> Subject: Re: [PATCH/RFC 09/12] drm: rcar-du: lvds: Fix companion's mode
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:06AM +0100, Fabrizio Castro wrote:
> > The companion encoder needs to be told to use the same
> > mode as the primary encoder.
> >
> > Fixes: e9e8798ab7b8 ("drm: rcar-du: lvds: Add support for dual-link mode")
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index edd63f5..7944ae9 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -415,8 +415,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >  		return;
> >
> >  	/* Enable the companion LVDS encoder in dual-link mode. */
> > -	if (lvds->dual_link && lvds->companion)
> > +	if (lvds->dual_link && lvds->companion) {
> > +		struct rcar_lvds *companion_lvds = bridge_to_rcar_lvds(
> > +							lvds->companion);
> > +		companion_lvds->mode = lvds->mode;
> >  		lvds->companion->funcs->enable(lvds->companion);
> > +	}
> 
> Would it make sense to do this in rcar_lvds_mode_set() instead, to keep
> the mode set code grouped in a single place ?

I need to looks into this, as the initialization of lvds1 is a bit special.
I'll get back to you.

Thanks,
Fab

> 
> >
> >  	/*
> >  	 * Hardcode the channels and control signals routing for now.
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 11/12] arm64: dts: renesas: cat874: Add definition for 12V regulator

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:30
> Subject: Re: [PATCH/RFC 11/12] arm64: dts: renesas: cat874: Add definition for 12V regulator
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:08AM +0100, Fabrizio Castro wrote:
> > Power rail "D12.0V" comes straight from the power barrel connector,
> > and it's used in both main board and sub board.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> I don't plan to take this in my tree without patch 12/12, so if you
> think the rest of the series won't be ready in time for v5.4, feel free
> to get this patch merged through Simon or Geert already.

Geert, would you be happy to take this patch?

Thanks,
Fab

> 
> > ---
> >  arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> > index 46a77ee..651383c 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> > @@ -65,6 +65,15 @@
> >  		reg = <0x0 0x48000000 0x0 0x78000000>;
> >  	};
> >
> > +	reg_12p0v: regulator-12p0v {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "D12.0V";
> > +		regulator-min-microvolt = <12000000>;
> > +		regulator-max-microvolt = <12000000>;
> > +		regulator-boot-on;
> > +		regulator-always-on;
> > +	};
> > +
> >  	sound: sound {
> >  		compatible = "simple-audio-card";
> >
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support

[Fabrizio Castro]

Hi Laurent,

Thank you for your feedback!

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:06
> Subject: Re: [PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
> > When in vertical stripe mode of operation, there is the option
> > of swapping even data and odd data on the two LVDS interfaces
> > used to drive the video output.
> > Add data swap support by exposing a new DT property named
> > "renesas,swap-data".
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ++++++++++++++++-------
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 3aeaf9e..c306fab 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -69,6 +69,7 @@ struct rcar_lvds {
> >
> >  	struct drm_bridge *companion;
> >  	bool dual_link;
> > +	bool stripe_swap_data;
> >  };
> >
> >  #define bridge_to_rcar_lvds(bridge) \
> > @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> >  	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> >
> >  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> > -		/*
> > -		 * Configure vertical stripe based on the mode of operation of
> > -		 * the connected device.
> > -		 */
> > -		rcar_lvds_write(lvds, LVDSTRIPE,
> > -				lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> > +		u32 lvdstripe = 0;
> > +
> > +		if (lvds->dual_link)
> > +			/*
> > +			 * Configure vertical stripe based on the mode of
> > +			 * operation of the connected device.
> > +			 */
> > +			lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?
> > +						       LVDSTRIPE_ST_SWAP : 0);
> 
> Would the following be simpler ?
> 
> 		lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0)
> 			  | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);
> 
> > +		rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);

I actually think I need to rework this patch slightly, because the user manual
says that ST_SWAP is reserved for LVD1STRIPE, so I need to make sure we
don't set it for LVDS1.

So perhaps, this could translate to something like:
If (lvds->dual_link)
	lvdstripe = LVDSTRIPE_ST_ON | (<swap-whatever> && lvds->companion) ? LVDSTRIPE_ST_SWAP : 0);

I don't think we should be setting LVDSTRIPE_ST_SWAP without LVDSTRIPE_ST_ON, this solution
would allow us to test lvds->dual_link only once, and will prevent us from setting LVDSTRIPE_ST_SWAP if
LVDSTRIPE_ST_ON is not set or if we are touching LVDS1.

What do you think?

Thanks,
Fab

> >  	}
> >
> >  	/*
> > @@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> >  		}
> >  	}
> >
> > -	if (lvds->dual_link)
> > +	if (lvds->dual_link) {
> > +		lvds->stripe_swap_data = of_property_read_bool(
> > +						lvds->dev->of_node,
> > +						"renesas,swap-data");
> >  		ret = rcar_lvds_parse_dt_companion(lvds);
> > +	}
> 
> As explained in the review of the corresponding DT bindings, I think
> this should be queried from the remote device rather than specified in
> DT.
> 
> >
> >  done:
> >  	of_node_put(local_output);
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data

[Laurent Pinchart]

Hi Fabrizio,

On Mon, Aug 05, 2019 at 08:59:51AM +0000, Fabrizio Castro wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Sent: 02 August 2019 08:44
> > Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
> > 
> > Hi Fabrizio,
> > 
> > Thank you for the patch.
> > 
> > On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
> > > R-Car D3, R-Car E3, and RZ/G2E support dual-link mode.
> > > In such a mode, the first LVDS encoder emits even data, and the
> > > second LVDS encoder emits odd data. This patch documents property
> > > renesas,swap-data, used to swap even and odd data around.
> > >
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > ---
> > >  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > index dece79e..8980179 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > @@ -52,6 +52,11 @@ Optional properties:
> > >    mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
> > >    and shall point to the second encoder to be used as a companion in dual-link
> > >    mode. It shall not be set for any other LVDS encoder.
> > > +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
> > > +  emits even data, and the second LVDS encoder emits odd data. When property
> > > +  renesas,swap-data is specified, the data emitted by the two encoders will be
> > > +  swapped around. This property can only be used in conjunction with property
> > > +  renesas,companion.
> > 
> > From an LVDS encoder point of view this is more a configuration option
> > than a description of the hardware. Wouldn't it be better for the LVDS
> > sink to report which of the odd or even pixels it expects on each of its
> > endpoints ?
> 
> Yes, that would be my preference too, and it would be better, I am just not entirely
> what's the best place for this information though
> 
> > The LVDS encoder driver could then query that at runtime and
> > configure itself accordingly. Ideally this should be queried through the
> > drm_bridge_timings structure (or through a similar mean), not through
> > DT. An LVDS sink that has a fixed mapping of odd/even pixels to
> > endpoints wouldn't need the information to be specified in DT at all.
> 
> Isn't drm_bridge_timings specific for bridges?

Its name makes it specific to bridges, but the information it contains
could equally apply to panels. I would thus use it for both, possibly
after renaming it.

-- 
Regards,

Laurent Pinchart

RE: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support

[Fabrizio Castro]

Hi Laurent,

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 02 August 2019 09:34
> Subject: Re: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support
> 
> Hi Fabrizio,
> 
> Thank you for the patch.
> 
> On Fri, Aug 02, 2019 at 08:34:09AM +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>
> > ---
> >  arch/arm64/boot/dts/renesas/Makefile               |   3 +-
> >  .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 112 +++++++++++++++++++++
> >  2 files changed, 114 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..d989998
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> > @@ -0,0 +1,112 @@
> > +// 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>;
> > +				lvds0_panel_in: endpoint {
> > +					remote-endpoint = <&lvds0_out>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				reg = <1>;
> > +				lvds1_panel_in: endpoint {
> > +					remote-endpoint = <&lvds1_out>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&gpio0 {
> > +	lvds-connector-en-gpio{
> > +		gpio-hog;
> > +		gpios = <17 GPIO_ACTIVE_HIGH>;
> > +		output-low;
> > +		line-name = "lvds-connector-en-gpio";
> > +	};
> 
> Any chance to specify this as the panel's enable signal in the panel DT
> node ?

I am not too sure, as this is not exactly an enable signal. When GP0_17 is low
then LVDS[01] are connected to the LVDS connector, when GP0_17 is high
then LVDS[01] are connected to the LT8918L.
Perhaps we should leave this fixed to low to avoid confusion?

Thanks,
Fab

> 
> > +};
> > +
> > +&lvds0 {
> > +	renesas,swap-data;
> 
> Let's discuss this property in reply to the DT bindings patch.
> 
> > +
> > +	ports {
> > +		port@1 {
> > +			lvds0_out: endpoint {
> > +				remote-endpoint = <&lvds0_panel_in>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&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 = <&lvds1_panel_in>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&pfc {
> > +	pwm5_pins: pwm5 {
> > +		groups = "pwm5_a";
> > +		function = "pwm5";
> > +	};
> > +};
> > +
> > +&pwm5 {
> > +	pinctrl-0 = <&pwm5_pins>;
> > +	pinctrl-names = "default";
> > +
> > +	status = "okay";
> > +};
> 
> I haven't reviewed pinouts in detail, but the patch otherwise looks sane
> to me. Another candidate for DT overlays though ;-)
> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support

[Fabrizio Castro]

Hi Geert,

Thank you for your feedback!

> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 02 August 2019 10:11
> Subject: Re: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support
> 
> Hi Fabrizio,
> 
> On Fri, Aug 2, 2019 at 9:35 AM Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> 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>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> > @@ -0,0 +1,112 @@
> 
> [...]
> 
> > +       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>;
> > +                               lvds0_panel_in: endpoint {
> > +                                       remote-endpoint = <&lvds0_out>;
> > +                               };
> > +                       };
> > +
> > +                       port@1 {
> > +                               reg = <1>;
> > +                               lvds1_panel_in: endpoint {
> > +                                       remote-endpoint = <&lvds1_out>;
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +};
> 
> [...]
> 
> > +&lvds0 {
> > +       renesas,swap-data;
> > +
> > +       ports {
> > +               port@1 {
> > +                       lvds0_out: endpoint {
> > +                               remote-endpoint = <&lvds0_panel_in>;
> > +                       };
> > +               };
> > +       };
> > +};
> > +
> > +&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 = <&lvds1_panel_in>;
> > +                       };
> > +               };
> > +       };
> > +};
> 
> Shouldn't the actual panel definition, and the lvds remote-endpoint setup,
> be extracted into a separate .dtsi, to be included here?
> 
> Cfr. arch/arm/boot/dts/r8a77xx-aa104xd12-panel.dtsi and
> arch/arm/boot/dts/r8a77xx-aa121td01-panel.dtsi.

It looks like those displays are commonly used with Marzen, Lager, and Koelsch
boards, I am not aware of any plans for reusing this particular panel.
Perhaps we should still make this more generic and create a .dtsi?

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/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels

[Laurent Pinchart]

Hi Fabrizio,

On Mon, Aug 05, 2019 at 09:12:34AM +0000, Fabrizio Castro wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Sent: 02 August 2019 09:20
> > Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
> > 
> > Hi Fabrizio,
> > 
> > Thank you for the patch.
> > 
> > On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
> > > If the display comes with two ports, assume it supports dual
> > > link.
> > >
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > ---
> > >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > index 2d54ae5..97c51c2 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> > >  			ret = -EPROBE_DEFER;
> > >  			goto done;
> > >  		}
> > > +		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> > > +			lvds->dual_link = of_graph_get_endpoint_count(remote)
> > > +					== 2;
> > 
> > This is a bit of a hack, as I think the information should be queried
> > from the panel, like we do for bridges. I'd say we can live with this
> > for now, but as the data swap flag should come from the panel as well,
> > we will need infrastructure for that, and we can as well through the
> > dual link flag there at the same time.
> 
> I totally agree, this is a nasty hack, my series is missing the infrastructure
> for describing this information
> 
> > I think we should use the drm_bridge_timings structure for this purpose,
> > as it would make life more difficult for users of drm_bridge and
> > drm_panel to have two different structures (especially when wrapping a
> > drm_panel with drm_panel_bridge_add()). The structure could be renamed
> > if desired.
> 
> I am not too sure using drm_bridge_timings for panels would make everybody
> happy? Is there any alternative? Perhaps this calls for a new struct we could
> embed in both drm_bridge_timings and some drm_panel_<whatever> data
> structure?

I think we could simply rename the structure, all its fields apply to
panels too.

> > >  	}
> > >
> > >  	if (lvds->dual_link) {

-- 
Regards,

Laurent Pinchart

RE: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data

[Fabrizio Castro]

Hi Laurent,

> From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf Of Laurent Pinchart
> Sent: 05 August 2019 10:36
> Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
> 
> Hi Fabrizio,
> 
> On Mon, Aug 05, 2019 at 08:59:51AM +0000, Fabrizio Castro wrote:
> > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Sent: 02 August 2019 08:44
> > > Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
> > >
> > > Hi Fabrizio,
> > >
> > > Thank you for the patch.
> > >
> > > On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
> > > > R-Car D3, R-Car E3, and RZ/G2E support dual-link mode.
> > > > In such a mode, the first LVDS encoder emits even data, and the
> > > > second LVDS encoder emits odd data. This patch documents property
> > > > renesas,swap-data, used to swap even and odd data around.
> > > >
> > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > > index dece79e..8980179 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > > > @@ -52,6 +52,11 @@ Optional properties:
> > > >    mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
> > > >    and shall point to the second encoder to be used as a companion in dual-link
> > > >    mode. It shall not be set for any other LVDS encoder.
> > > > +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
> > > > +  emits even data, and the second LVDS encoder emits odd data. When property
> > > > +  renesas,swap-data is specified, the data emitted by the two encoders will be
> > > > +  swapped around. This property can only be used in conjunction with property
> > > > +  renesas,companion.
> > >
> > > From an LVDS encoder point of view this is more a configuration option
> > > than a description of the hardware. Wouldn't it be better for the LVDS
> > > sink to report which of the odd or even pixels it expects on each of its
> > > endpoints ?
> >
> > Yes, that would be my preference too, and it would be better, I am just not entirely
> > what's the best place for this information though
> >
> > > The LVDS encoder driver could then query that at runtime and
> > > configure itself accordingly. Ideally this should be queried through the
> > > drm_bridge_timings structure (or through a similar mean), not through
> > > DT. An LVDS sink that has a fixed mapping of odd/even pixels to
> > > endpoints wouldn't need the information to be specified in DT at all.
> >
> > Isn't drm_bridge_timings specific for bridges?
> 
> Its name makes it specific to bridges, but the information it contains
> could equally apply to panels. I would thus use it for both, possibly
> after renaming it.

Will give this a try then.

Thanks,
Fab

> 
> --
> Regards,
> 
> Laurent Pinchart

RE: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels

[Fabrizio Castro]

Hi Laurent,

> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: 05 August 2019 10:49
> Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
> 
> Hi Fabrizio,
> 
> On Mon, Aug 05, 2019 at 09:12:34AM +0000, Fabrizio Castro wrote:
> > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Sent: 02 August 2019 09:20
> > > Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
> > >
> > > Hi Fabrizio,
> > >
> > > Thank you for the patch.
> > >
> > > On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
> > > > If the display comes with two ports, assume it supports dual
> > > > link.
> > > >
> > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > ---
> > > >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > > index 2d54ae5..97c51c2 100644
> > > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > > @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> > > >  			ret = -EPROBE_DEFER;
> > > >  			goto done;
> > > >  		}
> > > > +		if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> > > > +			lvds->dual_link = of_graph_get_endpoint_count(remote)
> > > > +					== 2;
> > >
> > > This is a bit of a hack, as I think the information should be queried
> > > from the panel, like we do for bridges. I'd say we can live with this
> > > for now, but as the data swap flag should come from the panel as well,
> > > we will need infrastructure for that, and we can as well through the
> > > dual link flag there at the same time.
> >
> > I totally agree, this is a nasty hack, my series is missing the infrastructure
> > for describing this information
> >
> > > I think we should use the drm_bridge_timings structure for this purpose,
> > > as it would make life more difficult for users of drm_bridge and
> > > drm_panel to have two different structures (especially when wrapping a
> > > drm_panel with drm_panel_bridge_add()). The structure could be renamed
> > > if desired.
> >
> > I am not too sure using drm_bridge_timings for panels would make everybody
> > happy? Is there any alternative? Perhaps this calls for a new struct we could
> > embed in both drm_bridge_timings and some drm_panel_<whatever> data
> > structure?
> 
> I think we could simply rename the structure, all its fields apply to
> panels too.

Ok, will give this a try.

Thanks,
Fab

> 
> > > >  	}
> > > >
> > > >  	if (lvds->dual_link) {
> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support

[Laurent Pinchart]

Hi Fabrizio,

On Mon, Aug 05, 2019 at 09:32:42AM +0000, Fabrizio Castro wrote:
> On 02 August 2019 09:06 Laurent Pinchart wrote:
> > On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
> > > When in vertical stripe mode of operation, there is the option
> > > of swapping even data and odd data on the two LVDS interfaces
> > > used to drive the video output.
> > > Add data swap support by exposing a new DT property named
> > > "renesas,swap-data".
> > >
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > ---
> > >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ++++++++++++++++-------
> > >  1 file changed, 16 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > index 3aeaf9e..c306fab 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > > @@ -69,6 +69,7 @@ struct rcar_lvds {
> > >
> > >  	struct drm_bridge *companion;
> > >  	bool dual_link;
> > > +	bool stripe_swap_data;
> > >  };
> > >
> > >  #define bridge_to_rcar_lvds(bridge) \
> > > @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> > >  	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> > >
> > >  	if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> > > -		/*
> > > -		 * Configure vertical stripe based on the mode of operation of
> > > -		 * the connected device.
> > > -		 */
> > > -		rcar_lvds_write(lvds, LVDSTRIPE,
> > > -				lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> > > +		u32 lvdstripe = 0;
> > > +
> > > +		if (lvds->dual_link)
> > > +			/*
> > > +			 * Configure vertical stripe based on the mode of
> > > +			 * operation of the connected device.
> > > +			 */
> > > +			lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?
> > > +						       LVDSTRIPE_ST_SWAP : 0);
> > 
> > Would the following be simpler ?
> > 
> > 		lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0)
> > 			  | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);
> > 
> > > +		rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
> 
> I actually think I need to rework this patch slightly, because the user manual
> says that ST_SWAP is reserved for LVD1STRIPE, so I need to make sure we
> don't set it for LVDS1.
> 
> So perhaps, this could translate to something like:
> If (lvds->dual_link)
> 	lvdstripe = LVDSTRIPE_ST_ON | (<swap-whatever> && lvds->companion) ? LVDSTRIPE_ST_SWAP : 0);
> 
> I don't think we should be setting LVDSTRIPE_ST_SWAP without LVDSTRIPE_ST_ON, this solution
> would allow us to test lvds->dual_link only once, and will prevent us from setting LVDSTRIPE_ST_SWAP if
> LVDSTRIPE_ST_ON is not set or if we are touching LVDS1.
> 
> What do you think?

I was thinking that lvds->stripe_swap_data should only be set when
lvds->dual_link is set and lvds->companion is non-NULL, so both are
roughly equivalent. It's a detail anyway, it doesn't matter too much.

> > >  	}
> > >
> > >  	/*
> > > @@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
> > >  		}
> > >  	}
> > >
> > > -	if (lvds->dual_link)
> > > +	if (lvds->dual_link) {
> > > +		lvds->stripe_swap_data = of_property_read_bool(
> > > +						lvds->dev->of_node,
> > > +						"renesas,swap-data");
> > >  		ret = rcar_lvds_parse_dt_companion(lvds);
> > > +	}
> > 
> > As explained in the review of the corresponding DT bindings, I think
> > this should be queried from the remote device rather than specified in
> > DT.
> > 
> > >
> > >  done:
> > >  	of_node_put(local_output);

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 12/12] arm64: dts: renesas: Add EK874 board with idk-2121wr display support

[Laurent Pinchart]

Hi Fabrizio,

On Mon, Aug 05, 2019 at 09:37:29AM +0000, Fabrizio Castro wrote:
> On 02 August 2019 09:34 Laurent Pinchart wrote:
> > On Fri, Aug 02, 2019 at 08:34:09AM +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>
> > > ---
> > >  arch/arm64/boot/dts/renesas/Makefile               |   3 +-
> > >  .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 112 +++++++++++++++++++++
> > >  2 files changed, 114 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..d989998
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
> > > @@ -0,0 +1,112 @@
> > > +// 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>;
> > > +				lvds0_panel_in: endpoint {
> > > +					remote-endpoint = <&lvds0_out>;
> > > +				};
> > > +			};
> > > +
> > > +			port@1 {
> > > +				reg = <1>;
> > > +				lvds1_panel_in: endpoint {
> > > +					remote-endpoint = <&lvds1_out>;
> > > +				};
> > > +			};
> > > +		};
> > > +	};
> > > +};
> > > +
> > > +&gpio0 {
> > > +	lvds-connector-en-gpio{
> > > +		gpio-hog;
> > > +		gpios = <17 GPIO_ACTIVE_HIGH>;
> > > +		output-low;
> > > +		line-name = "lvds-connector-en-gpio";
> > > +	};
> > 
> > Any chance to specify this as the panel's enable signal in the panel DT
> > node ?
> 
> I am not too sure, as this is not exactly an enable signal. When GP0_17 is low
> then LVDS[01] are connected to the LVDS connector, when GP0_17 is high
> then LVDS[01] are connected to the LT8918L.
> Perhaps we should leave this fixed to low to avoid confusion?

The line-name was confusing me. A GPIO hog is thus indeed the best
option. A comment that explains what the signal is for could help.

> > > +};
> > > +
> > > +&lvds0 {
> > > +	renesas,swap-data;
> > 
> > Let's discuss this property in reply to the DT bindings patch.
> > 
> > > +
> > > +	ports {
> > > +		port@1 {
> > > +			lvds0_out: endpoint {
> > > +				remote-endpoint = <&lvds0_panel_in>;
> > > +			};
> > > +		};
> > > +	};
> > > +};
> > > +
> > > +&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 = <&lvds1_panel_in>;
> > > +			};
> > > +		};
> > > +	};
> > > +};
> > > +
> > > +&pfc {
> > > +	pwm5_pins: pwm5 {
> > > +		groups = "pwm5_a";
> > > +		function = "pwm5";
> > > +	};
> > > +};
> > > +
> > > +&pwm5 {
> > > +	pinctrl-0 = <&pwm5_pins>;
> > > +	pinctrl-names = "default";
> > > +
> > > +	status = "okay";
> > > +};
> > 
> > I haven't reviewed pinouts in detail, but the patch otherwise looks sane
> > to me. Another candidate for DT overlays though ;-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 10/12] arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1

[Geert Uytterhoeven]

Hi Laurent,

On Fri, Aug 2, 2019 at 11:10 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Aug 02, 2019 at 11:03:54AM +0200, Geert Uytterhoeven wrote:
> > On Fri, Aug 2, 2019 at 10:27 AM Laurent Pinchart wrote:
> > > On Fri, Aug 02, 2019 at 08:34:07AM +0100, Fabrizio Castro wrote:
> > > > Add the new renesas,companion property to the LVDS0 node to point to the
> > > > companion LVDS encoder LVDS1.
> > > > Based on similar work from Laurent Pinchart for the r8a7799[05].
> > > >
> > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > and taken in my tree.
> >
> > Shouldn't this go through renesas-devel and arm-soc?
>
> I'm collecting multimedia-related DT patches for v5.4, but if you or
> Simon want to take this patch, it will save me from sending a pull
> request, so please go ahead :-)

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.4.

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/RFC 11/12] arm64: dts: renesas: cat874: Add definition for 12V regulator

[Geert Uytterhoeven]

Hi Fabrizio,

On Mon, Aug 5, 2019 at 11:17 AM Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Sent: 02 August 2019 09:30
> > Subject: Re: [PATCH/RFC 11/12] arm64: dts: renesas: cat874: Add definition for 12V regulator
> >
> > Hi Fabrizio,
> >
> > Thank you for the patch.
> >
> > On Fri, Aug 02, 2019 at 08:34:08AM +0100, Fabrizio Castro wrote:
> > > Power rail "D12.0V" comes straight from the power barrel connector,
> > > and it's used in both main board and sub board.
> > >
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > I don't plan to take this in my tree without patch 12/12, so if you
> > think the rest of the series won't be ready in time for v5.4, feel free
> > to get this patch merged through Simon or Geert already.
>
> Geert, would you be happy to take this patch?

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.4.

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