Skip to contentSkip to conversation

arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

PATCH In review Jeykumar Sankaran 3 participants 4 messages Dec 3, 2018 – Dec 4, 2018 3 messages not in the archive

Subsystems (3)
Device tree bindings Arm Qualcomm mailing list Arm Qualcomm support
lore
Share & tools
lore

Jeykumar Sankaran

author
#0 lore raw

PATCH v5 1/3 arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

DPU is short for the Display Processing Unit. It is the display controller on Qualcomm SDM845 chips.

This change adds MDSS and DSI nodes to enable display on the target device.

Changes in v2:
- Beefed up commit message
- Use SoC specific compatibles for mdss and dpu (Rob H)
- Use assigned-clocks to set initial clock frequency(Rob H)
Changes in v3:
- added IOMMU node
- Fix device naming (remove _phys)
- Use correct IRQ_TYPE in interrupt specifiers
Changes in v4:
- move mdss node to preserve the unit address sort order
- remove _clk suffix from dsi clocks
(both the comments are from Doug Anderson)
Changes in v5:
- Keep the device status "disabled" by default (Bjorn Andersson)
- Use MDSS_GDSC macro (Jordan)
- Fix phy-names (Jordan)
- List reg ranges in numerical order (Jordan)

Signed-off-by Jeykumar Sankaran <[email protected]>
Signed-off-by Sean Paul <[email protected]>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 203 +++++++++++++++++++++++++++++++++++
 1 file changed, 203 insertions(+)
arch/arm64/boot/dts/qcom/sdm845.dtsi +203 -0
index 1419b00..a16f1fe 100644
@@ -1256,6 +1256,209 @@
 			};
 		};
 
