[PATCH 00/15] drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel

[PATCH 00/15] drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel

[Marijn Suijten]

Bring up the SM6125 DPU now that all preliminary series (such as INTF
TE) have been merged (for me to test the hardware properly), and most
other conflicting work (barring ongoing catalog *improvements*) has made
its way in as well or is still being discussed.

The second part of the series complements that by immediately utilizing
this hardware in DT, and even enabling the MDSS/DSI nodes complete with
a 6.0" 1080x2520 panel for Sony's Seine PDX201 (Xperia 10 II).

The last patch ("sm6125-seine: Configure MDSS, DSI and panel") depends
on (an impending v2 of) my Sony panel collection series [1].

[1]: https://lore.kernel.org/linux-arm-msm/20230521-drm-panels-sony-v1-0-541c341d6bee@somainline.org/

---
Marijn Suijten (15):
      arm64: dts: qcom: sm6125: Sort spmi_bus node numerically by reg
      dt-bindings: clock: qcom,dispcc-sm6125: Remove unused GCC_DISP_AHB_CLK
      dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock
      dt-bindings: clock: qcom,dispcc-sm6125: Allow power-domains property
      dt-bindings: display/msm: dsi-controller-main: Document SM6125
      dt-bindings: display/msm: sc7180-dpu: Describe SM6125
      dt-bindings: display/msm: Add SM6125 MDSS
      drm/msm/dpu: Add SM6125 support
      drm/msm/mdss: Add SM6125 support
      dt-bindings: msm: dsi-phy-14nm: Document SM6125 variant
      drm/msm/dsi: Add 14nm phy configuration for SM6125
      arm64: dts: qcom: sm6125: Switch fixed xo_board clock to RPM XO clock
      arm64: dts: qcom: sm6125: Add dispcc node
      arm64: dts: qcom: sm6125: Add display hardware nodes
      arm64: dts: qcom: sm6125-seine: Configure MDSS, DSI and panel

 .../bindings/clock/qcom,dispcc-sm6125.yaml         |  17 +-
 .../bindings/display/msm/dsi-controller-main.yaml  |   2 +
 .../bindings/display/msm/dsi-phy-14nm.yaml         |   1 +
 .../bindings/display/msm/qcom,sc7180-dpu.yaml      |   1 +
 .../bindings/display/msm/qcom,sm6125-mdss.yaml     | 206 +++++++++++++++++
 .../dts/qcom/sm6125-sony-xperia-seine-pdx201.dts   |  59 +++++
 arch/arm64/boot/dts/qcom/sm6125.dtsi               | 244 +++++++++++++++++++--
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h | 173 +++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     |   6 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |   1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |   1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c              |   2 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h              |   1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c         |  15 ++
 drivers/gpu/drm/msm/msm_mdss.c                     |   8 +
 15 files changed, 712 insertions(+), 25 deletions(-)
---
base-commit: 8d2be868b42c08290509c60515865f4de24ea704
change-id: 20230624-sm6125-dpu-aedc9637ee7b

Best regards,
-- 
Marijn Suijten <marijn.suijten@somainline.org>

[PATCH 01/15] arm64: dts: qcom: sm6125: Sort spmi_bus node numerically by reg

[Marijn Suijten]

This node has always resided in the wrong spot, making it somewhat
harder to contribute new node entries while maintaining proper sorting
around it.  Move the node up to sit after hsusb_phy1 where it maintains
proper numerial sorting on the (first of its many) reg address property.

Fixes: cff4bbaf2a2d ("arm64: dts: qcom: Add support for SM6125")
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 arch/arm64/boot/dts/qcom/sm6125.dtsi | 38 ++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
index a596baa6ce3e..722dde560bec 100644
--- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
@@ -679,6 +679,24 @@ hsusb_phy1: phy@1613000 {
 			status = "disabled";
 		};
 
+		spmi_bus: spmi@1c40000 {
+			compatible = "qcom,spmi-pmic-arb";
+			reg = <0x01c40000 0x1100>,
+			      <0x01e00000 0x2000000>,
+			      <0x03e00000 0x100000>,
+			      <0x03f00000 0xa0000>,
+			      <0x01c0a000 0x26000>;
+			reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
+			interrupt-names = "periph_irq";
+			interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>;
+			qcom,ee = <0>;
+			qcom,channel = <0>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+			interrupt-controller;
+			#interrupt-cells = <4>;
+		};
+
 		rpm_msg_ram: sram@45f0000 {
 			compatible = "qcom,rpm-msg-ram";
 			reg = <0x045f0000 0x7000>;
@@ -1184,27 +1202,9 @@ sram@4690000 {
 			reg = <0x04690000 0x10000>;
 		};
 
-		spmi_bus: spmi@1c40000 {
-			compatible = "qcom,spmi-pmic-arb";
-			reg = <0x01c40000 0x1100>,
-			      <0x01e00000 0x2000000>,
-			      <0x03e00000 0x100000>,
-			      <0x03f00000 0xa0000>,
-			      <0x01c0a000 0x26000>;
-			reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
-			interrupt-names = "periph_irq";
-			interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>;
-			qcom,ee = <0>;
-			qcom,channel = <0>;
-			#address-cells = <2>;
-			#size-cells = <0>;
-			interrupt-controller;
-			#interrupt-cells = <4>;
-		};
-
 		apps_smmu: iommu@c600000 {
 			compatible = "qcom,sm6125-smmu-500", "qcom,smmu-500", "arm,mmu-500";
-			reg = <0xc600000 0x80000>;
+			reg = <0x0c600000 0x80000>;
 			interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,

-- 
2.41.0

[PATCH 02/15] dt-bindings: clock: qcom,dispcc-sm6125: Remove unused GCC_DISP_AHB_CLK

[Marijn Suijten]

The downsteam driver for dispcc only ever gets and puts this clock
without ever using it in the clocktree; this unnecessary workaround was
never ported to mainline, hence the driver doesn't consume this clock
and shouldn't be required by the bindings.

Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
index 8a210c4c5f82..2acf487d8a2f 100644
--- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
@@ -28,7 +28,6 @@ properties:
       - description: Pixel clock from DSI PHY1
       - description: Link clock from DP PHY
       - description: VCO DIV clock from DP PHY
-      - description: AHB config clock from GCC
 
   clock-names:
     items:
@@ -38,7 +37,6 @@ properties:
       - const: dsi1_phy_pll_out_dsiclk
       - const: dp_phy_pll_link_clk
       - const: dp_phy_pll_vco_div_clk
-      - const: cfg_ahb_clk
 
   '#clock-cells':
     const: 1
@@ -71,15 +69,13 @@ examples:
                <&dsi0_phy 1>,
                <&dsi1_phy 1>,
                <&dp_phy 0>,
-               <&dp_phy 1>,
-               <&gcc GCC_DISP_AHB_CLK>;
+               <&dp_phy 1>;
       clock-names = "bi_tcxo",
                     "dsi0_phy_pll_out_byteclk",
                     "dsi0_phy_pll_out_dsiclk",
                     "dsi1_phy_pll_out_dsiclk",
                     "dp_phy_pll_link_clk",
-                    "dp_phy_pll_vco_div_clk",
-                    "cfg_ahb_clk";
+                    "dp_phy_pll_vco_div_clk";
       #clock-cells = <1>;
       #power-domain-cells = <1>;
     };

-- 
2.41.0

[PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
be passed from DT, and should be required by the bindings.

Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
index 2acf487d8a2f..11ec154503a3 100644
--- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
@@ -23,6 +23,7 @@ properties:
   clocks:
     items:
       - description: Board XO source
+      - description: GPLL0 div source from GCC
       - description: Byte clock from DSI PHY0
       - description: Pixel clock from DSI PHY0
       - description: Pixel clock from DSI PHY1
@@ -32,6 +33,7 @@ properties:
   clock-names:
     items:
       - const: bi_tcxo
+      - const: gcc_disp_gpll0_div_clk_src
       - const: dsi0_phy_pll_out_byteclk
       - const: dsi0_phy_pll_out_dsiclk
       - const: dsi1_phy_pll_out_dsiclk
@@ -65,12 +67,14 @@ examples:
       compatible = "qcom,sm6125-dispcc";
       reg = <0x5f00000 0x20000>;
       clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
+               <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
                <&dsi0_phy 0>,
                <&dsi0_phy 1>,
                <&dsi1_phy 1>,
                <&dp_phy 0>,
                <&dp_phy 1>;
       clock-names = "bi_tcxo",
+                    "gcc_disp_gpll0_div_clk_src",
                     "dsi0_phy_pll_out_byteclk",
                     "dsi0_phy_pll_out_dsiclk",
                     "dsi1_phy_pll_out_dsiclk",

-- 
2.41.0

[PATCH 04/15] dt-bindings: clock: qcom,dispcc-sm6125: Allow power-domains property

[Marijn Suijten]

On SM6125 the dispcc block is gated behind VDDCX: allow this domain to
be configured.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
index 11ec154503a3..02796675e8f6 100644
--- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
@@ -46,6 +46,9 @@ properties:
   '#power-domain-cells':
     const: 1
 
+  power-domains:
+    maxItems: 1
+
   reg:
     maxItems: 1
 
@@ -63,6 +66,7 @@ examples:
   - |
     #include <dt-bindings/clock/qcom,rpmcc.h>
     #include <dt-bindings/clock/qcom,gcc-sm6125.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
     clock-controller@5f00000 {
       compatible = "qcom,sm6125-dispcc";
       reg = <0x5f00000 0x20000>;
@@ -80,6 +84,7 @@ examples:
                     "dsi1_phy_pll_out_dsiclk",
                     "dp_phy_pll_link_clk",
                     "dp_phy_pll_vco_div_clk";
+      power-domains = <&rpmpd SM6125_VDDCX>;
       #clock-cells = <1>;
       #power-domain-cells = <1>;
     };

-- 
2.41.0

[PATCH 05/15] dt-bindings: display/msm: dsi-controller-main: Document SM6125

[Marijn Suijten]

Document general compatibility of the DSI controller on SM6125.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index 01848bdd5873..23926c39407e 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -27,6 +27,7 @@ properties:
               - qcom,sdm660-dsi-ctrl
               - qcom,sdm845-dsi-ctrl
               - qcom,sm6115-dsi-ctrl
+              - qcom,sm6125-dsi-ctrl
               - qcom,sm6350-dsi-ctrl
               - qcom,sm6375-dsi-ctrl
               - qcom,sm8150-dsi-ctrl
@@ -301,6 +302,7 @@ allOf:
           contains:
             enum:
               - qcom,msm8998-dsi-ctrl
+              - qcom,sm6125-dsi-ctrl
               - qcom,sm6350-dsi-ctrl
     then:
       properties:

-- 
2.41.0

[PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Marijn Suijten]

SM6125 is identical to SM6375 except that while downstream also defines
a throttle clock, its presence results in timeouts whereas SM6375
requires it to not observe any timeouts.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml
index 630b11480496..6d2ba9a1cca1 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml
@@ -15,6 +15,7 @@ properties:
   compatible:
     enum:
       - qcom,sc7180-dpu
+      - qcom,sm6125-dpu
       - qcom,sm6350-dpu
       - qcom,sm6375-dpu
 

-- 
2.41.0

[PATCH 07/15] dt-bindings: display/msm: Add SM6125 MDSS

[Marijn Suijten]

Document the SM6125 MDSS.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 .../bindings/display/msm/qcom,sm6125-mdss.yaml     | 206 +++++++++++++++++++++
 1 file changed, 206 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm6125-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sm6125-mdss.yaml
new file mode 100644
index 000000000000..e4db05c4a464
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sm6125-mdss.yaml
@@ -0,0 +1,206 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/msm/qcom,sm6125-mdss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SM6125 Display MDSS
+
+maintainers:
+  - Marijn Suijten <marijn.suijten@somainline.org>
+
+description:
+  SM6125 MSM Mobile Display Subsystem (MDSS), which encapsulates sub-blocks
+  like DPU display controller, DSI and DP interfaces etc.
+
+$ref: /schemas/display/msm/mdss-common.yaml#
+
+properties:
+  compatible:
+    const: qcom,sm6125-mdss
+
+  clocks:
+    items:
+      - description: Display AHB clock from gcc
+      - description: Display AHB clock
+      - description: Display core clock
+
+  clock-names:
+    items:
+      - const: iface
+      - const: ahb
+      - const: core
+
+  iommus:
+    maxItems: 1
+
+  interconnects:
+    maxItems: 2
+
+  interconnect-names:
+    maxItems: 2
+
+patternProperties:
+  "^display-controller@[0-9a-f]+$":
+    type: object
+    properties:
+      compatible:
+        const: qcom,sm6125-dpu
+
+  "^dsi@[0-9a-f]+$":
+    type: object
+    properties:
+      compatible:
+        items:
+          - const: qcom,sm6125-dsi-ctrl
+          - const: qcom,mdss-dsi-ctrl
+
+  "^phy@[0-9a-f]+$":
+    type: object
+    properties:
+      compatible:
+        const: qcom,dsi-phy-14nm-6125
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,dispcc-sm6125.h>
+    #include <dt-bindings/clock/qcom,gcc-sm6125.h>
+    #include <dt-bindings/clock/qcom,rpmcc.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+
+    display-subsystem@5e00000 {
+        compatible = "qcom,sm6125-mdss";
+        reg = <0x05e00000 0x1000>;
+        reg-names = "mdss";
+
+        power-domains = <&dispcc MDSS_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 186 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-controller;
+        #interrupt-cells = <1>;
+
+        iommus = <&apps_smmu 0x400 0x0>;
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges;
+
+        display-controller@5e01000 {
+            compatible = "qcom,sm6125-dpu";
+            reg = <0x05e01000 0x83208>,
+                  <0x05eb0000 0x2008>;
+            reg-names = "mdp", "vbif";
+
+            clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
+                     <&dispcc DISP_CC_MDSS_AHB_CLK>,
+                     <&dispcc DISP_CC_MDSS_ROT_CLK>,
+                     <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
+                     <&dispcc DISP_CC_MDSS_MDP_CLK>,
+                     <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+            clock-names = "bus",
+                          "iface",
+                          "rot",
+                          "lut",
+                          "core",
+                          "vsync";
+            assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+            assigned-clock-rates = <19200000>;
+
+            operating-points-v2 = <&mdp_opp_table>;
+            power-domains = <&rpmpd SM6125_VDDCX>;
+
+            interrupt-parent = <&mdss>;
+            interrupts = <0>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    dpu_intf1_out: endpoint {
+                        remote-endpoint = <&mdss_dsi0_in>;
+                    };
+                };
+            };
+        };
+
+        dsi@5e94000 {
+            compatible = "qcom,sm6125-dsi-ctrl", "qcom,mdss-dsi-ctrl";
+            reg = <0x05e94000 0x400>;
+            reg-names = "dsi_ctrl";
+
+            interrupt-parent = <&mdss>;
+            interrupts = <4>;
+
+            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";
+
+            assigned-clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK_SRC>,
+                              <&dispcc DISP_CC_MDSS_PCLK0_CLK_SRC>;
+            assigned-clock-parents = <&mdss_dsi0_phy 0>, <&mdss_dsi0_phy 1>;
+
+            operating-points-v2 = <&dsi_opp_table>;
+            power-domains = <&rpmpd SM6125_VDDMX>;
+
+            phys = <&mdss_dsi0_phy>;
+            phy-names = "dsi";
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    mdss_dsi0_in: endpoint {
+                        remote-endpoint = <&dpu_intf1_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    mdss_dsi0_out: endpoint {
+                    };
+                };
+            };
+        };
+
+        phy@5e94400 {
+            compatible = "qcom,dsi-phy-14nm-6125";
+            reg = <0x05e94400 0x100>,
+                  <0x05e94500 0x300>,
+                  <0x05e94800 0x188>;
+            reg-names = "dsi_phy",
+                        "dsi_phy_lane",
+                        "dsi_pll";
+
+            #clock-cells = <1>;
+            #phy-cells = <0>;
+
+            clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+                     <&rpmcc RPM_SMD_XO_CLK_SRC>;
+            clock-names = "iface", "ref";
+        };
+    };
+...

-- 
2.41.0

[PATCH 08/15] drm/msm/dpu: Add SM6125 support

[Marijn Suijten]

Add definitions for the display hardware used on the Qualcomm SM6125
platform.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h | 173 +++++++++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     |   6 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |   1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |   1 +
 4 files changed, 181 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h
new file mode 100644
index 000000000000..39c698f793fe
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h
@@ -0,0 +1,173 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Marijn Suijten <marijn.suijten@somainline.org>. All rights reserved.
+ * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DPU_5_4_SM6125_H
+#define _DPU_5_4_SM6125_H
+
+static const struct dpu_caps sm6125_dpu_caps = {
+	.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
+	.max_mixer_blendstages = 0x6,
+	.has_dim_layer = true,
+	.has_idle_pc = true,
+	.max_linewidth = 2160,
+	.pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
+	.max_hdeci_exp = MAX_HORZ_DECIMATION,
+	.max_vdeci_exp = MAX_VERT_DECIMATION,
+};
+
+static const struct dpu_ubwc_cfg sm6125_ubwc_cfg = {
+	.ubwc_version = DPU_HW_UBWC_VER_10,
+	.highest_bank_bit = 0x1,
+	.ubwc_swizzle = 0x1,
+};
+
+static const struct dpu_mdp_cfg sm6125_mdp[] = {
+	{
+		.name = "top_0", .id = MDP_TOP,
+		.base = 0x0, .len = 0x45c,
+		.features = 0,
+		.clk_ctrls = {
+			[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x2ac, .bit_off = 0 },
+			[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x2ac, .bit_off = 8 },
+			[DPU_CLK_CTRL_DMA1] = { .reg_off = 0x2b4, .bit_off = 8 },
+		},
+	},
+};
+
+static const struct dpu_ctl_cfg sm6125_ctl[] = {
+	{
+		.name = "ctl_0", .id = CTL_0,
+		.base = 0x1000, .len = 0x1e0,
+		.features = BIT(DPU_CTL_ACTIVE_CFG),
+		.intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
+	}, {
+		.name = "ctl_1", .id = CTL_1,
+		.base = 0x1200, .len = 0x1e0,
+		.features = BIT(DPU_CTL_ACTIVE_CFG),
+		.intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10),
+	}, {
+		.name = "ctl_2", .id = CTL_2,
+		.base = 0x1400, .len = 0x1e0,
+		.features = BIT(DPU_CTL_ACTIVE_CFG),
+		.intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 11),
+	}, {
+		.name = "ctl_3", .id = CTL_3,
+		.base = 0x1600, .len = 0x1e0,
+		.features = BIT(DPU_CTL_ACTIVE_CFG),
+		.intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 12),
+	}, {
+		.name = "ctl_4", .id = CTL_4,
+		.base = 0x1800, .len = 0x1e0,
+		.features = BIT(DPU_CTL_ACTIVE_CFG),
+		.intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 13),
+	}, {
+		.name = "ctl_5", .id = CTL_5,
+		.base = 0x1a00, .len = 0x1e0,
+		.features = BIT(DPU_CTL_ACTIVE_CFG),
+		.intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 23),
+	},
+};
+
+static const struct dpu_sspp_cfg sm6125_sspp[] = {
+	SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, 0x1f0, VIG_SM6125_MASK,
+		sm6125_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
+	SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, 0x1f0, DMA_SDM845_MASK,
+		sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
+	SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x1f0, DMA_SDM845_MASK,
+		sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
+};
+
+static const struct dpu_lm_cfg sm6125_lm[] = {
+	LM_BLK("lm_0", LM_0, 0x44000, MIXER_QCM2290_MASK,
+		&sdm845_lm_sblk, PINGPONG_0, LM_1, DSPP_0),
+	LM_BLK("lm_1", LM_1, 0x45000, MIXER_QCM2290_MASK,
+		&sdm845_lm_sblk, PINGPONG_1, LM_0, 0),
+};
+
+static const struct dpu_dspp_cfg sm6125_dspp[] = {
+	DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_SC7180_MASK,
+		 &sm8150_dspp_sblk),
+};
+
+static const struct dpu_pingpong_cfg sm6125_pp[] = {
+	PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, 0, sdm845_pp_sblk,
+			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
+			-1),
+	PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, 0, sdm845_pp_sblk,
+			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
+			-1),
+};
+
+static const struct dpu_intf_cfg sm6125_intf[] = {
+	INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24,
+			INTF_SC7180_MASK,
+			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
+			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 25)),
+	INTF_BLK_DSI_TE("intf_1", INTF_1, 0x6a800, 0x2c0, INTF_DSI, 0, 24, INTF_SC7180_MASK,
+			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 26),
+			DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 27),
+			DPU_IRQ_IDX(MDP_INTF1_TEAR_INTR, 2)),
+};
+
+static const struct dpu_perf_cfg sm6125_perf_data = {
+	.max_bw_low = 4100000,
+	.max_bw_high = 4100000,
+	.min_core_ib = 2400000,
+	.min_llcc_ib = 800000,
+	.min_dram_ib = 800000,
+	.min_prefill_lines = 24,
+	.danger_lut_tbl = {0xf, 0xffff, 0x0},
+	.safe_lut_tbl = {0xfff8, 0xf000, 0xffff},
+	.qos_lut_tbl = {
+		{.nentry = ARRAY_SIZE(sm8150_qos_linear),
+		.entries = sm8150_qos_linear
+		},
+		{.nentry = ARRAY_SIZE(sc7180_qos_macrotile),
+		.entries = sc7180_qos_macrotile
+		},
+		{.nentry = ARRAY_SIZE(sc7180_qos_nrt),
+		.entries = sc7180_qos_nrt
+		},
+		/* TODO: macrotile-qseed is different from macrotile */
+	},
+	.cdp_cfg = {
+		{.rd_enable = 1, .wr_enable = 1},
+		{.rd_enable = 1, .wr_enable = 0}
+	},
+	.clk_inefficiency_factor = 105,
+	.bw_inefficiency_factor = 120,
+};
+
+const struct dpu_mdss_cfg dpu_sm6125_cfg = {
+	.caps = &sm6125_dpu_caps,
+	.ubwc = &sm6125_ubwc_cfg,
+	.mdp_count = ARRAY_SIZE(sm6125_mdp),
+	.mdp = sm6125_mdp,
+	.ctl_count = ARRAY_SIZE(sm6125_ctl),
+	.ctl = sm6125_ctl,
+	.sspp_count = ARRAY_SIZE(sm6125_sspp),
+	.sspp = sm6125_sspp,
+	.mixer_count = ARRAY_SIZE(sm6125_lm),
+	.mixer = sm6125_lm,
+	.dspp_count = ARRAY_SIZE(sm6125_dspp),
+	.dspp = sm6125_dspp,
+	.pingpong_count = ARRAY_SIZE(sm6125_pp),
+	.pingpong = sm6125_pp,
+	.intf_count = ARRAY_SIZE(sm6125_intf),
+	.intf = sm6125_intf,
+	.vbif_count = ARRAY_SIZE(sdm845_vbif),
+	.vbif = sdm845_vbif,
+	.perf = &sm6125_perf_data,
+	.mdss_irqs = BIT(MDP_SSPP_TOP0_INTR) | \
+		     BIT(MDP_SSPP_TOP0_INTR2) | \
+		     BIT(MDP_SSPP_TOP0_HIST_INTR) | \
+		     BIT(MDP_INTF0_INTR) | \
+		     BIT(MDP_INTF1_INTR) | \
+		     BIT(MDP_INTF1_TEAR_INTR),
+};
+
+#endif
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 0de507d4d7b7..8a02bbdaae8a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -33,6 +33,9 @@
 #define VIG_SC7180_MASK \
 	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4))
 
