Skip to contentSkip to conversation

arm64: dts: qcom: sc7180: Add Display Port dt node

PATCH Superseded Tanmay Shah 2 participants 3 messages Jun 18, 2020 – Jun 19, 2020

Subsystems (4)

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 arm64: dts: qcom: sc7180: Add Display Port dt node

Enable DP driver for sc7180.

This change depends-on following series:
https://patchwork.freedesktop.org/series/78583/
and https://patchwork.freedesktop.org/patch/351990/

Changes in v2:

- Add assigned-clocks and assigned-clock-parents
- Remove cell-index and pixel_rcg
- Change compatible to qcom,sc7180-dp

---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 57 ++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +53 -4
index 916401f7e87c..26fe623e3b0f 100644
@@ -2216,10 +2216,19 @@ ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
 
+					port@1 {
+						reg = <1>;
+						dpu_intf1_out: endpoint {
+							remote-endpoint =
+								 <&dsi0_in>;
+						};
+					};
+
 					port@0 {
 						reg = <0>;
-						dpu_intf1_out: endpoint {
-							remote-endpoint = <&dsi0_in>;
+						dpu_intf0_out: endpoint {
+							remote-endpoint =
+								 <&dp_in>;
 						};
 					};
 				};
@@ -2293,6 +2302,46 @@ dsi_phy: dsi-phy@ae94400 {
 			};
 		};
 
