[RESEND PATCH v2 0/5] Fix the parse_dt of exynos dsi and remove the OF graph

STALE3482d

9 messages, 2 authors, 2017-03-02 · open the first message on its own page

[RESEND PATCH v2 0/5] Fix the parse_dt of exynos dsi and remove the OF graph

From: Hoegeun Kwon <hidden>
Date: 2017-02-28 08:53:52

Hi All,

[Resend this v2 patches, because i have missing TO and CC.]

The dsi + panel is a parental relationship, so OF grpah is not needed.
Therefore, the current dsi_parse_dt function will throw an error,
because there is no linked OF graph for case such as fimd + dsi +
panel.

So the 1/5 patch parse the Pll, burst and esc clock frequency
properties in dsi_parse_dt and modified to create a bridge_node only
if there is an OF graph associated with dsi.

Also fixed the dts, which depend on the 1/5 patch. So removed the
ports node and move burst and esc clock frequency properties to the
parent (DSI node).

Changes for V2:
- Added the clear explanation for commit. (1/5 patch)
- Fixed it to the same subject as the actual work. (2/5 ~ 5/5 patches)

Best Regards,
Hoegeun

Hoegeun Kwon (5):
  drm/exynos: dsi: Fix the parse_dt function
  arm64: dts: exynos: Remove the OF graph from DSI node for exynos5433
    dts
  arm: dts: Remove the OF graph from DSI node for exynos3250 dts
  arm: dts: Remove the OF graph from DSI node for exynos4412 dts
  arm: dts: Remove the OF graph from DSI node for exynos4210 dts

 arch/arm/boot/dts/exynos3250-rinato.dts            | 23 ++--------------
 arch/arm/boot/dts/exynos4210-trats.dts             | 23 ++--------------
 arch/arm/boot/dts/exynos4412-trats2.dts            | 23 ++--------------
 .../boot/dts/exynos/exynos5433-tm2-common.dtsi     | 16 ++---------
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            | 32 ++++++----------------
 5 files changed, 16 insertions(+), 101 deletions(-)

-- 
1.9.1

[RESEND PATCH v2 3/5] arm: dts: Remove the OF graph from DSI node for exynos3250 dts

From: Hoegeun Kwon <hidden>
Date: 2017-02-28 08:30:34

The OF graph is not needed because the panel is a child of dsi. So
Remove the ports node and move burst and esc clock frequency
properties to the parent (DSI node).

Signed-off-by: Hoegeun Kwon <redacted>
Reviewed-by: Andrzej Hajda <redacted>
---
 arch/arm/boot/dts/exynos3250-rinato.dts | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
index 548413e..82e676a 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -215,24 +215,11 @@
 &dsi_0 {
 	vddcore-supply = <&ldo6_reg>;
 	vddio-supply = <&ldo6_reg>;
+	samsung,burst-clock-frequency = <250000000>;
+	samsung,esc-clock-frequency = <20000000>;
 	samsung,pll-clock-frequency = <24000000>;
 	status = "okay";
 
-	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port at 1 {
-			reg = <1>;
-
-			dsi_out: endpoint {
-				remote-endpoint = <&dsi_in>;
-				samsung,burst-clock-frequency = <250000000>;
-				samsung,esc-clock-frequency = <20000000>;
-			};
-		};
-	};
-
 	panel at 0 {
 		compatible = "samsung,s6e63j0x03";
 		reg = <0>;
@@ -262,12 +249,6 @@
 				vsync-len = <2>;
 			};
 		};
-
-		port {
-			dsi_in: endpoint {
-				remote-endpoint = <&dsi_out>;
-			};
-		};
 	};
 };
 
-- 
1.9.1

[RESEND PATCH v2 5/5] arm: dts: Remove the OF graph from DSI node for exynos4210 dts

From: Hoegeun Kwon <hidden>
Date: 2017-02-28 08:30:53

The OF graph is not needed because the panel is a child of dsi. So
Remove the ports node and move burst and esc clock frequency
properties to the parent (DSI node).

Signed-off-by: Hoegeun Kwon <redacted>
Reviewed-by: Andrzej Hajda <redacted>
---
 arch/arm/boot/dts/exynos4210-trats.dts | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts
index 0ca1b4d..9452bed 100644
--- a/arch/arm/boot/dts/exynos4210-trats.dts
+++ b/arch/arm/boot/dts/exynos4210-trats.dts
@@ -197,24 +197,11 @@
 &dsi_0 {
 	vddcore-supply = <&vusb_reg>;
 	vddio-supply = <&vmipi_reg>;
+	samsung,burst-clock-frequency = <500000000>;
+	samsung,esc-clock-frequency = <20000000>;
 	samsung,pll-clock-frequency = <24000000>;
 	status = "okay";
 
-	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port at 1 {
-			reg = <1>;
-
-			dsi_out: endpoint {
-				remote-endpoint = <&dsi_in>;
-				samsung,burst-clock-frequency = <500000000>;
-				samsung,esc-clock-frequency = <20000000>;
-			};
-		};
-	};
-
 	panel at 0 {
 		reg = <0>;
 		compatible = "samsung,s6e8aa0";
@@ -242,12 +229,6 @@
 				vsync-len = <2>;
 			};
 		};
-
-		port {
-			dsi_in: endpoint {
-				remote-endpoint = <&dsi_out>;
-			};
-		};
 	};
 };
 
-- 
1.9.1

[RESEND PATCH v2 4/5] arm: dts: Remove the OF graph from DSI node for exynos4412 dts

From: Hoegeun Kwon <hidden>
Date: 2017-02-28 08:33:08

The OF graph is not needed because the panel is a child of dsi. So
Remove the ports node and move burst and esc clock frequency
properties to the parent (DSI node).

Signed-off-by: Hoegeun Kwon <redacted>
Reviewed-by: Andrzej Hajda <redacted>
---
 arch/arm/boot/dts/exynos4412-trats2.dts | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
index 41ecd6d..86ce5e5 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -385,24 +385,11 @@
 &dsi_0 {
 	vddcore-supply = <&ldo8_reg>;
 	vddio-supply = <&ldo10_reg>;
+	samsung,burst-clock-frequency = <500000000>;
+	samsung,esc-clock-frequency = <20000000>;
 	samsung,pll-clock-frequency = <24000000>;
 	status = "okay";
 
-	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port at 1 {
-			reg = <1>;
-
-			dsi_out: endpoint {
-				remote-endpoint = <&dsi_in>;
-				samsung,burst-clock-frequency = <500000000>;
-				samsung,esc-clock-frequency = <20000000>;
-			};
-		};
-	};
-
 	panel at 0 {
 		compatible = "samsung,s6e8aa0";
 		reg = <0>;
@@ -430,12 +417,6 @@
 				vsync-len = <2>;
 			};
 		};
-
-		port {
-			dsi_in: endpoint {
-				remote-endpoint = <&dsi_out>;
-			};
-		};
 	};
 };
 
-- 
1.9.1

[RESEND PATCH v2 2/5] arm64: dts: exynos: Remove the OF graph from DSI node for exynos5433 dts

From: Hoegeun Kwon <hidden>
Date: 2017-02-28 08:49:25

The OF graph is not needed because the panel is a child of dsi. So
Remove the ports node and move burst and esc clock frequency
properties to the parent (DSI node).

Signed-off-by: Hoegeun Kwon <redacted>
Reviewed-by: Andrzej Hajda <redacted>
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index 6ce93a3..77ba238 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -298,23 +298,11 @@
 	status = "okay";
 	vddcore-supply = <&ldo6_reg>;
 	vddio-supply = <&ldo7_reg>;
+	samsung,burst-clock-frequency = <512000000>;
+	samsung,esc-clock-frequency = <16000000>;
 	samsung,pll-clock-frequency = <24000000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&te_irq>;