+#define VIG_SM6125_MASK \
+	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE))
+
 #define VIG_SC7180_MASK_SDMA \
 	(VIG_SC7180_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
 
@@ -348,6 +351,8 @@ static const struct dpu_sspp_sub_blks sc7280_vig_sblk_0 =
 
 static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
 				_VIG_SBLK("0", 2, DPU_SSPP_SCALER_QSEED4);
+static const struct dpu_sspp_sub_blks sm6125_vig_sblk_0 =
+				_VIG_SBLK("0", 3, DPU_SSPP_SCALER_QSEED3LITE);
 
 static const struct dpu_sspp_sub_blks sm8250_vig_sblk_0 =
 				_VIG_SBLK("0", 5, DPU_SSPP_SCALER_QSEED4);
@@ -762,6 +767,7 @@ static const struct dpu_qos_lut_entry sc7180_qos_nrt[] = {
 
 #include "catalog/dpu_5_0_sm8150.h"
 #include "catalog/dpu_5_1_sc8180x.h"
+#include "catalog/dpu_5_4_sm6125.h"
 
 #include "catalog/dpu_6_0_sm8250.h"
 #include "catalog/dpu_6_2_sc7180.h"
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index b860784ade72..4314235cb2b8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -861,6 +861,7 @@ extern const struct dpu_mdss_cfg dpu_sc8180x_cfg;
 extern const struct dpu_mdss_cfg dpu_sm8250_cfg;
 extern const struct dpu_mdss_cfg dpu_sc7180_cfg;
 extern const struct dpu_mdss_cfg dpu_sm6115_cfg;
+extern const struct dpu_mdss_cfg dpu_sm6125_cfg;
 extern const struct dpu_mdss_cfg dpu_sm6350_cfg;
 extern const struct dpu_mdss_cfg dpu_qcm2290_cfg;
 extern const struct dpu_mdss_cfg dpu_sm6375_cfg;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index aa8499de1b9f..a1c7ffb6dffb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1305,6 +1305,7 @@ static const struct of_device_id dpu_dt_match[] = {
 	{ .compatible = "qcom,sc8180x-dpu", .data = &dpu_sc8180x_cfg, },
 	{ .compatible = "qcom,sc8280xp-dpu", .data = &dpu_sc8280xp_cfg, },
 	{ .compatible = "qcom,sm6115-dpu", .data = &dpu_sm6115_cfg, },
+	{ .compatible = "qcom,sm6125-dpu", .data = &dpu_sm6125_cfg, },
 	{ .compatible = "qcom,sm6350-dpu", .data = &dpu_sm6350_cfg, },
 	{ .compatible = "qcom,sm6375-dpu", .data = &dpu_sm6375_cfg, },
 	{ .compatible = "qcom,sm8150-dpu", .data = &dpu_sm8150_cfg, },

-- 
2.41.0

[PATCH 09/15] drm/msm/mdss: Add SM6125 support

[Marijn Suijten]

SM6125's UBWC hardware decoder is version 3.0, and supports decoding
UBWC 1.0.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 05648c910c68..bf68bae23264 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -559,6 +559,13 @@ static const struct msm_mdss_data sm6115_data = {
 	.ubwc_static = 0x11f,
 };
 
+static const struct msm_mdss_data sm6125_data = {
+	.ubwc_version = UBWC_1_0,
+	.ubwc_dec_version = UBWC_3_0,
+	.ubwc_swizzle = 1,
+	.highest_bank_bit = 1,
+};
+
 static const struct msm_mdss_data sm8250_data = {
 	.ubwc_version = UBWC_4_0,
 	.ubwc_dec_version = UBWC_4_0,
@@ -579,6 +586,7 @@ static const struct of_device_id mdss_dt_match[] = {
 	{ .compatible = "qcom,sc8180x-mdss", .data = &sc8180x_data },
 	{ .compatible = "qcom,sc8280xp-mdss", .data = &sc8280xp_data },
 	{ .compatible = "qcom,sm6115-mdss", .data = &sm6115_data },
+	{ .compatible = "qcom,sm6125-mdss", .data = &sm6125_data },
 	{ .compatible = "qcom,sm6350-mdss", .data = &sm6350_data },
 	{ .compatible = "qcom,sm6375-mdss", .data = &sm6350_data },
 	{ .compatible = "qcom,sm8150-mdss", .data = &sm8150_data },

-- 
2.41.0

[PATCH 10/15] dt-bindings: msm: dsi-phy-14nm: Document SM6125 variant

[Marijn Suijten]

Document availability of the 14nm DSI PHY on SM6125.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
index a43e11d3b00d..60b590f21138 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
@@ -18,6 +18,7 @@ properties:
       - qcom,dsi-phy-14nm
       - qcom,dsi-phy-14nm-2290
       - qcom,dsi-phy-14nm-660
+      - qcom,dsi-phy-14nm-6125
       - qcom,dsi-phy-14nm-8953
 
   reg:

-- 
2.41.0

[PATCH 11/15] drm/msm/dsi: Add 14nm phy configuration for SM6125

[Marijn Suijten]

SM6125 features only a single PHY (despite a secondary PHY PLL source
being available to the disp_cc_mdss_pclk0_clk_src clock), and downstream
sources for this "trinket" SoC do not define the typical "vcca"
regulator to be available nor used.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c      |  2 ++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h      |  1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 15 +++++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index 9d5795c58a98..8688ed502dcf 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -559,6 +559,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
 	  .data = &dsi_phy_14nm_2290_cfgs },
 	{ .compatible = "qcom,dsi-phy-14nm-660",
 	  .data = &dsi_phy_14nm_660_cfgs },
+	{ .compatible = "qcom,dsi-phy-14nm-6125",
+	  .data = &dsi_phy_14nm_6125_cfgs },
 	{ .compatible = "qcom,dsi-phy-14nm-8953",
 	  .data = &dsi_phy_14nm_8953_cfgs },
 #endif
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index 8b640d174785..ebf915f5e6c6 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -52,6 +52,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_14nm_2290_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
index 3ce45b023e63..5d43c9ec69ae 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
@@ -1068,6 +1068,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = {
 	.num_dsi_phy = 2,
 };
 
+const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs = {
+	.has_phy_lane = true,
+	.ops = {
+		.enable = dsi_14nm_phy_enable,
+		.disable = dsi_14nm_phy_disable,
+		.pll_init = dsi_pll_14nm_init,
+		.save_pll_state = dsi_14nm_pll_save_state,
+		.restore_pll_state = dsi_14nm_pll_restore_state,
+	},
+	.min_pll_rate = VCO_MIN_RATE,
+	.max_pll_rate = VCO_MAX_RATE,
+	.io_start = { 0x5e94400 },
+	.num_dsi_phy = 1,
+};
+
 const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs = {
 	.has_phy_lane = true,
 	.regulator_data = dsi_phy_14nm_17mA_regulators,

-- 
2.41.0

[PATCH 12/15] arm64: dts: qcom: sm6125: Switch fixed xo_board clock to RPM XO clock

[Marijn Suijten]

We have a working RPM XO clock; no other driver except rpmcc should be
parenting directly to the fixed-factor xo_board clock nor should it be
reachable by that global name.  Remove the name to that effect, so that
every clock relation is explicitly defined in DTS.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 arch/arm64/boot/dts/qcom/sm6125.dtsi | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
index 722dde560bec..edb03508dba3 100644
--- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
@@ -22,7 +22,6 @@ xo_board: xo-board {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
 			clock-frequency = <19200000>;
-			clock-output-names = "xo_board";
 		};
 
 		sleep_clk: sleep-clk {
@@ -306,6 +305,8 @@ rpm_requests: rpm-requests {
 			rpmcc: clock-controller {
 				compatible = "qcom,rpmcc-sm6125", "qcom,rpmcc";
 				#clock-cells = <1>;
+				clocks = <&xo_board>;
+				clock-names = "xo";
 			};
 
 			rpmpd: power-controller {
@@ -713,7 +714,7 @@ sdhc_1: mmc@4744000 {
 
 			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
 				 <&gcc GCC_SDCC1_APPS_CLK>,
-				 <&xo_board>;
+				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
 			clock-names = "iface", "core", "xo";
 			iommus = <&apps_smmu 0x160 0x0>;
 
@@ -740,7 +741,7 @@ sdhc_2: mmc@4784000 {
 
 			clocks = <&gcc GCC_SDCC2_AHB_CLK>,
 				 <&gcc GCC_SDCC2_APPS_CLK>,
-				 <&xo_board>;
+				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
 			clock-names = "iface", "core", "xo";
 			iommus = <&apps_smmu 0x180 0x0>;
 

-- 
2.41.0

[PATCH 13/15] arm64: dts: qcom: sm6125: Add dispcc node

[Marijn Suijten]

Enable and configure the dispcc node on SM6125 for consumption by MDSS
later on.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 arch/arm64/boot/dts/qcom/sm6125.dtsi | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
index edb03508dba3..7d78b4e48ebe 100644
--- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
@@ -3,6 +3,7 @@
  * Copyright (c) 2021, Martin Botka <martin.botka@somainline.org>
  */
 
+#include <dt-bindings/clock/qcom,dispcc-sm6125.h>
 #include <dt-bindings/clock/qcom,gcc-sm6125.h>
 #include <dt-bindings/clock/qcom,rpmcc.h>
 #include <dt-bindings/dma/qcom-gpi.h>
@@ -1203,6 +1204,28 @@ sram@4690000 {
 			reg = <0x04690000 0x10000>;
 		};
 
+		dispcc: clock-controller@5f00000 {
+			compatible = "qcom,sm6125-dispcc";
+			reg = <0x05f00000 0x20000>;
+			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
+				 <0>,
+				 <0>,
+				 <0>,
+				 <0>,
+				 <0>,
+				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>;
+			clock-names = "bi_tcxo",
+				      "gcc_disp_gpll0_div_clk_src",
+				      "dsi0_phy_pll_out_byteclk",
+				      "dsi0_phy_pll_out_dsiclk",
+				      "dsi1_phy_pll_out_dsiclk",
+				      "dp_phy_pll_link_clk",
+				      "dp_phy_pll_vco_div_clk";
+			power-domains = <&rpmpd SM6125_VDDCX>;
+			#clock-cells = <1>;
+			#power-domain-cells = <1>;
+		};
+
 		apps_smmu: iommu@c600000 {
 			compatible = "qcom,sm6125-smmu-500", "qcom,smmu-500", "arm,mmu-500";
 			reg = <0x0c600000 0x80000>;

-- 
2.41.0

[PATCH 14/15] arm64: dts: qcom: sm6125: Add display hardware nodes

[Marijn Suijten]

Add the DT nodes that describe the MDSS hardware on SM6125, containing
one MDP (display controller) together with a single DSI and DSI PHY.  No
DisplayPort support is added for now.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 arch/arm64/boot/dts/qcom/sm6125.dtsi | 190 ++++++++++++++++++++++++++++++++++-
 1 file changed, 186 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
index 7d78b4e48ebe..6852dacf54e6 100644
--- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
@@ -1204,16 +1204,198 @@ sram@4690000 {
 			reg = <0x04690000 0x10000>;
 		};
 
+		mdss: display-subsystem@5e00000 {
+			compatible = "qcom,sm6125-mdss";
+			reg = <0x05e00000 0x1000>;
+			reg-names = "mdss";
+
+			power-domains = <&dispcc MDSS_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 186 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+
+			iommus = <&apps_smmu 0x400 0x0>;
+
+			status = "disabled";
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			mdss_mdp: display-controller@5e01000 {
+				compatible = "qcom,sm6125-dpu";
+				reg = <0x05e01000 0x83208>,
+				      <0x05eb0000 0x2008>;
+				reg-names = "mdp", "vbif";
+
+				clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
+					 <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&dispcc DISP_CC_MDSS_ROT_CLK>,
+					 <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
+					 <&dispcc DISP_CC_MDSS_MDP_CLK>,
+					 <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+				clock-names = "bus",
+					      "iface",
+					      "rot",
+					      "lut",
+					      "core",
+					      "vsync";
+				assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+				assigned-clock-rates = <19200000>;
+
+				operating-points-v2 = <&mdp_opp_table>;
+				power-domains = <&rpmpd SM6125_VDDCX>;
+
+				interrupt-parent = <&mdss>;
+				interrupts = <0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						dpu_intf1_out: endpoint {
+							remote-endpoint = <&mdss_dsi0_in>;
+						};
+					};
+				};
+
+				mdp_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					opp-192000000 {
+						opp-hz = /bits/ 64 <192000000>;
+						required-opps = <&rpmpd_opp_low_svs>;
+					};
+
+					opp-256000000 {
+						opp-hz = /bits/ 64 <256000000>;
+						required-opps = <&rpmpd_opp_svs>;
+					};
+
+					opp-307200000 {
+						opp-hz = /bits/ 64 <307200000>;
+						required-opps = <&rpmpd_opp_svs_plus>;
+					};
+
+					opp-384000000 {
+						opp-hz = /bits/ 64 <384000000>;
+						required-opps = <&rpmpd_opp_nom>;
+					};
+
+					opp-400000000 {
+						opp-hz = /bits/ 64 <400000000>;
+						required-opps = <&rpmpd_opp_turbo>;
+					};
+				};
+			};
+
+			mdss_dsi0: dsi@5e94000 {
+				compatible = "qcom,sm6125-dsi-ctrl", "qcom,mdss-dsi-ctrl";
+				reg = <0x05e94000 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupt-parent = <&mdss>;
+				interrupts = <4>;
+
+				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";
+
+				assigned-clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK_SRC>,
+						  <&dispcc DISP_CC_MDSS_PCLK0_CLK_SRC>;
+				assigned-clock-parents = <&mdss_dsi0_phy 0>, <&mdss_dsi0_phy 1>;
+
+				operating-points-v2 = <&dsi_opp_table>;
+				power-domains = <&rpmpd SM6125_VDDMX>;
+
+				phys = <&mdss_dsi0_phy>;
+				phy-names = "dsi";
+
+				status = "disabled";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						mdss_dsi0_in: endpoint {
+							remote-endpoint = <&dpu_intf1_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+						mdss_dsi0_out: endpoint {
+						};
+					};
+				};
+
+				dsi_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					opp-164000000 {
+						opp-hz = /bits/ 64 <164000000>;
+						required-opps = <&rpmpd_opp_low_svs>;
+					};
+
+					opp-187500000 {
+						opp-hz = /bits/ 64 <187500000>;
+						required-opps = <&rpmpd_opp_svs>;
+					};
+				};
+			};
+
+			mdss_dsi0_phy: phy@5e94400 {
+				compatible = "qcom,dsi-phy-14nm-6125";
+				reg = <0x05e94400 0x100>,
+				      <0x05e94500 0x300>,
+				      <0x05e94800 0x188>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmcc RPM_SMD_XO_CLK_SRC>;
+				clock-names = "iface", "ref";
+
+				status = "disabled";
+			};
+		};
+
 		dispcc: clock-controller@5f00000 {
 			compatible = "qcom,sm6125-dispcc";
 			reg = <0x05f00000 0x20000>;
 			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
+				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
+				 <&mdss_dsi0_phy 0>,
+				 <&mdss_dsi0_phy 1>,
 				 <0>,
 				 <0>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>;
+				 <0>;
 			clock-names = "bi_tcxo",
 				      "gcc_disp_gpll0_div_clk_src",
 				      "dsi0_phy_pll_out_byteclk",

-- 
2.41.0

[PATCH 15/15] arm64: dts: qcom: sm6125-seine: Configure MDSS, DSI and panel

[Marijn Suijten]

Enable MDSS and DSI, and configure the Samsung SOFEF01-M ams597ut01
6.0" 1080x2520 panel.

Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 .../dts/qcom/sm6125-sony-xperia-seine-pdx201.dts   | 59 ++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm6125-sony-xperia-seine-pdx201.dts b/arch/arm64/boot/dts/qcom/sm6125-sony-xperia-seine-pdx201.dts
index 9f8a9ef398a2..bdf7c15f9b83 100644
--- a/arch/arm64/boot/dts/qcom/sm6125-sony-xperia-seine-pdx201.dts
+++ b/arch/arm64/boot/dts/qcom/sm6125-sony-xperia-seine-pdx201.dts
@@ -179,6 +179,43 @@ &i2c3 {
 	/* Cirrus Logic CS35L41 boosted audio amplifier @ 40 */
 };
 
+&mdss {
+	status = "okay";
+};
+
+&mdss_dsi0 {
+	vdda-supply = <&pm6125_l18>;
+	status = "okay";
+
+	panel@0 {
+		compatible = "samsung,sofef01-m-ams597ut01";
+		reg = <0>;
+
+		reset-gpios = <&tlmm 90 GPIO_ACTIVE_LOW>;
+
+		vddio-supply = <&pm6125_l12>;
+
+		pinctrl-0 = <&sde_dsi_active &sde_te_active_sleep>;
+		pinctrl-1 = <&sde_dsi_sleep &sde_te_active_sleep>;
+		pinctrl-names = "default", "sleep";
+
+		port {
+			panel_in: endpoint {
+				remote-endpoint = <&mdss_dsi0_out>;
+			};
+		};
+	};
+};
+
+&mdss_dsi0_out {
+	remote-endpoint = <&panel_in>;
+	data-lanes = <0 1 2 3>;
+};
+
+&mdss_dsi0_phy {
+	status = "okay";
+};
+
 &pm6125_adc {
 	pinctrl-names = "default";
 	pinctrl-0 = <&camera_flash_therm &emmc_ufs_therm &rf_pa1_therm>;
@@ -469,6 +506,28 @@ vol_down_n: vol-down-n-state {
 		drive-strength = <2>;
 		bias-disable;
 	};
+
+	sde_te_active_sleep: sde-te-active-sleep-state {
+		pins = "gpio89";
+		function = "mdp_vsync";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+
+	sde_dsi_active: sde-dsi-active-state {
+		pins = "gpio90";
+		function = "gpio";
+		drive-strength = <8>;
+		bias-disable;
+	};
+
+	sde_dsi_sleep: sde-dsi-sleep-state {
+		pins = "gpio90";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+
 };
 
 &usb3 {

-- 
2.41.0

Re: [PATCH 00/15] drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel

[Konrad Dybcio]

On 24.06.2023 02:40, Marijn Suijten wrote:
> Bring up the SM6125 DPU now that all preliminary series (such as INTF
> TE) have been merged (for me to test the hardware properly)
We should not repeat the same mistake in the future.. Finding a
balance between releasing early and releasing what we can declare
working and tested code is hard, but we waaaaaaaay overstayed on
this one..

Konrad
, and most
> other conflicting work (barring ongoing catalog *improvements*) has made
> its way in as well or is still being discussed.

> 
> The second part of the series complements that by immediately utilizing
> this hardware in DT, and even enabling the MDSS/DSI nodes complete with
> a 6.0" 1080x2520 panel for Sony's Seine PDX201 (Xperia 10 II).
> 
> The last patch ("sm6125-seine: Configure MDSS, DSI and panel") depends
> on (an impending v2 of) my Sony panel collection series [1].
> 
> [1]: https://lore.kernel.org/linux-arm-msm/20230521-drm-panels-sony-v1-0-541c341d6bee@somainline.org/
> 
> ---
> Marijn Suijten (15):
>       arm64: dts: qcom: sm6125: Sort spmi_bus node numerically by reg
>       dt-bindings: clock: qcom,dispcc-sm6125: Remove unused GCC_DISP_AHB_CLK
>       dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock
>       dt-bindings: clock: qcom,dispcc-sm6125: Allow power-domains property
>       dt-bindings: display/msm: dsi-controller-main: Document SM6125
>       dt-bindings: display/msm: sc7180-dpu: Describe SM6125
>       dt-bindings: display/msm: Add SM6125 MDSS
>       drm/msm/dpu: Add SM6125 support
>       drm/msm/mdss: Add SM6125 support
>       dt-bindings: msm: dsi-phy-14nm: Document SM6125 variant
>       drm/msm/dsi: Add 14nm phy configuration for SM6125
>       arm64: dts: qcom: sm6125: Switch fixed xo_board clock to RPM XO clock
>       arm64: dts: qcom: sm6125: Add dispcc node
>       arm64: dts: qcom: sm6125: Add display hardware nodes
>       arm64: dts: qcom: sm6125-seine: Configure MDSS, DSI and panel
> 
>  .../bindings/clock/qcom,dispcc-sm6125.yaml         |  17 +-
>  .../bindings/display/msm/dsi-controller-main.yaml  |   2 +
>  .../bindings/display/msm/dsi-phy-14nm.yaml         |   1 +
>  .../bindings/display/msm/qcom,sc7180-dpu.yaml      |   1 +
>  .../bindings/display/msm/qcom,sm6125-mdss.yaml     | 206 +++++++++++++++++
>  .../dts/qcom/sm6125-sony-xperia-seine-pdx201.dts   |  59 +++++
>  arch/arm64/boot/dts/qcom/sm6125.dtsi               | 244 +++++++++++++++++++--
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h | 173 +++++++++++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     |   6 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |   1 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |   1 +
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c              |   2 +
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h              |   1 +
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c         |  15 ++
>  drivers/gpu/drm/msm/msm_mdss.c                     |   8 +
>  15 files changed, 712 insertions(+), 25 deletions(-)
> ---
> base-commit: 8d2be868b42c08290509c60515865f4de24ea704
> change-id: 20230624-sm6125-dpu-aedc9637ee7b
> 
> Best regards,

Re: [PATCH 01/15] arm64: dts: qcom: sm6125: Sort spmi_bus node numerically by reg

[Konrad Dybcio]

On 24.06.2023 02:40, Marijn Suijten wrote:
> This node has always resided in the wrong spot, making it somewhat
> harder to contribute new node entries while maintaining proper sorting
> around it.  Move the node up to sit after hsusb_phy1 where it maintains
> proper numerial
numerical

sorting on the (first of its many) reg address property.
> 
> Fixes: cff4bbaf2a2d ("arm64: dts: qcom: Add support for SM6125")
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm64/boot/dts/qcom/sm6125.dtsi | 38 ++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> index a596baa6ce3e..722dde560bec 100644
> --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> @@ -679,6 +679,24 @@ hsusb_phy1: phy@1613000 {
>  			status = "disabled";
>  		};
>  
> +		spmi_bus: spmi@1c40000 {
> +			compatible = "qcom,spmi-pmic-arb";
> +			reg = <0x01c40000 0x1100>,
> +			      <0x01e00000 0x2000000>,
> +			      <0x03e00000 0x100000>,
> +			      <0x03f00000 0xa0000>,
> +			      <0x01c0a000 0x26000>;
> +			reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
> +			interrupt-names = "periph_irq";
> +			interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>;
> +			qcom,ee = <0>;
> +			qcom,channel = <0>;
> +			#address-cells = <2>;
> +			#size-cells = <0>;
> +			interrupt-controller;
> +			#interrupt-cells = <4>;
> +		};
> +
>  		rpm_msg_ram: sram@45f0000 {
>  			compatible = "qcom,rpm-msg-ram";
>  			reg = <0x045f0000 0x7000>;
> @@ -1184,27 +1202,9 @@ sram@4690000 {
>  			reg = <0x04690000 0x10000>;
>  		};
>  
> -		spmi_bus: spmi@1c40000 {
> -			compatible = "qcom,spmi-pmic-arb";
> -			reg = <0x01c40000 0x1100>,
> -			      <0x01e00000 0x2000000>,
> -			      <0x03e00000 0x100000>,
> -			      <0x03f00000 0xa0000>,
> -			      <0x01c0a000 0x26000>;
> -			reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
> -			interrupt-names = "periph_irq";
> -			interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>;
> -			qcom,ee = <0>;
> -			qcom,channel = <0>;
> -			#address-cells = <2>;
> -			#size-cells = <0>;
> -			interrupt-controller;
> -			#interrupt-cells = <4>;
> -		};
> -
>  		apps_smmu: iommu@c600000 {
>  			compatible = "qcom,sm6125-smmu-500", "qcom,smmu-500", "arm,mmu-500";
> -			reg = <0xc600000 0x80000>;
> +			reg = <0x0c600000 0x80000>;
>  			interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,
> 

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Konrad Dybcio]

On 24.06.2023 02:41, Marijn Suijten wrote:
> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
> be passed from DT, and should be required by the bindings.
> 
> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
Ideally, you'd stick it at the bottom of the list, as the items: order
is part of the ABI

Konrad
>  Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> index 2acf487d8a2f..11ec154503a3 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> @@ -23,6 +23,7 @@ properties:
>    clocks:
>      items:
>        - description: Board XO source
> +      - description: GPLL0 div source from GCC
>        - description: Byte clock from DSI PHY0
>        - description: Pixel clock from DSI PHY0
>        - description: Pixel clock from DSI PHY1
> @@ -32,6 +33,7 @@ properties:
>    clock-names:
>      items:
>        - const: bi_tcxo
> +      - const: gcc_disp_gpll0_div_clk_src
>        - const: dsi0_phy_pll_out_byteclk
>        - const: dsi0_phy_pll_out_dsiclk
>        - const: dsi1_phy_pll_out_dsiclk
> @@ -65,12 +67,14 @@ examples:
>        compatible = "qcom,sm6125-dispcc";
>        reg = <0x5f00000 0x20000>;
>        clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> +               <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
>                 <&dsi0_phy 0>,
>                 <&dsi0_phy 1>,
>                 <&dsi1_phy 1>,
>                 <&dp_phy 0>,
>                 <&dp_phy 1>;
>        clock-names = "bi_tcxo",
> +                    "gcc_disp_gpll0_div_clk_src",
>                      "dsi0_phy_pll_out_byteclk",
>                      "dsi0_phy_pll_out_dsiclk",
>                      "dsi1_phy_pll_out_dsiclk",
> 

Re: [PATCH 08/15] drm/msm/dpu: Add SM6125 support

[Konrad Dybcio]

On 24.06.2023 02:41, Marijn Suijten wrote:
> Add definitions for the display hardware used on the Qualcomm SM6125
> platform.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
[...]

> +static const struct dpu_perf_cfg sm6125_perf_data = {
> +	.max_bw_low = 4100000,
> +	.max_bw_high = 4100000,
> +	.min_core_ib = 2400000,
> +	.min_llcc_ib = 800000,
While Dmitry will likely validate other values, I can tell you already
that this SoC has no LLCC.

Konrad
> +	.min_dram_ib = 800000,
> +	.min_prefill_lines = 24,
> +	.danger_lut_tbl = {0xf, 0xffff, 0x0},
> +	.safe_lut_tbl = {0xfff8, 0xf000, 0xffff},
> +	.qos_lut_tbl = {
> +		{.nentry = ARRAY_SIZE(sm8150_qos_linear),
> +		.entries = sm8150_qos_linear
> +		},
> +		{.nentry = ARRAY_SIZE(sc7180_qos_macrotile),
> +		.entries = sc7180_qos_macrotile
> +		},
> +		{.nentry = ARRAY_SIZE(sc7180_qos_nrt),
> +		.entries = sc7180_qos_nrt
> +		},
> +		/* TODO: macrotile-qseed is different from macrotile */
> +	},
> +	.cdp_cfg = {
> +		{.rd_enable = 1, .wr_enable = 1},
> +		{.rd_enable = 1, .wr_enable = 0}
> +	},
> +	.clk_inefficiency_factor = 105,
> +	.bw_inefficiency_factor = 120,
> +};
> +
> +const struct dpu_mdss_cfg dpu_sm6125_cfg = {
> +	.caps = &sm6125_dpu_caps,
> +	.ubwc = &sm6125_ubwc_cfg,
> +	.mdp_count = ARRAY_SIZE(sm6125_mdp),
> +	.mdp = sm6125_mdp,
> +	.ctl_count = ARRAY_SIZE(sm6125_ctl),
> +	.ctl = sm6125_ctl,
> +	.sspp_count = ARRAY_SIZE(sm6125_sspp),
> +	.sspp = sm6125_sspp,
> +	.mixer_count = ARRAY_SIZE(sm6125_lm),
> +	.mixer = sm6125_lm,
> +	.dspp_count = ARRAY_SIZE(sm6125_dspp),
> +	.dspp = sm6125_dspp,
> +	.pingpong_count = ARRAY_SIZE(sm6125_pp),
> +	.pingpong = sm6125_pp,
> +	.intf_count = ARRAY_SIZE(sm6125_intf),
> +	.intf = sm6125_intf,
> +	.vbif_count = ARRAY_SIZE(sdm845_vbif),
> +	.vbif = sdm845_vbif,
> +	.perf = &sm6125_perf_data,
> +	.mdss_irqs = BIT(MDP_SSPP_TOP0_INTR) | \
> +		     BIT(MDP_SSPP_TOP0_INTR2) | \
> +		     BIT(MDP_SSPP_TOP0_HIST_INTR) | \
> +		     BIT(MDP_INTF0_INTR) | \
> +		     BIT(MDP_INTF1_INTR) | \
> +		     BIT(MDP_INTF1_TEAR_INTR),
> +};
> +
> +#endif
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index 0de507d4d7b7..8a02bbdaae8a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -33,6 +33,9 @@
>  #define VIG_SC7180_MASK \
>  	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4))
>  
> +#define VIG_SM6125_MASK \
> +	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE))
> +
>  #define VIG_SC7180_MASK_SDMA \
>  	(VIG_SC7180_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
>  
> @@ -348,6 +351,8 @@ static const struct dpu_sspp_sub_blks sc7280_vig_sblk_0 =
>  
>  static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
>  				_VIG_SBLK("0", 2, DPU_SSPP_SCALER_QSEED4);
> +static const struct dpu_sspp_sub_blks sm6125_vig_sblk_0 =
> +				_VIG_SBLK("0", 3, DPU_SSPP_SCALER_QSEED3LITE);
>  
>  static const struct dpu_sspp_sub_blks sm8250_vig_sblk_0 =
>  				_VIG_SBLK("0", 5, DPU_SSPP_SCALER_QSEED4);
> @@ -762,6 +767,7 @@ static const struct dpu_qos_lut_entry sc7180_qos_nrt[] = {
>  
>  #include "catalog/dpu_5_0_sm8150.h"
>  #include "catalog/dpu_5_1_sc8180x.h"
> +#include "catalog/dpu_5_4_sm6125.h"
>  
>  #include "catalog/dpu_6_0_sm8250.h"
>  #include "catalog/dpu_6_2_sc7180.h"
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index b860784ade72..4314235cb2b8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -861,6 +861,7 @@ extern const struct dpu_mdss_cfg dpu_sc8180x_cfg;
>  extern const struct dpu_mdss_cfg dpu_sm8250_cfg;
>  extern const struct dpu_mdss_cfg dpu_sc7180_cfg;
>  extern const struct dpu_mdss_cfg dpu_sm6115_cfg;
> +extern const struct dpu_mdss_cfg dpu_sm6125_cfg;
>  extern const struct dpu_mdss_cfg dpu_sm6350_cfg;
>  extern const struct dpu_mdss_cfg dpu_qcm2290_cfg;
>  extern const struct dpu_mdss_cfg dpu_sm6375_cfg;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index aa8499de1b9f..a1c7ffb6dffb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1305,6 +1305,7 @@ static const struct of_device_id dpu_dt_match[] = {
>  	{ .compatible = "qcom,sc8180x-dpu", .data = &dpu_sc8180x_cfg, },
>  	{ .compatible = "qcom,sc8280xp-dpu", .data = &dpu_sc8280xp_cfg, },
>  	{ .compatible = "qcom,sm6115-dpu", .data = &dpu_sm6115_cfg, },
> +	{ .compatible = "qcom,sm6125-dpu", .data = &dpu_sm6125_cfg, },
>  	{ .compatible = "qcom,sm6350-dpu", .data = &dpu_sm6350_cfg, },
>  	{ .compatible = "qcom,sm6375-dpu", .data = &dpu_sm6375_cfg, },
>  	{ .compatible = "qcom,sm8150-dpu", .data = &dpu_sm8150_cfg, },
> 

Re: [PATCH 11/15] drm/msm/dsi: Add 14nm phy configuration for SM6125

[Konrad Dybcio]

On 24.06.2023 02:41, Marijn Suijten wrote:
> SM6125 features only a single PHY (despite a secondary PHY PLL source
> being available to the disp_cc_mdss_pclk0_clk_src clock), and downstream
> sources for this "trinket" SoC do not define the typical "vcca"
> regulator to be available nor used.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
The introduced ops are identical to 2290, modulo regulator..

But the regulator is absent on both (VDD_MX powers it instead), so
feel free to clean that up and reuse it ;)

Konrad
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c      |  2 ++
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h      |  1 +
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 15 +++++++++++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> index 9d5795c58a98..8688ed502dcf 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> @@ -559,6 +559,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
>  	  .data = &dsi_phy_14nm_2290_cfgs },
>  	{ .compatible = "qcom,dsi-phy-14nm-660",
>  	  .data = &dsi_phy_14nm_660_cfgs },
> +	{ .compatible = "qcom,dsi-phy-14nm-6125",
> +	  .data = &dsi_phy_14nm_6125_cfgs },
>  	{ .compatible = "qcom,dsi-phy-14nm-8953",
>  	  .data = &dsi_phy_14nm_8953_cfgs },
>  #endif
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> index 8b640d174785..ebf915f5e6c6 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> @@ -52,6 +52,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_2290_cfgs;
> +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs;
>  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> index 3ce45b023e63..5d43c9ec69ae 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> @@ -1068,6 +1068,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = {
>  	.num_dsi_phy = 2,
>  };
>  
> +const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs = {
> +	.has_phy_lane = true,
> +	.ops = {
> +		.enable = dsi_14nm_phy_enable,
> +		.disable = dsi_14nm_phy_disable,
> +		.pll_init = dsi_pll_14nm_init,
> +		.save_pll_state = dsi_14nm_pll_save_state,
> +		.restore_pll_state = dsi_14nm_pll_restore_state,
> +	},
> +	.min_pll_rate = VCO_MIN_RATE,
> +	.max_pll_rate = VCO_MAX_RATE,
> +	.io_start = { 0x5e94400 },
> +	.num_dsi_phy = 1,
> +};
> +
>  const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs = {
>  	.has_phy_lane = true,
>  	.regulator_data = dsi_phy_14nm_17mA_regulators,
> 

Re: [PATCH 12/15] arm64: dts: qcom: sm6125: Switch fixed xo_board clock to RPM XO clock

[Konrad Dybcio]

On 24.06.2023 02:41, Marijn Suijten wrote:
> We have a working RPM XO clock; no other driver except rpmcc should be
> parenting directly to the fixed-factor xo_board clock nor should it be
> reachable by that global name.  Remove the name to that effect, so that
> every clock relation is explicitly defined in DTS.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  arch/arm64/boot/dts/qcom/sm6125.dtsi | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> index 722dde560bec..edb03508dba3 100644
> --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> @@ -22,7 +22,6 @@ xo_board: xo-board {
>  			compatible = "fixed-clock";
>  			#clock-cells = <0>;
>  			clock-frequency = <19200000>;
> -			clock-output-names = "xo_board";
>  		};
>  
>  		sleep_clk: sleep-clk {
> @@ -306,6 +305,8 @@ rpm_requests: rpm-requests {
>  			rpmcc: clock-controller {
>  				compatible = "qcom,rpmcc-sm6125", "qcom,rpmcc";
>  				#clock-cells = <1>;
> +				clocks = <&xo_board>;
> +				clock-names = "xo";
>  			};
>  
>  			rpmpd: power-controller {
> @@ -713,7 +714,7 @@ sdhc_1: mmc@4744000 {
>  
>  			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
>  				 <&gcc GCC_SDCC1_APPS_CLK>,
> -				 <&xo_board>;
> +				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
>  			clock-names = "iface", "core", "xo";
>  			iommus = <&apps_smmu 0x160 0x0>;
>  
> @@ -740,7 +741,7 @@ sdhc_2: mmc@4784000 {
>  
>  			clocks = <&gcc GCC_SDCC2_AHB_CLK>,
>  				 <&gcc GCC_SDCC2_APPS_CLK>,
> -				 <&xo_board>;
> +				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
>  			clock-names = "iface", "core", "xo";
>  			iommus = <&apps_smmu 0x180 0x0>;
>  
> 

Re: [PATCH 13/15] arm64: dts: qcom: sm6125: Add dispcc node

[Konrad Dybcio]

On 24.06.2023 02:41, Marijn Suijten wrote:
> Enable and configure the dispcc node on SM6125 for consumption by MDSS
> later on.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  arch/arm64/boot/dts/qcom/sm6125.dtsi | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> index edb03508dba3..7d78b4e48ebe 100644
> --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> @@ -3,6 +3,7 @@
>   * Copyright (c) 2021, Martin Botka <martin.botka@somainline.org>
>   */
>  
> +#include <dt-bindings/clock/qcom,dispcc-sm6125.h>
>  #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>  #include <dt-bindings/clock/qcom,rpmcc.h>
>  #include <dt-bindings/dma/qcom-gpi.h>
> @@ -1203,6 +1204,28 @@ sram@4690000 {
>  			reg = <0x04690000 0x10000>;
>  		};
>  
> +		dispcc: clock-controller@5f00000 {
> +			compatible = "qcom,sm6125-dispcc";
> +			reg = <0x05f00000 0x20000>;
> +			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> +				 <0>,
are you..

> +				 <0>,
> +				 <0>,
> +				 <0>,
> +				 <0>,
> +				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>;
> +			clock-names = "bi_tcxo",
> +				      "gcc_disp_gpll0_div_clk_src",
..sure?

Konrad
> +				      "dsi0_phy_pll_out_byteclk",
> +				      "dsi0_phy_pll_out_dsiclk",
> +				      "dsi1_phy_pll_out_dsiclk",
> +				      "dp_phy_pll_link_clk",
> +				      "dp_phy_pll_vco_div_clk";
> +			power-domains = <&rpmpd SM6125_VDDCX>;
> +			#clock-cells = <1>;
> +			#power-domain-cells = <1>;
> +		};
> +
>  		apps_smmu: iommu@c600000 {
>  			compatible = "qcom,sm6125-smmu-500", "qcom,smmu-500", "arm,mmu-500";
>  			reg = <0x0c600000 0x80000>;
> 

Re: [PATCH 07/15] dt-bindings: display/msm: Add SM6125 MDSS

[Rob Herring]

On Sat, 24 Jun 2023 02:41:05 +0200, Marijn Suijten wrote:
> Document the SM6125 MDSS.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  .../bindings/display/msm/qcom,sm6125-mdss.yaml     | 206 +++++++++++++++++++++
>  1 file changed, 206 insertions(+)
> 

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:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,sm6125-mdss.example.dtb: dsi@5e94000: compatible: 'oneOf' conditional failed, one must be fixed:
	['qcom,sm6125-dsi-ctrl', 'qcom,mdss-dsi-ctrl'] is too long
	'qcom,sm6125-dsi-ctrl' is not one of ['qcom,apq8064-dsi-ctrl', 'qcom,msm8916-dsi-ctrl', 'qcom,msm8953-dsi-ctrl', 'qcom,msm8974-dsi-ctrl', 'qcom,msm8996-dsi-ctrl', 'qcom,msm8998-dsi-ctrl', 'qcom,qcm2290-dsi-ctrl', 'qcom,sc7180-dsi-ctrl', 'qcom,sc7280-dsi-ctrl', 'qcom,sdm660-dsi-ctrl', 'qcom,sdm845-dsi-ctrl', 'qcom,sm6115-dsi-ctrl', 'qcom,sm8150-dsi-ctrl', 'qcom,sm8250-dsi-ctrl', 'qcom,sm8350-dsi-ctrl', 'qcom,sm8450-dsi-ctrl', 'qcom,sm8550-dsi-ctrl']
	'qcom,sm6125-dsi-ctrl' is not one of ['qcom,dsi-ctrl-6g-qcm2290', 'qcom,mdss-dsi-ctrl']
	from schema $id: http://devicetree.org/schemas/display/msm/dsi-controller-main.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,sm6125-mdss.example.dtb: dsi@5e94000: Unevaluated properties are not allowed ('compatible' was unexpected)
	from schema $id: http://devicetree.org/schemas/display/msm/dsi-controller-main.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230624-sm6125-dpu-v1-7-1d5a638cebf2@somainline.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 14/15] arm64: dts: qcom: sm6125: Add display hardware nodes

[Konrad Dybcio]

On 24.06.2023 02:41, Marijn Suijten wrote:
> Add the DT nodes that describe the MDSS hardware on SM6125, containing
> one MDP (display controller) together with a single DSI and DSI PHY.  No
> DisplayPort support is added for now.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  arch/arm64/boot/dts/qcom/sm6125.dtsi | 190 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 186 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> index 7d78b4e48ebe..6852dacf54e6 100644
> --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> @@ -1204,16 +1204,198 @@ sram@4690000 {
>  			reg = <0x04690000 0x10000>;
>  		};
>  
> +		mdss: display-subsystem@5e00000 {
> +			compatible = "qcom,sm6125-mdss";
> +			reg = <0x05e00000 0x1000>;
> +			reg-names = "mdss";
> +
> +			power-domains = <&dispcc MDSS_GDSC>;
> +
> +			clocks = <&gcc GCC_DISP_AHB_CLK>,
> +				 <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +				 <&dispcc DISP_CC_MDSS_MDP_CLK>;
> +			clock-names = "iface", "ahb", "core";
One per line would be nice
> +
> +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +
> +			iommus = <&apps_smmu 0x400 0x0>;
> +
Every node has a different scramble of property orders :P

To roughly align with what I ask most people to do (and what I should
work on codifying and automating ehhh) would be:

[compat]
[reg]

[interrupt]

[clock]

[reset]
(btw there should be a MDSS_CORE reset at DISP_CC+0x2000, try writing 1 there
and see if MDSS dies)

[opp]
[pd]

[iommus]
[icc]

[phy]

[addresssizeranges]

[status]

> +			status = "disabled";
> +
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			mdss_mdp: display-controller@5e01000 {
> +				compatible = "qcom,sm6125-dpu";
> +				reg = <0x05e01000 0x83208>,
> +				      <0x05eb0000 0x2008>;
> +				reg-names = "mdp", "vbif";
> +
> +				clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> +					 <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +					 <&dispcc DISP_CC_MDSS_ROT_CLK>,
> +					 <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> +					 <&dispcc DISP_CC_MDSS_MDP_CLK>,
> +					 <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> +				clock-names = "bus",
> +					      "iface",
> +					      "rot",
> +					      "lut",
> +					      "core",
> +					      "vsync";
> +				assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> +				assigned-clock-rates = <19200000>;
> +
> +				operating-points-v2 = <&mdp_opp_table>;
> +				power-domains = <&rpmpd SM6125_VDDCX>;
> +
> +				interrupt-parent = <&mdss>;
> +				interrupts = <0>;
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						dpu_intf1_out: endpoint {
> +							remote-endpoint = <&mdss_dsi0_in>;
> +						};
> +					};
> +				};
> +
> +				mdp_opp_table: opp-table {
> +					compatible = "operating-points-v2";
> +
> +					opp-192000000 {
> +						opp-hz = /bits/ 64 <192000000>;
> +						required-opps = <&rpmpd_opp_low_svs>;
> +					};
> +
> +					opp-256000000 {
> +						opp-hz = /bits/ 64 <256000000>;
> +						required-opps = <&rpmpd_opp_svs>;
> +					};
> +
> +					opp-307200000 {
> +						opp-hz = /bits/ 64 <307200000>;
> +						required-opps = <&rpmpd_opp_svs_plus>;
> +					};
> +
> +					opp-384000000 {
> +						opp-hz = /bits/ 64 <384000000>;
> +						required-opps = <&rpmpd_opp_nom>;
> +					};
> +
> +					opp-400000000 {
> +						opp-hz = /bits/ 64 <400000000>;
> +						required-opps = <&rpmpd_opp_turbo>;
> +					};
> +				};
> +			};
> +
> +			mdss_dsi0: dsi@5e94000 {
> +				compatible = "qcom,sm6125-dsi-ctrl", "qcom,mdss-dsi-ctrl";
> +				reg = <0x05e94000 0x400>;
> +				reg-names = "dsi_ctrl";
> +
> +				interrupt-parent = <&mdss>;
> +				interrupts = <4>;
> +
> +				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";
> +
> +				assigned-clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK_SRC>,
> +						  <&dispcc DISP_CC_MDSS_PCLK0_CLK_SRC>;
> +				assigned-clock-parents = <&mdss_dsi0_phy 0>, <&mdss_dsi0_phy 1>;
> +
> +				operating-points-v2 = <&dsi_opp_table>;
> +				power-domains = <&rpmpd SM6125_VDDMX>;
CX

> +
> +				phys = <&mdss_dsi0_phy>;
> +				phy-names = "dsi";
> +
> +				status = "disabled";
> +
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						mdss_dsi0_in: endpoint {
> +							remote-endpoint = <&dpu_intf1_out>;
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +						mdss_dsi0_out: endpoint {
> +						};
> +					};
> +				};
> +
> +				dsi_opp_table: opp-table {
> +					compatible = "operating-points-v2";
> +
> +					opp-164000000 {
> +						opp-hz = /bits/ 64 <164000000>;
> +						required-opps = <&rpmpd_opp_low_svs>;
> +					};
> +
> +					opp-187500000 {
> +						opp-hz = /bits/ 64 <187500000>;
> +						required-opps = <&rpmpd_opp_svs>;
> +					};
> +				};
> +			};
> +
> +			mdss_dsi0_phy: phy@5e94400 {
> +				compatible = "qcom,dsi-phy-14nm-6125";
> +				reg = <0x05e94400 0x100>,
> +				      <0x05e94500 0x300>,
> +				      <0x05e94800 0x188>;
> +				reg-names = "dsi_phy",
> +					    "dsi_phy_lane",
> +					    "dsi_pll";
> +
> +				#clock-cells = <1>;
> +				#phy-cells = <0>;
> +
> +				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +					 <&rpmcc RPM_SMD_XO_CLK_SRC>;
> +				clock-names = "iface", "ref";
> +
> +				status = "disabled";
> +			};
> +		};
> +
>  		dispcc: clock-controller@5f00000 {
>  			compatible = "qcom,sm6125-dispcc";
>  			reg = <0x05f00000 0x20000>;
>  			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> +				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
> +				 <&mdss_dsi0_phy 0>,
> +				 <&mdss_dsi0_phy 1>,
>  				 <0>,
>  				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>;
> +				 <0>;
>  			clock-names = "bi_tcxo",
Wrong patch

Konrad
>  				      "gcc_disp_gpll0_div_clk_src",
>  				      "dsi0_phy_pll_out_byteclk",
> 

Re: [PATCH 15/15] arm64: dts: qcom: sm6125-seine: Configure MDSS, DSI and panel

[Konrad Dybcio]

On 24.06.2023 02:41, Marijn Suijten wrote:
> Enable MDSS and DSI, and configure the Samsung SOFEF01-M ams597ut01
> 6.0" 1080x2520 panel.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  .../dts/qcom/sm6125-sony-xperia-seine-pdx201.dts   | 59 ++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6125-sony-xperia-seine-pdx201.dts b/arch/arm64/boot/dts/qcom/sm6125-sony-xperia-seine-pdx201.dts
> index 9f8a9ef398a2..bdf7c15f9b83 100644
> --- a/arch/arm64/boot/dts/qcom/sm6125-sony-xperia-seine-pdx201.dts
> +++ b/arch/arm64/boot/dts/qcom/sm6125-sony-xperia-seine-pdx201.dts
> @@ -179,6 +179,43 @@ &i2c3 {
>  	/* Cirrus Logic CS35L41 boosted audio amplifier @ 40 */
>  };
>  
> +&mdss {
> +	status = "okay";
> +};
> +
> +&mdss_dsi0 {
> +	vdda-supply = <&pm6125_l18>;
> +	status = "okay";
> +
> +	panel@0 {
> +		compatible = "samsung,sofef01-m-ams597ut01";
> +		reg = <0>;
> +
> +		reset-gpios = <&tlmm 90 GPIO_ACTIVE_LOW>;
> +
> +		vddio-supply = <&pm6125_l12>;
> +
> +		pinctrl-0 = <&sde_dsi_active &sde_te_active_sleep>;
> +		pinctrl-1 = <&sde_dsi_sleep &sde_te_active_sleep>;
> +		pinctrl-names = "default", "sleep";
> +
> +		port {
> +			panel_in: endpoint {
> +				remote-endpoint = <&mdss_dsi0_out>;
> +			};
> +		};
> +	};
> +};
> +
> +&mdss_dsi0_out {
> +	remote-endpoint = <&panel_in>;
> +	data-lanes = <0 1 2 3>;
> +};
> +
> +&mdss_dsi0_phy {
> +	status = "okay";
> +};
> +
>  &pm6125_adc {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&camera_flash_therm &emmc_ufs_therm &rf_pa1_therm>;
> @@ -469,6 +506,28 @@ vol_down_n: vol-down-n-state {
>  		drive-strength = <2>;
>  		bias-disable;
>  	};
> +
> +	sde_te_active_sleep: sde-te-active-sleep-state {
> +		pins = "gpio89";
> +		function = "mdp_vsync";
> +		drive-strength = <2>;
> +		bias-pull-down;
> +	};
> +
> +	sde_dsi_active: sde-dsi-active-state {
> +		pins = "gpio90";
> +		function = "gpio";
> +		drive-strength = <8>;
> +		bias-disable;
> +	};
> +
> +	sde_dsi_sleep: sde-dsi-sleep-state {
> +		pins = "gpio90";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-down;
> +	};
s/sde/mdss as per Dmitry's recent request

Konrad
> +
>  };
>  
>  &usb3 {
> 

Re: [PATCH 02/15] dt-bindings: clock: qcom,dispcc-sm6125: Remove unused GCC_DISP_AHB_CLK

[Krzysztof Kozlowski]

On 24/06/2023 02:41, Marijn Suijten wrote:
> The downsteam driver for dispcc only ever gets and puts this clock
> without ever using it in the clocktree; this unnecessary workaround was
> never ported to mainline, hence the driver doesn't consume this clock
> and shouldn't be required by the bindings.
> 
> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---

In perfect would we would like to know whether hardware needs this clock
enabled/controlled, not whether some driver needs it. I understand
though that with lack of proper docs we rely on drivers, so:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Krzysztof Kozlowski]

On 24/06/2023 03:45, Konrad Dybcio wrote:
> On 24.06.2023 02:41, Marijn Suijten wrote:
>> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
>> be passed from DT, and should be required by the bindings.
>>
>> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>> ---
> Ideally, you'd stick it at the bottom of the list, as the items: order
> is part of the ABI

Yes, please add them to the end. Order is fixed.

Best regards,
Krzysztof

Re: [PATCH 04/15] dt-bindings: clock: qcom,dispcc-sm6125: Allow power-domains property

[Krzysztof Kozlowski]

On 24/06/2023 02:41, Marijn Suijten wrote:
> On SM6125 the dispcc block is gated behind VDDCX: allow this domain to
> be configured.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml | 5 +++++
>  1 file changed, 5 insertions(+)



Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 05/15] dt-bindings: display/msm: dsi-controller-main: Document SM6125

[Krzysztof Kozlowski]

On 24/06/2023 02:41, Marijn Suijten wrote:
> Document general compatibility of the DSI controller on SM6125.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 ++
>  1 file changed, 2 insertions(+)


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Krzysztof Kozlowski]

On 24/06/2023 02:41, Marijn Suijten wrote:
> SM6125 is identical to SM6375 except that while downstream also defines
> a throttle clock, its presence results in timeouts whereas SM6375
> requires it to not observe any timeouts.

Then it should not be allowed, so you need either "else:" block or
another "if: properties: compatible:" to disallow it. Because in current
patch it would be allowed.

Best regards,
Krzysztof

Re: [PATCH 07/15] dt-bindings: display/msm: Add SM6125 MDSS

[Krzysztof Kozlowski]

On 24/06/2023 02:41, Marijn Suijten wrote:
> Document the SM6125 MDSS.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 10/15] dt-bindings: msm: dsi-phy-14nm: Document SM6125 variant

[Krzysztof Kozlowski]

On 24/06/2023 02:41, Marijn Suijten wrote:
> Document availability of the 14nm DSI PHY on SM6125.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>  Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
> index a43e11d3b00d..60b590f21138 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
> @@ -18,6 +18,7 @@ properties:
>        - qcom,dsi-phy-14nm
>        - qcom,dsi-phy-14nm-2290
>        - qcom,dsi-phy-14nm-660
> +      - qcom,dsi-phy-14nm-6125

If there is going to be next version:
Should be rather ordered alphanumeric, so before 660.


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 10/15] dt-bindings: msm: dsi-phy-14nm: Document SM6125 variant

[Dmitry Baryshkov]

On 24/06/2023 03:41, Marijn Suijten wrote:
> Document availability of the 14nm DSI PHY on SM6125.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>   Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
> index a43e11d3b00d..60b590f21138 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
> @@ -18,6 +18,7 @@ properties:
>         - qcom,dsi-phy-14nm
>         - qcom,dsi-phy-14nm-2290
>         - qcom,dsi-phy-14nm-660
> +      - qcom,dsi-phy-14nm-6125

Should we start using standard scheme, so "qcom,sm6125-dsi-phy-14nm" ?

>         - qcom,dsi-phy-14nm-8953
-- 
With best wishes
Dmitry

Re: [PATCH 11/15] drm/msm/dsi: Add 14nm phy configuration for SM6125

[Dmitry Baryshkov]

On 24/06/2023 04:49, Konrad Dybcio wrote:
> On 24.06.2023 02:41, Marijn Suijten wrote:
>> SM6125 features only a single PHY (despite a secondary PHY PLL source
>> being available to the disp_cc_mdss_pclk0_clk_src clock), and downstream
>> sources for this "trinket" SoC do not define the typical "vcca"
>> regulator to be available nor used.
>>
>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>> ---
> The introduced ops are identical to 2290, modulo regulator..
> 
> But the regulator is absent on both (VDD_MX powers it instead), so
> feel free to clean that up and reuse it ;)

Could you please send a fix for qcm2290?

> 
> Konrad
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.c      |  2 ++
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.h      |  1 +
>>   drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 15 +++++++++++++++
>>   3 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> index 9d5795c58a98..8688ed502dcf 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>> @@ -559,6 +559,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
>>   	  .data = &dsi_phy_14nm_2290_cfgs },
>>   	{ .compatible = "qcom,dsi-phy-14nm-660",
>>   	  .data = &dsi_phy_14nm_660_cfgs },
>> +	{ .compatible = "qcom,dsi-phy-14nm-6125",
>> +	  .data = &dsi_phy_14nm_6125_cfgs },
>>   	{ .compatible = "qcom,dsi-phy-14nm-8953",
>>   	  .data = &dsi_phy_14nm_8953_cfgs },
>>   #endif
>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>> index 8b640d174785..ebf915f5e6c6 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>> @@ -52,6 +52,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_14nm_2290_cfgs;
>> +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs;
>>   extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
>> index 3ce45b023e63..5d43c9ec69ae 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
>> @@ -1068,6 +1068,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = {
>>   	.num_dsi_phy = 2,
>>   };
>>   
>> +const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs = {
>> +	.has_phy_lane = true,
>> +	.ops = {
>> +		.enable = dsi_14nm_phy_enable,
>> +		.disable = dsi_14nm_phy_disable,
>> +		.pll_init = dsi_pll_14nm_init,
>> +		.save_pll_state = dsi_14nm_pll_save_state,
>> +		.restore_pll_state = dsi_14nm_pll_restore_state,
>> +	},
>> +	.min_pll_rate = VCO_MIN_RATE,
>> +	.max_pll_rate = VCO_MAX_RATE,
>> +	.io_start = { 0x5e94400 },
>> +	.num_dsi_phy = 1,
>> +};
>> +
>>   const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs = {
>>   	.has_phy_lane = true,
>>   	.regulator_data = dsi_phy_14nm_17mA_regulators,
>>

-- 
With best wishes
Dmitry

Re: [PATCH 13/15] arm64: dts: qcom: sm6125: Add dispcc node

[Dmitry Baryshkov]

On 24/06/2023 03:41, Marijn Suijten wrote:
> Enable and configure the dispcc node on SM6125 for consumption by MDSS
> later on.
> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>   arch/arm64/boot/dts/qcom/sm6125.dtsi | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> index edb03508dba3..7d78b4e48ebe 100644
> --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> @@ -3,6 +3,7 @@
>    * Copyright (c) 2021, Martin Botka <martin.botka@somainline.org>
>    */
>   
> +#include <dt-bindings/clock/qcom,dispcc-sm6125.h>
>   #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>   #include <dt-bindings/clock/qcom,rpmcc.h>
>   #include <dt-bindings/dma/qcom-gpi.h>
> @@ -1203,6 +1204,28 @@ sram@4690000 {
>   			reg = <0x04690000 0x10000>;
>   		};
>   
> +		dispcc: clock-controller@5f00000 {
> +			compatible = "qcom,sm6125-dispcc";
> +			reg = <0x05f00000 0x20000>;
> +			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> +				 <0>,
> +				 <0>,
> +				 <0>,
> +				 <0>,
> +				 <0>,
> +				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>;

This clock is at the wrong position.

> +			clock-names = "bi_tcxo",
> +				      "gcc_disp_gpll0_div_clk_src",
> +				      "dsi0_phy_pll_out_byteclk",
> +				      "dsi0_phy_pll_out_dsiclk",
> +				      "dsi1_phy_pll_out_dsiclk",
> +				      "dp_phy_pll_link_clk",
> +				      "dp_phy_pll_vco_div_clk";
> +			power-domains = <&rpmpd SM6125_VDDCX>;
> +			#clock-cells = <1>;
> +			#power-domain-cells = <1>;
> +		};
> +
>   		apps_smmu: iommu@c600000 {
>   			compatible = "qcom,sm6125-smmu-500", "qcom,smmu-500", "arm,mmu-500";
>   			reg = <0x0c600000 0x80000>;
> 

-- 
With best wishes
Dmitry

Re: [PATCH 10/15] dt-bindings: msm: dsi-phy-14nm: Document SM6125 variant

[Krzysztof Kozlowski]

On 24/06/2023 15:48, Dmitry Baryshkov wrote:
> On 24/06/2023 03:41, Marijn Suijten wrote:
>> Document availability of the 14nm DSI PHY on SM6125.
>>
>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>> ---
>>   Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
>> index a43e11d3b00d..60b590f21138 100644
>> --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml
>> @@ -18,6 +18,7 @@ properties:
>>         - qcom,dsi-phy-14nm
>>         - qcom,dsi-phy-14nm-2290
>>         - qcom,dsi-phy-14nm-660
>> +      - qcom,dsi-phy-14nm-6125
> 
> Should we start using standard scheme, so "qcom,sm6125-dsi-phy-14nm" ?

I guess the earlier the better.

Best regards,
Krzysztof

Re: [PATCH 01/15] arm64: dts: qcom: sm6125: Sort spmi_bus node numerically by reg

[Marijn Suijten]

On 2023-06-24 03:43:21, Konrad Dybcio wrote:
> On 24.06.2023 02:40, Marijn Suijten wrote:
> > This node has always resided in the wrong spot, making it somewhat
> > harder to contribute new node entries while maintaining proper sorting
> > around it.  Move the node up to sit after hsusb_phy1 where it maintains
> > proper numerial
> numerical

Thanks.

> sorting on the (first of its many) reg address property.

Why was this continuation of the line not re-quoted?  Makes your reply
super-hard to read.

- Marijn

> > 
> > Fixes: cff4bbaf2a2d ("arm64: dts: qcom: Add support for SM6125")
> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> 
> Konrad
> >  arch/arm64/boot/dts/qcom/sm6125.dtsi | 38 ++++++++++++++++++------------------
> >  1 file changed, 19 insertions(+), 19 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > index a596baa6ce3e..722dde560bec 100644
> > --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > @@ -679,6 +679,24 @@ hsusb_phy1: phy@1613000 {
> >  			status = "disabled";
> >  		};
> >  
> > +		spmi_bus: spmi@1c40000 {
> > +			compatible = "qcom,spmi-pmic-arb";
> > +			reg = <0x01c40000 0x1100>,
> > +			      <0x01e00000 0x2000000>,
> > +			      <0x03e00000 0x100000>,
> > +			      <0x03f00000 0xa0000>,
> > +			      <0x01c0a000 0x26000>;
> > +			reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
> > +			interrupt-names = "periph_irq";
> > +			interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>;
> > +			qcom,ee = <0>;
> > +			qcom,channel = <0>;
> > +			#address-cells = <2>;
> > +			#size-cells = <0>;
> > +			interrupt-controller;
> > +			#interrupt-cells = <4>;
> > +		};
> > +
> >  		rpm_msg_ram: sram@45f0000 {
> >  			compatible = "qcom,rpm-msg-ram";
> >  			reg = <0x045f0000 0x7000>;
> > @@ -1184,27 +1202,9 @@ sram@4690000 {
> >  			reg = <0x04690000 0x10000>;
> >  		};
> >  
> > -		spmi_bus: spmi@1c40000 {
> > -			compatible = "qcom,spmi-pmic-arb";
> > -			reg = <0x01c40000 0x1100>,
> > -			      <0x01e00000 0x2000000>,
> > -			      <0x03e00000 0x100000>,
> > -			      <0x03f00000 0xa0000>,
> > -			      <0x01c0a000 0x26000>;
> > -			reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
> > -			interrupt-names = "periph_irq";
> > -			interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>;
> > -			qcom,ee = <0>;
> > -			qcom,channel = <0>;
> > -			#address-cells = <2>;
> > -			#size-cells = <0>;
> > -			interrupt-controller;
> > -			#interrupt-cells = <4>;
> > -		};
> > -
> >  		apps_smmu: iommu@c600000 {
> >  			compatible = "qcom,sm6125-smmu-500", "qcom,smmu-500", "arm,mmu-500";
> > -			reg = <0xc600000 0x80000>;
> > +			reg = <0x0c600000 0x80000>;
> >  			interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>,
> >  				     <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>,
> >  				     <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,
> > 

Re: [PATCH 00/15] drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel

[Marijn Suijten]

On 2023-06-24 03:42:46, Konrad Dybcio wrote:
> On 24.06.2023 02:40, Marijn Suijten wrote:
> > Bring up the SM6125 DPU now that all preliminary series (such as INTF
> > TE) have been merged (for me to test the hardware properly)
> We should not repeat the same mistake in the future.. Finding a
> balance between releasing early and releasing what we can declare
> working and tested code is hard, but we waaaaaaaay overstayed on
> this one..

I don't understand what you mean by "mistake" at all.  Yes the DPU
catalog portion of this series sat in my local branch for a very long
time.  Yes it had to be rebased on top of conflicts many many times.

However, that time has also been used to fix and extend DPU where
necessary, instead of submitting a half-broken or half-incomplete
catalog entry...

Re "we overstayed": you could have asked to clean up and send my patch,
so I don't take this as a mistake on my part as you are completely aware
of my time schedule ;)

> Konrad
> , and most

Also here, don't forget to re-quote my message if you break half-way in
the line.

> > other conflicting work (barring ongoing catalog *improvements*) has made
> > its way in as well or is still being discussed.
> 
> > 
> > The second part of the series complements that by immediately utilizing
> > this hardware in DT, and even enabling the MDSS/DSI nodes complete with
> > a 6.0" 1080x2520 panel for Sony's Seine PDX201 (Xperia 10 II).
> > 
> > The last patch ("sm6125-seine: Configure MDSS, DSI and panel") depends
> > on (an impending v2 of) my Sony panel collection series [1].
> > 
> > [1]: https://lore.kernel.org/linux-arm-msm/20230521-drm-panels-sony-v1-0-541c341d6bee@somainline.org/
> > 
> > ---
> > Marijn Suijten (15):
> >       arm64: dts: qcom: sm6125: Sort spmi_bus node numerically by reg
> >       dt-bindings: clock: qcom,dispcc-sm6125: Remove unused GCC_DISP_AHB_CLK
> >       dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock
> >       dt-bindings: clock: qcom,dispcc-sm6125: Allow power-domains property
> >       dt-bindings: display/msm: dsi-controller-main: Document SM6125
> >       dt-bindings: display/msm: sc7180-dpu: Describe SM6125
> >       dt-bindings: display/msm: Add SM6125 MDSS
> >       drm/msm/dpu: Add SM6125 support
> >       drm/msm/mdss: Add SM6125 support
> >       dt-bindings: msm: dsi-phy-14nm: Document SM6125 variant
> >       drm/msm/dsi: Add 14nm phy configuration for SM6125
> >       arm64: dts: qcom: sm6125: Switch fixed xo_board clock to RPM XO clock
> >       arm64: dts: qcom: sm6125: Add dispcc node
> >       arm64: dts: qcom: sm6125: Add display hardware nodes
> >       arm64: dts: qcom: sm6125-seine: Configure MDSS, DSI and panel
> > 
> >  .../bindings/clock/qcom,dispcc-sm6125.yaml         |  17 +-
> >  .../bindings/display/msm/dsi-controller-main.yaml  |   2 +
> >  .../bindings/display/msm/dsi-phy-14nm.yaml         |   1 +
> >  .../bindings/display/msm/qcom,sc7180-dpu.yaml      |   1 +
> >  .../bindings/display/msm/qcom,sm6125-mdss.yaml     | 206 +++++++++++++++++
> >  .../dts/qcom/sm6125-sony-xperia-seine-pdx201.dts   |  59 +++++
> >  arch/arm64/boot/dts/qcom/sm6125.dtsi               | 244 +++++++++++++++++++--
> >  .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h | 173 +++++++++++++++
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     |   6 +
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |   1 +
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |   1 +
> >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c              |   2 +
> >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h              |   1 +
> >  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c         |  15 ++
> >  drivers/gpu/drm/msm/msm_mdss.c                     |   8 +
> >  15 files changed, 712 insertions(+), 25 deletions(-)
> > ---
> > base-commit: 8d2be868b42c08290509c60515865f4de24ea704
> > change-id: 20230624-sm6125-dpu-aedc9637ee7b
> > 
> > Best regards,

Re: [PATCH 14/15] arm64: dts: qcom: sm6125: Add display hardware nodes

[Marijn Suijten]

On 2023-06-24 04:05:07, Konrad Dybcio wrote:
> On 24.06.2023 02:41, Marijn Suijten wrote:
> > Add the DT nodes that describe the MDSS hardware on SM6125, containing
> > one MDP (display controller) together with a single DSI and DSI PHY.  No
> > DisplayPort support is added for now.
> > 
> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm6125.dtsi | 190 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 186 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm6125.dtsi b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > index 7d78b4e48ebe..6852dacf54e6 100644
> > --- a/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm6125.dtsi
> > @@ -1204,16 +1204,198 @@ sram@4690000 {
> >  			reg = <0x04690000 0x10000>;
> >  		};
> >  
> > +		mdss: display-subsystem@5e00000 {
> > +			compatible = "qcom,sm6125-mdss";
> > +			reg = <0x05e00000 0x1000>;
> > +			reg-names = "mdss";
> > +
> > +			power-domains = <&dispcc MDSS_GDSC>;
> > +
> > +			clocks = <&gcc GCC_DISP_AHB_CLK>,
> > +				 <&dispcc DISP_CC_MDSS_AHB_CLK>,
> > +				 <&dispcc DISP_CC_MDSS_MDP_CLK>;
> > +			clock-names = "iface", "ahb", "core";
> One per line would be nice

Sure, it's all over the place elsewhere though.

> > +
> > +			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
> > +			interrupt-controller;
> > +			#interrupt-cells = <1>;
> > +
> > +			iommus = <&apps_smmu 0x400 0x0>;
> > +
> Every node has a different scramble of property orders :P
> 
> To roughly align with what I ask most people to do (and what I should
> work on codifying and automating ehhh) would be:

Exactly.  Until this automated - or even documented - there is barely
any point to manually deal with this.

> [compat]
> [reg]
> 
> [interrupt]

Also interrupt-cells here, since that's only related to
interrupt-controller; and not interrupts=<>;?

> 
> [clock]
> 
> [reset]
> (btw there should be a MDSS_CORE reset at DISP_CC+0x2000, try writing 1 there
> and see if MDSS dies)
> 
> [opp]
> [pd]
> 
> [iommus]
> [icc]
> 
> [phy]
> 
> [addresssizeranges]
> 
> [status]

Sure, have reordered them like this for v2.

> 
> > +			status = "disabled";
> > +
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +			ranges;
> > +
> > +			mdss_mdp: display-controller@5e01000 {
> > +				compatible = "qcom,sm6125-dpu";
> > +				reg = <0x05e01000 0x83208>,
> > +				      <0x05eb0000 0x2008>;
> > +				reg-names = "mdp", "vbif";
> > +
> > +				clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> > +					 <&dispcc DISP_CC_MDSS_AHB_CLK>,
> > +					 <&dispcc DISP_CC_MDSS_ROT_CLK>,
> > +					 <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> > +					 <&dispcc DISP_CC_MDSS_MDP_CLK>,
> > +					 <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> > +				clock-names = "bus",
> > +					      "iface",
> > +					      "rot",
> > +					      "lut",
> > +					      "core",
> > +					      "vsync";
> > +				assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> > +				assigned-clock-rates = <19200000>;
> > +
> > +				operating-points-v2 = <&mdp_opp_table>;
> > +				power-domains = <&rpmpd SM6125_VDDCX>;
> > +
> > +				interrupt-parent = <&mdss>;
> > +				interrupts = <0>;
> > +
> > +				ports {
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > +
> > +					port@0 {
> > +						reg = <0>;
> > +						dpu_intf1_out: endpoint {
> > +							remote-endpoint = <&mdss_dsi0_in>;
> > +						};
> > +					};
> > +				};
> > +
> > +				mdp_opp_table: opp-table {
> > +					compatible = "operating-points-v2";
> > +
> > +					opp-192000000 {
> > +						opp-hz = /bits/ 64 <192000000>;
> > +						required-opps = <&rpmpd_opp_low_svs>;
> > +					};
> > +
> > +					opp-256000000 {
> > +						opp-hz = /bits/ 64 <256000000>;
> > +						required-opps = <&rpmpd_opp_svs>;
> > +					};
> > +
> > +					opp-307200000 {
> > +						opp-hz = /bits/ 64 <307200000>;
> > +						required-opps = <&rpmpd_opp_svs_plus>;
> > +					};
> > +
> > +					opp-384000000 {
> > +						opp-hz = /bits/ 64 <384000000>;
> > +						required-opps = <&rpmpd_opp_nom>;
> > +					};
> > +
> > +					opp-400000000 {
> > +						opp-hz = /bits/ 64 <400000000>;
> > +						required-opps = <&rpmpd_opp_turbo>;
> > +					};
> > +				};
> > +			};
> > +
> > +			mdss_dsi0: dsi@5e94000 {
> > +				compatible = "qcom,sm6125-dsi-ctrl", "qcom,mdss-dsi-ctrl";
> > +				reg = <0x05e94000 0x400>;
> > +				reg-names = "dsi_ctrl";
> > +
> > +				interrupt-parent = <&mdss>;
> > +				interrupts = <4>;
> > +
> > +				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";
> > +
> > +				assigned-clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK_SRC>,
> > +						  <&dispcc DISP_CC_MDSS_PCLK0_CLK_SRC>;
> > +				assigned-clock-parents = <&mdss_dsi0_phy 0>, <&mdss_dsi0_phy 1>;
> > +
> > +				operating-points-v2 = <&dsi_opp_table>;
> > +				power-domains = <&rpmpd SM6125_VDDMX>;
> CX

Downstream says MX (and only on the phy... but it is the dsi node that
defines the corresponding rpmpd opps).

Do you have a source to back this up?

> 
> > +
> > +				phys = <&mdss_dsi0_phy>;
> > +				phy-names = "dsi";
> > +
> > +				status = "disabled";
> > +
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				ports {
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > +
> > +					port@0 {
> > +						reg = <0>;
> > +						mdss_dsi0_in: endpoint {
> > +							remote-endpoint = <&dpu_intf1_out>;
> > +						};
> > +					};
> > +
> > +					port@1 {
> > +						reg = <1>;
> > +						mdss_dsi0_out: endpoint {
> > +						};
> > +					};
> > +				};
> > +
> > +				dsi_opp_table: opp-table {
> > +					compatible = "operating-points-v2";
> > +
> > +					opp-164000000 {
> > +						opp-hz = /bits/ 64 <164000000>;
> > +						required-opps = <&rpmpd_opp_low_svs>;
> > +					};
> > +
> > +					opp-187500000 {
> > +						opp-hz = /bits/ 64 <187500000>;
> > +						required-opps = <&rpmpd_opp_svs>;
> > +					};
> > +				};
> > +			};
> > +
> > +			mdss_dsi0_phy: phy@5e94400 {
> > +				compatible = "qcom,dsi-phy-14nm-6125";
> > +				reg = <0x05e94400 0x100>,
> > +				      <0x05e94500 0x300>,
> > +				      <0x05e94800 0x188>;
> > +				reg-names = "dsi_phy",
> > +					    "dsi_phy_lane",
> > +					    "dsi_pll";
> > +
> > +				#clock-cells = <1>;
> > +				#phy-cells = <0>;
> > +
> > +				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> > +					 <&rpmcc RPM_SMD_XO_CLK_SRC>;
> > +				clock-names = "iface", "ref";
> > +
> > +				status = "disabled";
> > +			};
> > +		};
> > +
> >  		dispcc: clock-controller@5f00000 {
> >  			compatible = "qcom,sm6125-dispcc";
> >  			reg = <0x05f00000 0x20000>;
> >  			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> > +				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
> > +				 <&mdss_dsi0_phy 0>,
> > +				 <&mdss_dsi0_phy 1>,
> >  				 <0>,
> >  				 <0>,
> > -				 <0>,
> > -				 <0>,
> > -				 <0>,
> > -				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>;
> > +				 <0>;
> >  			clock-names = "bi_tcxo",
> Wrong patch

Yup, as said by many folks in many places.  It seems the --fixup went to
the wrong commit, even though the clock-names were fixup'ed into the
right one...

Have fixed this for v2.

- Marijn

> 
> Konrad
> >  				      "gcc_disp_gpll0_div_clk_src",
> >  				      "dsi0_phy_pll_out_byteclk",
> > 

Re: [PATCH 15/15] arm64: dts: qcom: sm6125-seine: Configure MDSS, DSI and panel

[Marijn Suijten]

On 2023-06-24 04:06:04, Konrad Dybcio wrote:
<snip>
> > +	sde_dsi_sleep: sde-dsi-sleep-state {
> > +		pins = "gpio90";
> > +		function = "gpio";
> > +		drive-strength = <2>;
> > +		bias-pull-down;
> > +	};
> s/sde/mdss as per Dmitry's recent request

Makes sense, SDE doesn't mean anything on mainline.

- Marijn

Re: [PATCH 02/15] dt-bindings: clock: qcom,dispcc-sm6125: Remove unused GCC_DISP_AHB_CLK

[Marijn Suijten]

On 2023-06-24 11:08:30, Krzysztof Kozlowski wrote:
> On 24/06/2023 02:41, Marijn Suijten wrote:
> > The downsteam driver for dispcc only ever gets and puts this clock
> > without ever using it in the clocktree; this unnecessary workaround was
> > never ported to mainline, hence the driver doesn't consume this clock
> > and shouldn't be required by the bindings.
> > 
> > Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> 
> In perfect would we would like to know whether hardware needs this clock
> enabled/controlled, not whether some driver needs it. I understand
> though that with lack of proper docs we rely on drivers, so:

It might only use this to figure out if those clocks have already probed
or are available.  The logic goes as follows:

	clk = devm_clk_get(&pdev->dev, "cfg_ahb_clk");
	if (IS_ERR(clk)) {
		if (PTR_ERR(clk) != -EPROBE_DEFER)
			dev_err(&pdev->dev, "Unable to get ahb clock handle\n");
		return PTR_ERR(clk);
	}
	devm_clk_put(&pdev->dev, clk);

Nothing else uses/parents cfg_ahb_clk.  Maybe with clk_ignore_unused or
similar, it gets turned on but never off again?

Indeed, a lack of documentation and comment from the manufacturer makes
it impossible to know, and ignoring it (as the driver already does)
works just fine.

> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks!

- Marijn

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-24 03:45:02, Konrad Dybcio wrote:
> On 24.06.2023 02:41, Marijn Suijten wrote:
> > The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
> > be passed from DT, and should be required by the bindings.
> > 
> > Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> Ideally, you'd stick it at the bottom of the list, as the items: order
> is part of the ABI

This isn't an ABI break, as this driver nor its bindings require/declare
a fixed order: they declare a relation between clocks and clock-names.

This orders the GCC clock just like other dispccs.  And the previous
patch dropped the unused cfg_ahb_clk from the bindings, so all bets are
off anyway.

- Marijn

> 
> Konrad
> >  Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> > index 2acf487d8a2f..11ec154503a3 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> > @@ -23,6 +23,7 @@ properties:
> >    clocks:
> >      items:
> >        - description: Board XO source
> > +      - description: GPLL0 div source from GCC
> >        - description: Byte clock from DSI PHY0
> >        - description: Pixel clock from DSI PHY0
> >        - description: Pixel clock from DSI PHY1
> > @@ -32,6 +33,7 @@ properties:
> >    clock-names:
> >      items:
> >        - const: bi_tcxo
> > +      - const: gcc_disp_gpll0_div_clk_src
> >        - const: dsi0_phy_pll_out_byteclk
> >        - const: dsi0_phy_pll_out_dsiclk
> >        - const: dsi1_phy_pll_out_dsiclk
> > @@ -65,12 +67,14 @@ examples:
> >        compatible = "qcom,sm6125-dispcc";
> >        reg = <0x5f00000 0x20000>;
> >        clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> > +               <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
> >                 <&dsi0_phy 0>,
> >                 <&dsi0_phy 1>,
> >                 <&dsi1_phy 1>,
> >                 <&dp_phy 0>,
> >                 <&dp_phy 1>;
> >        clock-names = "bi_tcxo",
> > +                    "gcc_disp_gpll0_div_clk_src",
> >                      "dsi0_phy_pll_out_byteclk",
> >                      "dsi0_phy_pll_out_dsiclk",
> >                      "dsi1_phy_pll_out_dsiclk",
> > 

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-24 11:08:54, Krzysztof Kozlowski wrote:
> On 24/06/2023 03:45, Konrad Dybcio wrote:
> > On 24.06.2023 02:41, Marijn Suijten wrote:
> >> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
> >> be passed from DT, and should be required by the bindings.
> >>
> >> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
> >> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> >> ---
> > Ideally, you'd stick it at the bottom of the list, as the items: order
> > is part of the ABI
> 
> Yes, please add them to the end. Order is fixed.

Disagreed for bindings that declare clock-names and when the driver
adheres to it, see my reply to Konrad's message.

- Marijn

Re: [PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Marijn Suijten]

On 2023-06-24 11:12:52, Krzysztof Kozlowski wrote:
> On 24/06/2023 02:41, Marijn Suijten wrote:
> > SM6125 is identical to SM6375 except that while downstream also defines
> > a throttle clock, its presence results in timeouts whereas SM6375
> > requires it to not observe any timeouts.
> 
> Then it should not be allowed, so you need either "else:" block or
> another "if: properties: compatible:" to disallow it. Because in current
> patch it would be allowed.

That means this binding is wrong/incomplete for all other SoCs then.
clock(-name)s has 6 items, and sets `minItems: 6`.  Only for sm6375-dpu
does it set `minItems: 7`, but an else case is missing.

Shall I send a Fixes: ed41005f5b7c ("dt-bindings: display/msm:
sc7180-dpu: Describe SM6350 and SM6375") for that, and should maxItems:
6 be the default under clock(-name)s or in an else:?

- Marijn

Re: [PATCH 08/15] drm/msm/dpu: Add SM6125 support

[Marijn Suijten]

On 2023-06-24 03:47:27, Konrad Dybcio wrote:
> On 24.06.2023 02:41, Marijn Suijten wrote:
> > Add definitions for the display hardware used on the Qualcomm SM6125
> > platform.
> > 
> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> [...]
> 
> > +static const struct dpu_perf_cfg sm6125_perf_data = {
> > +	.max_bw_low = 4100000,
> > +	.max_bw_high = 4100000,
> > +	.min_core_ib = 2400000,
> > +	.min_llcc_ib = 800000,
> While Dmitry will likely validate other values

Lovely.

> I can tell you already that this SoC has no LLCC.

Copy-paste error on downstream?

https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/LA.UM.9.11.c25/arch/arm64/boot/dts/qcom/trinket-sde.dtsi#L146

- Marijn

> 
> Konrad
> > +	.min_dram_ib = 800000,
> > +	.min_prefill_lines = 24,
> > +	.danger_lut_tbl = {0xf, 0xffff, 0x0},
> > +	.safe_lut_tbl = {0xfff8, 0xf000, 0xffff},
> > +	.qos_lut_tbl = {
> > +		{.nentry = ARRAY_SIZE(sm8150_qos_linear),
> > +		.entries = sm8150_qos_linear
> > +		},
> > +		{.nentry = ARRAY_SIZE(sc7180_qos_macrotile),
> > +		.entries = sc7180_qos_macrotile
> > +		},
> > +		{.nentry = ARRAY_SIZE(sc7180_qos_nrt),
> > +		.entries = sc7180_qos_nrt
> > +		},
> > +		/* TODO: macrotile-qseed is different from macrotile */
> > +	},
> > +	.cdp_cfg = {
> > +		{.rd_enable = 1, .wr_enable = 1},
> > +		{.rd_enable = 1, .wr_enable = 0}
> > +	},
> > +	.clk_inefficiency_factor = 105,
> > +	.bw_inefficiency_factor = 120,
> > +};
> > +
> > +const struct dpu_mdss_cfg dpu_sm6125_cfg = {
> > +	.caps = &sm6125_dpu_caps,
> > +	.ubwc = &sm6125_ubwc_cfg,
> > +	.mdp_count = ARRAY_SIZE(sm6125_mdp),
> > +	.mdp = sm6125_mdp,
> > +	.ctl_count = ARRAY_SIZE(sm6125_ctl),
> > +	.ctl = sm6125_ctl,
> > +	.sspp_count = ARRAY_SIZE(sm6125_sspp),
> > +	.sspp = sm6125_sspp,
> > +	.mixer_count = ARRAY_SIZE(sm6125_lm),
> > +	.mixer = sm6125_lm,
> > +	.dspp_count = ARRAY_SIZE(sm6125_dspp),
> > +	.dspp = sm6125_dspp,
> > +	.pingpong_count = ARRAY_SIZE(sm6125_pp),
> > +	.pingpong = sm6125_pp,
> > +	.intf_count = ARRAY_SIZE(sm6125_intf),
> > +	.intf = sm6125_intf,
> > +	.vbif_count = ARRAY_SIZE(sdm845_vbif),
> > +	.vbif = sdm845_vbif,
> > +	.perf = &sm6125_perf_data,
> > +	.mdss_irqs = BIT(MDP_SSPP_TOP0_INTR) | \
> > +		     BIT(MDP_SSPP_TOP0_INTR2) | \
> > +		     BIT(MDP_SSPP_TOP0_HIST_INTR) | \
> > +		     BIT(MDP_INTF0_INTR) | \
> > +		     BIT(MDP_INTF1_INTR) | \
> > +		     BIT(MDP_INTF1_TEAR_INTR),
> > +};
> > +
> > +#endif
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > index 0de507d4d7b7..8a02bbdaae8a 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > @@ -33,6 +33,9 @@
> >  #define VIG_SC7180_MASK \
> >  	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4))
> >  
> > +#define VIG_SM6125_MASK \
> > +	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE))
> > +
> >  #define VIG_SC7180_MASK_SDMA \
> >  	(VIG_SC7180_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
> >  
> > @@ -348,6 +351,8 @@ static const struct dpu_sspp_sub_blks sc7280_vig_sblk_0 =
> >  
> >  static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
> >  				_VIG_SBLK("0", 2, DPU_SSPP_SCALER_QSEED4);
> > +static const struct dpu_sspp_sub_blks sm6125_vig_sblk_0 =
> > +				_VIG_SBLK("0", 3, DPU_SSPP_SCALER_QSEED3LITE);
> >  
> >  static const struct dpu_sspp_sub_blks sm8250_vig_sblk_0 =
> >  				_VIG_SBLK("0", 5, DPU_SSPP_SCALER_QSEED4);
> > @@ -762,6 +767,7 @@ static const struct dpu_qos_lut_entry sc7180_qos_nrt[] = {
> >  
> >  #include "catalog/dpu_5_0_sm8150.h"
> >  #include "catalog/dpu_5_1_sc8180x.h"
> > +#include "catalog/dpu_5_4_sm6125.h"
> >  
> >  #include "catalog/dpu_6_0_sm8250.h"
> >  #include "catalog/dpu_6_2_sc7180.h"
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > index b860784ade72..4314235cb2b8 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > @@ -861,6 +861,7 @@ extern const struct dpu_mdss_cfg dpu_sc8180x_cfg;
> >  extern const struct dpu_mdss_cfg dpu_sm8250_cfg;
> >  extern const struct dpu_mdss_cfg dpu_sc7180_cfg;
> >  extern const struct dpu_mdss_cfg dpu_sm6115_cfg;
> > +extern const struct dpu_mdss_cfg dpu_sm6125_cfg;
> >  extern const struct dpu_mdss_cfg dpu_sm6350_cfg;
> >  extern const struct dpu_mdss_cfg dpu_qcm2290_cfg;
> >  extern const struct dpu_mdss_cfg dpu_sm6375_cfg;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index aa8499de1b9f..a1c7ffb6dffb 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -1305,6 +1305,7 @@ static const struct of_device_id dpu_dt_match[] = {
> >  	{ .compatible = "qcom,sc8180x-dpu", .data = &dpu_sc8180x_cfg, },
> >  	{ .compatible = "qcom,sc8280xp-dpu", .data = &dpu_sc8280xp_cfg, },
> >  	{ .compatible = "qcom,sm6115-dpu", .data = &dpu_sm6115_cfg, },
> > +	{ .compatible = "qcom,sm6125-dpu", .data = &dpu_sm6125_cfg, },
> >  	{ .compatible = "qcom,sm6350-dpu", .data = &dpu_sm6350_cfg, },
> >  	{ .compatible = "qcom,sm6375-dpu", .data = &dpu_sm6375_cfg, },
> >  	{ .compatible = "qcom,sm8150-dpu", .data = &dpu_sm8150_cfg, },
> > 

Re: [PATCH 11/15] drm/msm/dsi: Add 14nm phy configuration for SM6125

[Marijn Suijten]

On 2023-06-24 03:49:25, Konrad Dybcio wrote:
> On 24.06.2023 02:41, Marijn Suijten wrote:
> > SM6125 features only a single PHY (despite a secondary PHY PLL source
> > being available to the disp_cc_mdss_pclk0_clk_src clock), and downstream
> > sources for this "trinket" SoC do not define the typical "vcca"
> > regulator to be available nor used.
> > 
> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> The introduced ops are identical to 2290, modulo regulator..

Sure, I can create a "drop unused regulators from 14nm qcm2290 config"
and a second "reuse qcm2290 14nm dsi phy for sm6125" patch, instead of
this one.

> But the regulator is absent on both (VDD_MX powers it instead), so

In the DT patch you requested me to use CX instead of MX... Which one is
it?

Also note that I moved it from DSI PHY to DSI0 because that's where the
rpmpd opps reside.

- Marijn

> feel free to clean that up and reuse it ;)

> 
> Konrad
> >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c      |  2 ++
> >  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h      |  1 +
> >  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 15 +++++++++++++++
> >  3 files changed, 18 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> > index 9d5795c58a98..8688ed502dcf 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> > @@ -559,6 +559,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
> >  	  .data = &dsi_phy_14nm_2290_cfgs },
> >  	{ .compatible = "qcom,dsi-phy-14nm-660",
> >  	  .data = &dsi_phy_14nm_660_cfgs },
> > +	{ .compatible = "qcom,dsi-phy-14nm-6125",
> > +	  .data = &dsi_phy_14nm_6125_cfgs },
> >  	{ .compatible = "qcom,dsi-phy-14nm-8953",
> >  	  .data = &dsi_phy_14nm_8953_cfgs },
> >  #endif
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > index 8b640d174785..ebf915f5e6c6 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> > @@ -52,6 +52,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_2290_cfgs;
> > +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs;
> >  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> > index 3ce45b023e63..5d43c9ec69ae 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> > @@ -1068,6 +1068,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = {
> >  	.num_dsi_phy = 2,
> >  };
> >  
> > +const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs = {
> > +	.has_phy_lane = true,
> > +	.ops = {
> > +		.enable = dsi_14nm_phy_enable,
> > +		.disable = dsi_14nm_phy_disable,
> > +		.pll_init = dsi_pll_14nm_init,
> > +		.save_pll_state = dsi_14nm_pll_save_state,
> > +		.restore_pll_state = dsi_14nm_pll_restore_state,
> > +	},
> > +	.min_pll_rate = VCO_MIN_RATE,
> > +	.max_pll_rate = VCO_MAX_RATE,
> > +	.io_start = { 0x5e94400 },
> > +	.num_dsi_phy = 1,
> > +};
> > +
> >  const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs = {
> >  	.has_phy_lane = true,
> >  	.regulator_data = dsi_phy_14nm_17mA_regulators,
> > 

Re: [PATCH 08/15] drm/msm/dpu: Add SM6125 support

[Konrad Dybcio]

On 25.06.2023 22:19, Marijn Suijten wrote:
> On 2023-06-24 03:47:27, Konrad Dybcio wrote:
>> On 24.06.2023 02:41, Marijn Suijten wrote:
>>> Add definitions for the display hardware used on the Qualcomm SM6125
>>> platform.
>>>
>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>>> ---
>> [...]
>>
>>> +static const struct dpu_perf_cfg sm6125_perf_data = {
>>> +	.max_bw_low = 4100000,
>>> +	.max_bw_high = 4100000,
>>> +	.min_core_ib = 2400000,
>>> +	.min_llcc_ib = 800000,
>> While Dmitry will likely validate other values
> 
> Lovely.
> 
>> I can tell you already that this SoC has no LLCC.
> 
> Copy-paste error on downstream?
> 
> https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/blob/LA.UM.9.11.c25/arch/arm64/boot/dts/qcom/trinket-sde.dtsi#L146
> 
> - Marijn
Yes.

This code is bogus anyway and is just supposed to vote on DDR_FREQ_MIN

Konrad
> 
>>
>> Konrad
>>> +	.min_dram_ib = 800000,
>>> +	.min_prefill_lines = 24,
>>> +	.danger_lut_tbl = {0xf, 0xffff, 0x0},
>>> +	.safe_lut_tbl = {0xfff8, 0xf000, 0xffff},
>>> +	.qos_lut_tbl = {
>>> +		{.nentry = ARRAY_SIZE(sm8150_qos_linear),
>>> +		.entries = sm8150_qos_linear
>>> +		},
>>> +		{.nentry = ARRAY_SIZE(sc7180_qos_macrotile),
>>> +		.entries = sc7180_qos_macrotile
>>> +		},
>>> +		{.nentry = ARRAY_SIZE(sc7180_qos_nrt),
>>> +		.entries = sc7180_qos_nrt
>>> +		},
>>> +		/* TODO: macrotile-qseed is different from macrotile */
>>> +	},
>>> +	.cdp_cfg = {
>>> +		{.rd_enable = 1, .wr_enable = 1},
>>> +		{.rd_enable = 1, .wr_enable = 0}
>>> +	},
>>> +	.clk_inefficiency_factor = 105,
>>> +	.bw_inefficiency_factor = 120,
>>> +};
>>> +
>>> +const struct dpu_mdss_cfg dpu_sm6125_cfg = {
>>> +	.caps = &sm6125_dpu_caps,
>>> +	.ubwc = &sm6125_ubwc_cfg,
>>> +	.mdp_count = ARRAY_SIZE(sm6125_mdp),
>>> +	.mdp = sm6125_mdp,
>>> +	.ctl_count = ARRAY_SIZE(sm6125_ctl),
>>> +	.ctl = sm6125_ctl,
>>> +	.sspp_count = ARRAY_SIZE(sm6125_sspp),
>>> +	.sspp = sm6125_sspp,
>>> +	.mixer_count = ARRAY_SIZE(sm6125_lm),
>>> +	.mixer = sm6125_lm,
>>> +	.dspp_count = ARRAY_SIZE(sm6125_dspp),
>>> +	.dspp = sm6125_dspp,
>>> +	.pingpong_count = ARRAY_SIZE(sm6125_pp),
>>> +	.pingpong = sm6125_pp,
>>> +	.intf_count = ARRAY_SIZE(sm6125_intf),
>>> +	.intf = sm6125_intf,
>>> +	.vbif_count = ARRAY_SIZE(sdm845_vbif),
>>> +	.vbif = sdm845_vbif,
>>> +	.perf = &sm6125_perf_data,
>>> +	.mdss_irqs = BIT(MDP_SSPP_TOP0_INTR) | \
>>> +		     BIT(MDP_SSPP_TOP0_INTR2) | \
>>> +		     BIT(MDP_SSPP_TOP0_HIST_INTR) | \
>>> +		     BIT(MDP_INTF0_INTR) | \
>>> +		     BIT(MDP_INTF1_INTR) | \
>>> +		     BIT(MDP_INTF1_TEAR_INTR),
>>> +};
>>> +
>>> +#endif
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> index 0de507d4d7b7..8a02bbdaae8a 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>>> @@ -33,6 +33,9 @@
>>>  #define VIG_SC7180_MASK \
>>>  	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4))
>>>  
>>> +#define VIG_SM6125_MASK \
>>> +	(VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE))
>>> +
>>>  #define VIG_SC7180_MASK_SDMA \
>>>  	(VIG_SC7180_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
>>>  
>>> @@ -348,6 +351,8 @@ static const struct dpu_sspp_sub_blks sc7280_vig_sblk_0 =
>>>  
>>>  static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
>>>  				_VIG_SBLK("0", 2, DPU_SSPP_SCALER_QSEED4);
>>> +static const struct dpu_sspp_sub_blks sm6125_vig_sblk_0 =
>>> +				_VIG_SBLK("0", 3, DPU_SSPP_SCALER_QSEED3LITE);
>>>  
>>>  static const struct dpu_sspp_sub_blks sm8250_vig_sblk_0 =
>>>  				_VIG_SBLK("0", 5, DPU_SSPP_SCALER_QSEED4);
>>> @@ -762,6 +767,7 @@ static const struct dpu_qos_lut_entry sc7180_qos_nrt[] = {
>>>  
>>>  #include "catalog/dpu_5_0_sm8150.h"
>>>  #include "catalog/dpu_5_1_sc8180x.h"
>>> +#include "catalog/dpu_5_4_sm6125.h"
>>>  
>>>  #include "catalog/dpu_6_0_sm8250.h"
>>>  #include "catalog/dpu_6_2_sc7180.h"
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>>> index b860784ade72..4314235cb2b8 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>>> @@ -861,6 +861,7 @@ extern const struct dpu_mdss_cfg dpu_sc8180x_cfg;
>>>  extern const struct dpu_mdss_cfg dpu_sm8250_cfg;
>>>  extern const struct dpu_mdss_cfg dpu_sc7180_cfg;
>>>  extern const struct dpu_mdss_cfg dpu_sm6115_cfg;
>>> +extern const struct dpu_mdss_cfg dpu_sm6125_cfg;
>>>  extern const struct dpu_mdss_cfg dpu_sm6350_cfg;
>>>  extern const struct dpu_mdss_cfg dpu_qcm2290_cfg;
>>>  extern const struct dpu_mdss_cfg dpu_sm6375_cfg;
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> index aa8499de1b9f..a1c7ffb6dffb 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>>> @@ -1305,6 +1305,7 @@ static const struct of_device_id dpu_dt_match[] = {
>>>  	{ .compatible = "qcom,sc8180x-dpu", .data = &dpu_sc8180x_cfg, },
>>>  	{ .compatible = "qcom,sc8280xp-dpu", .data = &dpu_sc8280xp_cfg, },
>>>  	{ .compatible = "qcom,sm6115-dpu", .data = &dpu_sm6115_cfg, },
>>> +	{ .compatible = "qcom,sm6125-dpu", .data = &dpu_sm6125_cfg, },
>>>  	{ .compatible = "qcom,sm6350-dpu", .data = &dpu_sm6350_cfg, },
>>>  	{ .compatible = "qcom,sm6375-dpu", .data = &dpu_sm6375_cfg, },
>>>  	{ .compatible = "qcom,sm8150-dpu", .data = &dpu_sm8150_cfg, },
>>>

Re: [PATCH 00/15] drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel

[Konrad Dybcio]

On 25.06.2023 21:18, Marijn Suijten wrote:
> On 2023-06-24 03:42:46, Konrad Dybcio wrote:
>> On 24.06.2023 02:40, Marijn Suijten wrote:
>>> Bring up the SM6125 DPU now that all preliminary series (such as INTF
>>> TE) have been merged (for me to test the hardware properly)
>> We should not repeat the same mistake in the future.. Finding a
>> balance between releasing early and releasing what we can declare
>> working and tested code is hard, but we waaaaaaaay overstayed on
>> this one..
> 
> I don't understand what you mean by "mistake" at all.  Yes the DPU
> catalog portion of this series sat in my local branch for a very long
> time.  Yes it had to be rebased on top of conflicts many many times.
> 
> However, that time has also been used to fix and extend DPU where
> necessary, instead of submitting a half-broken or half-incomplete
> catalog entry...
> 
> Re "we overstayed": you could have asked to clean up and send my patch,
> so I don't take this as a mistake on my part as you are completely aware
> of my time schedule ;)
I didn't mean to pick on you. I just wanted to emphasize that a more
upstream-forward approach would have saved us quite some time on the
rebasing and cleaning-up front.



> 
>> Konrad
>> , and most
> 
> Also here, don't forget to re-quote my message if you break half-way in
> the line.
Ugh. All the time I've been doing this I thought thunderfox was smart
enough to do it for me. Apparently not and you're the 1st one to point
that out.

Konrad
> 
>>> other conflicting work (barring ongoing catalog *improvements*) has made
>>> its way in as well or is still being discussed.
>>
>>>
>>> The second part of the series complements that by immediately utilizing
>>> this hardware in DT, and even enabling the MDSS/DSI nodes complete with
>>> a 6.0" 1080x2520 panel for Sony's Seine PDX201 (Xperia 10 II).
>>>
>>> The last patch ("sm6125-seine: Configure MDSS, DSI and panel") depends
>>> on (an impending v2 of) my Sony panel collection series [1].
>>>
>>> [1]: https://lore.kernel.org/linux-arm-msm/20230521-drm-panels-sony-v1-0-541c341d6bee@somainline.org/
>>>
>>> ---
>>> Marijn Suijten (15):
>>>       arm64: dts: qcom: sm6125: Sort spmi_bus node numerically by reg
>>>       dt-bindings: clock: qcom,dispcc-sm6125: Remove unused GCC_DISP_AHB_CLK
>>>       dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock
>>>       dt-bindings: clock: qcom,dispcc-sm6125: Allow power-domains property
>>>       dt-bindings: display/msm: dsi-controller-main: Document SM6125
>>>       dt-bindings: display/msm: sc7180-dpu: Describe SM6125
>>>       dt-bindings: display/msm: Add SM6125 MDSS
>>>       drm/msm/dpu: Add SM6125 support
>>>       drm/msm/mdss: Add SM6125 support
>>>       dt-bindings: msm: dsi-phy-14nm: Document SM6125 variant
>>>       drm/msm/dsi: Add 14nm phy configuration for SM6125
>>>       arm64: dts: qcom: sm6125: Switch fixed xo_board clock to RPM XO clock
>>>       arm64: dts: qcom: sm6125: Add dispcc node
>>>       arm64: dts: qcom: sm6125: Add display hardware nodes
>>>       arm64: dts: qcom: sm6125-seine: Configure MDSS, DSI and panel
>>>
>>>  .../bindings/clock/qcom,dispcc-sm6125.yaml         |  17 +-
>>>  .../bindings/display/msm/dsi-controller-main.yaml  |   2 +
>>>  .../bindings/display/msm/dsi-phy-14nm.yaml         |   1 +
>>>  .../bindings/display/msm/qcom,sc7180-dpu.yaml      |   1 +
>>>  .../bindings/display/msm/qcom,sm6125-mdss.yaml     | 206 +++++++++++++++++
>>>  .../dts/qcom/sm6125-sony-xperia-seine-pdx201.dts   |  59 +++++
>>>  arch/arm64/boot/dts/qcom/sm6125.dtsi               | 244 +++++++++++++++++++--
>>>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_4_sm6125.h | 173 +++++++++++++++
>>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     |   6 +
>>>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |   1 +
>>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |   1 +
>>>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c              |   2 +
>>>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h              |   1 +
>>>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c         |  15 ++
>>>  drivers/gpu/drm/msm/msm_mdss.c                     |   8 +
>>>  15 files changed, 712 insertions(+), 25 deletions(-)
>>> ---
>>> base-commit: 8d2be868b42c08290509c60515865f4de24ea704
>>> change-id: 20230624-sm6125-dpu-aedc9637ee7b
>>>
>>> Best regards,

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Konrad Dybcio]

On 25.06.2023 21:48, Marijn Suijten wrote:
> On 2023-06-24 03:45:02, Konrad Dybcio wrote:
>> On 24.06.2023 02:41, Marijn Suijten wrote:
>>> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
>>> be passed from DT, and should be required by the bindings.
>>>
>>> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>>> ---
>> Ideally, you'd stick it at the bottom of the list, as the items: order
>> is part of the ABI
> 
> This isn't an ABI break, as this driver nor its bindings require/declare
> a fixed order: they declare a relation between clocks and clock-names.
Bindings describe the ABI, drivers implement compliant code flow.

> 
> This orders the GCC clock just like other dispccs.  And the previous
> patch dropped the unused cfg_ahb_clk from the bindings, so all bets are
> off anyway.
Thinking about it again, the binding has not been consumed by any upstream
DT to date, so it should (tm) be fine to let it slide..

Konrad
> 
> - Marijn
> 
>>
>> Konrad
>>>  Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>> index 2acf487d8a2f..11ec154503a3 100644
>>> --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>> @@ -23,6 +23,7 @@ properties:
>>>    clocks:
>>>      items:
>>>        - description: Board XO source
>>> +      - description: GPLL0 div source from GCC
>>>        - description: Byte clock from DSI PHY0
>>>        - description: Pixel clock from DSI PHY0
>>>        - description: Pixel clock from DSI PHY1
>>> @@ -32,6 +33,7 @@ properties:
>>>    clock-names:
>>>      items:
>>>        - const: bi_tcxo
>>> +      - const: gcc_disp_gpll0_div_clk_src
>>>        - const: dsi0_phy_pll_out_byteclk
>>>        - const: dsi0_phy_pll_out_dsiclk
>>>        - const: dsi1_phy_pll_out_dsiclk
>>> @@ -65,12 +67,14 @@ examples:
>>>        compatible = "qcom,sm6125-dispcc";
>>>        reg = <0x5f00000 0x20000>;
>>>        clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>>> +               <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
>>>                 <&dsi0_phy 0>,
>>>                 <&dsi0_phy 1>,
>>>                 <&dsi1_phy 1>,
>>>                 <&dp_phy 0>,
>>>                 <&dp_phy 1>;
>>>        clock-names = "bi_tcxo",
>>> +                    "gcc_disp_gpll0_div_clk_src",
>>>                      "dsi0_phy_pll_out_byteclk",
>>>                      "dsi0_phy_pll_out_dsiclk",
>>>                      "dsi1_phy_pll_out_dsiclk",
>>>

Re: [PATCH 11/15] drm/msm/dsi: Add 14nm phy configuration for SM6125

[Konrad Dybcio]

On 25.06.2023 22:23, Marijn Suijten wrote:
> On 2023-06-24 03:49:25, Konrad Dybcio wrote:
>> On 24.06.2023 02:41, Marijn Suijten wrote:
>>> SM6125 features only a single PHY (despite a secondary PHY PLL source
>>> being available to the disp_cc_mdss_pclk0_clk_src clock), and downstream
>>> sources for this "trinket" SoC do not define the typical "vcca"
>>> regulator to be available nor used.
>>>
>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>>> ---
>> The introduced ops are identical to 2290, modulo regulator..
> 
> Sure, I can create a "drop unused regulators from 14nm qcm2290 config"
> and a second "reuse qcm2290 14nm dsi phy for sm6125" patch, instead of
> this one.
Please do.

> 
>> But the regulator is absent on both (VDD_MX powers it instead), so
> 
> In the DT patch you requested me to use CX instead of MX... Which one is
> it?
You're confusing DSI host with DSI PHY.

> 
> Also note that I moved it from DSI PHY to DSI0 because that's where the
> rpmpd opps reside.
Both of them need their separate power lines to be active!

Also, OPP is not necessary for genpd activation.

Konrad
> 
> - Marijn
> 
>> feel free to clean that up and reuse it ;)
> 
>>
>> Konrad
>>>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c      |  2 ++
>>>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h      |  1 +
>>>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 15 +++++++++++++++
>>>  3 files changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>>> index 9d5795c58a98..8688ed502dcf 100644
>>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
>>> @@ -559,6 +559,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
>>>  	  .data = &dsi_phy_14nm_2290_cfgs },
>>>  	{ .compatible = "qcom,dsi-phy-14nm-660",
>>>  	  .data = &dsi_phy_14nm_660_cfgs },
>>> +	{ .compatible = "qcom,dsi-phy-14nm-6125",
>>> +	  .data = &dsi_phy_14nm_6125_cfgs },
>>>  	{ .compatible = "qcom,dsi-phy-14nm-8953",
>>>  	  .data = &dsi_phy_14nm_8953_cfgs },
>>>  #endif
>>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>>> index 8b640d174785..ebf915f5e6c6 100644
>>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
>>> @@ -52,6 +52,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
>>>  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
>>>  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
>>>  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_2290_cfgs;
>>> +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs;
>>>  extern const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs;
>>>  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs;
>>>  extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
>>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
>>> index 3ce45b023e63..5d43c9ec69ae 100644
>>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
>>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
>>> @@ -1068,6 +1068,21 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs = {
>>>  	.num_dsi_phy = 2,
>>>  };
>>>  
>>> +const struct msm_dsi_phy_cfg dsi_phy_14nm_6125_cfgs = {
>>> +	.has_phy_lane = true,
>>> +	.ops = {
>>> +		.enable = dsi_14nm_phy_enable,
>>> +		.disable = dsi_14nm_phy_disable,
>>> +		.pll_init = dsi_pll_14nm_init,
>>> +		.save_pll_state = dsi_14nm_pll_save_state,
>>> +		.restore_pll_state = dsi_14nm_pll_restore_state,
>>> +	},
>>> +	.min_pll_rate = VCO_MIN_RATE,
>>> +	.max_pll_rate = VCO_MAX_RATE,
>>> +	.io_start = { 0x5e94400 },
>>> +	.num_dsi_phy = 1,
>>> +};
>>> +
>>>  const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs = {
>>>  	.has_phy_lane = true,
>>>  	.regulator_data = dsi_phy_14nm_17mA_regulators,
>>>

Re: [PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Dmitry Baryshkov]

On 24/06/2023 03:41, Marijn Suijten wrote:
> SM6125 is identical to SM6375 except that while downstream also defines
> a throttle clock, its presence results in timeouts whereas SM6375
> requires it to not observe any timeouts.

I see that the vendor DTS still references this clock.

Abhinav, Tanya, do possibly know what can be wrong here?

> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>   Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml
> index 630b11480496..6d2ba9a1cca1 100644
> --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml
> @@ -15,6 +15,7 @@ properties:
>     compatible:
>       enum:
>         - qcom,sc7180-dpu
> +      - qcom,sm6125-dpu
>         - qcom,sm6350-dpu
>         - qcom,sm6375-dpu
>   
> 

-- 
With best wishes
Dmitry

Re: [PATCH 00/15] drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel

[Marijn Suijten]

On 2023-06-26 11:41:39, Konrad Dybcio wrote:
> On 25.06.2023 21:18, Marijn Suijten wrote:
> > On 2023-06-24 03:42:46, Konrad Dybcio wrote:
> >> On 24.06.2023 02:40, Marijn Suijten wrote:
> >>> Bring up the SM6125 DPU now that all preliminary series (such as INTF
> >>> TE) have been merged (for me to test the hardware properly)
> >> We should not repeat the same mistake in the future.. Finding a
> >> balance between releasing early and releasing what we can declare
> >> working and tested code is hard, but we waaaaaaaay overstayed on
> >> this one..
> > 
> > I don't understand what you mean by "mistake" at all.  Yes the DPU
> > catalog portion of this series sat in my local branch for a very long
> > time.  Yes it had to be rebased on top of conflicts many many times.
> > 
> > However, that time has also been used to fix and extend DPU where
> > necessary, instead of submitting a half-broken or half-incomplete
> > catalog entry...
> > 
> > Re "we overstayed": you could have asked to clean up and send my patch,
> > so I don't take this as a mistake on my part as you are completely aware
> > of my time schedule ;)
> I didn't mean to pick on you. I just wanted to emphasize that a more
> upstream-forward approach would have saved us quite some time on the
> rebasing and cleaning-up front.

That is how it comes across ;) - our dream is all about upstream-first
but as you know this becomes a mess really quickly when things are
blocked on dependencies and you're working on 5 different features and
testing across ±8 different Sony platforms on ±14 different devices at
once... all in a limited portion of free time.

Fwiw cleaning-up would have had to happen either way, and would have
taken the same amount of time regardless of whether this series is
submitted now or two months ago.

> >> Konrad
> >> , and most
> > 
> > Also here, don't forget to re-quote my message if you break half-way in
> > the line.
> Ugh. All the time I've been doing this I thought thunderfox was smart
> enough to do it for me. Apparently not and you're the 1st one to point
> that out.

You're welcome!
(Though I thought it should be visible in Thunderburd, unless you're not
 in plaintext mode?  Does it still show the "this is quoted" line in
 front of the broken sentence?)

 - Marijn

Re: [PATCH 00/15] drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel

[Konrad Dybcio]

On 26.06.2023 16:17, Marijn Suijten wrote:
> On 2023-06-26 11:41:39, Konrad Dybcio wrote:
>> On 25.06.2023 21:18, Marijn Suijten wrote:
>>> On 2023-06-24 03:42:46, Konrad Dybcio wrote:
>>>> On 24.06.2023 02:40, Marijn Suijten wrote:
>>>>> Bring up the SM6125 DPU now that all preliminary series (such as INTF
>>>>> TE) have been merged (for me to test the hardware properly)
>>>> We should not repeat the same mistake in the future.. Finding a
>>>> balance between releasing early and releasing what we can declare
>>>> working and tested code is hard, but we waaaaaaaay overstayed on
>>>> this one..
>>>
>>> I don't understand what you mean by "mistake" at all.  Yes the DPU
>>> catalog portion of this series sat in my local branch for a very long
>>> time.  Yes it had to be rebased on top of conflicts many many times.
>>>
>>> However, that time has also been used to fix and extend DPU where
>>> necessary, instead of submitting a half-broken or half-incomplete
>>> catalog entry...
>>>
>>> Re "we overstayed": you could have asked to clean up and send my patch,
>>> so I don't take this as a mistake on my part as you are completely aware
>>> of my time schedule ;)
>> I didn't mean to pick on you. I just wanted to emphasize that a more
>> upstream-forward approach would have saved us quite some time on the
>> rebasing and cleaning-up front.
> 
> That is how it comes across ;) - our dream is all about upstream-first
> but as you know this becomes a mess really quickly when things are
> blocked on dependencies and you're working on 5 different features and
> testing across ±8 different Sony platforms on ±14 different devices at
> once... all in a limited portion of free time.
> 
> Fwiw cleaning-up would have had to happen either way, and would have
> taken the same amount of time regardless of whether this series is
> submitted now or two months ago.
> 
>>>> Konrad
>>>> , and most
>>>
>>> Also here, don't forget to re-quote my message if you break half-way in
>>> the line.
>> Ugh. All the time I've been doing this I thought thunderfox was smart
>> enough to do it for me. Apparently not and you're the 1st one to point
>> that out.
> 
> You're welcome!
> (Though I thought it should be visible in Thunderburd, unless you're not
>  in plaintext mode?  Does it still show the "this is quoted" line in
>  front of the broken sentence?)
It doesn't, but the text stays blue (as if it was)

Konrad
> 
>  - Marijn

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-26 11:43:39, Konrad Dybcio wrote:
> On 25.06.2023 21:48, Marijn Suijten wrote:
> > On 2023-06-24 03:45:02, Konrad Dybcio wrote:
> >> On 24.06.2023 02:41, Marijn Suijten wrote:
> >>> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
> >>> be passed from DT, and should be required by the bindings.
> >>>
> >>> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
> >>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> >>> ---
> >> Ideally, you'd stick it at the bottom of the list, as the items: order
> >> is part of the ABI
> > 
> > This isn't an ABI break, as this driver nor its bindings require/declare
> > a fixed order: they declare a relation between clocks and clock-names.
> Bindings describe the ABI, drivers implement compliant code flow.

That is how bindings are supposed to be...  However typically the driver
is written/ported first and then the bindings are simply created to
reflect this, and sometimes (as is the case with this patch)
incorrectly.

That, together with a lack of DTS and known-working device with it
(which is why I'm submitting driver+bindings+dts in one series now!)
makes us shoot ourselves in the foot by locking everyone into an ABI
that makes no sense.

> > This orders the GCC clock just like other dispccs.  And the previous
> > patch dropped the unused cfg_ahb_clk from the bindings, so all bets are
> > off anyway.
> Thinking about it again, the binding has not been consumed by any upstream
> DT to date, so it should (tm) be fine to let it slide..

Exactly, I hope/doubt anyone was already using these incomplete
bindings.  And again: the ABI here is the name->phandle mapping, the
order Does Not Matter™.  So I hope we can let it slide (otherwise the
previous patch shouldd have been NAK'ed as well??)

(Unless you are SM6115 which uses index-based mapping and does not
 define clock-names at all)

- Marijn

> Konrad
> > 
> > - Marijn
> > 
> >>
> >> Konrad
> >>>  Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>> index 2acf487d8a2f..11ec154503a3 100644
> >>> --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>> @@ -23,6 +23,7 @@ properties:
> >>>    clocks:
> >>>      items:
> >>>        - description: Board XO source
> >>> +      - description: GPLL0 div source from GCC
> >>>        - description: Byte clock from DSI PHY0
> >>>        - description: Pixel clock from DSI PHY0
> >>>        - description: Pixel clock from DSI PHY1
> >>> @@ -32,6 +33,7 @@ properties:
> >>>    clock-names:
> >>>      items:
> >>>        - const: bi_tcxo
> >>> +      - const: gcc_disp_gpll0_div_clk_src
> >>>        - const: dsi0_phy_pll_out_byteclk
> >>>        - const: dsi0_phy_pll_out_dsiclk
> >>>        - const: dsi1_phy_pll_out_dsiclk
> >>> @@ -65,12 +67,14 @@ examples:
> >>>        compatible = "qcom,sm6125-dispcc";
> >>>        reg = <0x5f00000 0x20000>;
> >>>        clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> >>> +               <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
> >>>                 <&dsi0_phy 0>,
> >>>                 <&dsi0_phy 1>,
> >>>                 <&dsi1_phy 1>,
> >>>                 <&dp_phy 0>,
> >>>                 <&dp_phy 1>;
> >>>        clock-names = "bi_tcxo",
> >>> +                    "gcc_disp_gpll0_div_clk_src",
> >>>                      "dsi0_phy_pll_out_byteclk",
> >>>                      "dsi0_phy_pll_out_dsiclk",
> >>>                      "dsi1_phy_pll_out_dsiclk",
> >>>

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Krzysztof Kozlowski]

On 25/06/2023 21:48, Marijn Suijten wrote:
> On 2023-06-24 11:08:54, Krzysztof Kozlowski wrote:
>> On 24/06/2023 03:45, Konrad Dybcio wrote:
>>> On 24.06.2023 02:41, Marijn Suijten wrote:
>>>> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
>>>> be passed from DT, and should be required by the bindings.
>>>>
>>>> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
>>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>>>> ---
>>> Ideally, you'd stick it at the bottom of the list, as the items: order
>>> is part of the ABI
>>
>> Yes, please add them to the end. Order is fixed.
> 
> Disagreed for bindings that declare clock-names and when the driver
> adheres to it, see my reply to Konrad's message.

That's the generic rule, with some exceptions of course. Whether one
chosen driver (chosen system and chosen version of that system) adheres
or not, does not change it. Other driver behaves differently and ABI is
for everyone, not only for your specific version of Linux driver.

Follow the rule.

Best regards,
Krzysztof

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Krzysztof Kozlowski]

On 26/06/2023 16:26, Marijn Suijten wrote:
> On 2023-06-26 11:43:39, Konrad Dybcio wrote:
>> On 25.06.2023 21:48, Marijn Suijten wrote:
>>> On 2023-06-24 03:45:02, Konrad Dybcio wrote:
>>>> On 24.06.2023 02:41, Marijn Suijten wrote:
>>>>> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
>>>>> be passed from DT, and should be required by the bindings.
>>>>>
>>>>> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
>>>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>>>>> ---
>>>> Ideally, you'd stick it at the bottom of the list, as the items: order
>>>> is part of the ABI
>>>
>>> This isn't an ABI break, as this driver nor its bindings require/declare
>>> a fixed order: they declare a relation between clocks and clock-names.
>> Bindings describe the ABI, drivers implement compliant code flow.
> 
> That is how bindings are supposed to be...  However typically the driver
> is written/ported first and then the bindings are simply created to

Your development process does not matter for the bindings. Whatever you
decide to do "typically" is your choice, although of course I understand
why you do it like that. You can argument the same that "I never create
bindings in my process, so the driver defines the ABI".

> reflect this, and sometimes (as is the case with this patch)
> incorrectly.
> 
> That, together with a lack of DTS and known-working device with it
> (which is why I'm submitting driver+bindings+dts in one series now!)
> makes us shoot ourselves in the foot by locking everyone into an ABI
> that makes no sense.

No one is locked into the ABI. SoC maintainer decides on this. However
unjustified ABI breaking or not caring about it at all is not the way to
go. It is not the correct process.

> 
>>> This orders the GCC clock just like other dispccs.  And the previous
>>> patch dropped the unused cfg_ahb_clk from the bindings, so all bets are
>>> off anyway.
>> Thinking about it again, the binding has not been consumed by any upstream
>> DT to date, so it should (tm) be fine to let it slide..
> 
> Exactly, I hope/doubt anyone was already using these incomplete
> bindings.  And again: the ABI here is the name->phandle mapping, the
> order Does Not Matter™.

No, it's not. Your one driver does not define the ABI. There are many
different drivers, many different operating systems and other software
components.

Best regards,
Krzysztof

Re: [PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Krzysztof Kozlowski]

On 25/06/2023 21:52, Marijn Suijten wrote:
> On 2023-06-24 11:12:52, Krzysztof Kozlowski wrote:
>> On 24/06/2023 02:41, Marijn Suijten wrote:
>>> SM6125 is identical to SM6375 except that while downstream also defines
>>> a throttle clock, its presence results in timeouts whereas SM6375
>>> requires it to not observe any timeouts.
>>
>> Then it should not be allowed, so you need either "else:" block or
>> another "if: properties: compatible:" to disallow it. Because in current
>> patch it would be allowed.
> 
> That means this binding is wrong/incomplete for all other SoCs then.
> clock(-name)s has 6 items, and sets `minItems: 6`.  Only for sm6375-dpu
> does it set `minItems: 7`, but an else case is missing.

Ask the author why it is done like this.

> 
> Shall I send a Fixes: ed41005f5b7c ("dt-bindings: display/msm:
> sc7180-dpu: Describe SM6350 and SM6375") for that, and should maxItems:
> 6 be the default under clock(-name)s or in an else:?

There is no bug to fix. Or at least it is not yet known. Whether other
devices should be constrained as well - sure, sounds reasonable, but I
did not check the code exactly.

We talk here about this patch.

Best regards,
Krzysztof

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-26 18:15:13, Krzysztof Kozlowski wrote:
> On 26/06/2023 16:26, Marijn Suijten wrote:
> > On 2023-06-26 11:43:39, Konrad Dybcio wrote:
> >> On 25.06.2023 21:48, Marijn Suijten wrote:
> >>> On 2023-06-24 03:45:02, Konrad Dybcio wrote:
> >>>> On 24.06.2023 02:41, Marijn Suijten wrote:
> >>>>> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
> >>>>> be passed from DT, and should be required by the bindings.
> >>>>>
> >>>>> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
> >>>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> >>>>> ---
> >>>> Ideally, you'd stick it at the bottom of the list, as the items: order
> >>>> is part of the ABI
> >>>
> >>> This isn't an ABI break, as this driver nor its bindings require/declare
> >>> a fixed order: they declare a relation between clocks and clock-names.
> >> Bindings describe the ABI, drivers implement compliant code flow.
> > 
> > That is how bindings are supposed to be...  However typically the driver
> > is written/ported first and then the bindings are simply created to
> 
> Your development process does not matter for the bindings. Whatever you
> decide to do "typically" is your choice, although of course I understand
> why you do it like that. You can argument the same that "I never create
> bindings in my process, so the driver defines the ABI".

This is not my process, nor my choice.

> > reflect this, and sometimes (as is the case with this patch)
> > incorrectly.
> > 
> > That, together with a lack of DTS and known-working device with it
> > (which is why I'm submitting driver+bindings+dts in one series now!)
> > makes us shoot ourselves in the foot by locking everyone into an ABI
> > that makes no sense.
> 
> No one is locked into the ABI. SoC maintainer decides on this. However
> unjustified ABI breaking or not caring about it at all is not the way to
> go. It is not the correct process.

It definitely sounds like it.

> >>> This orders the GCC clock just like other dispccs.  And the previous
> >>> patch dropped the unused cfg_ahb_clk from the bindings, so all bets are
> >>> off anyway.
> >> Thinking about it again, the binding has not been consumed by any upstream
> >> DT to date, so it should (tm) be fine to let it slide..
> > 
> > Exactly, I hope/doubt anyone was already using these incomplete
> > bindings.  And again: the ABI here is the name->phandle mapping, the
> > order Does Not Matter™.
> 
> No, it's not. Your one driver does not define the ABI. There are many
> different drivers, many different operating systems and other software
> components.

You missed the point entirely.

The point is that someone contributed a dt-bindings patch that does not
represent the hardware (hence not the driver for that hardware either).
It was taken without objection.  So now what?  We are stuck with a
broken ABI that does not allow us to describe the hardware?

If there are many different drivers and other OSes, why are we solely
responsible for describing broken bindings?  Why were there no
objections elsewhere that this does not allow us to describe the
hardware in question?

Note that the person signing off on and sending that initial dt-bindings
patch for qcom,dispcc-sm6125.yaml is me, by the way.

- Marijn

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-26 18:10:44, Krzysztof Kozlowski wrote:
> On 25/06/2023 21:48, Marijn Suijten wrote:
> > On 2023-06-24 11:08:54, Krzysztof Kozlowski wrote:
> >> On 24/06/2023 03:45, Konrad Dybcio wrote:
> >>> On 24.06.2023 02:41, Marijn Suijten wrote:
> >>>> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
> >>>> be passed from DT, and should be required by the bindings.
> >>>>
> >>>> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
> >>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> >>>> ---
> >>> Ideally, you'd stick it at the bottom of the list, as the items: order
> >>> is part of the ABI
> >>
> >> Yes, please add them to the end. Order is fixed.
> > 
> > Disagreed for bindings that declare clock-names and when the driver
> > adheres to it, see my reply to Konrad's message.
> 
> That's the generic rule, with some exceptions of course. Whether one
> chosen driver (chosen system and chosen version of that system) adheres
> or not, does not change it. Other driver behaves differently and ABI is
> for everyone, not only for your specific version of Linux driver.
> 
> Follow the rule.

This has no relation to the driver (just that our driver adheres to the
bindings, as it is supposed to be).  The bindings define a mapping from
a clock-names=<> entry to a clock on the same index in the clocks=<>
array.  That relation remains the same with this change.

- Marijn

Re: [PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Marijn Suijten]

On 2023-06-26 18:16:58, Krzysztof Kozlowski wrote:
> On 25/06/2023 21:52, Marijn Suijten wrote:
> > On 2023-06-24 11:12:52, Krzysztof Kozlowski wrote:
> >> On 24/06/2023 02:41, Marijn Suijten wrote:
> >>> SM6125 is identical to SM6375 except that while downstream also defines
> >>> a throttle clock, its presence results in timeouts whereas SM6375
> >>> requires it to not observe any timeouts.
> >>
> >> Then it should not be allowed, so you need either "else:" block or
> >> another "if: properties: compatible:" to disallow it. Because in current
> >> patch it would be allowed.
> > 
> > That means this binding is wrong/incomplete for all other SoCs then.
> > clock(-name)s has 6 items, and sets `minItems: 6`.  Only for sm6375-dpu

Of course meant to say that clock(-name)s has **7** items, not 6.

> > does it set `minItems: 7`, but an else case is missing.
> 
> Ask the author why it is done like this.

Konrad, can you clarify why other 

> > Shall I send a Fixes: ed41005f5b7c ("dt-bindings: display/msm:
> > sc7180-dpu: Describe SM6350 and SM6375") for that, and should maxItems:
> > 6 be the default under clock(-name)s or in an else:?
> 
> There is no bug to fix. Or at least it is not yet known. Whether other
> devices should be constrained as well - sure, sounds reasonable, but I
> did not check the code exactly.

I don't know either, but we need this information to decide whether to
use `maxItems: 6`:

1. Directly on the property;
2. In an `else:` case on the current `if: sm6375-dpu` (should have the
   same effect as 1., afaik);
3. In a second `if:` case that lists all SoCS explicitly.

Since we don't have this information, I think option 3. is the right way
to go, setting `maxItems: 6` for qcom,sm6125-dpu.

However, it is not yet understood why downstream is able to use the
throttle clock without repercussions.

> We talk here about this patch.

We used this patch to discover that other SoCs are similarly
unconstrained.  But if you don't want me to look into it, by all means!
Saves me a lot of time.  So I will go with option 3.

- Marijn

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Krzysztof Kozlowski]

On 26/06/2023 19:49, Marijn Suijten wrote:
> On 2023-06-26 18:10:44, Krzysztof Kozlowski wrote:
>> On 25/06/2023 21:48, Marijn Suijten wrote:
>>> On 2023-06-24 11:08:54, Krzysztof Kozlowski wrote:
>>>> On 24/06/2023 03:45, Konrad Dybcio wrote:
>>>>> On 24.06.2023 02:41, Marijn Suijten wrote:
>>>>>> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
>>>>>> be passed from DT, and should be required by the bindings.
>>>>>>
>>>>>> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
>>>>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>>>>>> ---
>>>>> Ideally, you'd stick it at the bottom of the list, as the items: order
>>>>> is part of the ABI
>>>>
>>>> Yes, please add them to the end. Order is fixed.
>>>
>>> Disagreed for bindings that declare clock-names and when the driver
>>> adheres to it, see my reply to Konrad's message.
>>
>> That's the generic rule, with some exceptions of course. Whether one
>> chosen driver (chosen system and chosen version of that system) adheres
>> or not, does not change it. Other driver behaves differently and ABI is
>> for everyone, not only for your specific version of Linux driver.
>>
>> Follow the rule.
> 
> This has no relation to the driver (just that our driver adheres to the
> bindings, as it is supposed to be).  The bindings define a mapping from
> a clock-names=<> entry to a clock on the same index in the clocks=<>
> array.  That relation remains the same with this change.

Not really, binding also defines the list of clocks - their order and
specific entries. This changes.

Best regards,
Krzysztof

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-26 20:29:37, Krzysztof Kozlowski wrote:
> On 26/06/2023 19:49, Marijn Suijten wrote:
> > On 2023-06-26 18:10:44, Krzysztof Kozlowski wrote:
> >> On 25/06/2023 21:48, Marijn Suijten wrote:
> >>> On 2023-06-24 11:08:54, Krzysztof Kozlowski wrote:
> >>>> On 24/06/2023 03:45, Konrad Dybcio wrote:
> >>>>> On 24.06.2023 02:41, Marijn Suijten wrote:
> >>>>>> The "gcc_disp_gpll0_div_clk_src" clock is consumed by the driver, will
> >>>>>> be passed from DT, and should be required by the bindings.
> >>>>>>
> >>>>>> Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings")
> >>>>>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> >>>>>> ---
> >>>>> Ideally, you'd stick it at the bottom of the list, as the items: order
> >>>>> is part of the ABI
> >>>>
> >>>> Yes, please add them to the end. Order is fixed.
> >>>
> >>> Disagreed for bindings that declare clock-names and when the driver
> >>> adheres to it, see my reply to Konrad's message.
> >>
> >> That's the generic rule, with some exceptions of course. Whether one
> >> chosen driver (chosen system and chosen version of that system) adheres
> >> or not, does not change it. Other driver behaves differently and ABI is
> >> for everyone, not only for your specific version of Linux driver.
> >>
> >> Follow the rule.
> > 
> > This has no relation to the driver (just that our driver adheres to the
> > bindings, as it is supposed to be).  The bindings define a mapping from
> > a clock-names=<> entry to a clock on the same index in the clocks=<>
> > array.  That relation remains the same with this change.
> 
> Not really, binding also defines the list of clocks - their order and
> specific entries. This changes.

And so it does in "dt-bindings: clock: qcom,dispcc-sm6125: Remove unused
GCC_DISP_AHB_CLK"?

- Marijn

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-26 20:51:38, Marijn Suijten wrote:
<snip>
> > Not really, binding also defines the list of clocks - their order and
> > specific entries. This changes.
> 
> And so it does in "dt-bindings: clock: qcom,dispcc-sm6125: Remove unused
> GCC_DISP_AHB_CLK"?

Never mind: it is the last item so the order of the other items doesn't
change.  The total number of items decreases though, which sounds like
an ABI-break too?

- Marijn

Re: [PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Konrad Dybcio]

On 26.06.2023 19:54, Marijn Suijten wrote:
> On 2023-06-26 18:16:58, Krzysztof Kozlowski wrote:
>> On 25/06/2023 21:52, Marijn Suijten wrote:
>>> On 2023-06-24 11:12:52, Krzysztof Kozlowski wrote:
>>>> On 24/06/2023 02:41, Marijn Suijten wrote:
>>>>> SM6125 is identical to SM6375 except that while downstream also defines
>>>>> a throttle clock, its presence results in timeouts whereas SM6375
>>>>> requires it to not observe any timeouts.
>>>>
>>>> Then it should not be allowed, so you need either "else:" block or
>>>> another "if: properties: compatible:" to disallow it. Because in current
>>>> patch it would be allowed.
>>>
>>> That means this binding is wrong/incomplete for all other SoCs then.
>>> clock(-name)s has 6 items, and sets `minItems: 6`.  Only for sm6375-dpu
> 
> Of course meant to say that clock(-name)s has **7** items, not 6.
> 
>>> does it set `minItems: 7`, but an else case is missing.
>>
>> Ask the author why it is done like this.
> 
> Konrad, can you clarify why other 
6375 needs the throttle clk and the clock(-names) are strongly ordered
so having minItems: 6 discards the last entry

Konrad
> 
>>> Shall I send a Fixes: ed41005f5b7c ("dt-bindings: display/msm:
>>> sc7180-dpu: Describe SM6350 and SM6375") for that, and should maxItems:
>>> 6 be the default under clock(-name)s or in an else:?
>>
>> There is no bug to fix. Or at least it is not yet known. Whether other
>> devices should be constrained as well - sure, sounds reasonable, but I
>> did not check the code exactly.
> 
> I don't know either, but we need this information to decide whether to
> use `maxItems: 6`:
> 
> 1. Directly on the property;
> 2. In an `else:` case on the current `if: sm6375-dpu` (should have the
>    same effect as 1., afaik);
> 3. In a second `if:` case that lists all SoCS explicitly.
> 
> Since we don't have this information, I think option 3. is the right way
> to go, setting `maxItems: 6` for qcom,sm6125-dpu.
> 
> However, it is not yet understood why downstream is able to use the
> throttle clock without repercussions.
> 
>> We talk here about this patch.
> 
> We used this patch to discover that other SoCs are similarly
> unconstrained.  But if you don't want me to look into it, by all means!
> Saves me a lot of time.  So I will go with option 3.
> 
> - Marijn

Re: [PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Marijn Suijten]

On 2023-06-26 20:57:51, Konrad Dybcio wrote:
> On 26.06.2023 19:54, Marijn Suijten wrote:
> > On 2023-06-26 18:16:58, Krzysztof Kozlowski wrote:
> >> On 25/06/2023 21:52, Marijn Suijten wrote:
> >>> On 2023-06-24 11:12:52, Krzysztof Kozlowski wrote:
> >>>> On 24/06/2023 02:41, Marijn Suijten wrote:
> >>>>> SM6125 is identical to SM6375 except that while downstream also defines
> >>>>> a throttle clock, its presence results in timeouts whereas SM6375
> >>>>> requires it to not observe any timeouts.
> >>>>
> >>>> Then it should not be allowed, so you need either "else:" block or
> >>>> another "if: properties: compatible:" to disallow it. Because in current
> >>>> patch it would be allowed.
> >>>
> >>> That means this binding is wrong/incomplete for all other SoCs then.
> >>> clock(-name)s has 6 items, and sets `minItems: 6`.  Only for sm6375-dpu
> > 
> > Of course meant to say that clock(-name)s has **7** items, not 6.
> > 
> >>> does it set `minItems: 7`, but an else case is missing.
> >>
> >> Ask the author why it is done like this.
> > 
> > Konrad, can you clarify why other 

(Looks like I forgot to complete this sentence before sending,
apologies)

> 6375 needs the throttle clk and the clock(-names) are strongly ordered
> so having minItems: 6 discards the last entry

The question is whether or not we should have maxItems: 6 to disallow
the clock from being passed: right now it is optional and either is
allowed for any !6375 SoC.

- Marijn

> 
> Konrad
> > 
> >>> Shall I send a Fixes: ed41005f5b7c ("dt-bindings: display/msm:
> >>> sc7180-dpu: Describe SM6350 and SM6375") for that, and should maxItems:
> >>> 6 be the default under clock(-name)s or in an else:?
> >>
> >> There is no bug to fix. Or at least it is not yet known. Whether other
> >> devices should be constrained as well - sure, sounds reasonable, but I
> >> did not check the code exactly.
> > 
> > I don't know either, but we need this information to decide whether to
> > use `maxItems: 6`:
> > 
> > 1. Directly on the property;
> > 2. In an `else:` case on the current `if: sm6375-dpu` (should have the
> >    same effect as 1., afaik);
> > 3. In a second `if:` case that lists all SoCS explicitly.
> > 
> > Since we don't have this information, I think option 3. is the right way
> > to go, setting `maxItems: 6` for qcom,sm6125-dpu.
> > 
> > However, it is not yet understood why downstream is able to use the
> > throttle clock without repercussions.
> > 
> >> We talk here about this patch.
> > 
> > We used this patch to discover that other SoCs are similarly
> > unconstrained.  But if you don't want me to look into it, by all means!
> > Saves me a lot of time.  So I will go with option 3.
> > 
> > - Marijn

Re: [PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Konrad Dybcio]

On 26.06.2023 22:28, Marijn Suijten wrote:
> On 2023-06-26 20:57:51, Konrad Dybcio wrote:
>> On 26.06.2023 19:54, Marijn Suijten wrote:
>>> On 2023-06-26 18:16:58, Krzysztof Kozlowski wrote:
>>>> On 25/06/2023 21:52, Marijn Suijten wrote:
>>>>> On 2023-06-24 11:12:52, Krzysztof Kozlowski wrote:
>>>>>> On 24/06/2023 02:41, Marijn Suijten wrote:
>>>>>>> SM6125 is identical to SM6375 except that while downstream also defines
>>>>>>> a throttle clock, its presence results in timeouts whereas SM6375
>>>>>>> requires it to not observe any timeouts.
>>>>>>
>>>>>> Then it should not be allowed, so you need either "else:" block or
>>>>>> another "if: properties: compatible:" to disallow it. Because in current
>>>>>> patch it would be allowed.
>>>>>
>>>>> That means this binding is wrong/incomplete for all other SoCs then.
>>>>> clock(-name)s has 6 items, and sets `minItems: 6`.  Only for sm6375-dpu
>>>
>>> Of course meant to say that clock(-name)s has **7** items, not 6.
>>>
>>>>> does it set `minItems: 7`, but an else case is missing.
>>>>
>>>> Ask the author why it is done like this.
>>>
>>> Konrad, can you clarify why other 
> 
> (Looks like I forgot to complete this sentence before sending,
> apologies)
> 
>> 6375 needs the throttle clk and the clock(-names) are strongly ordered
>> so having minItems: 6 discards the last entry
> 
> The question is whether or not we should have maxItems: 6 to disallow
> the clock from being passed: right now it is optional and either is
> allowed for any !6375 SoC.
That's a very good question. I don't have a 7180 to test, but for
you it seems to cause inexplicable issues on 6125..

Konrad
> 
> - Marijn
> 
>>
>> Konrad
>>>
>>>>> Shall I send a Fixes: ed41005f5b7c ("dt-bindings: display/msm:
>>>>> sc7180-dpu: Describe SM6350 and SM6375") for that, and should maxItems:
>>>>> 6 be the default under clock(-name)s or in an else:?
>>>>
>>>> There is no bug to fix. Or at least it is not yet known. Whether other
>>>> devices should be constrained as well - sure, sounds reasonable, but I
>>>> did not check the code exactly.
>>>
>>> I don't know either, but we need this information to decide whether to
>>> use `maxItems: 6`:
>>>
>>> 1. Directly on the property;
>>> 2. In an `else:` case on the current `if: sm6375-dpu` (should have the
>>>    same effect as 1., afaik);
>>> 3. In a second `if:` case that lists all SoCS explicitly.
>>>
>>> Since we don't have this information, I think option 3. is the right way
>>> to go, setting `maxItems: 6` for qcom,sm6125-dpu.
>>>
>>> However, it is not yet understood why downstream is able to use the
>>> throttle clock without repercussions.
>>>
>>>> We talk here about this patch.
>>>
>>> We used this patch to discover that other SoCs are similarly
>>> unconstrained.  But if you don't want me to look into it, by all means!
>>> Saves me a lot of time.  So I will go with option 3.
>>>
>>> - Marijn

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Krzysztof Kozlowski]

On 26/06/2023 20:53, Marijn Suijten wrote:
> On 2023-06-26 20:51:38, Marijn Suijten wrote:
> <snip>
>>> Not really, binding also defines the list of clocks - their order and
>>> specific entries. This changes.
>>
>> And so it does in "dt-bindings: clock: qcom,dispcc-sm6125: Remove unused
>> GCC_DISP_AHB_CLK"?
> 
> Never mind: it is the last item so the order of the other items doesn't
> change.  The total number of items decreases though, which sounds like
> an ABI-break too?

How does it break? Old DTS works exactly the same, doesn't it?

Best regards,
Krzysztof

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-27 08:24:41, Krzysztof Kozlowski wrote:
> On 26/06/2023 20:53, Marijn Suijten wrote:
> > On 2023-06-26 20:51:38, Marijn Suijten wrote:
> > <snip>
> >>> Not really, binding also defines the list of clocks - their order and
> >>> specific entries. This changes.
> >>
> >> And so it does in "dt-bindings: clock: qcom,dispcc-sm6125: Remove unused
> >> GCC_DISP_AHB_CLK"?
> > 
> > Never mind: it is the last item so the order of the other items doesn't
> > change.  The total number of items decreases though, which sounds like
> > an ABI-break too?
> 
> How does it break? Old DTS works exactly the same, doesn't it?

So deleting a new item at the end does not matter.  But what if I respin
this patch to add the new clock _at the end_, which will then be at the
same index as the previous GCC_DISP_AHB_CLK?

- Marijn

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Krzysztof Kozlowski]

On 27/06/2023 08:54, Marijn Suijten wrote:
> On 2023-06-27 08:24:41, Krzysztof Kozlowski wrote:
>> On 26/06/2023 20:53, Marijn Suijten wrote:
>>> On 2023-06-26 20:51:38, Marijn Suijten wrote:
>>> <snip>
>>>>> Not really, binding also defines the list of clocks - their order and
>>>>> specific entries. This changes.
>>>>
>>>> And so it does in "dt-bindings: clock: qcom,dispcc-sm6125: Remove unused
>>>> GCC_DISP_AHB_CLK"?
>>>
>>> Never mind: it is the last item so the order of the other items doesn't
>>> change.  The total number of items decreases though, which sounds like
>>> an ABI-break too?
>>
>> How does it break? Old DTS works exactly the same, doesn't it?
> 
> So deleting a new item at the end does not matter.  But what if I respin
> this patch to add the new clock _at the end_, which will then be at the
> same index as the previous GCC_DISP_AHB_CLK?

I think you know the answer, right? What do you want to prove? That two
independent changes can have together negative effect? We know this.

Best regards,
Krzysztof

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-27 09:29:53, Krzysztof Kozlowski wrote:
> On 27/06/2023 08:54, Marijn Suijten wrote:
> > On 2023-06-27 08:24:41, Krzysztof Kozlowski wrote:
> >> On 26/06/2023 20:53, Marijn Suijten wrote:
> >>> On 2023-06-26 20:51:38, Marijn Suijten wrote:
> >>> <snip>
> >>>>> Not really, binding also defines the list of clocks - their order and
> >>>>> specific entries. This changes.
> >>>>
> >>>> And so it does in "dt-bindings: clock: qcom,dispcc-sm6125: Remove unused
> >>>> GCC_DISP_AHB_CLK"?
> >>>
> >>> Never mind: it is the last item so the order of the other items doesn't
> >>> change.  The total number of items decreases though, which sounds like
> >>> an ABI-break too?
> >>
> >> How does it break? Old DTS works exactly the same, doesn't it?
> > 
> > So deleting a new item at the end does not matter.  But what if I respin
> > this patch to add the new clock _at the end_, which will then be at the
> > same index as the previous GCC_DISP_AHB_CLK?
> 
> I think you know the answer, right? What do you want to prove? That two
> independent changes can have together negative effect? We know this.

The question is whether this is allowed?

- Marijn

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Krzysztof Kozlowski]

On 27/06/2023 09:49, Marijn Suijten wrote:
> On 2023-06-27 09:29:53, Krzysztof Kozlowski wrote:
>> On 27/06/2023 08:54, Marijn Suijten wrote:
>>> On 2023-06-27 08:24:41, Krzysztof Kozlowski wrote:
>>>> On 26/06/2023 20:53, Marijn Suijten wrote:
>>>>> On 2023-06-26 20:51:38, Marijn Suijten wrote:
>>>>> <snip>
>>>>>>> Not really, binding also defines the list of clocks - their order and
>>>>>>> specific entries. This changes.
>>>>>>
>>>>>> And so it does in "dt-bindings: clock: qcom,dispcc-sm6125: Remove unused
>>>>>> GCC_DISP_AHB_CLK"?
>>>>>
>>>>> Never mind: it is the last item so the order of the other items doesn't
>>>>> change.  The total number of items decreases though, which sounds like
>>>>> an ABI-break too?
>>>>
>>>> How does it break? Old DTS works exactly the same, doesn't it?
>>>
>>> So deleting a new item at the end does not matter.  But what if I respin
>>> this patch to add the new clock _at the end_, which will then be at the
>>> same index as the previous GCC_DISP_AHB_CLK?
>>
>> I think you know the answer, right? What do you want to prove? That two
>> independent changes can have together negative effect? We know this.
> 
> The question is whether this is allowed?

That would be an ABI break and I already explained if it is or is not
allowed.

Best regards,
Krzysztof

Re: [PATCH 09/15] drm/msm/mdss: Add SM6125 support

[Dmitry Baryshkov]

On 24/06/2023 03:41, Marijn Suijten wrote:
> SM6125's UBWC hardware decoder is version 3.0, and supports decoding
> UBWC 1.0.

I think it's UBWC encoder version, see 
https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/display-kernel.lnx.5.15.1.r17-rel/msm/sde/sde_hw_top.c?ref_type=heads#L357

This is a part of 
https://patchwork.freedesktop.org/patch/538279/?series=118074&rev=1

(no, you don't have to rebase on that patchset, it is not reviewed yet).

> 
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
>   drivers/gpu/drm/msm/msm_mdss.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 05648c910c68..bf68bae23264 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -559,6 +559,13 @@ static const struct msm_mdss_data sm6115_data = {
>   	.ubwc_static = 0x11f,
>   };
>   
> +static const struct msm_mdss_data sm6125_data = {
> +	.ubwc_version = UBWC_1_0,
> +	.ubwc_dec_version = UBWC_3_0,
> +	.ubwc_swizzle = 1,
> +	.highest_bank_bit = 1,
> +};
> +
>   static const struct msm_mdss_data sm8250_data = {
>   	.ubwc_version = UBWC_4_0,
>   	.ubwc_dec_version = UBWC_4_0,
> @@ -579,6 +586,7 @@ static const struct of_device_id mdss_dt_match[] = {
>   	{ .compatible = "qcom,sc8180x-mdss", .data = &sc8180x_data },
>   	{ .compatible = "qcom,sc8280xp-mdss", .data = &sc8280xp_data },
>   	{ .compatible = "qcom,sm6115-mdss", .data = &sm6115_data },
> +	{ .compatible = "qcom,sm6125-mdss", .data = &sm6125_data },
>   	{ .compatible = "qcom,sm6350-mdss", .data = &sm6350_data },
>   	{ .compatible = "qcom,sm6375-mdss", .data = &sm6350_data },
>   	{ .compatible = "qcom,sm8150-mdss", .data = &sm8150_data },
> 

-- 
With best wishes
Dmitry

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-27 10:21:12, Krzysztof Kozlowski wrote:
> On 27/06/2023 09:49, Marijn Suijten wrote:
> > On 2023-06-27 09:29:53, Krzysztof Kozlowski wrote:
> >> On 27/06/2023 08:54, Marijn Suijten wrote:
> >>> On 2023-06-27 08:24:41, Krzysztof Kozlowski wrote:
> >>>> On 26/06/2023 20:53, Marijn Suijten wrote:
> >>>>> On 2023-06-26 20:51:38, Marijn Suijten wrote:
> >>>>> <snip>
> >>>>>>> Not really, binding also defines the list of clocks - their order and
> >>>>>>> specific entries. This changes.
> >>>>>>
> >>>>>> And so it does in "dt-bindings: clock: qcom,dispcc-sm6125: Remove unused
> >>>>>> GCC_DISP_AHB_CLK"?
> >>>>>
> >>>>> Never mind: it is the last item so the order of the other items doesn't
> >>>>> change.  The total number of items decreases though, which sounds like
> >>>>> an ABI-break too?
> >>>>
> >>>> How does it break? Old DTS works exactly the same, doesn't it?
> >>>
> >>> So deleting a new item at the end does not matter.  But what if I respin
> >>> this patch to add the new clock _at the end_, which will then be at the
> >>> same index as the previous GCC_DISP_AHB_CLK?
> >>
> >> I think you know the answer, right? What do you want to prove? That two
> >> independent changes can have together negative effect? We know this.
> > 
> > The question is whether this is allowed?
> 
> That would be an ABI break and I already explained if it is or is not
> allowed.

How should we solve it then, if we cannot remove GCC_DISP_AHB_CLK in one
patch and add GCC_DISP_GPLL0_DIV_CLK_SRC **at the end** in the next
patch?  Keep an empty spot at the original index of GCC_DISP_AHB_CLK?

- Marijn

Re: [PATCH 09/15] drm/msm/mdss: Add SM6125 support

[Marijn Suijten]

On 2023-06-27 11:49:07, Dmitry Baryshkov wrote:
> On 24/06/2023 03:41, Marijn Suijten wrote:
> > SM6125's UBWC hardware decoder is version 3.0, and supports decoding
> > UBWC 1.0.
> 
> I think it's UBWC encoder version, see 
> https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/display-kernel.lnx.5.15.1.r17-rel/msm/sde/sde_hw_top.c?ref_type=heads#L357
> 
> This is a part of 
> https://patchwork.freedesktop.org/patch/538279/?series=118074&rev=1
> 
> (no, you don't have to rebase on that patchset, it is not reviewed yet).

Thanks for clarifying this.  I always thought that there only was a
(decoder) hardware version, that is able to decode a specific format
version of the UBWC data format.  And that it was confusingly using the
same enum.

I will reword the message.

(Also didn't really see why MDSS would have to _encode_ to UBWC, for
 readbacks?)

- Marijn

> > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> > ---
> >   drivers/gpu/drm/msm/msm_mdss.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> > index 05648c910c68..bf68bae23264 100644
> > --- a/drivers/gpu/drm/msm/msm_mdss.c
> > +++ b/drivers/gpu/drm/msm/msm_mdss.c
> > @@ -559,6 +559,13 @@ static const struct msm_mdss_data sm6115_data = {
> >   	.ubwc_static = 0x11f,
> >   };
> >   
> > +static const struct msm_mdss_data sm6125_data = {
> > +	.ubwc_version = UBWC_1_0,
> > +	.ubwc_dec_version = UBWC_3_0,
> > +	.ubwc_swizzle = 1,
> > +	.highest_bank_bit = 1,
> > +};
> > +
> >   static const struct msm_mdss_data sm8250_data = {
> >   	.ubwc_version = UBWC_4_0,
> >   	.ubwc_dec_version = UBWC_4_0,
> > @@ -579,6 +586,7 @@ static const struct of_device_id mdss_dt_match[] = {
> >   	{ .compatible = "qcom,sc8180x-mdss", .data = &sc8180x_data },
> >   	{ .compatible = "qcom,sc8280xp-mdss", .data = &sc8280xp_data },
> >   	{ .compatible = "qcom,sm6115-mdss", .data = &sm6115_data },
> > +	{ .compatible = "qcom,sm6125-mdss", .data = &sm6125_data },
> >   	{ .compatible = "qcom,sm6350-mdss", .data = &sm6350_data },
> >   	{ .compatible = "qcom,sm6375-mdss", .data = &sm6350_data },
> >   	{ .compatible = "qcom,sm8150-mdss", .data = &sm8150_data },
> > 
> 
> -- 
> With best wishes
> Dmitry
> 

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Krzysztof Kozlowski]

On 27/06/2023 11:02, Marijn Suijten wrote:
>>>>> So deleting a new item at the end does not matter.  But what if I respin
>>>>> this patch to add the new clock _at the end_, which will then be at the
>>>>> same index as the previous GCC_DISP_AHB_CLK?
>>>>
>>>> I think you know the answer, right? What do you want to prove? That two
>>>> independent changes can have together negative effect? We know this.
>>>
>>> The question is whether this is allowed?
>>
>> That would be an ABI break and I already explained if it is or is not
>> allowed.
> 
> How should we solve it then, if we cannot remove GCC_DISP_AHB_CLK in one
> patch and add GCC_DISP_GPLL0_DIV_CLK_SRC **at the end** in the next
> patch?  Keep an empty spot at the original index of GCC_DISP_AHB_CLK?

I don't know if you are trolling me or really asking question, so just
in case it is the latter:

"No one is locked into the ABI. SoC maintainer decides on this. "

Also:
https://lore.kernel.org/linux-arm-msm/20230608152759.GA2721945-robh@kernel.org/

https://lore.kernel.org/linux-arm-msm/CAL_JsqKOq+PdjUPVYqdC7QcjGxp-KbAG_O9e+zrfY7k-wRr1QQ@mail.gmail.com/

https://lore.kernel.org/linux-arm-msm/20220602143245.GA2256965-robh@kernel.org/

https://lore.kernel.org/linux-arm-msm/20220601202452.GA365963-robh@kernel.org/

Any many more.

Best regards,
Krzysztof

Re: [PATCH 03/15] dt-bindings: clock: qcom,dispcc-sm6125: Require GCC PLL0 DIV clock

[Marijn Suijten]

On 2023-06-27 11:07:22, Krzysztof Kozlowski wrote:
> On 27/06/2023 11:02, Marijn Suijten wrote:
> >>>>> So deleting a new item at the end does not matter.  But what if I respin
> >>>>> this patch to add the new clock _at the end_, which will then be at the
> >>>>> same index as the previous GCC_DISP_AHB_CLK?
> >>>>
> >>>> I think you know the answer, right? What do you want to prove? That two
> >>>> independent changes can have together negative effect? We know this.
> >>>
> >>> The question is whether this is allowed?
> >>
> >> That would be an ABI break and I already explained if it is or is not
> >> allowed.
> > 
> > How should we solve it then, if we cannot remove GCC_DISP_AHB_CLK in one
> > patch and add GCC_DISP_GPLL0_DIV_CLK_SRC **at the end** in the next
> > patch?  Keep an empty spot at the original index of GCC_DISP_AHB_CLK?
> 
> I don't know if you are trolling me or really asking question, so just
> in case it is the latter:

Apologies if it comes across that way, but I am genuinely
misunderstanding what is and is not allowed as part of this ABI...

> "No one is locked into the ABI. SoC maintainer decides on this. "

Especially if it is up to the SoC mantainer.

> Also:
> https://lore.kernel.org/linux-arm-msm/20230608152759.GA2721945-robh@kernel.org/
> 
> https://lore.kernel.org/linux-arm-msm/CAL_JsqKOq+PdjUPVYqdC7QcjGxp-KbAG_O9e+zrfY7k-wRr1QQ@mail.gmail.com/
> 
> https://lore.kernel.org/linux-arm-msm/20220602143245.GA2256965-robh@kernel.org/
> 
> https://lore.kernel.org/linux-arm-msm/20220601202452.GA365963-robh@kernel.org/
> 
> Any many more.

In that sense the question above is not for you, but for the SoC
maintainer?  Whom, I hope, will say that we can be lenient in changing
the ABI for a platform that is only slowly being brought up by a bunch
of community developers and unlikely to be touched by anyone else.

Thanks for helping out so far!

- Marijn

Re: [Freedreno] [PATCH 06/15] dt-bindings: display/msm: sc7180-dpu: Describe SM6125

[Abhinav Kumar]

On 6/26/2023 7:04 AM, Dmitry Baryshkov wrote:
> On 24/06/2023 03:41, Marijn Suijten wrote:
>> SM6125 is identical to SM6375 except that while downstream also defines
>> a throttle clock, its presence results in timeouts whereas SM6375
>> requires it to not observe any timeouts.
> 
> I see that the vendor DTS still references this clock.
> 
> Abhinav, Tanya, do possibly know what can be wrong here?
> 

 From display side, we just enable it without any specific vote. Seeing 
timeouts without it makes sense but not with it.

I dont have experience with this family of chipsets and this clock is 
specific to this family of chipsets.

Will reach out to folks who might have a better idea about this clock 
and update with possible suggestions.

Unless ... tanya has more suggestions.

>>
>> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
>> ---
>>   Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml | 
>> 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml 
>> b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml
>> index 630b11480496..6d2ba9a1cca1 100644
>> --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml
>> @@ -15,6 +15,7 @@ properties:
>>     compatible:
>>       enum:
>>         - qcom,sc7180-dpu
>> +      - qcom,sm6125-dpu
>>         - qcom,sm6350-dpu
>>         - qcom,sm6375-dpu
>>
>