Re: [PATCH 2/2] arm64: dts: qcom: msm8953: add MDSS

From: Dmitry Baryshkov
Date: Tue Jun 14 2022 - 16:41:45 EST


On 11/06/2022 12:36, Luca Weiss wrote:
Hi Dmitry,

thanks for the feedback!

On Samstag, 11. Juni 2022 01:20:31 CEST Dmitry Baryshkov wrote:
On 11/06/2022 01:53, Luca Weiss wrote:
From: Vladimir Lypak <vladimir.lypak@xxxxxxxxx>

Add the MDSS, MDP and DSI nodes that are found on msm8953 SoC.

IOMMU is not added because support for it isn't yet upstream and MDSS
works fine without IOMMU on 8953.

Signed-off-by: Vladimir Lypak <vladimir.lypak@xxxxxxxxx>
Signed-off-by: Luca Weiss <luca@xxxxxxxxx>

Looks good, few minor nits below.

---

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

diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi
b/arch/arm64/boot/dts/qcom/msm8953.dtsi index ffc3ec2cd3bc..a2aca3d05899
100644
--- a/arch/arm64/boot/dts/qcom/msm8953.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi
@@ -726,6 +726,208 @@ tcsr_phy_clk_scheme_sel: syscon@193f044 {

reg = <0x193f044 0x4>;

};

+ mdss: mdss@1a00000 {
+ compatible = "qcom,mdss";
+
+ reg = <0x1a00000 0x1000>,
+ <0x1ab0000 0x1040>;
+ reg-names = "mdss_phys",
+ "vbif_phys";
+
+ power-domains = <&gcc MDSS_GDSC>;
+ interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ clocks = <&gcc GCC_MDSS_AHB_CLK>,
+ <&gcc GCC_MDSS_AXI_CLK>,
+ <&gcc GCC_MDSS_VSYNC_CLK>;

Please also add GCC_MDSS_MDP_CLK at the end of this array. It might be
required to read HW_REV register.

I don't think if I add that clock with the name "core" (or similar) that it
would be used by the driver:

It is not (now), but I have plans to use it. Please consider adding it now, so that we won't have to change it later.


https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/msm/msm_mdss.c#L274-L302

Regards
Luca


+ clock-names = "iface",
+ "bus",
+ "vsync";
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;

status = "disabled";

+
+ mdp: mdp@1a01000 {
+ compatible = "qcom,mdp5";
+ reg = <0x1a01000 0x89000>;
+ reg-names = "mdp_phys";
+
+ interrupt-parent = <&mdss>;
+ interrupts = <0>;
+
+ power-domains = <&gcc MDSS_GDSC>;
+
+ clocks = <&gcc GCC_MDSS_AHB_CLK>,
+ <&gcc GCC_MDSS_AXI_CLK>,
+ <&gcc GCC_MDSS_MDP_CLK>,
+ <&gcc GCC_MDSS_VSYNC_CLK>;
+ clock-names = "iface",
+ "bus",
+ "core",
+ "vsync";
+
+ // iommus = <&apps_iommu 0xc00 0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ mdp5_intf1_out: endpoint {
+ remote-endpoint = <&dsi0_in>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ mdp5_intf2_out: endpoint {
+ remote-endpoint = <&dsi1_in>;
+ };
+ };
+ };
+ };
+
+ dsi0: dsi@1a94000 {
+ compatible = "qcom,mdss-dsi-ctrl";
+ reg = <0x1a94000 0x400>;
+ reg-names = "dsi_ctrl";
+
+ interrupt-parent = <&mdss>;
+ interrupts = <4>;
+
+ assigned-clocks = <&gcc BYTE0_CLK_SRC>,
+ <&gcc PCLK0_CLK_SRC>;
+ assigned-clock-parents = <&dsi0_phy 0>,
+ <&dsi0_phy 1>;
+
+ clocks = <&gcc GCC_MDSS_MDP_CLK>,
+ <&gcc GCC_MDSS_AHB_CLK>,
+ <&gcc GCC_MDSS_AXI_CLK>,
+ <&gcc GCC_MDSS_BYTE0_CLK>,
+ <&gcc GCC_MDSS_PCLK0_CLK>,
+ <&gcc GCC_MDSS_ESC0_CLK>;
+ clock-names = "mdp_core",
+ "iface",
+ "bus",
+ "byte",
+ "pixel",
+ "core";
+
+ phys = <&dsi0_phy>;
+ phy-names = "dsi";
+
+ #address-cells = <1>;
+ #size-cells = <0>;

status = "disabled";

+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dsi0_in: endpoint {
+ remote-endpoint = <&mdp5_intf1_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dsi0_out: endpoint {
+ };
+ };
+ };
+ };
+
+ dsi0_phy: dsi-phy@1a94400 {
+ compatible = "qcom,dsi-phy-14nm-8953";
+ reg = <0x1a94400 0x100>,
+ <0x1a94500 0x300>,
+ <0x1a94800 0x188>;
+ reg-names = "dsi_phy",
+ "dsi_phy_lane",
+ "dsi_pll";
+
+ #clock-cells = <1>;
+ #phy-cells = <0>;

status = "disabled";

+
+ clocks = <&gcc GCC_MDSS_AHB_CLK>, <&xo_board>;
+ clock-names = "iface", "ref";
+ };
+
+ dsi1: dsi@1a96000 {
+ compatible = "qcom,mdss-dsi-ctrl";
+ reg = <0x1a96000 0x400>;
+ reg-names = "dsi_ctrl";
+
+ interrupt-parent = <&mdss>;
+ interrupts = <5>;
+
+ assigned-clocks = <&gcc BYTE1_CLK_SRC>,
+ <&gcc PCLK1_CLK_SRC>;
+ assigned-clock-parents = <&dsi1_phy 0>,
+ <&dsi1_phy 1>;
+
+ clocks = <&gcc GCC_MDSS_MDP_CLK>,
+ <&gcc GCC_MDSS_AHB_CLK>,
+ <&gcc GCC_MDSS_AXI_CLK>,
+ <&gcc GCC_MDSS_BYTE1_CLK>,
+ <&gcc GCC_MDSS_PCLK1_CLK>,
+ <&gcc GCC_MDSS_ESC1_CLK>;
+ clock-names = "mdp_core",
+ "iface",
+ "bus",
+ "byte",
+ "pixel",
+ "core";
+
+ phys = <&dsi1_phy>;
+ phy-names = "dsi";
+
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dsi1_in: endpoint {
+ remote-endpoint = <&mdp5_intf2_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dsi1_out: endpoint {
+ };
+ };
+ };
+ };
+
+ dsi1_phy: dsi-phy@1a96400 {
+ compatible = "qcom,dsi-phy-14nm-8953";
+ reg = <0x1a96400 0x100>,
+ <0x1a96500 0x300>,
+ <0x1a96800 0x188>;
+ reg-names = "dsi_phy",
+ "dsi_phy_lane",
+ "dsi_pll";
+
+ #clock-cells = <1>;
+ #phy-cells = <0>;
+
+ clocks = <&gcc GCC_MDSS_AHB_CLK>, <&xo_board>;
+ clock-names = "iface", "ref";
+
+ status = "disabled";
+ };
+ };
+

spmi_bus: spmi@200f000 {

compatible = "qcom,spmi-pmic-arb";
reg = <0x200f000 0x1000>,






--
With best wishes
Dmitry