-
-	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port at 1 {
-			reg = <1>;
-
-			dsi_out: endpoint {
-				samsung,burst-clock-frequency = <512000000>;
-				samsung,esc-clock-frequency = <16000000>;
-			};
-		};
-	};
 };
 
 &hdmi {
-- 
1.9.1

[RESEND PATCH v2 1/5] drm/exynos: dsi: Fix the parse_dt function

From: Hoegeun Kwon <hidden>
Date: 2017-02-28 08:53:49

The dsi + panel is a parental relationship, so OF grpah is not needed.
Therefore, the current dsi_parse_dt function will throw an error,
because there is no linked OF graph for case such as fimd + dsi +
panel. So this patch parse the Pll, burst and esc clock frequency
properties in dsi_parse_dt and modified to create a bridge_node only
if there is an OF graph associated with dsi.
So I think the ABI breakage is needed.

Signed-off-by: Hoegeun Kwon <redacted>
Reviewed-by: Andrzej Hajda <redacted>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index f5c04d0..2d4e118 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1652,39 +1652,23 @@ static int exynos_dsi_parse_dt(struct exynos_dsi *dsi)
 	if (ret < 0)
 		return ret;
 
-	ep = of_graph_get_endpoint_by_regs(node, DSI_PORT_OUT, 0);
-	if (!ep) {
-		dev_err(dev, "no output port with endpoint specified\n");
-		return -EINVAL;
-	}
-
-	ret = exynos_dsi_of_read_u32(ep, "samsung,burst-clock-frequency",
+	ret = exynos_dsi_of_read_u32(node, "samsung,burst-clock-frequency",
 				     &dsi->burst_clk_rate);
 	if (ret < 0)
-		goto end;
+		return ret;
 
-	ret = exynos_dsi_of_read_u32(ep, "samsung,esc-clock-frequency",
+	ret = exynos_dsi_of_read_u32(node, "samsung,esc-clock-frequency",
 				     &dsi->esc_clk_rate);
 	if (ret < 0)
-		goto end;
-
-	of_node_put(ep);
+		return ret;
 
 	ep = of_graph_get_next_endpoint(node, NULL);
-	if (!ep) {
-		ret = -EINVAL;
-		goto end;
-	}
-
-	dsi->bridge_node = of_graph_get_remote_port_parent(ep);
-	if (!dsi->bridge_node) {
-		ret = -EINVAL;
-		goto end;
+	if (ep) {
+		dsi->bridge_node = of_graph_get_remote_port_parent(ep);
+		of_node_put(ep);
 	}
-end:
-	of_node_put(ep);
 
-	return ret;
+	return 0;
 }
 
 static int exynos_dsi_bind(struct device *dev, struct device *master,
-- 
1.9.1

Re: [RESEND PATCH v2 0/5] Fix the parse_dt of exynos dsi and remove the OF graph

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2017-02-28 09:58:59

On Tue, Feb 28, 2017 at 10:17 AM, Hoegeun Kwon [off-list ref] wrote:
Hi All,

[Resend this v2 patches, because i have missing TO and CC.]

The dsi + panel is a parental relationship, so OF grpah is not needed.
Therefore, the current dsi_parse_dt function will throw an error,
because there is no linked OF graph for case such as fimd + dsi +
panel.

So the 1/5 patch parse the Pll, burst and esc clock frequency
properties in dsi_parse_dt and modified to create a bridge_node only
if there is an OF graph associated with dsi.

Also fixed the dts, which depend on the 1/5 patch. So removed the
ports node and move burst and esc clock frequency properties to the
parent (DSI node).
Discussions in previous thread lead us to bisectability problem.
Bisectability in regular driver changes is one thing but in case of
driver + DTS the gap is much bigger. DTS will go through separate tree
and branches. How do you want to solve the problem?

Best regards,
Krzysztof

Re: [RESEND PATCH v2 0/5] Fix the parse_dt of exynos dsi and remove the OF graph

From: Hoegeun Kwon <hidden>
Date: 2017-03-02 01:44:28

On 02/28/2017 06:58 PM, Krzysztof Kozlowski wrote:
On Tue, Feb 28, 2017 at 10:17 AM, Hoegeun Kwon [off-list ref] wrote:
quoted
Hi All,

[Resend this v2 patches, because i have missing TO and CC.]

The dsi + panel is a parental relationship, so OF grpah is not needed.
Therefore, the current dsi_parse_dt function will throw an error,
because there is no linked OF graph for case such as fimd + dsi +
panel.

So the 1/5 patch parse the Pll, burst and esc clock frequency
properties in dsi_parse_dt and modified to create a bridge_node only
if there is an OF graph associated with dsi.

Also fixed the dts, which depend on the 1/5 patch. So removed the
ports node and move burst and esc clock frequency properties to the
parent (DSI node).
Discussions in previous thread lead us to bisectability problem.
Bisectability in regular driver changes is one thing but in case of
driver + DTS the gap is much bigger. DTS will go through separate tree
and branches. How do you want to solve the problem?
Sorry for the delay in reply, Mar 1st was the holiday.
I thought of two solutions.

1. squash the patches in a single patch

2. split the dts related patches so that the first part adds the:
+    samsung,burst-clock-frequency = <512000000>;
+    samsung,esc-clock-frequency = <16000000>;

and the second part at the end removes the 'port' node
So it consists of 6 patches in total.

Which do you think is better?
If you have any other ideas, could you tell me?

Best Regards,
Hoegeun
Best regards,
Krzysztof

Re: [RESEND PATCH v2 0/5] Fix the parse_dt of exynos dsi and remove the OF graph

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2017-03-02 07:37:13

On Thu, Mar 2, 2017 at 3:44 AM, Hoegeun Kwon [off-list ref] wrote:
On 02/28/2017 06:58 PM, Krzysztof Kozlowski wrote:
quoted
Discussions in previous thread lead us to bisectability problem.
Bisectability in regular driver changes is one thing but in case of
driver + DTS the gap is much bigger. DTS will go through separate tree
and branches. How do you want to solve the problem?

Sorry for the delay in reply, Mar 1st was the holiday.
I thought of two solutions.

1. squash the patches in a single patch
No, for the same reason. DTS code/patches have to go through arm-soc
DTS branch without mixing with any driver changes. Otherwise arm-soc
guys are angry.
2. split the dts related patches so that the first part adds the:
+    samsung,burst-clock-frequency = <512000000>;
+    samsung,esc-clock-frequency = <16000000>;

and the second part at the end removes the 'port' node
So it consists of 6 patches in total.
That's a solution. The remaining DTS patches would go in next release...

Another solution would be for this release cycle:
        if (of_property_does_not_exist(node, "samsung,burst-clock-frequency") {
                // Fallback to old parsing mode
                node = of_graph_get_endpoint_by_regs(node, DSI_PORT_OUT, 0);
                if (!node) {
                        dev_err(dev, "no burst-clock-frequency nor
output port with endpoint specified\n");
                        return -EINVAL;
                }
        }
        ret = exynos_dsi_of_read_u32(node, "samsung,burst-clock-frequency",
                                     &dsi->burst_clk_rate);
        ...

...and in next release the DTS patches would go in. This would give
you also DTB backward compatibility. However still DTS could be
applied later, after driver changes gets into mainline.

Personally I would prefer your solution #2 (with separate DTS patch
adding new properties). Does it sound reasonable for Inki?

Thanks for looking into this problem.

Best regards,
Krzysztof
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help