+		msm_dp: displayport-controller@ae90000{
+			status = "ok";
+			compatible = "qcom,sc7180-dp";
+
+			reg = <0 0xae90000 0 0x1400>;
+			reg-names = "dp_controller";
+
+			interrupt-parent = <&mdss>;
+			interrupts = <12 0>;
+
+			clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
+				 <&dispcc DISP_CC_MDSS_DP_LINK_CLK>,
+				 <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>,
+				 <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>;
+			clock-names = "core_aux", "ctrl_link",
+				      "ctrl_link_iface", "stream_pixel";
+			#clock-cells = <1>;
+			assigned-clocks = <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
+			assigned-clock-parents = <&msm_dp 1>;
+
+			data-lanes = <0 1>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					dp_in: endpoint {
+						remote-endpoint =
+							 <&dpu_intf0_out>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					dp_out: endpoint { };
+				};
+			};
+		};
+
 		dispcc: clock-controller@af00000 {
 			compatible = "qcom,sc7180-dispcc";
 			reg = <0 0x0af00000 0 0x200000>;
@@ -2300,8 +2349,8 @@ dispcc: clock-controller@af00000 {
 				 <&gcc GCC_DISP_GPLL0_CLK_SRC>,
 				 <&dsi_phy 0>,
 				 <&dsi_phy 1>,
-				 <0>,
-				 <0>;
+				 <&msm_dp 0>,
+				 <&msm_dp 1>;
 			clock-names = "bi_tcxo",
 				      "gcc_disp_gpll0_clk_src",
 				      "dsi0_phy_pll_out_byteclk",
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#1 lore raw

Quoting Tanmay Shah (2020-06-18 16:21:13)

Quoted 1 line

Enable DP driver for sc7180.

Add DP device node on sc7180? This isn't a driver.

Quoted 32 lines

This change depends-on following series:
https://patchwork.freedesktop.org/series/78583/
and https://patchwork.freedesktop.org/patch/351990/

Changes in v2:

- Add assigned-clocks and assigned-clock-parents
- Remove cell-index and pixel_rcg
- Change compatible to qcom,sc7180-dp

---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 57 ++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +8 -0
index 916401f7e87c..26fe623e3b0f 100644
@@ -2216,10 +2216,19 @@ ports {
                                        #address-cells = <1>;
                                        #size-cells = <0>;
 
+                                       port@1 {
+                                               reg = <1>;
+                                               dpu_intf1_out: endpoint {
+                                                       remote-endpoint =
+                                                                <&dsi0_in>;
+                                               };
+                                       };
+

arch/arm64/boot/dts/qcom/sc7180.dtsi ports { · line 2234

Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +3 -2
                                        port@0 {
                                                reg = <0>;
-                                               dpu_intf1_out: endpoint {
-                                                       remote-endpoint = <&dsi0_in>;
+                                               dpu_intf0_out: endpoint {
+                                                       remote-endpoint =
+                                                                <&dp_in>;
                                                };
                                        };
                                };

I thought this wasn't supposed to change? At least according to the binding it shouldn't be needed.

arch/arm64/boot/dts/qcom/sc7180.dtsi dsi_phy: dsi-phy@ae94400 { · line 2306

Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +2 -0
@@ -2293,6 +2302,46 @@ dsi_phy: dsi-phy@ae94400 {
                        };
                };
 
+               msm_dp: displayport-controller@ae90000{
+                       status = "ok";

Please use status = "disabled";

arch/arm64/boot/dts/qcom/sc7180.dtsi dsi_phy: dsi-phy@ae94400 { · line 2312

Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +6 -0
+                       compatible = "qcom,sc7180-dp";
+
+                       reg = <0 0xae90000 0 0x1400>;
+                       reg-names = "dp_controller";
+
+                       interrupt-parent = <&mdss>;

Any reason why this isn't under the mdss node like the other display device nodes?

arch/arm64/boot/dts/qcom/sc7180.dtsi dsi_phy: dsi-phy@ae94400 { · line 2325

Quoted 13 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +13 -0
+                       interrupts = <12 0>;
+
+                       clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
+                                <&dispcc DISP_CC_MDSS_DP_LINK_CLK>,
+                                <&dispcc DISP_CC_MDSS_DP_LINK_INTF_CLK>,
+                                <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>;
+                       clock-names = "core_aux", "ctrl_link",
+                                     "ctrl_link_iface", "stream_pixel";
+                       #clock-cells = <1>;
+                       assigned-clocks = <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
+                       assigned-clock-parents = <&msm_dp 1>;
+
+                       data-lanes = <0 1>;

This can and should be left to the board files. At the SoC level my understanding is that there are four lanes possible, so no need to artificially limit it here.

arch/arm64/boot/dts/qcom/sc7180.dtsi dsi_phy: dsi-phy@ae94400 { · line 2334

Quoted 9 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +9 -0
+
+                       ports {
+                               #address-cells = <1>;
+                               #size-cells = <0>;
+                               port@0 {
+                                       reg = <0>;
+                                       dp_in: endpoint {
+                                               remote-endpoint =
+                                                        <&dpu_intf0_out>;

I'd prefer these were on one line, regardless of the 80 character line limit/suggestion.

Quoted 13 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +10 -0
+                                       };
+                               };
+
+                               port@1 {
+                                       reg = <1>;
+                                       dp_out: endpoint { };
+                               };
+                       };
+               };
+
                dispcc: clock-controller@af00000 {
                        compatible = "qcom,sc7180-dispcc";
                        reg = <0 0x0af00000 0 0x200000>;
#2 lore raw

Thanks Stephen for reviews.
I will post new change addressing your all comments.

Quoted 5 lines · Stephen Boyd

Quoting Tanmay Shah (2020-06-18 16:21:13)

Quoted 1 line

Enable DP driver for sc7180.

Add DP device node on sc7180? This isn't a driver.

Done.

Quoted 46 lines
Quoted 46 lines

This change depends-on following series:
https://patchwork.freedesktop.org/series/78583/
and https://patchwork.freedesktop.org/patch/351990/

Changes in v2:

- Add assigned-clocks and assigned-clock-parents
- Remove cell-index and pixel_rcg
- Change compatible to qcom,sc7180-dp

---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 57 
++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +4 -0
index 916401f7e87c..26fe623e3b0f 100644
@@ -2216,10 +2216,19 @@ ports {
                                        #address-cells = <1>;
                                        #size-cells = <0>;
 
+                                       port@1 {
+                                               reg = <1>;
+                                               dpu_intf1_out: 
 endpoint {
+                                                       

remote-endpoint =
+
<&dsi0_in>;

arch/arm64/boot/dts/qcom/sc7180.dtsi +3 -1
+                                               };
+                                       };
+
                                        port@0 {
                                                reg = <0>;
-                                               dpu_intf1_out: 

endpoint {
-
remote-endpoint = <&dsi0_in>;
+ dpu_intf0_out:
endpoint {
+
remote-endpoint =

Quoted 9 lines
Quoted 5 lines

+

<&dp_in>;
                                                };
                                        };
                                };

I thought this wasn't supposed to change? At least according to the binding it shouldn't be needed.

Thanks. Yes it should be port@2 as per new dpu-sc7180.yaml bindings in below patch.
https://patchwork.freedesktop.org/patch/371087/?series=78583&rev=1

Quoted 9 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi dsi_phy: dsi-phy@ae94400 { · line 2306

Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +2 -0
@@ -2293,6 +2302,46 @@ dsi_phy: dsi-phy@ae94400 {
                        };
                };
 
+               msm_dp: displayport-controller@ae90000{
+                       status = "ok";

Please use status = "disabled";

Ok got it. Board's DT will enable it.

Quoted 10 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi dsi_phy: dsi-phy@ae94400 { · line 2312

Quoted 6 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +6 -0
+                       compatible = "qcom,sc7180-dp";
+
+                       reg = <0 0xae90000 0 0x1400>;
+                       reg-names = "dp_controller";
+
+                       interrupt-parent = <&mdss>;

Any reason why this isn't under the mdss node like the other display device nodes?

Done. Moved msm_dp node under mdss.

Quoted 12 lines
Quoted 12 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +10 -0
+                       interrupts = <12 0>;
+
+                       clocks = <&dispcc DISP_CC_MDSS_DP_AUX_CLK>,
+                                <&dispcc DISP_CC_MDSS_DP_LINK_CLK>,
+                                <&dispcc 
 DISP_CC_MDSS_DP_LINK_INTF_CLK>,
+                                <&dispcc DISP_CC_MDSS_DP_PIXEL_CLK>;
+                       clock-names = "core_aux", "ctrl_link",
+                                     "ctrl_link_iface", 
 "stream_pixel";
+                       #clock-cells = <1>;
+                       assigned-clocks = <&dispcc 
Quoted 9 lines

arch/arm64/boot/dts/qcom/sc7180.dtsi dsi_phy: dsi-phy@ae94400 { · line 2325

Quoted 4 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +3 -0
 DISP_CC_MDSS_DP_PIXEL_CLK_SRC>;
+                       assigned-clock-parents = <&msm_dp 1>;
+
+                       data-lanes = <0 1>;

This can and should be left to the board files. At the SoC level my understanding is that there are four lanes possible, so no need to artificially limit it here.

Removed from here. Driver changes I will be posting accordingly.

Quoted 11 lines
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +9 -0
+
+                       ports {
+                               #address-cells = <1>;
+                               #size-cells = <0>;
+                               port@0 {
+                                       reg = <0>;
+                                       dp_in: endpoint {
+                                               remote-endpoint =
+                                                        

<&dpu_intf0_out>;

Quoted 3 lines

I'd prefer these were on one line, regardless of the 80 character line limit/suggestion.

ok done.

Quoted 13 lines
Quoted 13 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +10 -0
+                                       };
+                               };
+
+                               port@1 {
+                                       reg = <1>;
+                                       dp_out: endpoint { };
+                               };
+                       };
+               };
+
                dispcc: clock-controller@af00000 {
                        compatible = "qcom,sc7180-dispcc";
                        reg = <0 0x0af00000 0 0x200000>;