linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280
@ 2021-08-18 10:27 Krishna Manikandan
  2021-08-18 10:27 ` [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes Krishna Manikandan
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Krishna Manikandan @ 2021-08-18 10:27 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, linux-kernel
  Cc: Krishna Manikandan, kalyan_t, sbillaka, abhinavk, robdclark,
	swboyd, bjorn.andersson, khsieh, rajeevny, freedreno, dri-devel,
	robh+dt

MSM Mobile Display Subsystem (MDSS) encapsulates sub-blocks
like DPU display controller, DSI, EDP etc. Add required DPU
device tree bindings for SC7280.

Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
---
 .../bindings/display/msm/dpu-sc7280.yaml           | 228 +++++++++++++++++++++
 1 file changed, 228 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml

diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml
new file mode 100644
index 0000000..3d256c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml
@@ -0,0 +1,228 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/msm/dpu-sc7280.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Display DPU dt properties for SC7280 target
+
+maintainers:
+  - Krishna Manikandan <mkrishn@codeaurora.org>
+
+description: |
+  Device tree bindings for MSM Mobile Display Subsystem(MDSS) that encapsulates
+  sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree
+  bindings of MDSS and DPU are mentioned for SC7280 target.
+
+properties:
+  compatible:
+    items:
+      - const: qcom,sc7280-mdss
+
+  reg:
+    maxItems: 1
+
+  reg-names:
+    const: mdss
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Display AHB clock from gcc
+      - description: Display AHB clock from dispcc
+      - description: Display core clock
+
+  clock-names:
+    items:
+      - const: iface
+      - const: ahb
+      - const: core
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#address-cells": true
+
+  "#size-cells": true
+
+  "#interrupt-cells":
+    const: 1
+
+  iommus:
+    items:
+      - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0
+
+  ranges: true
+
+  interconnects:
+    items:
+      - description: Interconnect path specifying the port ids for data bus
+
+  interconnect-names:
+    const: mdp0-mem
+
+patternProperties:
+  "^display-controller@[0-9a-f]+$":
+    type: object
+    description: Node containing the properties of DPU.
+
+    properties:
+      compatible:
+        items:
+          - const: qcom,sc7280-dpu
+
+      reg:
+        items:
+          - description: Address offset and size for mdp register set
+          - description: Address offset and size for vbif register set
+
+      reg-names:
+        items:
+          - const: mdp
+          - const: vbif
+
+      clocks:
+        items:
+          - description: Display hf axi clock
+          - description: Display sf axi clock
+          - description: Display ahb clock
+          - description: Display lut clock
+          - description: Display core clock
+          - description: Display vsync clock
+
+      clock-names:
+        items:
+          - const: bus
+          - const: nrt_bus
+          - const: iface
+          - const: lut
+          - const: core
+          - const: vsync
+
+      interrupts:
+        maxItems: 1
+
+      power-domains:
+        maxItems: 1
+
+      operating-points-v2: true
+
+      ports:
+        $ref: /schemas/graph.yaml#/properties/ports
+        description: |
+          Contains the list of output ports from DPU device. These ports
+          connect to interfaces that are external to the DPU hardware,
+          such as DSI, DP etc. Each output port contains an endpoint that
+          describes how it is connected to an external interface.
+
+        properties:
+          port@0:
+            $ref: /schemas/graph.yaml#/properties/port
+            description: DPU_INTF1 (DSI)
+
+          port@1:
+            $ref: /schemas/graph.yaml#/properties/port
+            description: DPU_INTF5 (EDP)
+
+        required:
+          - port@0
+
+    required:
+      - compatible
+      - reg
+      - reg-names
+      - clocks
+      - interrupts
+      - power-domains
+      - operating-points-v2
+      - ports
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - power-domains
+  - clocks
+  - interrupts
+  - interrupt-controller
+  - iommus
+  - ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,dispcc-sc7280.h>
+    #include <dt-bindings/clock/qcom,gcc-sc7280.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interconnect/qcom,sc7280.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+
+    display-subsystem@ae00000 {
+         #address-cells = <1>;
+         #size-cells = <1>;
+         compatible = "qcom,sc7280-mdss";
+         reg = <0xae00000 0x1000>;
+         reg-names = "mdss";
+         power-domains = <&dispcc DISP_CC_MDSS_CORE_GDSC>;
+         clocks = <&gcc GCC_DISP_AHB_CLK>,
+                  <&dispcc DISP_CC_MDSS_AHB_CLK>,
+                  <&dispcc DISP_CC_MDSS_MDP_CLK>;
+         clock-names = "iface", "ahb", "core";
+
+         interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+         interrupt-controller;
+         #interrupt-cells = <1>;
+
+         interconnects = <&mmss_noc MASTER_MDP0 &mc_virt SLAVE_EBI1>;
+         interconnect-names = "mdp0-mem";
+
+         iommus = <&apps_smmu 0x900 0x402>;
+         ranges;
+
+         display-controller@ae01000 {
+                   compatible = "qcom,sc7280-dpu";
+                   reg = <0x0ae01000 0x8f000>,
+                         <0x0aeb0000 0x2008>;
+
+                   reg-names = "mdp", "vbif";
+
+                   clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
+                            <&gcc GCC_DISP_SF_AXI_CLK>,
+                            <&dispcc DISP_CC_MDSS_AHB_CLK>,
+                            <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
+                            <&dispcc DISP_CC_MDSS_MDP_CLK>,
+                            <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+                   clock-names = "bus", "nrt_bus", "iface", "lut", "core",
+                                 "vsync";
+
+                   interrupt-parent = <&mdss>;
+                   interrupts = <0>;
+                   power-domains = <&rpmhpd SC7280_CX>;
+                   operating-points-v2 = <&mdp_opp_table>;
+
+                   ports {
+                           #address-cells = <1>;
+                           #size-cells = <0>;
+
+                           port@0 {
+                                   reg = <0>;
+                                   dpu_intf1_out: endpoint {
+                                                  remote-endpoint = <&dsi0_in>;
+                                   };
+                           };
+
+                           port@1 {
+                                   reg = <1>;
+                                   dpu_intf5_out: endpoint {
+                                                  remote-endpoint = <&edp_in>;
+                                   };
+                           };
+                   };
+         };
+    };
+...
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes
  2021-08-18 10:27 [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Krishna Manikandan
@ 2021-08-18 10:27 ` Krishna Manikandan
  2021-08-18 19:57   ` Stephen Boyd
  2021-08-18 10:27 ` [PATCH v1 3/4] arm64: dts: qcom: sc7280: Add DSI display nodes Krishna Manikandan
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Krishna Manikandan @ 2021-08-18 10:27 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, linux-kernel
  Cc: Krishna Manikandan, kalyan_t, sbillaka, abhinavk, robdclark,
	swboyd, bjorn.andersson, khsieh, rajeevny, freedreno, dri-devel,
	robh+dt

Add mdss and mdp DT nodes for sc7280.

Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 85 ++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 53a21d0..fd7ff1c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -5,6 +5,7 @@
  * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
  */
 
+#include <dt-bindings/clock/qcom,dispcc-sc7280.h>
 #include <dt-bindings/clock/qcom,gcc-sc7280.h>
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/interconnect/qcom,sc7280.h>
@@ -1424,6 +1425,90 @@
 			#power-domain-cells = <1>;
 		};
 
+		mdss: mdss@ae00000 {
+			compatible = "qcom,sc7280-mdss";
+			reg = <0 0x0ae00000 0 0x1000>;
+			reg-names = "mdss";
+
+			power-domains = <&dispcc DISP_CC_MDSS_CORE_GDSC>;
+
+			clocks = <&gcc GCC_DISP_AHB_CLK>,
+				 <&dispcc DISP_CC_MDSS_AHB_CLK>,
+				<&dispcc DISP_CC_MDSS_MDP_CLK>;
+			clock-names = "iface", "ahb", "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>;
+
+			interconnects = <&mmss_noc MASTER_MDP0 0 &mc_virt SLAVE_EBI1 0>;
+			interconnect-names = "mdp0-mem";
+
+			iommus = <&apps_smmu 0x900 0x402>;
+
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+
+			status = "disabled";
+
+			mdp: mdp@ae01000 {
+				compatible = "qcom,sc7280-dpu";
+				reg = <0 0x0ae01000 0 0x8f030>,
+					<0 0x0aeb0000 0 0x2008>;
+				reg-names = "mdp", "vbif";
+
+				clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
+					<&gcc GCC_DISP_SF_AXI_CLK>,
+					<&dispcc DISP_CC_MDSS_AHB_CLK>,
+					<&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
+					<&dispcc DISP_CC_MDSS_MDP_CLK>,
+					<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+				clock-names = "bus", "nrt_bus", "iface", "lut", "core",
+					      "vsync";
+				assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>,
+						<&dispcc DISP_CC_MDSS_VSYNC_CLK>,
+						<&dispcc DISP_CC_MDSS_AHB_CLK>;
+				assigned-clock-rates = <300000000>,
+							<19200000>,
+							<19200000>;
+				operating-points-v2 = <&mdp_opp_table>;
+				power-domains = <&rpmhpd SC7280_CX>;
+
+				interrupt-parent = <&mdss>;
+				interrupts = <0>;
+
+				status = "disabled";
+
+				mdp_opp_table: mdp-opp-table {
+					compatible = "operating-points-v2";
+
+					opp-200000000 {
+						opp-hz = /bits/ 64 <200000000>;
+						required-opps = <&rpmhpd_opp_low_svs>;
+					};
+
+					opp-300000000 {
+						opp-hz = /bits/ 64 <300000000>;
+						required-opps = <&rpmhpd_opp_svs>;
+					};
+
+					opp-380000000 {
+						opp-hz = /bits/ 64 <380000000>;
+						required-opps = <&rpmhpd_opp_svs_l1>;
+					};
+
+					opp-506666667 {
+						opp-hz = /bits/ 64 <506666667>;
+						required-opps = <&rpmhpd_opp_nom>;
+					};
+				};
+			};
+		};
+
 		pdc: interrupt-controller@b220000 {
 			compatible = "qcom,sc7280-pdc", "qcom,pdc";
 			reg = <0 0x0b220000 0 0x30000>;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 3/4] arm64: dts: qcom: sc7280: Add DSI display nodes
  2021-08-18 10:27 [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Krishna Manikandan
  2021-08-18 10:27 ` [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes Krishna Manikandan
@ 2021-08-18 10:27 ` Krishna Manikandan
  2021-08-18 16:35   ` Matthias Kaehlcke
  2021-08-18 19:59   ` Stephen Boyd
  2021-08-18 10:27 ` [PATCH v1 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes Krishna Manikandan
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Krishna Manikandan @ 2021-08-18 10:27 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, linux-kernel
  Cc: Rajeev Nandan, kalyan_t, sbillaka, abhinavk, robdclark, swboyd,
	bjorn.andersson, khsieh, freedreno, dri-devel, robh+dt

From: Rajeev Nandan <rajeevny@codeaurora.org>

Add DSI controller and PHY nodes for sc7280.

Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 101 +++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index fd7ff1c..aadf55d 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1483,6 +1483,18 @@
 
 				status = "disabled";
 
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						dpu_intf1_out: endpoint {
+							remote-endpoint = <&dsi0_in>;
+						};
+					};
+				};
+
 				mdp_opp_table: mdp-opp-table {
 					compatible = "operating-points-v2";
 
@@ -1507,6 +1519,95 @@
 					};
 				};
 			};
+
+			dsi0: dsi@ae94000 {
+				compatible = "qcom,mdss-dsi-ctrl";
+				reg = <0 0x0ae94000 0 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>,
+					 <&gcc GCC_DISP_HF_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+
+				operating-points-v2 = <&dsi_opp_table>;
+				power-domains = <&rpmhpd SC7280_CX>;
+
+				phys = <&dsi_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 = <&dpu_intf1_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+						dsi0_out: endpoint {
+						};
+					};
+				};
+
+				dsi_opp_table: dsi-opp-table {
+					compatible = "operating-points-v2";
+
+					opp-187500000 {
+						opp-hz = /bits/ 64 <187500000>;
+						required-opps = <&rpmhpd_opp_low_svs>;
+					};
+
+					opp-300000000 {
+						opp-hz = /bits/ 64 <300000000>;
+						required-opps = <&rpmhpd_opp_svs>;
+					};
+
+					opp-358000000 {
+						opp-hz = /bits/ 64 <358000000>;
+						required-opps = <&rpmhpd_opp_svs_l1>;
+					};
+				};
+			};
+
+			dsi_phy: dsi-phy@ae94400 {
+				compatible = "qcom,sc7280-dsi-phy-7nm";
+				reg = <0 0x0ae94400 0 0x200>,
+				      <0 0x0ae94600 0 0x280>,
+				      <0 0x0ae94900 0 0x280>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface", "ref";
+
+				status = "disabled";
+			};
 		};
 
 		pdc: interrupt-controller@b220000 {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v1 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes
  2021-08-18 10:27 [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Krishna Manikandan
  2021-08-18 10:27 ` [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes Krishna Manikandan
  2021-08-18 10:27 ` [PATCH v1 3/4] arm64: dts: qcom: sc7280: Add DSI display nodes Krishna Manikandan
@ 2021-08-18 10:27 ` Krishna Manikandan
  2021-08-18 16:14   ` Matthias Kaehlcke
  2021-08-18 20:03   ` Stephen Boyd
  2021-08-18 16:14 ` [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Rob Herring
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: Krishna Manikandan @ 2021-08-18 10:27 UTC (permalink / raw)
  To: linux-arm-msm, devicetree, linux-kernel
  Cc: Sankeerth Billakanti, kalyan_t, abhinavk, robdclark, swboyd,
	bjorn.andersson, khsieh, rajeevny, freedreno, dri-devel, robh+dt

From: Sankeerth Billakanti <sbillaka@codeaurora.org>

Add edp controller and phy DT nodes for sc7280.

Signed-off-by: Sankeerth Billakanti <sbillaka@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 127 ++++++++++++++++++++++++++++++++++-
 1 file changed, 126 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index aadf55d..5be318e 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1412,7 +1412,7 @@
 			reg = <0 0xaf00000 0 0x20000>;
 			clocks = <&rpmhcc RPMH_CXO_CLK>,
 				 <&gcc GCC_DISP_GPLL0_CLK_SRC>,
-				 <0>, <0>, <0>, <0>, <0>, <0>;
+				 <0>, <0>, <0>, <0>, <&edp_phy 0>, <&edp_phy 1>;
 			clock-names = "bi_tcxo", "gcc_disp_gpll0_clk",
 				      "dsi0_phy_pll_out_byteclk",
 				      "dsi0_phy_pll_out_dsiclk",
@@ -1493,6 +1493,12 @@
 							remote-endpoint = <&dsi0_in>;
 						};
 					};
+					port@1 {
+						reg = <1>;
+						dpu_intf5_out: endpoint {
+							remote-endpoint = <&edp_in>;
+						};
+					};
 				};
 
 				mdp_opp_table: mdp-opp-table {
@@ -1608,6 +1614,101 @@
 
 				status = "disabled";
 			};
+
+			msm_edp: edp@aea0000 {
+				status = "disabled";
+				compatible = "qcom,sc7280-edp";
+				reg = <0 0xaea0000 0 0x200>,
+				      <0 0xaea0200 0 0x200>,
+				      <0 0xaea0400 0 0xc00>,
+				      <0 0xaea1000 0 0x400>;
+
+				interrupt-parent = <&mdss>;
+				interrupts = <14 IRQ_TYPE_NONE>;
+
+				clocks = <&rpmhcc RPMH_CXO_CLK>,
+					 <&gcc GCC_EDP_CLKREF_EN>,
+					 <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&dispcc DISP_CC_MDSS_EDP_AUX_CLK>,
+					 <&dispcc DISP_CC_MDSS_EDP_LINK_CLK>,
+					 <&dispcc DISP_CC_MDSS_EDP_LINK_INTF_CLK>,
+					 <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK>;
+				clock-names = "core_xo", "core_ref",
+					      "core_iface", "core_aux", "ctrl_link",
+					      "ctrl_link_iface", "stream_pixel";
+				#clock-cells = <1>;
+				assigned-clocks = <&dispcc DISP_CC_MDSS_EDP_LINK_CLK_SRC>,
+						  <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK_SRC>;
+				assigned-clock-parents = <&edp_phy 0>, <&edp_phy 1>;
+
+				phys = <&edp_phy>;
+				phy-names = "dp";
+
+				vdda-1p2-supply = <&vreg_l6b_1p2>;
+				vdda-0p9-supply = <&vreg_l10c_0p8>;
+				operating-points-v2 = <&edp_opp_table>;
+				power-domains = <&rpmhpd SC7280_CX>;
+
+				pinctrl-names = "default";
+				pinctrl-0 = <&edp_hot_plug_det>, <&edp_panel_power_on>;
+
+				panel-bklt-gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>;
+				panel-pwm-gpio = <&pm8350c_gpios 8 GPIO_ACTIVE_HIGH>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					port@0 {
+						reg = <0>;
+						edp_in: endpoint {
+							remote-endpoint = <&dpu_intf5_out>;
+						};
+					};
+				};
+
+				edp_opp_table: edp-opp-table {
+					compatible = "operating-points-v2";
+
+					opp-160000000 {
+						opp-hz = /bits/ 64 <160000000>;
+						required-opps = <&rpmhpd_opp_low_svs>;
+					};
+
+					opp-270000000 {
+						opp-hz = /bits/ 64 <270000000>;
+						required-opps = <&rpmhpd_opp_svs>;
+					};
+
+					opp-540000000 {
+						opp-hz = /bits/ 64 <540000000>;
+						required-opps = <&rpmhpd_opp_nom>;
+					};
+
+					opp-810000000 {
+						opp-hz = /bits/ 64 <810000000>;
+						required-opps = <&rpmhpd_opp_nom>;
+					};
+				};
+			};
+
+			edp_phy: phy@aec2000 {
+				status = "disabled";
+				compatible = "qcom,sc7280-edp-phy";
+				reg = <0 0xaec2a00 0 0x19c>,
+				      <0 0xaec2200 0 0xa0>,
+				      <0 0xaec2600 0 0xa0>,
+				      <0 0xaec2000 0 0x1c0>;
+
+				clocks = <&rpmhcc RPMH_CXO_CLK>,
+					 <&gcc GCC_EDP_CLKREF_EN>;
+				clock-names = "aux", "cfg_ahb";
+
+				vdda-pll-supply = <&vreg_l6b_1p2>;
+				vdda-phy-supply = <&vreg_l10c_0p8>;
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+			};
 		};
 
 		pdc: interrupt-controller@b220000 {
@@ -1704,6 +1805,30 @@
 				function = "qup13";
 			};
 
+			edp_hot_plug_det: edp-hot-plug-det {
+				pinmux {
+					pins = "gpio60";
+					function = "edp_hot";
+				};
+				pinconf {
+					pins = "gpio60";
+					bias-pull-down;
+					input-enable;
+				};
+			};
+
+			edp_panel_power_on: edp-panel-power-on {
+				pinmux {
+					pins = "gpio80";
+					function = "gpio";
+				};
+				pinconf {
+					pins = "gpio80";
+					bias-disable;
+					output-high;
+				};
+			};
+
 			sdc1_on: sdc1-on {
 				clk {
 					pins = "sdc1_clk";
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280
  2021-08-18 10:27 [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Krishna Manikandan
                   ` (2 preceding siblings ...)
  2021-08-18 10:27 ` [PATCH v1 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes Krishna Manikandan
@ 2021-08-18 16:14 ` Rob Herring
  2021-08-18 19:56 ` Stephen Boyd
  2021-08-18 20:07 ` Stephen Boyd
  5 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2021-08-18 16:14 UTC (permalink / raw)
  To: Krishna Manikandan
  Cc: rajeevny, freedreno, devicetree, bjorn.andersson, kalyan_t,
	robh+dt, abhinavk, dri-devel, linux-kernel, robdclark,
	linux-arm-msm, khsieh, swboyd, sbillaka

On Wed, 18 Aug 2021 15:57:01 +0530, Krishna Manikandan wrote:
> MSM Mobile Display Subsystem (MDSS) encapsulates sub-blocks
> like DPU display controller, DSI, EDP etc. Add required DPU
> device tree bindings for SC7280.
> 
> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
> ---
>  .../bindings/display/msm/dpu-sc7280.yaml           | 228 +++++++++++++++++++++
>  1 file changed, 228 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/display/msm/dpu-sc7280.example.dts:19:18: fatal error: dt-bindings/clock/qcom,dispcc-sc7280.h: No such file or directory
   19 |         #include <dt-bindings/clock/qcom,dispcc-sc7280.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/display/msm/dpu-sc7280.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1419: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1517976

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes
  2021-08-18 10:27 ` [PATCH v1 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes Krishna Manikandan
@ 2021-08-18 16:14   ` Matthias Kaehlcke
  2021-08-18 20:03   ` Stephen Boyd
  1 sibling, 0 replies; 17+ messages in thread
From: Matthias Kaehlcke @ 2021-08-18 16:14 UTC (permalink / raw)
  To: Krishna Manikandan
  Cc: linux-arm-msm, devicetree, linux-kernel, Sankeerth Billakanti,
	kalyan_t, abhinavk, robdclark, swboyd, bjorn.andersson, khsieh,
	rajeevny, freedreno, dri-devel, robh+dt, satya priya

On Wed, Aug 18, 2021 at 03:57:04PM +0530, Krishna Manikandan wrote:
> From: Sankeerth Billakanti <sbillaka@codeaurora.org>
> 
> Add edp controller and phy DT nodes for sc7280.
> 
> Signed-off-by: Sankeerth Billakanti <sbillaka@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 127 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 126 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index aadf55d..5be318e 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1412,7 +1412,7 @@
>  			reg = <0 0xaf00000 0 0x20000>;
>  			clocks = <&rpmhcc RPMH_CXO_CLK>,
>  				 <&gcc GCC_DISP_GPLL0_CLK_SRC>,
> -				 <0>, <0>, <0>, <0>, <0>, <0>;
> +				 <0>, <0>, <0>, <0>, <&edp_phy 0>, <&edp_phy 1>;
>  			clock-names = "bi_tcxo", "gcc_disp_gpll0_clk",
>  				      "dsi0_phy_pll_out_byteclk",
>  				      "dsi0_phy_pll_out_dsiclk",
> @@ -1493,6 +1493,12 @@
>  							remote-endpoint = <&dsi0_in>;
>  						};
>  					};
> +					port@1 {
> +						reg = <1>;
> +						dpu_intf5_out: endpoint {
> +							remote-endpoint = <&edp_in>;
> +						};
> +					};
>  				};
>  
>  				mdp_opp_table: mdp-opp-table {
> @@ -1608,6 +1614,101 @@
>  
>  				status = "disabled";
>  			};
> +
> +			msm_edp: edp@aea0000 {
> +				status = "disabled";
> +				compatible = "qcom,sc7280-edp";
> +				reg = <0 0xaea0000 0 0x200>,
> +				      <0 0xaea0200 0 0x200>,
> +				      <0 0xaea0400 0 0xc00>,
> +				      <0 0xaea1000 0 0x400>;
> +
> +				interrupt-parent = <&mdss>;
> +				interrupts = <14 IRQ_TYPE_NONE>;
> +
> +				clocks = <&rpmhcc RPMH_CXO_CLK>,
> +					 <&gcc GCC_EDP_CLKREF_EN>,
> +					 <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +					 <&dispcc DISP_CC_MDSS_EDP_AUX_CLK>,
> +					 <&dispcc DISP_CC_MDSS_EDP_LINK_CLK>,
> +					 <&dispcc DISP_CC_MDSS_EDP_LINK_INTF_CLK>,
> +					 <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK>;
> +				clock-names = "core_xo", "core_ref",
> +					      "core_iface", "core_aux", "ctrl_link",
> +					      "ctrl_link_iface", "stream_pixel";
> +				#clock-cells = <1>;
> +				assigned-clocks = <&dispcc DISP_CC_MDSS_EDP_LINK_CLK_SRC>,
> +						  <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK_SRC>;
> +				assigned-clock-parents = <&edp_phy 0>, <&edp_phy 1>;
> +
> +				phys = <&edp_phy>;
> +				phy-names = "dp";
> +
> +				vdda-1p2-supply = <&vreg_l6b_1p2>;
> +				vdda-0p9-supply = <&vreg_l10c_0p8>;

These regulators are defined in the board .dts (sc7280-idp.dts), hence the SoC
.dtsi shouldn't depend on them. My impression is that pm7325.dtsi and pm8350c.dtsi
should include definitions for regulators that supply basic SoC blocks. If the
configuration can vary depending on the SoC there could be SoC specific includes
for each PMIC. If a board uses a different configuration it could overwrite the
PMIC .dtsi settings.

> +				operating-points-v2 = <&edp_opp_table>;
> +				power-domains = <&rpmhpd SC7280_CX>;
> +
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&edp_hot_plug_det>, <&edp_panel_power_on>;
> +
> +				panel-bklt-gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>;
> +				panel-pwm-gpio = <&pm8350c_gpios 8 GPIO_ACTIVE_HIGH>;

The pins are board specific, hence they shouldn't be configured in the .dtsi of
the SoC.

> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					port@0 {
> +						reg = <0>;
> +						edp_in: endpoint {
> +							remote-endpoint = <&dpu_intf5_out>;
> +						};
> +					};
> +				};
> +
> +				edp_opp_table: edp-opp-table {
> +					compatible = "operating-points-v2";
> +
> +					opp-160000000 {
> +						opp-hz = /bits/ 64 <160000000>;
> +						required-opps = <&rpmhpd_opp_low_svs>;
> +					};
> +
> +					opp-270000000 {
> +						opp-hz = /bits/ 64 <270000000>;
> +						required-opps = <&rpmhpd_opp_svs>;
> +					};
> +
> +					opp-540000000 {
> +						opp-hz = /bits/ 64 <540000000>;
> +						required-opps = <&rpmhpd_opp_nom>;
> +					};
> +
> +					opp-810000000 {
> +						opp-hz = /bits/ 64 <810000000>;
> +						required-opps = <&rpmhpd_opp_nom>;
> +					};
> +				};
> +			};
> +
> +			edp_phy: phy@aec2000 {
> +				status = "disabled";
> +				compatible = "qcom,sc7280-edp-phy";
> +				reg = <0 0xaec2a00 0 0x19c>,
> +				      <0 0xaec2200 0 0xa0>,
> +				      <0 0xaec2600 0 0xa0>,
> +				      <0 0xaec2000 0 0x1c0>;
> +
> +				clocks = <&rpmhcc RPMH_CXO_CLK>,
> +					 <&gcc GCC_EDP_CLKREF_EN>;
> +				clock-names = "aux", "cfg_ahb";
> +
> +				vdda-pll-supply = <&vreg_l6b_1p2>;
> +				vdda-phy-supply = <&vreg_l10c_0p8>;
> +
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +			};
>  		};
>  
>  		pdc: interrupt-controller@b220000 {
> @@ -1704,6 +1805,30 @@
>  				function = "qup13";
>  			};
>  
> +			edp_hot_plug_det: edp-hot-plug-det {
> +				pinmux {
> +					pins = "gpio60";
> +					function = "edp_hot";
> +				};
> +				pinconf {
> +					pins = "gpio60";
> +					bias-pull-down;
> +					input-enable;
> +				};

There should be no separate 'pinmux' and 'pinconf' nodes, see other entries
in this section.

> +			};
> +
> +			edp_panel_power_on: edp-panel-power-on {
> +				pinmux {
> +					pins = "gpio80";
> +					function = "gpio";
> +				};
> +				pinconf {
> +					pins = "gpio80";
> +					bias-disable;
> +					output-high;
> +				};
> +			};

IIUC this could be any GPIO and hence should be configured by the board file.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/4] arm64: dts: qcom: sc7280: Add DSI display nodes
  2021-08-18 10:27 ` [PATCH v1 3/4] arm64: dts: qcom: sc7280: Add DSI display nodes Krishna Manikandan
@ 2021-08-18 16:35   ` Matthias Kaehlcke
  2021-08-18 19:59   ` Stephen Boyd
  1 sibling, 0 replies; 17+ messages in thread
From: Matthias Kaehlcke @ 2021-08-18 16:35 UTC (permalink / raw)
  To: Krishna Manikandan
  Cc: linux-arm-msm, devicetree, linux-kernel, Rajeev Nandan, kalyan_t,
	sbillaka, abhinavk, robdclark, swboyd, bjorn.andersson, khsieh,
	freedreno, dri-devel, robh+dt

On Wed, Aug 18, 2021 at 03:57:03PM +0530, Krishna Manikandan wrote:
> From: Rajeev Nandan <rajeevny@codeaurora.org>
> 
> Add DSI controller and PHY nodes for sc7280.
> 
> Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>

You should sign off patches you send, even if you aren't the original author.

> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 101 +++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index fd7ff1c..aadf55d 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1483,6 +1483,18 @@
>  
>  				status = "disabled";
>  
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						dpu_intf1_out: endpoint {
> +							remote-endpoint = <&dsi0_in>;
> +						};
> +					};
> +				};
> +
>  				mdp_opp_table: mdp-opp-table {
>  					compatible = "operating-points-v2";
>  
> @@ -1507,6 +1519,95 @@
>  					};
>  				};
>  			};
> +
> +			dsi0: dsi@ae94000 {
> +				compatible = "qcom,mdss-dsi-ctrl";
> +				reg = <0 0x0ae94000 0 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>,
> +					 <&gcc GCC_DISP_HF_AXI_CLK>;
> +				clock-names = "byte",
> +					      "byte_intf",
> +					      "pixel",
> +					      "core",
> +					      "iface",
> +					      "bus";
> +
> +				operating-points-v2 = <&dsi_opp_table>;
> +				power-domains = <&rpmhpd SC7280_CX>;
> +
> +				phys = <&dsi_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 = <&dpu_intf1_out>;
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +						dsi0_out: endpoint {
> +						};
> +					};
> +				};
> +
> +				dsi_opp_table: dsi-opp-table {
> +					compatible = "operating-points-v2";
> +
> +					opp-187500000 {
> +						opp-hz = /bits/ 64 <187500000>;
> +						required-opps = <&rpmhpd_opp_low_svs>;
> +					};
> +
> +					opp-300000000 {
> +						opp-hz = /bits/ 64 <300000000>;
> +						required-opps = <&rpmhpd_opp_svs>;
> +					};
> +
> +					opp-358000000 {
> +						opp-hz = /bits/ 64 <358000000>;
> +						required-opps = <&rpmhpd_opp_svs_l1>;
> +					};
> +				};
> +			};
> +
> +			dsi_phy: dsi-phy@ae94400 {
> +				compatible = "qcom,sc7280-dsi-phy-7nm";
> +				reg = <0 0x0ae94400 0 0x200>,
> +				      <0 0x0ae94600 0 0x280>,
> +				      <0 0x0ae94900 0 0x280>;
> +				reg-names = "dsi_phy",
> +					    "dsi_phy_lane",
> +					    "dsi_pll";
> +
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +
> +				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +					 <&rpmhcc RPMH_CXO_CLK>;
> +				clock-names = "iface", "ref";
> +
> +				status = "disabled";
> +			};

I'm not an expect, but this looks sane to me and it's very similar to the
SC7180 config.

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280
  2021-08-18 10:27 [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Krishna Manikandan
                   ` (3 preceding siblings ...)
  2021-08-18 16:14 ` [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Rob Herring
@ 2021-08-18 19:56 ` Stephen Boyd
  2021-08-18 20:07 ` Stephen Boyd
  5 siblings, 0 replies; 17+ messages in thread
From: Stephen Boyd @ 2021-08-18 19:56 UTC (permalink / raw)
  To: Krishna Manikandan, devicetree, linux-arm-msm, linux-kernel
  Cc: kalyan_t, sbillaka, abhinavk, robdclark, bjorn.andersson, khsieh,
	rajeevny, freedreno, dri-devel, robh+dt

Quoting Krishna Manikandan (2021-08-18 03:27:01)
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml
> new file mode 100644
> index 0000000..3d256c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7280.yaml
> @@ -0,0 +1,228 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/msm/dpu-sc7280.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Display DPU dt properties for SC7280 target

Drop "target"?

> +
> +maintainers:
> +  - Krishna Manikandan <mkrishn@codeaurora.org>
> +
> +description: |
> +  Device tree bindings for MSM Mobile Display Subsystem(MDSS) that encapsulates

Space after Subsystem please.

> +  sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree
> +  bindings of MDSS and DPU are mentioned for SC7280 target.

Drop "target"?

> +
> +properties:
> +  compatible:
> +    items:

Will there be anymore? If not, drop items and only have const.

> +      - const: qcom,sc7280-mdss
> +
> +  reg:
> +    maxItems: 1
> +
> +  reg-names:
> +    const: mdss
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Display AHB clock from gcc
> +      - description: Display AHB clock from dispcc
> +      - description: Display core clock
> +
> +  clock-names:
> +    items:
> +      - const: iface
> +      - const: ahb
> +      - const: core
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  "#address-cells": true
> +
> +  "#size-cells": true
> +
> +  "#interrupt-cells":
> +    const: 1
> +
> +  iommus:
> +    items:
> +      - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0
> +
> +  ranges: true
> +
> +  interconnects:
> +    items:
> +      - description: Interconnect path specifying the port ids for data bus
> +
> +  interconnect-names:
> +    const: mdp0-mem
> +
> +patternProperties:
> +  "^display-controller@[0-9a-f]+$":
> +    type: object
> +    description: Node containing the properties of DPU.
> +
> +    properties:
> +      compatible:
> +        items:
> +          - const: qcom,sc7280-dpu

Will there be anymore? If not, drop items and only have const.

> +
> +      reg:
> +        items:
> +          - description: Address offset and size for mdp register set
> +          - description: Address offset and size for vbif register set
> +
> +      reg-names:
> +        items:
> +          - const: mdp
> +          - const: vbif
> +
> +      clocks:
> +        items:
> +          - description: Display hf axi clock
> +          - description: Display sf axi clock
> +          - description: Display ahb clock
> +          - description: Display lut clock
> +          - description: Display core clock
> +          - description: Display vsync clock
> +
> +      clock-names:
> +        items:
> +          - const: bus
> +          - const: nrt_bus
> +          - const: iface
> +          - const: lut
> +          - const: core
> +          - const: vsync
> +
> +      interrupts:
> +        maxItems: 1
> +
> +      power-domains:
> +        maxItems: 1
> +
> +      operating-points-v2: true
> +
> +      ports:
> +        $ref: /schemas/graph.yaml#/properties/ports
> +        description: |
> +          Contains the list of output ports from DPU device. These ports
> +          connect to interfaces that are external to the DPU hardware,
> +          such as DSI, DP etc. Each output port contains an endpoint that
> +          describes how it is connected to an external interface.
> +
> +        properties:
> +          port@0:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description: DPU_INTF1 (DSI)
> +
> +          port@1:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description: DPU_INTF5 (EDP)
> +
> +        required:
> +          - port@0
> +
> +    required:
> +      - compatible
> +      - reg
> +      - reg-names
> +      - clocks
> +      - interrupts
> +      - power-domains
> +      - operating-points-v2
> +      - ports
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - power-domains
> +  - clocks
> +  - interrupts
> +  - interrupt-controller
> +  - iommus
> +  - ranges
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,dispcc-sc7280.h>
> +    #include <dt-bindings/clock/qcom,gcc-sc7280.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interconnect/qcom,sc7280.h>
> +    #include <dt-bindings/power/qcom-rpmpd.h>
> +
> +    display-subsystem@ae00000 {

Maybe just 'subsystem' as that is generic enough.

> +         #address-cells = <1>;
> +         #size-cells = <1>;
> +         compatible = "qcom,sc7280-mdss";
> +         reg = <0xae00000 0x1000>;
> +         reg-names = "mdss";
> +         power-domains = <&dispcc DISP_CC_MDSS_CORE_GDSC>;
> +         clocks = <&gcc GCC_DISP_AHB_CLK>,
> +                  <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +                  <&dispcc DISP_CC_MDSS_MDP_CLK>;
> +         clock-names = "iface", "ahb", "core";

Can these names be one per line? It makes it easier to match to the
clocks property above.

> +
> +         interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> +         interrupt-controller;
> +         #interrupt-cells = <1>;
> +
> +         interconnects = <&mmss_noc MASTER_MDP0 &mc_virt SLAVE_EBI1>;
> +         interconnect-names = "mdp0-mem";
> +
> +         iommus = <&apps_smmu 0x900 0x402>;
> +         ranges;
> +
> +         display-controller@ae01000 {
> +                   compatible = "qcom,sc7280-dpu";
> +                   reg = <0x0ae01000 0x8f000>,
> +                         <0x0aeb0000 0x2008>;
> +
> +                   reg-names = "mdp", "vbif";
> +
> +                   clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> +                            <&gcc GCC_DISP_SF_AXI_CLK>,
> +                            <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +                            <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> +                            <&dispcc DISP_CC_MDSS_MDP_CLK>,
> +                            <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> +                   clock-names = "bus", "nrt_bus", "iface", "lut", "core",
> +                                 "vsync";

Can these names be one per line? It makes it easier to match to the
clocks property above.

> +
> +                   interrupt-parent = <&mdss>;
> +                   interrupts = <0>;
> +                   power-domains = <&rpmhpd SC7280_CX>;
> +                   operating-points-v2 = <&mdp_opp_table>;
> +
> +                   ports {
> +                           #address-cells = <1>;
> +                           #size-cells = <0>;
> +
> +                           port@0 {
> +                                   reg = <0>;
> +                                   dpu_intf1_out: endpoint {
> +                                                  remote-endpoint = <&dsi0_in>;

Tabbed one too many times.

> +                                   };
> +                           };
> +
> +                           port@1 {
> +                                   reg = <1>;
> +                                   dpu_intf5_out: endpoint {
> +                                                  remote-endpoint = <&edp_in>;

Tabbed one too many times.

> +                                   };
> +                           };
> +                   };
> +         };
> +    };

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes
  2021-08-18 10:27 ` [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes Krishna Manikandan
@ 2021-08-18 19:57   ` Stephen Boyd
  2021-09-30 11:56     ` mkrishn
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2021-08-18 19:57 UTC (permalink / raw)
  To: Krishna Manikandan, devicetree, linux-arm-msm, linux-kernel
  Cc: kalyan_t, sbillaka, abhinavk, robdclark, bjorn.andersson, khsieh,
	rajeevny, freedreno, dri-devel, robh+dt

Quoting Krishna Manikandan (2021-08-18 03:27:02)
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 53a21d0..fd7ff1c 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
>   */
>
> +#include <dt-bindings/clock/qcom,dispcc-sc7280.h>
>  #include <dt-bindings/clock/qcom,gcc-sc7280.h>
>  #include <dt-bindings/clock/qcom,rpmh.h>
>  #include <dt-bindings/interconnect/qcom,sc7280.h>
> @@ -1424,6 +1425,90 @@
>                         #power-domain-cells = <1>;
>                 };
>
> +               mdss: mdss@ae00000 {

subsystem@ae00000

> +                       compatible = "qcom,sc7280-mdss";
> +                       reg = <0 0x0ae00000 0 0x1000>;
> +                       reg-names = "mdss";
> +
> +                       power-domains = <&dispcc DISP_CC_MDSS_CORE_GDSC>;
> +
> +                       clocks = <&gcc GCC_DISP_AHB_CLK>,
> +                                <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +                               <&dispcc DISP_CC_MDSS_MDP_CLK>;
> +                       clock-names = "iface", "ahb", "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>;
> +
> +                       interconnects = <&mmss_noc MASTER_MDP0 0 &mc_virt SLAVE_EBI1 0>;
> +                       interconnect-names = "mdp0-mem";
> +
> +                       iommus = <&apps_smmu 0x900 0x402>;
> +
> +                       #address-cells = <2>;
> +                       #size-cells = <2>;
> +                       ranges;
> +
> +                       status = "disabled";
> +
> +                       mdp: mdp@ae01000 {

display-controller@ae01000

> +                               compatible = "qcom,sc7280-dpu";
> +                               reg = <0 0x0ae01000 0 0x8f030>,
> +                                       <0 0x0aeb0000 0 0x2008>;
> +                               reg-names = "mdp", "vbif";
> +
> +                               clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> +                                       <&gcc GCC_DISP_SF_AXI_CLK>,
> +                                       <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +                                       <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> +                                       <&dispcc DISP_CC_MDSS_MDP_CLK>,
> +                                       <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> +                               clock-names = "bus", "nrt_bus", "iface", "lut", "core",
> +                                             "vsync";

One line per string please.

> +                               assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>,
> +                                               <&dispcc DISP_CC_MDSS_VSYNC_CLK>,
> +                                               <&dispcc DISP_CC_MDSS_AHB_CLK>;
> +                               assigned-clock-rates = <300000000>,
> +                                                       <19200000>,
> +                                                       <19200000>;
> +                               operating-points-v2 = <&mdp_opp_table>;
> +                               power-domains = <&rpmhpd SC7280_CX>;
> +
> +                               interrupt-parent = <&mdss>;
> +                               interrupts = <0>;
> +
> +                               status = "disabled";
> +
> +                               mdp_opp_table: mdp-opp-table {

mdp_opp_table: opp-table {

> +                                       compatible = "operating-points-v2";
> +
> +                                       opp-200000000 {
> +                                               opp-hz = /bits/ 64 <200000000>;
> +                                               required-opps = <&rpmhpd_opp_low_svs>;
> +                                       };
> +
> +                                       opp-300000000 {
> +                                               opp-hz = /bits/ 64 <300000000>;
> +                                               required-opps = <&rpmhpd_opp_svs>;
> +                                       };
> +
> +                                       opp-380000000 {
> +                                               opp-hz = /bits/ 64 <380000000>;
> +                                               required-opps = <&rpmhpd_opp_svs_l1>;
> +                                       };
> +
> +                                       opp-506666667 {
> +                                               opp-hz = /bits/ 64 <506666667>;
> +                                               required-opps = <&rpmhpd_opp_nom>;
> +                                       };
> +                               };
> +                       };
> +               };
> +

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 3/4] arm64: dts: qcom: sc7280: Add DSI display nodes
  2021-08-18 10:27 ` [PATCH v1 3/4] arm64: dts: qcom: sc7280: Add DSI display nodes Krishna Manikandan
  2021-08-18 16:35   ` Matthias Kaehlcke
@ 2021-08-18 19:59   ` Stephen Boyd
  1 sibling, 0 replies; 17+ messages in thread
From: Stephen Boyd @ 2021-08-18 19:59 UTC (permalink / raw)
  To: Krishna Manikandan, devicetree, linux-arm-msm, linux-kernel
  Cc: Rajeev Nandan, kalyan_t, sbillaka, abhinavk, robdclark,
	bjorn.andersson, khsieh, freedreno, dri-devel, robh+dt

Quoting Krishna Manikandan (2021-08-18 03:27:03)
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index fd7ff1c..aadf55d 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1507,6 +1519,95 @@
>                                         };
>                                 };
>                         };
> +
> +                       dsi0: dsi@ae94000 {
> +                               compatible = "qcom,mdss-dsi-ctrl";
> +                               reg = <0 0x0ae94000 0 0x400>;
> +                               reg-names = "dsi_ctrl";
> +
> +                               interrupt-parent = <&mdss>;
> +                               interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;

Drop flags as the #interrupt-cells is 0 for mdss

> +
> +                               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>,
> +                                        <&gcc GCC_DISP_HF_AXI_CLK>;
> +                               clock-names = "byte",
> +                                             "byte_intf",
> +                                             "pixel",
> +                                             "core",
> +                                             "iface",
> +                                             "bus";
> +
> +                               operating-points-v2 = <&dsi_opp_table>;
> +                               power-domains = <&rpmhpd SC7280_CX>;
> +
> +                               phys = <&dsi_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 = <&dpu_intf1_out>;
> +                                               };
> +                                       };
> +
> +                                       port@1 {
> +                                               reg = <1>;
> +                                               dsi0_out: endpoint {
> +                                               };
> +                                       };
> +                               };
> +
> +                               dsi_opp_table: dsi-opp-table {

dsi_opp_table: opp-table {

> +                                       compatible = "operating-points-v2";
> +
> +                                       opp-187500000 {
> +                                               opp-hz = /bits/ 64 <187500000>;
> +                                               required-opps = <&rpmhpd_opp_low_svs>;
> +                                       };
> +
> +                                       opp-300000000 {
> +                                               opp-hz = /bits/ 64 <300000000>;
> +                                               required-opps = <&rpmhpd_opp_svs>;
> +                                       };
> +
> +                                       opp-358000000 {
> +                                               opp-hz = /bits/ 64 <358000000>;
> +                                               required-opps = <&rpmhpd_opp_svs_l1>;
> +                                       };
> +                               };
> +                       };
> +
> +                       dsi_phy: dsi-phy@ae94400 {

phy@ae94400

> +                               compatible = "qcom,sc7280-dsi-phy-7nm";
> +                               reg = <0 0x0ae94400 0 0x200>,
> +                                     <0 0x0ae94600 0 0x280>,
> +                                     <0 0x0ae94900 0 0x280>;
> +                               reg-names = "dsi_phy",
> +                                           "dsi_phy_lane",
> +                                           "dsi_pll";
> +
> +                               #clock-cells = <1>;
> +                               #phy-cells = <0>;
> +
> +                               clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +                                        <&rpmhcc RPMH_CXO_CLK>;
> +                               clock-names = "iface", "ref";
> +
> +                               status = "disabled";
> +                       };

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes
  2021-08-18 10:27 ` [PATCH v1 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes Krishna Manikandan
  2021-08-18 16:14   ` Matthias Kaehlcke
@ 2021-08-18 20:03   ` Stephen Boyd
  1 sibling, 0 replies; 17+ messages in thread
From: Stephen Boyd @ 2021-08-18 20:03 UTC (permalink / raw)
  To: Krishna Manikandan, devicetree, linux-arm-msm, linux-kernel
  Cc: Sankeerth Billakanti, kalyan_t, abhinavk, robdclark,
	bjorn.andersson, khsieh, rajeevny, freedreno, dri-devel, robh+dt

Quoting Krishna Manikandan (2021-08-18 03:27:04)
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index aadf55d..5be318e 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1412,7 +1412,7 @@
>                         reg = <0 0xaf00000 0 0x20000>;
>                         clocks = <&rpmhcc RPMH_CXO_CLK>,
>                                  <&gcc GCC_DISP_GPLL0_CLK_SRC>,
> -                                <0>, <0>, <0>, <0>, <0>, <0>;
> +                                <0>, <0>, <0>, <0>, <&edp_phy 0>, <&edp_phy 1>;
>                         clock-names = "bi_tcxo", "gcc_disp_gpll0_clk",
>                                       "dsi0_phy_pll_out_byteclk",
>                                       "dsi0_phy_pll_out_dsiclk",
> @@ -1493,6 +1493,12 @@
>                                                         remote-endpoint = <&dsi0_in>;
>                                                 };
>                                         };

Newline here please.

> +                                       port@1 {
> +                                               reg = <1>;
> +                                               dpu_intf5_out: endpoint {
> +                                                       remote-endpoint = <&edp_in>;
> +                                               };
> +                                       };
>                                 };
>
>                                 mdp_opp_table: mdp-opp-table {
> @@ -1608,6 +1614,101 @@
>
>                                 status = "disabled";
>                         };
> +
> +                       msm_edp: edp@aea0000 {
> +                               status = "disabled";

Please pick a place to put status disabled. I don't know what qcom
maintainers want, but please be consistent.

> +                               compatible = "qcom,sc7280-edp";
> +                               reg = <0 0xaea0000 0 0x200>,
> +                                     <0 0xaea0200 0 0x200>,
> +                                     <0 0xaea0400 0 0xc00>,
> +                                     <0 0xaea1000 0 0x400>;
> +
> +                               interrupt-parent = <&mdss>;
> +                               interrupts = <14 IRQ_TYPE_NONE>;

Drop flags.

> +
> +                               clocks = <&rpmhcc RPMH_CXO_CLK>,
> +                                        <&gcc GCC_EDP_CLKREF_EN>,
> +                                        <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +                                        <&dispcc DISP_CC_MDSS_EDP_AUX_CLK>,
> +                                        <&dispcc DISP_CC_MDSS_EDP_LINK_CLK>,
> +                                        <&dispcc DISP_CC_MDSS_EDP_LINK_INTF_CLK>,
> +                                        <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK>;
> +                               clock-names = "core_xo", "core_ref",
> +                                             "core_iface", "core_aux", "ctrl_link",
> +                                             "ctrl_link_iface", "stream_pixel";

One line per string please.

> +                               #clock-cells = <1>;
> +                               assigned-clocks = <&dispcc DISP_CC_MDSS_EDP_LINK_CLK_SRC>,
> +                                                 <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK_SRC>;
> +                               assigned-clock-parents = <&edp_phy 0>, <&edp_phy 1>;
> +
> +                               phys = <&edp_phy>;
> +                               phy-names = "dp";
> +
> +                               vdda-1p2-supply = <&vreg_l6b_1p2>;
> +                               vdda-0p9-supply = <&vreg_l10c_0p8>;

Can this be done here? Probably needs to move to the board dts/dtsi
file.

> +                               operating-points-v2 = <&edp_opp_table>;
> +                               power-domains = <&rpmhpd SC7280_CX>;
> +
> +                               pinctrl-names = "default";
> +                               pinctrl-0 = <&edp_hot_plug_det>, <&edp_panel_power_on>;
> +
> +                               panel-bklt-gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>;
> +                               panel-pwm-gpio = <&pm8350c_gpios 8 GPIO_ACTIVE_HIGH>;

Please no panel-bklt-gpio and panel-pwm-gpio properties.

> +
> +                               ports {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       port@0 {
> +                                               reg = <0>;
> +                                               edp_in: endpoint {
> +                                                       remote-endpoint = <&dpu_intf5_out>;
> +                                               };
> +                                       };
> +                               };
> +
> +                               edp_opp_table: edp-opp-table {

edp_opp_table: opp-table {

> +                                       compatible = "operating-points-v2";
> +
> +                                       opp-160000000 {
> +                                               opp-hz = /bits/ 64 <160000000>;
> +                                               required-opps = <&rpmhpd_opp_low_svs>;
> +                                       };
> +
> +                                       opp-270000000 {
> +                                               opp-hz = /bits/ 64 <270000000>;
> +                                               required-opps = <&rpmhpd_opp_svs>;
> +                                       };
> +
> +                                       opp-540000000 {
> +                                               opp-hz = /bits/ 64 <540000000>;
> +                                               required-opps = <&rpmhpd_opp_nom>;
> +                                       };
> +
> +                                       opp-810000000 {
> +                                               opp-hz = /bits/ 64 <810000000>;
> +                                               required-opps = <&rpmhpd_opp_nom>;
> +                                       };
> +                               };
> +                       };
> +
> +                       edp_phy: phy@aec2000 {

Good job on using phy!

> +                               status = "disabled";
> +                               compatible = "qcom,sc7280-edp-phy";
> +                               reg = <0 0xaec2a00 0 0x19c>,
> +                                     <0 0xaec2200 0 0xa0>,
> +                                     <0 0xaec2600 0 0xa0>,
> +                                     <0 0xaec2000 0 0x1c0>;
> +
> +                               clocks = <&rpmhcc RPMH_CXO_CLK>,
> +                                        <&gcc GCC_EDP_CLKREF_EN>;
> +                               clock-names = "aux", "cfg_ahb";
> +
> +                               vdda-pll-supply = <&vreg_l6b_1p2>;
> +                               vdda-phy-supply = <&vreg_l10c_0p8>;

Again, still question the ability to put this here vs. in IDP file.

> +
> +                               #clock-cells = <1>;
> +                               #phy-cells = <0>;
> +                       };
>                 };
>
>                 pdc: interrupt-controller@b220000 {
> @@ -1704,6 +1805,30 @@
>                                 function = "qup13";
>                         };
>
> +                       edp_hot_plug_det: edp-hot-plug-det {
> +                               pinmux {
> +                                       pins = "gpio60";
> +                                       function = "edp_hot";
> +                               };
> +                               pinconf {
> +                                       pins = "gpio60";
> +                                       bias-pull-down;
> +                                       input-enable;
> +                               };

Move pinconf stuff to board file (and combine nodes as mka stated).

> +                       };
> +
> +                       edp_panel_power_on: edp-panel-power-on {
> +                               pinmux {
> +                                       pins = "gpio80";
> +                                       function = "gpio";
> +                               };
> +                               pinconf {
> +                                       pins = "gpio80";
> +                                       bias-disable;
> +                                       output-high;
> +                               };
> +                       };
> +
>                         sdc1_on: sdc1-on {
>                                 clk {
>                                         pins = "sdc1_clk";
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280
  2021-08-18 10:27 [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Krishna Manikandan
                   ` (4 preceding siblings ...)
  2021-08-18 19:56 ` Stephen Boyd
@ 2021-08-18 20:07 ` Stephen Boyd
  5 siblings, 0 replies; 17+ messages in thread
From: Stephen Boyd @ 2021-08-18 20:07 UTC (permalink / raw)
  To: Krishna Manikandan, devicetree, linux-arm-msm, linux-kernel
  Cc: kalyan_t, sbillaka, abhinavk, robdclark, bjorn.andersson, khsieh,
	rajeevny, freedreno, dri-devel, robh+dt

Quoting Krishna Manikandan (2021-08-18 03:27:01)
> MSM Mobile Display Subsystem (MDSS) encapsulates sub-blocks
> like DPU display controller, DSI, EDP etc. Add required DPU
> device tree bindings for SC7280.
>
> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
> ---

Please send a cover-letter next time.

Do you have the display port dts bits and driver code ready too? Can it
be part of this patch series?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes
  2021-08-18 19:57   ` Stephen Boyd
@ 2021-09-30 11:56     ` mkrishn
  2021-09-30 17:58       ` Stephen Boyd
  0 siblings, 1 reply; 17+ messages in thread
From: mkrishn @ 2021-09-30 11:56 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: devicetree, linux-arm-msm, linux-kernel, kalyan_t, sbillaka,
	abhinavk, robdclark, bjorn.andersson, khsieh, rajeevny,
	freedreno, dri-devel, robh+dt

On 2021-08-19 01:27, Stephen Boyd wrote:
> Quoting Krishna Manikandan (2021-08-18 03:27:02)
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 53a21d0..fd7ff1c 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -5,6 +5,7 @@
>>   * Copyright (c) 2020-2021, The Linux Foundation. All rights 
>> reserved.
>>   */
>> 
>> +#include <dt-bindings/clock/qcom,dispcc-sc7280.h>
>>  #include <dt-bindings/clock/qcom,gcc-sc7280.h>
>>  #include <dt-bindings/clock/qcom,rpmh.h>
>>  #include <dt-bindings/interconnect/qcom,sc7280.h>
>> @@ -1424,6 +1425,90 @@
>>                         #power-domain-cells = <1>;
>>                 };
>> 
>> +               mdss: mdss@ae00000 {
> 
> subsystem@ae00000
> 
>> +                       compatible = "qcom,sc7280-mdss";
>> +                       reg = <0 0x0ae00000 0 0x1000>;
>> +                       reg-names = "mdss";
>> +
>> +                       power-domains = <&dispcc 
>> DISP_CC_MDSS_CORE_GDSC>;
>> +
>> +                       clocks = <&gcc GCC_DISP_AHB_CLK>,
>> +                                <&dispcc DISP_CC_MDSS_AHB_CLK>,
>> +                               <&dispcc DISP_CC_MDSS_MDP_CLK>;
>> +                       clock-names = "iface", "ahb", "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>;
>> +
>> +                       interconnects = <&mmss_noc MASTER_MDP0 0 
>> &mc_virt SLAVE_EBI1 0>;
>> +                       interconnect-names = "mdp0-mem";
>> +
>> +                       iommus = <&apps_smmu 0x900 0x402>;
>> +
>> +                       #address-cells = <2>;
>> +                       #size-cells = <2>;
>> +                       ranges;
>> +
>> +                       status = "disabled";
>> +
>> +                       mdp: mdp@ae01000 {
> 
> display-controller@ae01000

Stephen,
    In the current driver code, there is a substring comparison for "mdp" 
in device node name as part of probe sequence. If "mdp" is not present 
in the node name, it will
    return an error resulting in probe failure. Can we continue using mdp 
as nodename instead of display controller?

Thanks,
Krishna


> 
>> +                               compatible = "qcom,sc7280-dpu";
>> +                               reg = <0 0x0ae01000 0 0x8f030>,
>> +                                       <0 0x0aeb0000 0 0x2008>;
>> +                               reg-names = "mdp", "vbif";
>> +
>> +                               clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
>> +                                       <&gcc GCC_DISP_SF_AXI_CLK>,
>> +                                       <&dispcc 
>> DISP_CC_MDSS_AHB_CLK>,
>> +                                       <&dispcc 
>> DISP_CC_MDSS_MDP_LUT_CLK>,
>> +                                       <&dispcc 
>> DISP_CC_MDSS_MDP_CLK>,
>> +                                       <&dispcc 
>> DISP_CC_MDSS_VSYNC_CLK>;
>> +                               clock-names = "bus", "nrt_bus", 
>> "iface", "lut", "core",
>> +                                             "vsync";
> 
> One line per string please.
> 
>> +                               assigned-clocks = <&dispcc 
>> DISP_CC_MDSS_MDP_CLK>,
>> +                                               <&dispcc 
>> DISP_CC_MDSS_VSYNC_CLK>,
>> +                                               <&dispcc 
>> DISP_CC_MDSS_AHB_CLK>;
>> +                               assigned-clock-rates = <300000000>,
>> +                                                       <19200000>,
>> +                                                       <19200000>;
>> +                               operating-points-v2 = 
>> <&mdp_opp_table>;
>> +                               power-domains = <&rpmhpd SC7280_CX>;
>> +
>> +                               interrupt-parent = <&mdss>;
>> +                               interrupts = <0>;
>> +
>> +                               status = "disabled";
>> +
>> +                               mdp_opp_table: mdp-opp-table {
> 
> mdp_opp_table: opp-table {
> 
>> +                                       compatible = 
>> "operating-points-v2";
>> +
>> +                                       opp-200000000 {
>> +                                               opp-hz = /bits/ 64 
>> <200000000>;
>> +                                               required-opps = 
>> <&rpmhpd_opp_low_svs>;
>> +                                       };
>> +
>> +                                       opp-300000000 {
>> +                                               opp-hz = /bits/ 64 
>> <300000000>;
>> +                                               required-opps = 
>> <&rpmhpd_opp_svs>;
>> +                                       };
>> +
>> +                                       opp-380000000 {
>> +                                               opp-hz = /bits/ 64 
>> <380000000>;
>> +                                               required-opps = 
>> <&rpmhpd_opp_svs_l1>;
>> +                                       };
>> +
>> +                                       opp-506666667 {
>> +                                               opp-hz = /bits/ 64 
>> <506666667>;
>> +                                               required-opps = 
>> <&rpmhpd_opp_nom>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes
  2021-09-30 11:56     ` mkrishn
@ 2021-09-30 17:58       ` Stephen Boyd
  2021-10-01  6:39         ` mkrishn
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2021-09-30 17:58 UTC (permalink / raw)
  To: mkrishn
  Cc: devicetree, linux-arm-msm, linux-kernel, kalyan_t, sbillaka,
	abhinavk, robdclark, bjorn.andersson, khsieh, rajeevny,
	freedreno, dri-devel, robh+dt

Quoting mkrishn@codeaurora.org (2021-09-30 04:56:59)
> On 2021-08-19 01:27, Stephen Boyd wrote:
> > Quoting Krishna Manikandan (2021-08-18 03:27:02)
> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> index 53a21d0..fd7ff1c 100644
> >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> +
> >> +                       status = "disabled";
> >> +
> >> +                       mdp: mdp@ae01000 {
> >
> > display-controller@ae01000
>
> Stephen,
>     In the current driver code, there is a substring comparison for "mdp"
> in device node name as part of probe sequence. If "mdp" is not present
> in the node name, it will
>     return an error resulting in probe failure. Can we continue using mdp
> as nodename instead of display controller?
>

Can we fix the driver to not look for node names and look for compatible
strings instead? It took me a minute to find compare_name_mdp() in
drivers/gpu/drm/msm/msm_drv.c to understand what you're talking about.
Perhaps looking for qcom,mdp5 in there will be sufficient instead of
looking at the node name.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes
  2021-09-30 17:58       ` Stephen Boyd
@ 2021-10-01  6:39         ` mkrishn
  2021-10-05  1:51           ` Stephen Boyd
  0 siblings, 1 reply; 17+ messages in thread
From: mkrishn @ 2021-10-01  6:39 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: devicetree, linux-arm-msm, linux-kernel, kalyan_t, sbillaka,
	abhinavk, robdclark, bjorn.andersson, khsieh, rajeevny,
	freedreno, dri-devel, robh+dt

On 2021-09-30 23:28, Stephen Boyd wrote:
> Quoting mkrishn@codeaurora.org (2021-09-30 04:56:59)
>> On 2021-08-19 01:27, Stephen Boyd wrote:
>> > Quoting Krishna Manikandan (2021-08-18 03:27:02)
>> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> >> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> >> index 53a21d0..fd7ff1c 100644
>> >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> >> +
>> >> +                       status = "disabled";
>> >> +
>> >> +                       mdp: mdp@ae01000 {
>> >
>> > display-controller@ae01000
>> 
>> Stephen,
>>     In the current driver code, there is a substring comparison for 
>> "mdp"
>> in device node name as part of probe sequence. If "mdp" is not present
>> in the node name, it will
>>     return an error resulting in probe failure. Can we continue using 
>> mdp
>> as nodename instead of display controller?
>> 
> 
> Can we fix the driver to not look for node names and look for 
> compatible
> strings instead? It took me a minute to find compare_name_mdp() in
> drivers/gpu/drm/msm/msm_drv.c to understand what you're talking about.
> Perhaps looking for qcom,mdp5 in there will be sufficient instead of
> looking at the node name.

Sure Stephen. I will make the necessary changes in msm_drv.c to look for 
compatible string instead of node name.
Can I include these two changes (changing mdp--> display controller and 
msm_drv.c changes) in a separate series ?

Thanks,
Krishna

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes
  2021-10-01  6:39         ` mkrishn
@ 2021-10-05  1:51           ` Stephen Boyd
  2021-10-05  9:52             ` mkrishn
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2021-10-05  1:51 UTC (permalink / raw)
  To: mkrishn
  Cc: devicetree, linux-arm-msm, linux-kernel, kalyan_t, sbillaka,
	abhinavk, robdclark, bjorn.andersson, khsieh, rajeevny,
	freedreno, dri-devel, robh+dt

Quoting mkrishn@codeaurora.org (2021-09-30 23:39:07)
> On 2021-09-30 23:28, Stephen Boyd wrote:
> > Quoting mkrishn@codeaurora.org (2021-09-30 04:56:59)
> >> On 2021-08-19 01:27, Stephen Boyd wrote:
> >> > Quoting Krishna Manikandan (2021-08-18 03:27:02)
> >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> >> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> >> index 53a21d0..fd7ff1c 100644
> >> >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> >> >> +
> >> >> +                       status = "disabled";
> >> >> +
> >> >> +                       mdp: mdp@ae01000 {
> >> >
> >> > display-controller@ae01000
> >>
> >> Stephen,
> >>     In the current driver code, there is a substring comparison for
> >> "mdp"
> >> in device node name as part of probe sequence. If "mdp" is not present
> >> in the node name, it will
> >>     return an error resulting in probe failure. Can we continue using
> >> mdp
> >> as nodename instead of display controller?
> >>
> >
> > Can we fix the driver to not look for node names and look for
> > compatible
> > strings instead? It took me a minute to find compare_name_mdp() in
> > drivers/gpu/drm/msm/msm_drv.c to understand what you're talking about.
> > Perhaps looking for qcom,mdp5 in there will be sufficient instead of
> > looking at the node name.
>
> Sure Stephen. I will make the necessary changes in msm_drv.c to look for
> compatible string instead of node name.
> Can I include these two changes (changing mdp--> display controller and
> msm_drv.c changes) in a separate series ?
>

Sure. So you'll send the drm driver change now and we'll get the DT
change after that with the more generic node name?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes
  2021-10-05  1:51           ` Stephen Boyd
@ 2021-10-05  9:52             ` mkrishn
  0 siblings, 0 replies; 17+ messages in thread
From: mkrishn @ 2021-10-05  9:52 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: devicetree, linux-arm-msm, linux-kernel, kalyan_t, sbillaka,
	abhinavk, robdclark, bjorn.andersson, khsieh, rajeevny,
	freedreno, dri-devel, robh+dt

On 2021-10-05 07:21, Stephen Boyd wrote:
> Quoting mkrishn@codeaurora.org (2021-09-30 23:39:07)
>> On 2021-09-30 23:28, Stephen Boyd wrote:
>> > Quoting mkrishn@codeaurora.org (2021-09-30 04:56:59)
>> >> On 2021-08-19 01:27, Stephen Boyd wrote:
>> >> > Quoting Krishna Manikandan (2021-08-18 03:27:02)
>> >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> >> >> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> >> >> index 53a21d0..fd7ff1c 100644
>> >> >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> >> >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> >> >> +
>> >> >> +                       status = "disabled";
>> >> >> +
>> >> >> +                       mdp: mdp@ae01000 {
>> >> >
>> >> > display-controller@ae01000
>> >>
>> >> Stephen,
>> >>     In the current driver code, there is a substring comparison for
>> >> "mdp"
>> >> in device node name as part of probe sequence. If "mdp" is not present
>> >> in the node name, it will
>> >>     return an error resulting in probe failure. Can we continue using
>> >> mdp
>> >> as nodename instead of display controller?
>> >>
>> >
>> > Can we fix the driver to not look for node names and look for
>> > compatible
>> > strings instead? It took me a minute to find compare_name_mdp() in
>> > drivers/gpu/drm/msm/msm_drv.c to understand what you're talking about.
>> > Perhaps looking for qcom,mdp5 in there will be sufficient instead of
>> > looking at the node name.
>> 
>> Sure Stephen. I will make the necessary changes in msm_drv.c to look 
>> for
>> compatible string instead of node name.
>> Can I include these two changes (changing mdp--> display controller 
>> and
>> msm_drv.c changes) in a separate series ?
>> 
> 
> Sure. So you'll send the drm driver change now and we'll get the DT
> change after that with the more generic node name?

Yes Stephen.I have raised the change to fix the driver issue.
https://patchwork.kernel.org/project/linux-arm-msm/patch/1633427071-19523-1-git-send-email-mkrishn@codeaurora.org/

Regards,
Krishna


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-10-05  9:53 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18 10:27 [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Krishna Manikandan
2021-08-18 10:27 ` [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes Krishna Manikandan
2021-08-18 19:57   ` Stephen Boyd
2021-09-30 11:56     ` mkrishn
2021-09-30 17:58       ` Stephen Boyd
2021-10-01  6:39         ` mkrishn
2021-10-05  1:51           ` Stephen Boyd
2021-10-05  9:52             ` mkrishn
2021-08-18 10:27 ` [PATCH v1 3/4] arm64: dts: qcom: sc7280: Add DSI display nodes Krishna Manikandan
2021-08-18 16:35   ` Matthias Kaehlcke
2021-08-18 19:59   ` Stephen Boyd
2021-08-18 10:27 ` [PATCH v1 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes Krishna Manikandan
2021-08-18 16:14   ` Matthias Kaehlcke
2021-08-18 20:03   ` Stephen Boyd
2021-08-18 16:14 ` [PATCH v1 1/4] dt-bindings: msm: add DT bindings for sc7280 Rob Herring
2021-08-18 19:56 ` Stephen Boyd
2021-08-18 20:07 ` Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).