+		mdss: mdss@ae00000 {
+			compatible = "qcom,sdm845-mdss";
+			reg = <0xae00000 0x1000>;
+			reg-names = "mdss";
+
+			power-domains = <&dispcc MDSS_GDSC>;
+
+			clocks = <&gcc GCC_DISP_AHB_CLK>,
+				 <&gcc GCC_DISP_AXI_CLK>,
+				 <&dispcc DISP_CC_MDSS_MDP_CLK>;
+			clock-names = "iface", "bus", "core";
+
+			assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
+			assigned-clock-rates = <300000000>;
+
+			interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+
+			iommus = <&apps_smmu 0x880 0x8>,
+			         <&apps_smmu 0xc80 0x8>;
+
+			status = "disabled";
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			mdss_mdp: mdp@ae01000 {
+				compatible = "qcom,sdm845-dpu";
+				reg = <0x0ae01000 0x8f000>,
+				      <0x0aeb0000 0x2008>;
+				reg-names = "mdp", "vbif";
+
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&dispcc DISP_CC_MDSS_AXI_CLK>,
+					 <&dispcc DISP_CC_MDSS_MDP_CLK>,
+					 <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+				clock-names = "iface", "bus", "core", "vsync";
+
+				assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>,
+						  <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+				assigned-clock-rates = <300000000>,
+						       <19200000>;
+
+				interrupt-parent = <&mdss>;
+				interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						dpu_intf1_out: endpoint {
+							remote-endpoint = <&dsi0_in>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+						dpu_intf2_out: endpoint {
+							remote-endpoint = <&dsi1_in>;
+						};
+					};
+				};
+			};
+
+			dsi0: dsi@ae94000 {
+				compatible = "qcom,mdss-dsi-ctrl";
+				reg = <0xae94000 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupt-parent = <&mdss>;
+				interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+
+				clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
+					 <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
+					 <&dispcc DISP_CC_MDSS_PCLK0_CLK>,
+					 <&dispcc DISP_CC_MDSS_ESC0_CLK>,
+					 <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&dispcc DISP_CC_MDSS_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+
+				phys = <&dsi0_phy>;
+				phy-names = "dsi0";
+
+				status = "disabled";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						dsi0_in: endpoint {
+							remote-endpoint = <&dpu_intf1_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+						dsi0_out: endpoint {
+						};
+					};
+				};
+			};
+
+			dsi0_phy: dsi-phy@ae94400 {
+				compatible = "qcom,dsi-phy-10nm";
+				reg = <0xae94400 0x200>,
+				      <0xae94600 0x280>,
+				      <0xae94a00 0x1e0>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
+				clock-names = "iface";
+
+				status = "disabled";
+			};
+
+			dsi1: dsi@ae96000 {
+				compatible = "qcom,mdss-dsi-ctrl";
+				reg = <0xae96000 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupt-parent = <&mdss>;
+				interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+
+				clocks = <&dispcc DISP_CC_MDSS_BYTE1_CLK>,
+					 <&dispcc DISP_CC_MDSS_BYTE1_INTF_CLK>,
+					 <&dispcc DISP_CC_MDSS_PCLK1_CLK>,
+					 <&dispcc DISP_CC_MDSS_ESC1_CLK>,
+					 <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&dispcc DISP_CC_MDSS_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+
+				phys = <&dsi1_phy>;
+				phy-names = "dsi1";
+
+				status = "disabled";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						dsi1_in: endpoint {
+							remote-endpoint = <&dpu_intf2_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+						dsi1_out: endpoint {
+						};
+					};
+				};
+			};
+
+			dsi1_phy: dsi-phy@ae96400 {
+				compatible = "qcom,dsi-phy-10nm";
+				reg = <0xae96400 0x200>,
+				      <0xae96600 0x280>,
+				      <0xae96a00 0x10e>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
+				clock-names = "iface";
+
+				status = "disabled";
+			};
+		};
+
 		dispcc: clock-controller@af00000 {
 			compatible = "qcom,sdm845-dispcc";
 			reg = <0xaf00000 0x10000>;
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

A message that is not in the archive (sent off-list or not yet received): 1543876054-12998-1-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org

#2 lore raw

Hi,

Quoted 13 lines · Jeykumar Sankaran
arch/arm64/boot/dts/qcom/sdm845.dtsi +13 -0
+                       dsi0: dsi@ae94000 {
+                               compatible = "qcom,mdss-dsi-ctrl";
+                               reg = <0xae94000 0x400>;
+                               reg-names = "dsi_ctrl";
+
+                               interrupt-parent = <&mdss>;
+                               interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+
+                               clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
+                                        <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
+                                        <&dispcc DISP_CC_MDSS_PCLK0_CLK>,
+                                        <&dispcc DISP_CC_MDSS_ESC0_CLK>,
+                                        <&dispcc DISP_CC_MDSS_AHB_CLK>,

arch/arm64/boot/dts/qcom/sdm845.dtsi line 1351

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +10 -0
+                                        <&dispcc DISP_CC_MDSS_AXI_CLK>;
+                               clock-names = "byte",
+                                             "byte_intf",
+                                             "pixel",
+                                             "core",
+                                             "iface",
+                                             "bus";
+
+                               phys = <&dsi0_phy>;
+                               phy-names = "dsi0";

I'm pretty sure that this should just be "dsi" and the one below in dsi1 should also be called "dsi". +Jordan should confirm.

Quoted 15 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +15 -0
+                       dsi1: dsi@ae96000 {
+                               compatible = "qcom,mdss-dsi-ctrl";
+                               reg = <0xae96000 0x400>;
+                               reg-names = "dsi_ctrl";
+
+                               interrupt-parent = <&mdss>;
+                               interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+
+                               clocks = <&dispcc DISP_CC_MDSS_BYTE1_CLK>,
+                                        <&dispcc DISP_CC_MDSS_BYTE1_INTF_CLK>,
+                                        <&dispcc DISP_CC_MDSS_PCLK1_CLK>,
+                                        <&dispcc DISP_CC_MDSS_ESC1_CLK>,
+                                        <&dispcc DISP_CC_MDSS_AHB_CLK>,
+                                        <&dispcc DISP_CC_MDSS_AXI_CLK>;
+                               clock-names = "byte",

arch/arm64/boot/dts/qcom/sdm845.dtsi line 1419

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +10 -0
+                                             "byte_intf",
+                                             "pixel",
+                                             "core",
+                                             "iface",
+                                             "bus";
+
+                               phys = <&dsi1_phy>;
+                               phy-names = "dsi1";
+
+                               status = "disabled";

This "disabled" is causing me problems. I don't actually need "dsi1" but if I don't enable "dsi1" then my display doesn't come up. :( I ran out of time to debug but I wonder if this is this the standard thing where DRM needs to wait for all the components to probe until it can finish? If nobody on this list just knows I'll dig tomorrow and confirm that my memory isn't faulty and see what we've done about this in the past.

One last note: it's pretty weird that you sent out only 1/3 and not 2/3 and 3/3. If you're not ready to send out MTP stuff yet then you should send out v6 as just a singleton patch.

-Doug
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

A message that is not in the archive (sent off-list or not yet received): CAD=FV=WKtf0Ba2Fk-V8kktp6KPJh_6DcMQSVF=ynoAA4nkqKBQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#4 lore raw
Quoted 28 lines · Doug Anderson

Hi,

Quoted 24 lines · Jeykumar Sankaran
arch/arm64/boot/dts/qcom/sdm845.dtsi +9 -0
+                       dsi0: dsi@ae94000 {
+                               compatible = "qcom,mdss-dsi-ctrl";
+                               reg = <0xae94000 0x400>;
+                               reg-names = "dsi_ctrl";
+
+                               interrupt-parent = <&mdss>;
+                               interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+
+                               clocks = <&dispcc

DISP_CC_MDSS_BYTE0_CLK>,
+ <&dispcc
DISP_CC_MDSS_BYTE0_INTF_CLK>,
+ <&dispcc
DISP_CC_MDSS_PCLK0_CLK>,
+ <&dispcc
DISP_CC_MDSS_ESC0_CLK>,
+ <&dispcc
DISP_CC_MDSS_AHB_CLK>,
+ <&dispcc
DISP_CC_MDSS_AXI_CLK>;

arch/arm64/boot/dts/qcom/sdm845.dtsi +4 -0
+                               clock-names = "byte",
+                                             "byte_intf",
+                                             "pixel",
+                                             "core",
Quoted 9 lines

arch/arm64/boot/dts/qcom/sdm845.dtsi line 1351

Quoted 5 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +5 -0
+                                             "iface",
+                                             "bus";
+
+                               phys = <&dsi0_phy>;
+                               phy-names = "dsi0";

I'm pretty sure that this should just be "dsi" and the one below in dsi1 should also be called "dsi". +Jordan should confirm.

Makes sense. I can fix that!

Quoted 33 lines
Quoted 22 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +9 -0
+                       dsi1: dsi@ae96000 {
+                               compatible = "qcom,mdss-dsi-ctrl";
+                               reg = <0xae96000 0x400>;
+                               reg-names = "dsi_ctrl";
+
+                               interrupt-parent = <&mdss>;
+                               interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+
+                               clocks = <&dispcc

DISP_CC_MDSS_BYTE1_CLK>,
+ <&dispcc
DISP_CC_MDSS_BYTE1_INTF_CLK>,
+ <&dispcc
DISP_CC_MDSS_PCLK1_CLK>,
+ <&dispcc
DISP_CC_MDSS_ESC1_CLK>,
+ <&dispcc
DISP_CC_MDSS_AHB_CLK>,
+ <&dispcc
DISP_CC_MDSS_AXI_CLK>;

arch/arm64/boot/dts/qcom/sdm845.dtsi +1 -0
+                               clock-names = "byte",

arch/arm64/boot/dts/qcom/sdm845.dtsi line 1419

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +10 -0
+                                             "byte_intf",
+                                             "pixel",
+                                             "core",
+                                             "iface",
+                                             "bus";
+
+                               phys = <&dsi1_phy>;
+                               phy-names = "dsi1";
+
+                               status = "disabled";
Quoted 8 lines

This "disabled" is causing me problems. I don't actually need "dsi1" but if I don't enable "dsi1" then my display doesn't come up. :( I ran out of time to debug but I wonder if this is this the standard thing where DRM needs to wait for all the components to probe until it can finish? If nobody on this list just knows I'll dig tomorrow and confirm that my memory isn't faulty and see what we've done about this in the past.

https://patchwork.kernel.org/patch/10467895/

Can you try out with this change (reviewed but not merged yet). It validates the nodes before adding to the DSI list.

Quoted 5 lines

One last note: it's pretty weird that you sent out only 1/3 and not 2/3 and 3/3. If you're not ready to send out MTP stuff yet then you should send out v6 as just a singleton patch.

Yes. I was trying to separate this one out as an independent change. Sandeep is working on the comments on removing the pinctrl nodes and updated mtp nodes. He should be posting 2/3 and 3/3 in the next couple of days.

Thanks,
Jeykumar S.

Quoted 3 lines

-Doug

Signature · 5 lines
Jeykumar S
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

A message that is not in the archive (sent off-list or not yet received): e9207da13c30cae15cd2aba77078d08b-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org

#6 lore raw

Hi,

Quoted 40 lines · Jeykumar Sankaran
Quoted 32 lines
Quoted 21 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +9 -0
+                       dsi1: dsi@ae96000 {
+                               compatible = "qcom,mdss-dsi-ctrl";
+                               reg = <0xae96000 0x400>;
+                               reg-names = "dsi_ctrl";
+
+                               interrupt-parent = <&mdss>;
+                               interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+
+                               clocks = <&dispcc

DISP_CC_MDSS_BYTE1_CLK>,
+ <&dispcc
DISP_CC_MDSS_BYTE1_INTF_CLK>,
+ <&dispcc
DISP_CC_MDSS_PCLK1_CLK>,
+ <&dispcc
DISP_CC_MDSS_ESC1_CLK>,
+ <&dispcc
DISP_CC_MDSS_AHB_CLK>,
+ <&dispcc
DISP_CC_MDSS_AXI_CLK>;

arch/arm64/boot/dts/qcom/sdm845.dtsi +1 -0
+                               clock-names = "byte",

arch/arm64/boot/dts/qcom/sdm845.dtsi line 1419

Quoted 10 lines
arch/arm64/boot/dts/qcom/sdm845.dtsi +10 -0
+                                             "byte_intf",
+                                             "pixel",
+                                             "core",
+                                             "iface",
+                                             "bus";
+
+                               phys = <&dsi1_phy>;
+                               phy-names = "dsi1";
+
+                               status = "disabled";
Quoted 8 lines

This "disabled" is causing me problems. I don't actually need "dsi1" but if I don't enable "dsi1" then my display doesn't come up. :( I ran out of time to debug but I wonder if this is this the standard thing where DRM needs to wait for all the components to probe until it can finish? If nobody on this list just knows I'll dig tomorrow and confirm that my memory isn't faulty and see what we've done about this in the past.

Quoted 5 lines

https://patchwork.kernel.org/patch/10467895/

Can you try out with this change (reviewed but not merged yet). It validates
the nodes before adding to the DSI list.

No, that doesn't fix it. I also don't see your printout.

OK, found the problem and posted a patch. See
<https://lkml.kernel.org/r/[email protected]>.
Please test and review if you are able.

Quoted 7 lines
Quoted 3 lines

One last note: it's pretty weird that you sent out only 1/3 and not 2/3 and 3/3. If you're not ready to send out MTP stuff yet then you should send out v6 as just a singleton patch.

Yes. I was trying to separate this one out as an independent change. Sandeep
is working on the comments on removing the pinctrl nodes and updated mtp nodes. He should be posting 2/3 and 3/3 in the next couple of days.

OK, good to know. Probably 2/3 and 3/3 will be squashed anyway since (as I suggested in the review) they should both be touching the MTP device tree file.

...in any case, you should send yours out as a singleton patch and then you don't have to guess how many patches might or might not be sent out later. Sandeep can send out his patch and say it depends on yours.

-Doug
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno