[PATCH v3] Add display nodes to SDM845 dtsi

[PATCH v3] Add display nodes to SDM845 dtsi

[Jeykumar Sankaran]

Reviving the patch posted by Sean initially.

This patch set adds MDSS and DSI nodes to SDM845 dtsi to enable display. The
patches are tested on SDM845 MTP platform using the kernel based on [1].

Part of the dependent drivers are already posted on list. Rest of the 
dependencies are met using using downstream version of the driver(s)
which are yet to make it to the list. 

References to the driver patches used for testing:

display controller: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/qcom/sdm845.dtsi?id=40019e8452fe76867bdb2e7
WLED: https://patchwork.kernel.org/project/linux-arm-msm/list/?series=11023&archive=both&state=*
Panel: https://patchwork.freedesktop.org/series/50657/
iommu: https://patchwork.kernel.org/patch/10534999/

[1] https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/log/?h=integration-linux-qcomlt

Thanks and Regards,
Jeykumar S.


Jeykumar Sankaran (1):
  arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 191 +++++++++++++++++++++++++++++++++++
 1 file changed, 191 insertions(+)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

[Jeykumar Sankaran]

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

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 191 +++++++++++++++++++++++++++++++++++
 1 file changed, 191 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0c9a2aa..dd612ac 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -978,6 +978,197 @@
 			#thermal-sensor-cells = <1>;
 		};
 
+		mdss: mdss@ae00000 {
+			compatible = "qcom,sdm845-mdss";
+			reg = <0xae00000 0x1000>;
+			reg-names = "mdss";
+
+			power-domains = <&dispcc 0>;
+
+			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>;
+
+			#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>;
+
+				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_clk",
+					      "byte_intf_clk",
+					      "pixel_clk",
+					      "core_clk",
+					      "iface_clk",
+					      "bus_clk";
+
+				phys = <&dsi0_phy>;
+				phy-names = "dsi-phy";
+
+				#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>,
+				      <0xae94a00 0x1e0>,
+				      <0xae94600 0x280>;
+				reg-names = "dsi_phy",
+					    "dsi_pll",
+					    "dsi_phy_lane";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
+				clock-names = "iface_clk";
+			};
+
+			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_clk",
+					      "byte_intf_clk",
+					      "pixel_clk",
+					      "core_clk",
+					      "iface_clk",
+					      "bus_clk";
+
+				phys = <&dsi1_phy>;
+				phy-names = "dsi-phy";
+
+				#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>,
+				      <0xae96a00 0x10e>,
+				      <0xae96600 0x280>;
+				reg-names = "dsi_phy",
+					    "dsi_pll",
+					    "dsi_phy_lane";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
+				clock-names = "iface_clk";
+			};
+		};
+
 		spmi_bus: spmi@c440000 {
 			compatible = "qcom,spmi-pmic-arb";
 			reg = <0xc440000 0x1100>,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

[Doug Anderson]

Hi,

On Thu, Nov 1, 2018 at 7:25 PM Jeykumar Sankaran <jsanka@codeaurora.org> wrote:
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -978,6 +978,197 @@
>                         #thermal-sensor-cells = <1>;
>                 };

Note that your patch doesn't cleanly apply to Andy's tree.  Context
here shows that you have it between the thermal sensor and the SPMI
bus but the "qcom,sdm845-aoss-cc" node is there in.  It's easy enough
to resolve, but it's usually considered nice to post patches that the
maintainers can apply.

When resolving this myself, I realized that you have the node in the
wrong place anyway.  Nodes with a unit address should be sorted by
unit address.  As you unit address is ae00000 you should be right
above 'dispcc: clock-controller@af00000'.


> +               mdss: mdss@ae00000 {
> +                       compatible = "qcom,sdm845-mdss";
> +                       reg = <0xae00000 0x1000>;
> +                       reg-names = "mdss";
> +
> +                       power-domains = <&dispcc 0>;
> +
> +                       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>;
> +
> +                       #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>;
> +
> +                               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_clk",
> +                                             "byte_intf_clk",
> +                                             "pixel_clk",
> +                                             "core_clk",
> +                                             "iface_clk",
> +                                             "bus_clk";

msm_dsi ae94000.dsi: Using legacy clk name binding.  Use "iface"
instead of "iface_clk"
msm_dsi ae94000.dsi: Using legacy clk name binding.  Use "bus" instead
of "bus_clk"
msm_dsi ae94000.dsi: Using legacy clk name binding.  Use "byte"
instead of "byte_clk"
msm_dsi ae94000.dsi: Using legacy clk name binding.  Use "pixel"
instead of "pixel_clk"
msm_dsi ae94000.dsi: Using legacy clk name binding.  Use "core"
instead of "core_clk"
msm_dsi ae94000.dsi: Using legacy clk name binding.  Use "byte_intf"
instead of "byte_intf_clk"


> +
> +                               phys = <&dsi0_phy>;
> +                               phy-names = "dsi-phy";
> +
> +                               #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>,
> +                                     <0xae94a00 0x1e0>,
> +                                     <0xae94600 0x280>;
> +                               reg-names = "dsi_phy",
> +                                           "dsi_pll",
> +                                           "dsi_phy_lane";
> +
> +                               #clock-cells = <1>;
> +                               #phy-cells = <0>;
> +
> +                               clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
> +                               clock-names = "iface_clk";

msm_dsi_phy ae94400.dsi-phy: Using legacy clk name binding.  Use
"iface" instead of "iface_clk"

> +                       };
> +
> +                       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_clk",
> +                                             "byte_intf_clk",
> +                                             "pixel_clk",
> +                                             "core_clk",
> +                                             "iface_clk",
> +                                             "bus_clk";

More places to remove "_clk"

> +
> +                               phys = <&dsi1_phy>;
> +                               phy-names = "dsi-phy";
> +
> +                               #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>,
> +                                     <0xae96a00 0x10e>,
> +                                     <0xae96600 0x280>;
> +                               reg-names = "dsi_phy",
> +                                           "dsi_pll",
> +                                           "dsi_phy_lane";
> +
> +                               #clock-cells = <1>;
> +                               #phy-cells = <0>;
> +
> +                               clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
> +                               clock-names = "iface_clk";

...and another "_clk" to remove.


-Doug
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

[Bjorn Andersson]

On Thu 01 Nov 19:25 PDT 2018, Jeykumar Sankaran wrote:

> 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.
> 

Happy to see support reaching the level where we can introduce this in
the dtsi!

But, not all devices has a display and not all devices with display has
DSI, so I would suggest that you mark these status = "disabled" and
enable them in the board dts.

Regards,
Bjorn

> 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
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 191 +++++++++++++++++++++++++++++++++++
>  1 file changed, 191 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0c9a2aa..dd612ac 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -978,6 +978,197 @@
>  			#thermal-sensor-cells = <1>;
>  		};
>  
> +		mdss: mdss@ae00000 {
> +			compatible = "qcom,sdm845-mdss";
> +			reg = <0xae00000 0x1000>;
> +			reg-names = "mdss";
> +
> +			power-domains = <&dispcc 0>;
> +
> +			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>;
> +
> +			#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>;
> +
> +				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_clk",
> +					      "byte_intf_clk",
> +					      "pixel_clk",
> +					      "core_clk",
> +					      "iface_clk",
> +					      "bus_clk";
> +
> +				phys = <&dsi0_phy>;
> +				phy-names = "dsi-phy";
> +
> +				#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>,
> +				      <0xae94a00 0x1e0>,
> +				      <0xae94600 0x280>;
> +				reg-names = "dsi_phy",
> +					    "dsi_pll",
> +					    "dsi_phy_lane";
> +
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +
> +				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
> +				clock-names = "iface_clk";
> +			};
> +
> +			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_clk",
> +					      "byte_intf_clk",
> +					      "pixel_clk",
> +					      "core_clk",
> +					      "iface_clk",
> +					      "bus_clk";
> +
> +				phys = <&dsi1_phy>;
> +				phy-names = "dsi-phy";
> +
> +				#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>,
> +				      <0xae96a00 0x10e>,
> +				      <0xae96600 0x280>;
> +				reg-names = "dsi_phy",
> +					    "dsi_pll",
> +					    "dsi_phy_lane";
> +
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +
> +				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
> +				clock-names = "iface_clk";
> +			};
> +		};
> +
>  		spmi_bus: spmi@c440000 {
>  			compatible = "qcom,spmi-pmic-arb";
>  			reg = <0xc440000 0x1100>,
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

[Jordan Crouse]

On Thu, Nov 01, 2018 at 07:25:23PM -0700, Jeykumar Sankaran wrote:
> 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
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 191 +++++++++++++++++++++++++++++++++++
>  1 file changed, 191 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0c9a2aa..dd612ac 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -978,6 +978,197 @@
>  			#thermal-sensor-cells = <1>;
>  		};
>  
> +		mdss: mdss@ae00000 {
> +			compatible = "qcom,sdm845-mdss";
> +			reg = <0xae00000 0x1000>;
> +			reg-names = "mdss";
> +
> +			power-domains = <&dispcc 0>;

Would the powers-that-be in the power domain prefer this to have a symbolic
value? I see MDSS_GDSC in the bindings header.

> +
> +			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>;
> +
> +			#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>;
> +
> +				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_clk",
> +					      "byte_intf_clk",
> +					      "pixel_clk",
> +					      "core_clk",
> +					      "iface_clk",
> +					      "bus_clk";

We will need to remove  the _clk extension and fix the code if needed.

> +
> +				phys = <&dsi0_phy>;
> +				phy-names = "dsi-phy";

This too, I reckon.

> +				#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>,
> +				      <0xae94a00 0x1e0>,
> +				      <0xae94600 0x280>;

I think it would be better if these were in numerical order.

> +				reg-names = "dsi_phy",
> +					    "dsi_pll",
> +					    "dsi_phy_lane";
> +
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +
> +				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
> +				clock-names = "iface_clk";

Remove _clk extension.

> +			};
> +
> +			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_clk",
> +					      "byte_intf_clk",
> +					      "pixel_clk",
> +					      "core_clk",
> +					      "iface_clk",
> +					      "bus_clk";

And here.

> +
> +				phys = <&dsi1_phy>;
> +				phy-names = "dsi-phy";

And here.

> +
> +				#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>,
> +				      <0xae96a00 0x10e>,
> +				      <0xae96600 0x280>;

Numerical order would be preferred.

> +				reg-names = "dsi_phy",
> +					    "dsi_pll",
> +					    "dsi_phy_lane";
> +
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +
> +				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
> +				clock-names = "iface_clk";

And one more extension.

> +			};
> +		};
> +
>  		spmi_bus: spmi@c440000 {
>  			compatible = "qcom,spmi-pmic-arb";
>  			reg = <0xc440000 0x1100>,

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

[Doug Anderson]

Hi,

On Wed, Nov 28, 2018 at 3:44 PM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>
> On Thu, Nov 01, 2018 at 07:25:23PM -0700, Jeykumar Sankaran wrote:
> > 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
> >
> > Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 191 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 191 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 0c9a2aa..dd612ac 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -978,6 +978,197 @@
> >                       #thermal-sensor-cells = <1>;
> >               };
> >
> > +             mdss: mdss@ae00000 {
> > +                     compatible = "qcom,sdm845-mdss";
> > +                     reg = <0xae00000 0x1000>;
> > +                     reg-names = "mdss";
> > +
> > +                     power-domains = <&dispcc 0>;
>
> Would the powers-that-be in the power domain prefer this to have a symbolic
> value? I see MDSS_GDSC in the bindings header.

You're one patch version behind.  Can you look at:

https://patchwork.kernel.org/patch/10666253/ AKA
https://lkml.kernel.org/r/1541197576-19730-2-git-send-email-jsanka@codeaurora.org

...that addresses the _clk issues and I commented about the #define too.


> > +                             phys = <&dsi0_phy>;
> > +                             phy-names = "dsi-phy";
>
> This too, I reckon.

I didn't notice this one, so it's still "dsi-phy" in v4.


> > +                     dsi0_phy: dsi-phy@ae94400 {
> > +                             compatible = "qcom,dsi-phy-10nm";
> > +                             reg = <0xae94400 0x200>,
> > +                                   <0xae94a00 0x1e0>,
> > +                                   <0xae94600 0x280>;
>
> I think it would be better if these were in numerical order.

Still broken in v4.


Also Bjorn's thought about adding a 'status = "disabled"' also makes sense.

So I guess it's time for a v5 Jeykumar.


-Doug
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno