devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML
@ 2021-07-03 15:18 David Heidelberg
  2021-07-04  8:18 ` Dmitry Baryshkov
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: David Heidelberg @ 2021-07-03 15:18 UTC (permalink / raw)
  To: smasetty, masneyb, robdclark, jordan
  Cc: devicetree, linux-arm-msm, David Heidelberg

This warning cannot be fixed by conversion, since this naming is already used.
Documentation/devicetree/bindings/display/msm/gpu.example.dt.yaml: gpu@5000000: interconnect-names: ['gfx-mem'] is too short

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 .../devicetree/bindings/display/msm/gpu.txt   | 157 -----------
 .../devicetree/bindings/display/msm/gpu.yaml  | 256 ++++++++++++++++++
 2 files changed, 256 insertions(+), 157 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/msm/gpu.txt
 create mode 100644 Documentation/devicetree/bindings/display/msm/gpu.yaml

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt
deleted file mode 100644
index 090dcb3fc34d..000000000000
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ /dev/null
@@ -1,157 +0,0 @@
-Qualcomm adreno/snapdragon GPU
-
-Required properties:
-- compatible: "qcom,adreno-XYZ.W", "qcom,adreno" or
-	      "amd,imageon-XYZ.W", "amd,imageon"
-    for example: "qcom,adreno-306.0", "qcom,adreno"
-  Note that you need to list the less specific "qcom,adreno" (since this
-  is what the device is matched on), in addition to the more specific
-  with the chip-id.
-  If "amd,imageon" is used, there should be no top level msm device.
-- reg: Physical base address and length of the controller's registers.
-- interrupts: The interrupt signal from the gpu.
-- clocks: device clocks (if applicable)
-  See ../clocks/clock-bindings.txt for details.
-- clock-names: the following clocks are required by a3xx, a4xx and a5xx
-  cores:
-  * "core"
-  * "iface"
-  * "mem_iface"
-  For GMU attached devices the GPU clocks are not used and are not required. The
-  following devices should not list clocks:
-   - qcom,adreno-630.2
-- iommus: optional phandle to an adreno iommu instance
-- operating-points-v2: optional phandle to the OPP operating points
-- interconnects: optional phandle to an interconnect provider.  See
-  ../interconnect/interconnect.txt for details. Some A3xx and all A4xx platforms
-  will have two paths; all others will have one path.
-- interconnect-names: The names of the interconnect paths that correspond to the
-  interconnects property. Values must be gfx-mem and ocmem.
-- qcom,gmu: For GMU attached devices a phandle to the GMU device that will
-  control the power for the GPU. Applicable targets:
-    - qcom,adreno-630.2
-- zap-shader: For a5xx and a6xx devices this node contains a memory-region that
-  points to reserved memory to store the zap shader that can be used to help
-  bring the GPU out of secure mode.
-- firmware-name: optional property of the 'zap-shader' node, listing the
-  relative path of the device specific zap firmware.
-- sram: phandle to the On Chip Memory (OCMEM) that's present on some a3xx and
-        a4xx Snapdragon SoCs. See
-        Documentation/devicetree/bindings/sram/qcom,ocmem.yaml.
-
-Optional properties:
-- #cooling-cells: The value must be 2. For details, please refer
-	Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml.
-
-Example 3xx/4xx:
-
-/ {
-	...
-
-	gpu: adreno@fdb00000 {
-		compatible = "qcom,adreno-330.2",
-		             "qcom,adreno";
-		reg = <0xfdb00000 0x10000>;
-		reg-names = "kgsl_3d0_reg_memory";
-		interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "kgsl_3d0_irq";
-		clock-names = "core",
-		              "iface",
-		              "mem_iface";
-		clocks = <&mmcc OXILI_GFX3D_CLK>,
-		         <&mmcc OXILICX_AHB_CLK>,
-		         <&mmcc OXILICX_AXI_CLK>;
-		sram = <&gpu_sram>;
-		power-domains = <&mmcc OXILICX_GDSC>;
-		operating-points-v2 = <&gpu_opp_table>;
-		iommus = <&gpu_iommu 0>;
-		#cooling-cells = <2>;
-	};
-
-	gpu_sram: ocmem@fdd00000 {
-		compatible = "qcom,msm8974-ocmem";
-
-		reg = <0xfdd00000 0x2000>,
-		      <0xfec00000 0x180000>;
-		reg-names = "ctrl",
-		            "mem";
-
-		clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
-		         <&mmcc OCMEMCX_OCMEMNOC_CLK>;
-		clock-names = "core",
-		              "iface";
-
-		#address-cells = <1>;
-		#size-cells = <1>;
-
-		gpu_sram: gpu-sram@0 {
-			reg = <0x0 0x100000>;
-			ranges = <0 0 0xfec00000 0x100000>;
-		};
-	};
-};
-
-Example a6xx (with GMU):
-
-/ {
-	...
-
-	gpu@5000000 {
-		compatible = "qcom,adreno-630.2", "qcom,adreno";
-		#stream-id-cells = <16>;
-
-		reg = <0x5000000 0x40000>, <0x509e000 0x10>;
-		reg-names = "kgsl_3d0_reg_memory", "cx_mem";
-
-		#cooling-cells = <2>;
-
-		/*
-		 * Look ma, no clocks! The GPU clocks and power are
-		 * controlled entirely by the GMU
-		 */
-
-		interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
-
-		iommus = <&adreno_smmu 0>;
-
-		operating-points-v2 = <&gpu_opp_table>;
-
-		interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>;
-		interconnect-names = "gfx-mem";
-
-		gpu_opp_table: opp-table {
-			compatible = "operating-points-v2";
-
-			opp-430000000 {
-				opp-hz = /bits/ 64 <430000000>;
-				opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
-				opp-peak-kBps = <5412000>;
-			};
-
-			opp-355000000 {
-				opp-hz = /bits/ 64 <355000000>;
-				opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
-				opp-peak-kBps = <3072000>;
-			};
-
-			opp-267000000 {
-				opp-hz = /bits/ 64 <267000000>;
-				opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
-				opp-peak-kBps = <3072000>;
-			};
-
-			opp-180000000 {
-				opp-hz = /bits/ 64 <180000000>;
-				opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
-				opp-peak-kBps = <1804000>;
-			};
-		};
-
-		qcom,gmu = <&gmu>;
-
-		zap-shader {
-			memory-region = <&zap_shader_region>;
-			firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn"
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
new file mode 100644
index 000000000000..4315482e0b12
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
@@ -0,0 +1,256 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: "http://devicetree.org/schemas/display/msm/gpu.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Devicetree bindings for the Adreno or Snapdragon GPUs
+
+maintainers:
+  - Rob Clark <robdclark@gmail.com>
+
+description: |
+  These bindings describe the GPUs
+
+properties:
+  compatible:
+    anyOf:
+      - items:
+          - pattern: '^qcom,adreno-[3-6][0-9][0-9].[0-9]$'
+          - const: qcom,adreno
+      - items:
+          - pattern: '^amd,imageon-200.[0-1]$'
+          - const: amd,imageon
+
+  clocks:
+    maxItems: 3
+
+  clock-names:
+    maxItems: 3
+
+  reg:
+    minItems: 1
+    maxItems: 2
+    description: Physical base address and length of the controller's registers.
+
+  reg-names:
+    minItems: 1
+    maxItems: 2
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    maxItems: 1
+
+  interconnects:
+    minItems: 1
+    maxItems: 2
+    description: |
+      optional phandle to an interconnect provider. See
+      ../interconnect/interconnect.txt for details.
+      Some A3xx and all A4xx platforms will have two paths;
+      all others will have one path.
+
+  interconnect-names:
+    items:
+      - const: gfx-mem
+      - const: ocmem
+    description: |
+      the names of the interconnect paths that correspond to
+      the interconnects property
+
+  iommus:
+    maxItems: 1
+
+  sram:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 1
+    maxItems: 4
+    description: |
+      phandles to one or more reserved on-chip SRAM regions.
+      phandle to the On Chip Memory (OCMEM) that's present on some a3xx and
+      a4xx Snapdragon SoCs. See
+      Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
+
+  operating-points-v2: true
+  opp-table: true
+
+  power-domains:
+    maxItems: 1
+
+  zap-shader:
+    description: |
+      For a5xx and a6xx devices this node contains a memory-region that
+      points to reserved memory to store the zap shader that can be used to
+      help bring the GPU out of secure mode.
+
+  "#cooling-cells":
+    const: 2
+    description: |
+      For details, please refer
+      Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
+
+  qcom,gmu:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      for GMU attached devices a phandle to the GMU device that will
+      control the power for the GPU
+
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            pattern: '^qcom,adreno-[3-5][0-9][0-9].[0-9]$'
+
+    then:
+      properties:
+        clocks:
+          items:
+            - description: GPU Core clock
+            - description: GPU Interface clock
+            - description: GPU Memory Interface clock
+
+        clock-names:
+          items:
+            - const: core
+            - const: iface
+            - const: mem_iface
+      required:
+        - clocks
+        - clock-names
+
+examples:
+  - |
+
+    // Example a3xx/4xx:
+
+    #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
+    #include <dt-bindings/clock/qcom,rpmcc.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    gpu: adreno@fdb00000 {
+        compatible = "qcom,adreno-330.2", "qcom,adreno";
+
+        reg = <0xfdb00000 0x10000>;
+        reg-names = "kgsl_3d0_reg_memory";
+
+        clock-names = "core", "iface", "mem_iface";
+        clocks = <&mmcc OXILI_GFX3D_CLK>,
+                 <&mmcc OXILICX_AHB_CLK>,
+                 <&mmcc OXILICX_AXI_CLK>;
+
+        interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "kgsl_3d0_irq";
+
+        sram = <&gpu_sram>;
+        power-domains = <&mmcc OXILICX_GDSC>;
+        operating-points-v2 = <&gpu_opp_table>;
+        iommus = <&gpu_iommu 0>;
+        #cooling-cells = <2>;
+    };
+
+    ocmem@fdd00000 {
+        compatible = "qcom,msm8974-ocmem";
+
+        reg = <0xfdd00000 0x2000>,
+              <0xfec00000 0x180000>;
+        reg-names = "ctrl", "mem";
+
+        clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
+                 <&mmcc OCMEMCX_OCMEMNOC_CLK>;
+        clock-names = "core", "iface";
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges = <0 0xfec00000 0x100000>;
+
+        gpu_sram: gpu-sram@0 {
+            reg = <0x0 0x100000>;
+        };
+    };
+  - |
+
+    // Example a6xx (with GMU):
+
+    #include <dt-bindings/clock/qcom,gpucc-sdm845.h>
+    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interconnect/qcom,sdm845.h>
+
+    reserved-memory {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        zap_shader_region: gpu@8f200000 {
+            compatible = "shared-dma-pool";
+            reg = <0x0 0x90b00000 0x0 0xa00000>;
+            no-map;
+        };
+    };
+
+    gpu@5000000 {
+        compatible = "qcom,adreno-630.2", "qcom,adreno";
+
+        reg = <0x5000000 0x40000>, <0x509e000 0x10>;
+        reg-names = "kgsl_3d0_reg_memory", "cx_mem";
+
+        #cooling-cells = <2>;
+
+        interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
+
+        iommus = <&adreno_smmu 0>;
+
+        operating-points-v2 = <&gpu_opp_table>;
+
+        interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>;
+        interconnect-names = "gfx-mem";
+
+        qcom,gmu = <&gmu>;
+
+        gpu_opp_table: opp-table {
+            compatible = "operating-points-v2";
+
+            opp-430000000 {
+                opp-hz = /bits/ 64 <430000000>;
+                opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
+                opp-peak-kBps = <5412000>;
+            };
+
+            opp-355000000 {
+                opp-hz = /bits/ 64 <355000000>;
+                opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
+                opp-peak-kBps = <3072000>;
+            };
+
+            opp-267000000 {
+                opp-hz = /bits/ 64 <267000000>;
+                opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+                opp-peak-kBps = <3072000>;
+            };
+
+            opp-180000000 {
+                opp-hz = /bits/ 64 <180000000>;
+                opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
+                opp-peak-kBps = <1804000>;
+            };
+        };
+
+        zap-shader {
+            memory-region = <&zap_shader_region>;
+            firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn";
+        };
+    };
-- 
2.30.2


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

* Re: [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML
  2021-07-03 15:18 [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML David Heidelberg
@ 2021-07-04  8:18 ` Dmitry Baryshkov
  2021-07-04  9:31   ` David Heidelberg
  2021-07-12 14:51 ` Rob Herring
  2021-07-12 15:18 ` Rob Clark
  2 siblings, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2021-07-04  8:18 UTC (permalink / raw)
  To: David Heidelberg
  Cc: Sharat Masetty, masneyb, Rob Clark, Jordan Crouse,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:DRM DRIVER FOR MSM ADRENO GPU

On Sat, 3 Jul 2021 at 18:19, David Heidelberg <david@ixit.cz> wrote:
>
> This warning cannot be fixed by conversion, since this naming is already used.
> Documentation/devicetree/bindings/display/msm/gpu.example.dt.yaml: gpu@5000000: interconnect-names: ['gfx-mem'] is too short

It is not a problem of the name length. You've declared that
interconnect-names would contain two items (gfx-mem and ocmem), but
add just one.

>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  .../devicetree/bindings/display/msm/gpu.txt   | 157 -----------
>  .../devicetree/bindings/display/msm/gpu.yaml  | 256 ++++++++++++++++++
>  2 files changed, 256 insertions(+), 157 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/display/msm/gpu.txt
>  create mode 100644 Documentation/devicetree/bindings/display/msm/gpu.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt
> deleted file mode 100644
> index 090dcb3fc34d..000000000000
> --- a/Documentation/devicetree/bindings/display/msm/gpu.txt
> +++ /dev/null
> @@ -1,157 +0,0 @@
> -Qualcomm adreno/snapdragon GPU
> -
> -Required properties:
> -- compatible: "qcom,adreno-XYZ.W", "qcom,adreno" or
> -             "amd,imageon-XYZ.W", "amd,imageon"
> -    for example: "qcom,adreno-306.0", "qcom,adreno"
> -  Note that you need to list the less specific "qcom,adreno" (since this
> -  is what the device is matched on), in addition to the more specific
> -  with the chip-id.
> -  If "amd,imageon" is used, there should be no top level msm device.
> -- reg: Physical base address and length of the controller's registers.
> -- interrupts: The interrupt signal from the gpu.
> -- clocks: device clocks (if applicable)
> -  See ../clocks/clock-bindings.txt for details.
> -- clock-names: the following clocks are required by a3xx, a4xx and a5xx
> -  cores:
> -  * "core"
> -  * "iface"
> -  * "mem_iface"
> -  For GMU attached devices the GPU clocks are not used and are not required. The
> -  following devices should not list clocks:
> -   - qcom,adreno-630.2
> -- iommus: optional phandle to an adreno iommu instance
> -- operating-points-v2: optional phandle to the OPP operating points
> -- interconnects: optional phandle to an interconnect provider.  See
> -  ../interconnect/interconnect.txt for details. Some A3xx and all A4xx platforms
> -  will have two paths; all others will have one path.
> -- interconnect-names: The names of the interconnect paths that correspond to the
> -  interconnects property. Values must be gfx-mem and ocmem.
> -- qcom,gmu: For GMU attached devices a phandle to the GMU device that will
> -  control the power for the GPU. Applicable targets:
> -    - qcom,adreno-630.2
> -- zap-shader: For a5xx and a6xx devices this node contains a memory-region that
> -  points to reserved memory to store the zap shader that can be used to help
> -  bring the GPU out of secure mode.
> -- firmware-name: optional property of the 'zap-shader' node, listing the
> -  relative path of the device specific zap firmware.
> -- sram: phandle to the On Chip Memory (OCMEM) that's present on some a3xx and
> -        a4xx Snapdragon SoCs. See
> -        Documentation/devicetree/bindings/sram/qcom,ocmem.yaml.
> -
> -Optional properties:
> -- #cooling-cells: The value must be 2. For details, please refer
> -       Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml.
> -
> -Example 3xx/4xx:
> -
> -/ {
> -       ...
> -
> -       gpu: adreno@fdb00000 {
> -               compatible = "qcom,adreno-330.2",
> -                            "qcom,adreno";
> -               reg = <0xfdb00000 0x10000>;
> -               reg-names = "kgsl_3d0_reg_memory";
> -               interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> -               interrupt-names = "kgsl_3d0_irq";
> -               clock-names = "core",
> -                             "iface",
> -                             "mem_iface";
> -               clocks = <&mmcc OXILI_GFX3D_CLK>,
> -                        <&mmcc OXILICX_AHB_CLK>,
> -                        <&mmcc OXILICX_AXI_CLK>;
> -               sram = <&gpu_sram>;
> -               power-domains = <&mmcc OXILICX_GDSC>;
> -               operating-points-v2 = <&gpu_opp_table>;
> -               iommus = <&gpu_iommu 0>;
> -               #cooling-cells = <2>;
> -       };
> -
> -       gpu_sram: ocmem@fdd00000 {
> -               compatible = "qcom,msm8974-ocmem";
> -
> -               reg = <0xfdd00000 0x2000>,
> -                     <0xfec00000 0x180000>;
> -               reg-names = "ctrl",
> -                           "mem";
> -
> -               clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
> -                        <&mmcc OCMEMCX_OCMEMNOC_CLK>;
> -               clock-names = "core",
> -                             "iface";
> -
> -               #address-cells = <1>;
> -               #size-cells = <1>;
> -
> -               gpu_sram: gpu-sram@0 {
> -                       reg = <0x0 0x100000>;
> -                       ranges = <0 0 0xfec00000 0x100000>;
> -               };
> -       };
> -};
> -
> -Example a6xx (with GMU):
> -
> -/ {
> -       ...
> -
> -       gpu@5000000 {
> -               compatible = "qcom,adreno-630.2", "qcom,adreno";
> -               #stream-id-cells = <16>;
> -
> -               reg = <0x5000000 0x40000>, <0x509e000 0x10>;
> -               reg-names = "kgsl_3d0_reg_memory", "cx_mem";
> -
> -               #cooling-cells = <2>;
> -
> -               /*
> -                * Look ma, no clocks! The GPU clocks and power are
> -                * controlled entirely by the GMU
> -                */
> -
> -               interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> -
> -               iommus = <&adreno_smmu 0>;
> -
> -               operating-points-v2 = <&gpu_opp_table>;
> -
> -               interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>;
> -               interconnect-names = "gfx-mem";
> -
> -               gpu_opp_table: opp-table {
> -                       compatible = "operating-points-v2";
> -
> -                       opp-430000000 {
> -                               opp-hz = /bits/ 64 <430000000>;
> -                               opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> -                               opp-peak-kBps = <5412000>;
> -                       };
> -
> -                       opp-355000000 {
> -                               opp-hz = /bits/ 64 <355000000>;
> -                               opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
> -                               opp-peak-kBps = <3072000>;
> -                       };
> -
> -                       opp-267000000 {
> -                               opp-hz = /bits/ 64 <267000000>;
> -                               opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> -                               opp-peak-kBps = <3072000>;
> -                       };
> -
> -                       opp-180000000 {
> -                               opp-hz = /bits/ 64 <180000000>;
> -                               opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> -                               opp-peak-kBps = <1804000>;
> -                       };
> -               };
> -
> -               qcom,gmu = <&gmu>;
> -
> -               zap-shader {
> -                       memory-region = <&zap_shader_region>;
> -                       firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn"
> -               };
> -       };
> -};
> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> new file mode 100644
> index 000000000000..4315482e0b12
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> @@ -0,0 +1,256 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: "http://devicetree.org/schemas/display/msm/gpu.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Devicetree bindings for the Adreno or Snapdragon GPUs
> +
> +maintainers:
> +  - Rob Clark <robdclark@gmail.com>
> +
> +description: |
> +  These bindings describe the GPUs
> +
> +properties:
> +  compatible:
> +    anyOf:
> +      - items:
> +          - pattern: '^qcom,adreno-[3-6][0-9][0-9].[0-9]$'
> +          - const: qcom,adreno
> +      - items:
> +          - pattern: '^amd,imageon-200.[0-1]$'
> +          - const: amd,imageon
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    maxItems: 3
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2
> +    description: Physical base address and length of the controller's registers.
> +
> +  reg-names:
> +    minItems: 1
> +    maxItems: 2
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-names:
> +    maxItems: 1
> +
> +  interconnects:
> +    minItems: 1
> +    maxItems: 2
> +    description: |
> +      optional phandle to an interconnect provider. See
> +      ../interconnect/interconnect.txt for details.
> +      Some A3xx and all A4xx platforms will have two paths;
> +      all others will have one path.
> +
> +  interconnect-names:
> +    items:
> +      - const: gfx-mem
> +      - const: ocmem
> +    description: |
> +      the names of the interconnect paths that correspond to
> +      the interconnects property
> +
> +  iommus:
> +    maxItems: 1
> +
> +  sram:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    minItems: 1
> +    maxItems: 4
> +    description: |
> +      phandles to one or more reserved on-chip SRAM regions.
> +      phandle to the On Chip Memory (OCMEM) that's present on some a3xx and
> +      a4xx Snapdragon SoCs. See
> +      Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> +
> +  operating-points-v2: true
> +  opp-table: true
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  zap-shader:
> +    description: |
> +      For a5xx and a6xx devices this node contains a memory-region that
> +      points to reserved memory to store the zap shader that can be used to
> +      help bring the GPU out of secure mode.
> +
> +  "#cooling-cells":
> +    const: 2
> +    description: |
> +      For details, please refer
> +      Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
> +
> +  qcom,gmu:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      for GMU attached devices a phandle to the GMU device that will
> +      control the power for the GPU
> +
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            pattern: '^qcom,adreno-[3-5][0-9][0-9].[0-9]$'
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: GPU Core clock
> +            - description: GPU Interface clock
> +            - description: GPU Memory Interface clock
> +
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: iface
> +            - const: mem_iface
> +      required:
> +        - clocks
> +        - clock-names
> +
> +examples:
> +  - |
> +
> +    // Example a3xx/4xx:
> +
> +    #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> +    #include <dt-bindings/clock/qcom,rpmcc.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    gpu: adreno@fdb00000 {
> +        compatible = "qcom,adreno-330.2", "qcom,adreno";
> +
> +        reg = <0xfdb00000 0x10000>;
> +        reg-names = "kgsl_3d0_reg_memory";
> +
> +        clock-names = "core", "iface", "mem_iface";
> +        clocks = <&mmcc OXILI_GFX3D_CLK>,
> +                 <&mmcc OXILICX_AHB_CLK>,
> +                 <&mmcc OXILICX_AXI_CLK>;
> +
> +        interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-names = "kgsl_3d0_irq";
> +
> +        sram = <&gpu_sram>;
> +        power-domains = <&mmcc OXILICX_GDSC>;
> +        operating-points-v2 = <&gpu_opp_table>;
> +        iommus = <&gpu_iommu 0>;
> +        #cooling-cells = <2>;
> +    };
> +
> +    ocmem@fdd00000 {
> +        compatible = "qcom,msm8974-ocmem";
> +
> +        reg = <0xfdd00000 0x2000>,
> +              <0xfec00000 0x180000>;
> +        reg-names = "ctrl", "mem";
> +
> +        clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
> +                 <&mmcc OCMEMCX_OCMEMNOC_CLK>;
> +        clock-names = "core", "iface";
> +
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0 0xfec00000 0x100000>;
> +
> +        gpu_sram: gpu-sram@0 {
> +            reg = <0x0 0x100000>;
> +        };
> +    };
> +  - |
> +
> +    // Example a6xx (with GMU):
> +
> +    #include <dt-bindings/clock/qcom,gpucc-sdm845.h>
> +    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> +    #include <dt-bindings/power/qcom-rpmpd.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interconnect/qcom,sdm845.h>
> +
> +    reserved-memory {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        zap_shader_region: gpu@8f200000 {
> +            compatible = "shared-dma-pool";
> +            reg = <0x0 0x90b00000 0x0 0xa00000>;
> +            no-map;
> +        };
> +    };
> +
> +    gpu@5000000 {
> +        compatible = "qcom,adreno-630.2", "qcom,adreno";
> +
> +        reg = <0x5000000 0x40000>, <0x509e000 0x10>;
> +        reg-names = "kgsl_3d0_reg_memory", "cx_mem";
> +
> +        #cooling-cells = <2>;
> +
> +        interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> +
> +        iommus = <&adreno_smmu 0>;
> +
> +        operating-points-v2 = <&gpu_opp_table>;
> +
> +        interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>;
> +        interconnect-names = "gfx-mem";
> +
> +        qcom,gmu = <&gmu>;
> +
> +        gpu_opp_table: opp-table {
> +            compatible = "operating-points-v2";
> +
> +            opp-430000000 {
> +                opp-hz = /bits/ 64 <430000000>;
> +                opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> +                opp-peak-kBps = <5412000>;
> +            };
> +
> +            opp-355000000 {
> +                opp-hz = /bits/ 64 <355000000>;
> +                opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
> +                opp-peak-kBps = <3072000>;
> +            };
> +
> +            opp-267000000 {
> +                opp-hz = /bits/ 64 <267000000>;
> +                opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> +                opp-peak-kBps = <3072000>;
> +            };
> +
> +            opp-180000000 {
> +                opp-hz = /bits/ 64 <180000000>;
> +                opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> +                opp-peak-kBps = <1804000>;
> +            };
> +        };
> +
> +        zap-shader {
> +            memory-region = <&zap_shader_region>;
> +            firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn";
> +        };
> +    };
> --
> 2.30.2
>


-- 
With best wishes
Dmitry

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

* Re: [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML
  2021-07-04  8:18 ` Dmitry Baryshkov
@ 2021-07-04  9:31   ` David Heidelberg
  0 siblings, 0 replies; 6+ messages in thread
From: David Heidelberg @ 2021-07-04  9:31 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Sharat Masetty, masneyb, Rob Clark, Jordan Crouse,
	OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Thank you Dmitry,

I'll fix it for [v2]. I'll wait a little bit for some feedback now.
Best regards
David Heidelberg

On Sun, Jul 4 2021 at 11:18:36 +0300, Dmitry Baryshkov 
<dmitry.baryshkov@linaro.org> wrote:
> On Sat, 3 Jul 2021 at 18:19, David Heidelberg <david@ixit.cz> wrote:
>> 
>>  This warning cannot be fixed by conversion, since this naming is 
>> already used.
>>  Documentation/devicetree/bindings/display/msm/gpu.example.dt.yaml: 
>> gpu@5000000: interconnect-names: ['gfx-mem'] is too short
> 
> It is not a problem of the name length. You've declared that
> interconnect-names would contain two items (gfx-mem and ocmem), but
> add just one.
> 
>> 
>>  Signed-off-by: David Heidelberg <david@ixit.cz>
>>  ---
>>   .../devicetree/bindings/display/msm/gpu.txt   | 157 -----------
>>   .../devicetree/bindings/display/msm/gpu.yaml  | 256 
>> ++++++++++++++++++
>>   2 files changed, 256 insertions(+), 157 deletions(-)
>>   delete mode 100644 
>> Documentation/devicetree/bindings/display/msm/gpu.txt
>>   create mode 100644 
>> Documentation/devicetree/bindings/display/msm/gpu.yaml
>> 
>>  diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt 
>> b/Documentation/devicetree/bindings/display/msm/gpu.txt
>>  deleted file mode 100644
>>  index 090dcb3fc34d..000000000000
>>  --- a/Documentation/devicetree/bindings/display/msm/gpu.txt
>>  +++ /dev/null
>>  @@ -1,157 +0,0 @@
>>  -Qualcomm adreno/snapdragon GPU
>>  -
>>  -Required properties:
>>  -- compatible: "qcom,adreno-XYZ.W", "qcom,adreno" or
>>  -             "amd,imageon-XYZ.W", "amd,imageon"
>>  -    for example: "qcom,adreno-306.0", "qcom,adreno"
>>  -  Note that you need to list the less specific "qcom,adreno" 
>> (since this
>>  -  is what the device is matched on), in addition to the more 
>> specific
>>  -  with the chip-id.
>>  -  If "amd,imageon" is used, there should be no top level msm 
>> device.
>>  -- reg: Physical base address and length of the controller's 
>> registers.
>>  -- interrupts: The interrupt signal from the gpu.
>>  -- clocks: device clocks (if applicable)
>>  -  See ../clocks/clock-bindings.txt for details.
>>  -- clock-names: the following clocks are required by a3xx, a4xx and 
>> a5xx
>>  -  cores:
>>  -  * "core"
>>  -  * "iface"
>>  -  * "mem_iface"
>>  -  For GMU attached devices the GPU clocks are not used and are not 
>> required. The
>>  -  following devices should not list clocks:
>>  -   - qcom,adreno-630.2
>>  -- iommus: optional phandle to an adreno iommu instance
>>  -- operating-points-v2: optional phandle to the OPP operating points
>>  -- interconnects: optional phandle to an interconnect provider.  See
>>  -  ../interconnect/interconnect.txt for details. Some A3xx and all 
>> A4xx platforms
>>  -  will have two paths; all others will have one path.
>>  -- interconnect-names: The names of the interconnect paths that 
>> correspond to the
>>  -  interconnects property. Values must be gfx-mem and ocmem.
>>  -- qcom,gmu: For GMU attached devices a phandle to the GMU device 
>> that will
>>  -  control the power for the GPU. Applicable targets:
>>  -    - qcom,adreno-630.2
>>  -- zap-shader: For a5xx and a6xx devices this node contains a 
>> memory-region that
>>  -  points to reserved memory to store the zap shader that can be 
>> used to help
>>  -  bring the GPU out of secure mode.
>>  -- firmware-name: optional property of the 'zap-shader' node, 
>> listing the
>>  -  relative path of the device specific zap firmware.
>>  -- sram: phandle to the On Chip Memory (OCMEM) that's present on 
>> some a3xx and
>>  -        a4xx Snapdragon SoCs. See
>>  -        Documentation/devicetree/bindings/sram/qcom,ocmem.yaml.
>>  -
>>  -Optional properties:
>>  -- #cooling-cells: The value must be 2. For details, please refer
>>  -       
>> Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml.
>>  -
>>  -Example 3xx/4xx:
>>  -
>>  -/ {
>>  -       ...
>>  -
>>  -       gpu: adreno@fdb00000 {
>>  -               compatible = "qcom,adreno-330.2",
>>  -                            "qcom,adreno";
>>  -               reg = <0xfdb00000 0x10000>;
>>  -               reg-names = "kgsl_3d0_reg_memory";
>>  -               interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
>>  -               interrupt-names = "kgsl_3d0_irq";
>>  -               clock-names = "core",
>>  -                             "iface",
>>  -                             "mem_iface";
>>  -               clocks = <&mmcc OXILI_GFX3D_CLK>,
>>  -                        <&mmcc OXILICX_AHB_CLK>,
>>  -                        <&mmcc OXILICX_AXI_CLK>;
>>  -               sram = <&gpu_sram>;
>>  -               power-domains = <&mmcc OXILICX_GDSC>;
>>  -               operating-points-v2 = <&gpu_opp_table>;
>>  -               iommus = <&gpu_iommu 0>;
>>  -               #cooling-cells = <2>;
>>  -       };
>>  -
>>  -       gpu_sram: ocmem@fdd00000 {
>>  -               compatible = "qcom,msm8974-ocmem";
>>  -
>>  -               reg = <0xfdd00000 0x2000>,
>>  -                     <0xfec00000 0x180000>;
>>  -               reg-names = "ctrl",
>>  -                           "mem";
>>  -
>>  -               clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
>>  -                        <&mmcc OCMEMCX_OCMEMNOC_CLK>;
>>  -               clock-names = "core",
>>  -                             "iface";
>>  -
>>  -               #address-cells = <1>;
>>  -               #size-cells = <1>;
>>  -
>>  -               gpu_sram: gpu-sram@0 {
>>  -                       reg = <0x0 0x100000>;
>>  -                       ranges = <0 0 0xfec00000 0x100000>;
>>  -               };
>>  -       };
>>  -};
>>  -
>>  -Example a6xx (with GMU):
>>  -
>>  -/ {
>>  -       ...
>>  -
>>  -       gpu@5000000 {
>>  -               compatible = "qcom,adreno-630.2", "qcom,adreno";
>>  -               #stream-id-cells = <16>;
>>  -
>>  -               reg = <0x5000000 0x40000>, <0x509e000 0x10>;
>>  -               reg-names = "kgsl_3d0_reg_memory", "cx_mem";
>>  -
>>  -               #cooling-cells = <2>;
>>  -
>>  -               /*
>>  -                * Look ma, no clocks! The GPU clocks and power are
>>  -                * controlled entirely by the GMU
>>  -                */
>>  -
>>  -               interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
>>  -
>>  -               iommus = <&adreno_smmu 0>;
>>  -
>>  -               operating-points-v2 = <&gpu_opp_table>;
>>  -
>>  -               interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos 
>> SLAVE_EBI1>;
>>  -               interconnect-names = "gfx-mem";
>>  -
>>  -               gpu_opp_table: opp-table {
>>  -                       compatible = "operating-points-v2";
>>  -
>>  -                       opp-430000000 {
>>  -                               opp-hz = /bits/ 64 <430000000>;
>>  -                               opp-level = 
>> <RPMH_REGULATOR_LEVEL_SVS_L1>;
>>  -                               opp-peak-kBps = <5412000>;
>>  -                       };
>>  -
>>  -                       opp-355000000 {
>>  -                               opp-hz = /bits/ 64 <355000000>;
>>  -                               opp-level = 
>> <RPMH_REGULATOR_LEVEL_SVS>;
>>  -                               opp-peak-kBps = <3072000>;
>>  -                       };
>>  -
>>  -                       opp-267000000 {
>>  -                               opp-hz = /bits/ 64 <267000000>;
>>  -                               opp-level = 
>> <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>>  -                               opp-peak-kBps = <3072000>;
>>  -                       };
>>  -
>>  -                       opp-180000000 {
>>  -                               opp-hz = /bits/ 64 <180000000>;
>>  -                               opp-level = 
>> <RPMH_REGULATOR_LEVEL_MIN_SVS>;
>>  -                               opp-peak-kBps = <1804000>;
>>  -                       };
>>  -               };
>>  -
>>  -               qcom,gmu = <&gmu>;
>>  -
>>  -               zap-shader {
>>  -                       memory-region = <&zap_shader_region>;
>>  -                       firmware-name = 
>> "qcom/LENOVO/81JL/qcdxkmsuc850.mbn"
>>  -               };
>>  -       };
>>  -};
>>  diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml 
>> b/Documentation/devicetree/bindings/display/msm/gpu.yaml
>>  new file mode 100644
>>  index 000000000000..4315482e0b12
>>  --- /dev/null
>>  +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
>>  @@ -0,0 +1,256 @@
>>  +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>  +%YAML 1.2
>>  +---
>>  +
>>  +$id: "http://devicetree.org/schemas/display/msm/gpu.yaml#"
>>  +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>>  +
>>  +title: Devicetree bindings for the Adreno or Snapdragon GPUs
>>  +
>>  +maintainers:
>>  +  - Rob Clark <robdclark@gmail.com>
>>  +
>>  +description: |
>>  +  These bindings describe the GPUs
>>  +
>>  +properties:
>>  +  compatible:
>>  +    anyOf:
>>  +      - items:
>>  +          - pattern: '^qcom,adreno-[3-6][0-9][0-9].[0-9]$'
>>  +          - const: qcom,adreno
>>  +      - items:
>>  +          - pattern: '^amd,imageon-200.[0-1]$'
>>  +          - const: amd,imageon
>>  +
>>  +  clocks:
>>  +    maxItems: 3
>>  +
>>  +  clock-names:
>>  +    maxItems: 3
>>  +
>>  +  reg:
>>  +    minItems: 1
>>  +    maxItems: 2
>>  +    description: Physical base address and length of the 
>> controller's registers.
>>  +
>>  +  reg-names:
>>  +    minItems: 1
>>  +    maxItems: 2
>>  +
>>  +  interrupts:
>>  +    maxItems: 1
>>  +
>>  +  interrupt-names:
>>  +    maxItems: 1
>>  +
>>  +  interconnects:
>>  +    minItems: 1
>>  +    maxItems: 2
>>  +    description: |
>>  +      optional phandle to an interconnect provider. See
>>  +      ../interconnect/interconnect.txt for details.
>>  +      Some A3xx and all A4xx platforms will have two paths;
>>  +      all others will have one path.
>>  +
>>  +  interconnect-names:
>>  +    items:
>>  +      - const: gfx-mem
>>  +      - const: ocmem
>>  +    description: |
>>  +      the names of the interconnect paths that correspond to
>>  +      the interconnects property
>>  +
>>  +  iommus:
>>  +    maxItems: 1
>>  +
>>  +  sram:
>>  +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>  +    minItems: 1
>>  +    maxItems: 4
>>  +    description: |
>>  +      phandles to one or more reserved on-chip SRAM regions.
>>  +      phandle to the On Chip Memory (OCMEM) that's present on some 
>> a3xx and
>>  +      a4xx Snapdragon SoCs. See
>>  +      Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
>>  +
>>  +  operating-points-v2: true
>>  +  opp-table: true
>>  +
>>  +  power-domains:
>>  +    maxItems: 1
>>  +
>>  +  zap-shader:
>>  +    description: |
>>  +      For a5xx and a6xx devices this node contains a memory-region 
>> that
>>  +      points to reserved memory to store the zap shader that can 
>> be used to
>>  +      help bring the GPU out of secure mode.
>>  +
>>  +  "#cooling-cells":
>>  +    const: 2
>>  +    description: |
>>  +      For details, please refer
>>  +      
>> Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
>>  +
>>  +  qcom,gmu:
>>  +    $ref: /schemas/types.yaml#/definitions/phandle
>>  +    description: |
>>  +      for GMU attached devices a phandle to the GMU device that 
>> will
>>  +      control the power for the GPU
>>  +
>>  +
>>  +required:
>>  +  - compatible
>>  +  - reg
>>  +  - interrupts
>>  +
>>  +additionalProperties: false
>>  +
>>  +allOf:
>>  +  - if:
>>  +      properties:
>>  +        compatible:
>>  +          contains:
>>  +            pattern: '^qcom,adreno-[3-5][0-9][0-9].[0-9]$'
>>  +
>>  +    then:
>>  +      properties:
>>  +        clocks:
>>  +          items:
>>  +            - description: GPU Core clock
>>  +            - description: GPU Interface clock
>>  +            - description: GPU Memory Interface clock
>>  +
>>  +        clock-names:
>>  +          items:
>>  +            - const: core
>>  +            - const: iface
>>  +            - const: mem_iface
>>  +      required:
>>  +        - clocks
>>  +        - clock-names
>>  +
>>  +examples:
>>  +  - |
>>  +
>>  +    // Example a3xx/4xx:
>>  +
>>  +    #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
>>  +    #include <dt-bindings/clock/qcom,rpmcc.h>
>>  +    #include <dt-bindings/interrupt-controller/irq.h>
>>  +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>  +
>>  +    gpu: adreno@fdb00000 {
>>  +        compatible = "qcom,adreno-330.2", "qcom,adreno";
>>  +
>>  +        reg = <0xfdb00000 0x10000>;
>>  +        reg-names = "kgsl_3d0_reg_memory";
>>  +
>>  +        clock-names = "core", "iface", "mem_iface";
>>  +        clocks = <&mmcc OXILI_GFX3D_CLK>,
>>  +                 <&mmcc OXILICX_AHB_CLK>,
>>  +                 <&mmcc OXILICX_AXI_CLK>;
>>  +
>>  +        interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
>>  +        interrupt-names = "kgsl_3d0_irq";
>>  +
>>  +        sram = <&gpu_sram>;
>>  +        power-domains = <&mmcc OXILICX_GDSC>;
>>  +        operating-points-v2 = <&gpu_opp_table>;
>>  +        iommus = <&gpu_iommu 0>;
>>  +        #cooling-cells = <2>;
>>  +    };
>>  +
>>  +    ocmem@fdd00000 {
>>  +        compatible = "qcom,msm8974-ocmem";
>>  +
>>  +        reg = <0xfdd00000 0x2000>,
>>  +              <0xfec00000 0x180000>;
>>  +        reg-names = "ctrl", "mem";
>>  +
>>  +        clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
>>  +                 <&mmcc OCMEMCX_OCMEMNOC_CLK>;
>>  +        clock-names = "core", "iface";
>>  +
>>  +        #address-cells = <1>;
>>  +        #size-cells = <1>;
>>  +        ranges = <0 0xfec00000 0x100000>;
>>  +
>>  +        gpu_sram: gpu-sram@0 {
>>  +            reg = <0x0 0x100000>;
>>  +        };
>>  +    };
>>  +  - |
>>  +
>>  +    // Example a6xx (with GMU):
>>  +
>>  +    #include <dt-bindings/clock/qcom,gpucc-sdm845.h>
>>  +    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
>>  +    #include <dt-bindings/power/qcom-rpmpd.h>
>>  +    #include <dt-bindings/interrupt-controller/irq.h>
>>  +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>  +    #include <dt-bindings/interconnect/qcom,sdm845.h>
>>  +
>>  +    reserved-memory {
>>  +        #address-cells = <2>;
>>  +        #size-cells = <2>;
>>  +
>>  +        zap_shader_region: gpu@8f200000 {
>>  +            compatible = "shared-dma-pool";
>>  +            reg = <0x0 0x90b00000 0x0 0xa00000>;
>>  +            no-map;
>>  +        };
>>  +    };
>>  +
>>  +    gpu@5000000 {
>>  +        compatible = "qcom,adreno-630.2", "qcom,adreno";
>>  +
>>  +        reg = <0x5000000 0x40000>, <0x509e000 0x10>;
>>  +        reg-names = "kgsl_3d0_reg_memory", "cx_mem";
>>  +
>>  +        #cooling-cells = <2>;
>>  +
>>  +        interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
>>  +
>>  +        iommus = <&adreno_smmu 0>;
>>  +
>>  +        operating-points-v2 = <&gpu_opp_table>;
>>  +
>>  +        interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos 
>> SLAVE_EBI1>;
>>  +        interconnect-names = "gfx-mem";
>>  +
>>  +        qcom,gmu = <&gmu>;
>>  +
>>  +        gpu_opp_table: opp-table {
>>  +            compatible = "operating-points-v2";
>>  +
>>  +            opp-430000000 {
>>  +                opp-hz = /bits/ 64 <430000000>;
>>  +                opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
>>  +                opp-peak-kBps = <5412000>;
>>  +            };
>>  +
>>  +            opp-355000000 {
>>  +                opp-hz = /bits/ 64 <355000000>;
>>  +                opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
>>  +                opp-peak-kBps = <3072000>;
>>  +            };
>>  +
>>  +            opp-267000000 {
>>  +                opp-hz = /bits/ 64 <267000000>;
>>  +                opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
>>  +                opp-peak-kBps = <3072000>;
>>  +            };
>>  +
>>  +            opp-180000000 {
>>  +                opp-hz = /bits/ 64 <180000000>;
>>  +                opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
>>  +                opp-peak-kBps = <1804000>;
>>  +            };
>>  +        };
>>  +
>>  +        zap-shader {
>>  +            memory-region = <&zap_shader_region>;
>>  +            firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn";
>>  +        };
>>  +    };
>>  --
>>  2.30.2
>> 
> 
> 
> --
> With best wishes
> Dmitry



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

* Re: [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML
  2021-07-03 15:18 [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML David Heidelberg
  2021-07-04  8:18 ` Dmitry Baryshkov
@ 2021-07-12 14:51 ` Rob Herring
  2021-07-12 15:08   ` Rob Clark
  2021-07-12 15:18 ` Rob Clark
  2 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2021-07-12 14:51 UTC (permalink / raw)
  To: David Heidelberg
  Cc: smasetty, masneyb, robdclark, jordan, devicetree, linux-arm-msm

On Sat, Jul 03, 2021 at 05:18:35PM +0200, David Heidelberg wrote:
> This warning cannot be fixed by conversion, since this naming is already used.
> Documentation/devicetree/bindings/display/msm/gpu.example.dt.yaml: gpu@5000000: interconnect-names: ['gfx-mem'] is too short
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  .../devicetree/bindings/display/msm/gpu.txt   | 157 -----------
>  .../devicetree/bindings/display/msm/gpu.yaml  | 256 ++++++++++++++++++
>  2 files changed, 256 insertions(+), 157 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/display/msm/gpu.txt
>  create mode 100644 Documentation/devicetree/bindings/display/msm/gpu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt
> deleted file mode 100644
> index 090dcb3fc34d..000000000000
> --- a/Documentation/devicetree/bindings/display/msm/gpu.txt
> +++ /dev/null
> @@ -1,157 +0,0 @@
> -Qualcomm adreno/snapdragon GPU
> -
> -Required properties:
> -- compatible: "qcom,adreno-XYZ.W", "qcom,adreno" or
> -	      "amd,imageon-XYZ.W", "amd,imageon"
> -    for example: "qcom,adreno-306.0", "qcom,adreno"
> -  Note that you need to list the less specific "qcom,adreno" (since this
> -  is what the device is matched on), in addition to the more specific
> -  with the chip-id.
> -  If "amd,imageon" is used, there should be no top level msm device.
> -- reg: Physical base address and length of the controller's registers.
> -- interrupts: The interrupt signal from the gpu.
> -- clocks: device clocks (if applicable)
> -  See ../clocks/clock-bindings.txt for details.
> -- clock-names: the following clocks are required by a3xx, a4xx and a5xx
> -  cores:
> -  * "core"
> -  * "iface"
> -  * "mem_iface"
> -  For GMU attached devices the GPU clocks are not used and are not required. The
> -  following devices should not list clocks:
> -   - qcom,adreno-630.2
> -- iommus: optional phandle to an adreno iommu instance
> -- operating-points-v2: optional phandle to the OPP operating points
> -- interconnects: optional phandle to an interconnect provider.  See
> -  ../interconnect/interconnect.txt for details. Some A3xx and all A4xx platforms
> -  will have two paths; all others will have one path.
> -- interconnect-names: The names of the interconnect paths that correspond to the
> -  interconnects property. Values must be gfx-mem and ocmem.
> -- qcom,gmu: For GMU attached devices a phandle to the GMU device that will
> -  control the power for the GPU. Applicable targets:
> -    - qcom,adreno-630.2
> -- zap-shader: For a5xx and a6xx devices this node contains a memory-region that
> -  points to reserved memory to store the zap shader that can be used to help
> -  bring the GPU out of secure mode.
> -- firmware-name: optional property of the 'zap-shader' node, listing the
> -  relative path of the device specific zap firmware.
> -- sram: phandle to the On Chip Memory (OCMEM) that's present on some a3xx and
> -        a4xx Snapdragon SoCs. See
> -        Documentation/devicetree/bindings/sram/qcom,ocmem.yaml.
> -
> -Optional properties:
> -- #cooling-cells: The value must be 2. For details, please refer
> -	Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml.
> -
> -Example 3xx/4xx:
> -
> -/ {
> -	...
> -
> -	gpu: adreno@fdb00000 {
> -		compatible = "qcom,adreno-330.2",
> -		             "qcom,adreno";
> -		reg = <0xfdb00000 0x10000>;
> -		reg-names = "kgsl_3d0_reg_memory";
> -		interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> -		interrupt-names = "kgsl_3d0_irq";
> -		clock-names = "core",
> -		              "iface",
> -		              "mem_iface";
> -		clocks = <&mmcc OXILI_GFX3D_CLK>,
> -		         <&mmcc OXILICX_AHB_CLK>,
> -		         <&mmcc OXILICX_AXI_CLK>;
> -		sram = <&gpu_sram>;
> -		power-domains = <&mmcc OXILICX_GDSC>;
> -		operating-points-v2 = <&gpu_opp_table>;
> -		iommus = <&gpu_iommu 0>;
> -		#cooling-cells = <2>;
> -	};
> -
> -	gpu_sram: ocmem@fdd00000 {
> -		compatible = "qcom,msm8974-ocmem";
> -
> -		reg = <0xfdd00000 0x2000>,
> -		      <0xfec00000 0x180000>;
> -		reg-names = "ctrl",
> -		            "mem";
> -
> -		clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
> -		         <&mmcc OCMEMCX_OCMEMNOC_CLK>;
> -		clock-names = "core",
> -		              "iface";
> -
> -		#address-cells = <1>;
> -		#size-cells = <1>;
> -
> -		gpu_sram: gpu-sram@0 {
> -			reg = <0x0 0x100000>;
> -			ranges = <0 0 0xfec00000 0x100000>;
> -		};
> -	};
> -};
> -
> -Example a6xx (with GMU):
> -
> -/ {
> -	...
> -
> -	gpu@5000000 {
> -		compatible = "qcom,adreno-630.2", "qcom,adreno";
> -		#stream-id-cells = <16>;
> -
> -		reg = <0x5000000 0x40000>, <0x509e000 0x10>;
> -		reg-names = "kgsl_3d0_reg_memory", "cx_mem";
> -
> -		#cooling-cells = <2>;
> -
> -		/*
> -		 * Look ma, no clocks! The GPU clocks and power are
> -		 * controlled entirely by the GMU
> -		 */
> -
> -		interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> -
> -		iommus = <&adreno_smmu 0>;
> -
> -		operating-points-v2 = <&gpu_opp_table>;
> -
> -		interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>;
> -		interconnect-names = "gfx-mem";
> -
> -		gpu_opp_table: opp-table {
> -			compatible = "operating-points-v2";
> -
> -			opp-430000000 {
> -				opp-hz = /bits/ 64 <430000000>;
> -				opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> -				opp-peak-kBps = <5412000>;
> -			};
> -
> -			opp-355000000 {
> -				opp-hz = /bits/ 64 <355000000>;
> -				opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
> -				opp-peak-kBps = <3072000>;
> -			};
> -
> -			opp-267000000 {
> -				opp-hz = /bits/ 64 <267000000>;
> -				opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> -				opp-peak-kBps = <3072000>;
> -			};
> -
> -			opp-180000000 {
> -				opp-hz = /bits/ 64 <180000000>;
> -				opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> -				opp-peak-kBps = <1804000>;
> -			};
> -		};
> -
> -		qcom,gmu = <&gmu>;
> -
> -		zap-shader {
> -			memory-region = <&zap_shader_region>;
> -			firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn"
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> new file mode 100644
> index 000000000000..4315482e0b12
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> @@ -0,0 +1,256 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: "http://devicetree.org/schemas/display/msm/gpu.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Devicetree bindings for the Adreno or Snapdragon GPUs
> +
> +maintainers:
> +  - Rob Clark <robdclark@gmail.com>
> +
> +description: |
> +  These bindings describe the GPUs

Describe what this h/w is/does. The 'title' tells me more than this 
sentence.

> +
> +properties:
> +  compatible:
> +    anyOf:

How can both be true? Use 'oneOf'.

> +      - items:
> +          - pattern: '^qcom,adreno-[3-6][0-9][0-9].[0-9]$'
> +          - const: qcom,adreno
> +      - items:
> +          - pattern: '^amd,imageon-200.[0-1]$'
> +          - const: amd,imageon
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    maxItems: 3
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2
> +    description: Physical base address and length of the controller's registers.

Drop description. That's every 'reg'.

> +
> +  reg-names:
> +    minItems: 1
> +    maxItems: 2

Need defined names.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-names:
> +    maxItems: 1
> +
> +  interconnects:
> +    minItems: 1
> +    maxItems: 2
> +    description: |
> +      optional phandle to an interconnect provider. See
> +      ../interconnect/interconnect.txt for details.
> +      Some A3xx and all A4xx platforms will have two paths;
> +      all others will have one path.
> +
> +  interconnect-names:

minItems: 1

to fix your warning.

> +    items:
> +      - const: gfx-mem
> +      - const: ocmem
> +    description: |
> +      the names of the interconnect paths that correspond to
> +      the interconnects property
> +
> +  iommus:
> +    maxItems: 1
> +
> +  sram:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    minItems: 1
> +    maxItems: 4
> +    description: |
> +      phandles to one or more reserved on-chip SRAM regions.
> +      phandle to the On Chip Memory (OCMEM) that's present on some a3xx and
> +      a4xx Snapdragon SoCs. See
> +      Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> +
> +  operating-points-v2: true
> +  opp-table: true
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  zap-shader:
> +    description: |
> +      For a5xx and a6xx devices this node contains a memory-region that
> +      points to reserved memory to store the zap shader that can be used to
> +      help bring the GPU out of secure mode.

Needs a type.

> +
> +  "#cooling-cells":
> +    const: 2
> +    description: |
> +      For details, please refer
> +      Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml

Drop this.

> +
> +  qcom,gmu:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      for GMU attached devices a phandle to the GMU device that will
> +      control the power for the GPU
> +
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            pattern: '^qcom,adreno-[3-5][0-9][0-9].[0-9]$'

Would be simpler to use just 'qcom,adreno' here.

> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: GPU Core clock
> +            - description: GPU Interface clock
> +            - description: GPU Memory Interface clock
> +
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: iface
> +            - const: mem_iface
> +      required:
> +        - clocks
> +        - clock-names

What do we have for clocks if this is false?

> +
> +examples:
> +  - |
> +
> +    // Example a3xx/4xx:
> +
> +    #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> +    #include <dt-bindings/clock/qcom,rpmcc.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    gpu: adreno@fdb00000 {
> +        compatible = "qcom,adreno-330.2", "qcom,adreno";
> +
> +        reg = <0xfdb00000 0x10000>;
> +        reg-names = "kgsl_3d0_reg_memory";
> +
> +        clock-names = "core", "iface", "mem_iface";
> +        clocks = <&mmcc OXILI_GFX3D_CLK>,
> +                 <&mmcc OXILICX_AHB_CLK>,
> +                 <&mmcc OXILICX_AXI_CLK>;
> +
> +        interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-names = "kgsl_3d0_irq";
> +
> +        sram = <&gpu_sram>;
> +        power-domains = <&mmcc OXILICX_GDSC>;
> +        operating-points-v2 = <&gpu_opp_table>;
> +        iommus = <&gpu_iommu 0>;
> +        #cooling-cells = <2>;
> +    };
> +
> +    ocmem@fdd00000 {
> +        compatible = "qcom,msm8974-ocmem";
> +
> +        reg = <0xfdd00000 0x2000>,
> +              <0xfec00000 0x180000>;
> +        reg-names = "ctrl", "mem";
> +
> +        clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
> +                 <&mmcc OCMEMCX_OCMEMNOC_CLK>;
> +        clock-names = "core", "iface";
> +
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges = <0 0xfec00000 0x100000>;
> +
> +        gpu_sram: gpu-sram@0 {
> +            reg = <0x0 0x100000>;
> +        };
> +    };
> +  - |
> +
> +    // Example a6xx (with GMU):
> +
> +    #include <dt-bindings/clock/qcom,gpucc-sdm845.h>
> +    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> +    #include <dt-bindings/power/qcom-rpmpd.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interconnect/qcom,sdm845.h>
> +
> +    reserved-memory {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        zap_shader_region: gpu@8f200000 {
> +            compatible = "shared-dma-pool";
> +            reg = <0x0 0x90b00000 0x0 0xa00000>;
> +            no-map;
> +        };
> +    };
> +
> +    gpu@5000000 {
> +        compatible = "qcom,adreno-630.2", "qcom,adreno";
> +
> +        reg = <0x5000000 0x40000>, <0x509e000 0x10>;
> +        reg-names = "kgsl_3d0_reg_memory", "cx_mem";
> +
> +        #cooling-cells = <2>;
> +
> +        interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> +
> +        iommus = <&adreno_smmu 0>;
> +
> +        operating-points-v2 = <&gpu_opp_table>;
> +
> +        interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>;
> +        interconnect-names = "gfx-mem";
> +
> +        qcom,gmu = <&gmu>;
> +
> +        gpu_opp_table: opp-table {
> +            compatible = "operating-points-v2";
> +
> +            opp-430000000 {
> +                opp-hz = /bits/ 64 <430000000>;
> +                opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> +                opp-peak-kBps = <5412000>;
> +            };
> +
> +            opp-355000000 {
> +                opp-hz = /bits/ 64 <355000000>;
> +                opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
> +                opp-peak-kBps = <3072000>;
> +            };
> +
> +            opp-267000000 {
> +                opp-hz = /bits/ 64 <267000000>;
> +                opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> +                opp-peak-kBps = <3072000>;
> +            };
> +
> +            opp-180000000 {
> +                opp-hz = /bits/ 64 <180000000>;
> +                opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> +                opp-peak-kBps = <1804000>;
> +            };
> +        };
> +
> +        zap-shader {
> +            memory-region = <&zap_shader_region>;
> +            firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn";
> +        };
> +    };
> -- 
> 2.30.2
> 
> 

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

* Re: [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML
  2021-07-12 14:51 ` Rob Herring
@ 2021-07-12 15:08   ` Rob Clark
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Clark @ 2021-07-12 15:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Heidelberg, Sharat Masetty, Brian Masney, Rob Clark,
	Jordan Crouse,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm

On Mon, Jul 12, 2021 at 7:51 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sat, Jul 03, 2021 at 05:18:35PM +0200, David Heidelberg wrote:
> > This warning cannot be fixed by conversion, since this naming is already used.
> > Documentation/devicetree/bindings/display/msm/gpu.example.dt.yaml: gpu@5000000: interconnect-names: ['gfx-mem'] is too short
> >
> > Signed-off-by: David Heidelberg <david@ixit.cz>
> > ---
> >  .../devicetree/bindings/display/msm/gpu.txt   | 157 -----------
> >  .../devicetree/bindings/display/msm/gpu.yaml  | 256 ++++++++++++++++++
> >  2 files changed, 256 insertions(+), 157 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/display/msm/gpu.txt
> >  create mode 100644 Documentation/devicetree/bindings/display/msm/gpu.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt
> > deleted file mode 100644
> > index 090dcb3fc34d..000000000000
> > --- a/Documentation/devicetree/bindings/display/msm/gpu.txt
> > +++ /dev/null
> > @@ -1,157 +0,0 @@
> > -Qualcomm adreno/snapdragon GPU
> > -
> > -Required properties:
> > -- compatible: "qcom,adreno-XYZ.W", "qcom,adreno" or
> > -           "amd,imageon-XYZ.W", "amd,imageon"
> > -    for example: "qcom,adreno-306.0", "qcom,adreno"
> > -  Note that you need to list the less specific "qcom,adreno" (since this
> > -  is what the device is matched on), in addition to the more specific
> > -  with the chip-id.
> > -  If "amd,imageon" is used, there should be no top level msm device.
> > -- reg: Physical base address and length of the controller's registers.
> > -- interrupts: The interrupt signal from the gpu.
> > -- clocks: device clocks (if applicable)
> > -  See ../clocks/clock-bindings.txt for details.
> > -- clock-names: the following clocks are required by a3xx, a4xx and a5xx
> > -  cores:
> > -  * "core"
> > -  * "iface"
> > -  * "mem_iface"
> > -  For GMU attached devices the GPU clocks are not used and are not required. The
> > -  following devices should not list clocks:
> > -   - qcom,adreno-630.2
> > -- iommus: optional phandle to an adreno iommu instance
> > -- operating-points-v2: optional phandle to the OPP operating points
> > -- interconnects: optional phandle to an interconnect provider.  See
> > -  ../interconnect/interconnect.txt for details. Some A3xx and all A4xx platforms
> > -  will have two paths; all others will have one path.
> > -- interconnect-names: The names of the interconnect paths that correspond to the
> > -  interconnects property. Values must be gfx-mem and ocmem.
> > -- qcom,gmu: For GMU attached devices a phandle to the GMU device that will
> > -  control the power for the GPU. Applicable targets:
> > -    - qcom,adreno-630.2
> > -- zap-shader: For a5xx and a6xx devices this node contains a memory-region that
> > -  points to reserved memory to store the zap shader that can be used to help
> > -  bring the GPU out of secure mode.
> > -- firmware-name: optional property of the 'zap-shader' node, listing the
> > -  relative path of the device specific zap firmware.
> > -- sram: phandle to the On Chip Memory (OCMEM) that's present on some a3xx and
> > -        a4xx Snapdragon SoCs. See
> > -        Documentation/devicetree/bindings/sram/qcom,ocmem.yaml.
> > -
> > -Optional properties:
> > -- #cooling-cells: The value must be 2. For details, please refer
> > -     Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml.
> > -
> > -Example 3xx/4xx:
> > -
> > -/ {
> > -     ...
> > -
> > -     gpu: adreno@fdb00000 {
> > -             compatible = "qcom,adreno-330.2",
> > -                          "qcom,adreno";
> > -             reg = <0xfdb00000 0x10000>;
> > -             reg-names = "kgsl_3d0_reg_memory";
> > -             interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> > -             interrupt-names = "kgsl_3d0_irq";
> > -             clock-names = "core",
> > -                           "iface",
> > -                           "mem_iface";
> > -             clocks = <&mmcc OXILI_GFX3D_CLK>,
> > -                      <&mmcc OXILICX_AHB_CLK>,
> > -                      <&mmcc OXILICX_AXI_CLK>;
> > -             sram = <&gpu_sram>;
> > -             power-domains = <&mmcc OXILICX_GDSC>;
> > -             operating-points-v2 = <&gpu_opp_table>;
> > -             iommus = <&gpu_iommu 0>;
> > -             #cooling-cells = <2>;
> > -     };
> > -
> > -     gpu_sram: ocmem@fdd00000 {
> > -             compatible = "qcom,msm8974-ocmem";
> > -
> > -             reg = <0xfdd00000 0x2000>,
> > -                   <0xfec00000 0x180000>;
> > -             reg-names = "ctrl",
> > -                         "mem";
> > -
> > -             clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
> > -                      <&mmcc OCMEMCX_OCMEMNOC_CLK>;
> > -             clock-names = "core",
> > -                           "iface";
> > -
> > -             #address-cells = <1>;
> > -             #size-cells = <1>;
> > -
> > -             gpu_sram: gpu-sram@0 {
> > -                     reg = <0x0 0x100000>;
> > -                     ranges = <0 0 0xfec00000 0x100000>;
> > -             };
> > -     };
> > -};
> > -
> > -Example a6xx (with GMU):
> > -
> > -/ {
> > -     ...
> > -
> > -     gpu@5000000 {
> > -             compatible = "qcom,adreno-630.2", "qcom,adreno";
> > -             #stream-id-cells = <16>;
> > -
> > -             reg = <0x5000000 0x40000>, <0x509e000 0x10>;
> > -             reg-names = "kgsl_3d0_reg_memory", "cx_mem";
> > -
> > -             #cooling-cells = <2>;
> > -
> > -             /*
> > -              * Look ma, no clocks! The GPU clocks and power are
> > -              * controlled entirely by the GMU
> > -              */
> > -
> > -             interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> > -
> > -             iommus = <&adreno_smmu 0>;
> > -
> > -             operating-points-v2 = <&gpu_opp_table>;
> > -
> > -             interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>;
> > -             interconnect-names = "gfx-mem";
> > -
> > -             gpu_opp_table: opp-table {
> > -                     compatible = "operating-points-v2";
> > -
> > -                     opp-430000000 {
> > -                             opp-hz = /bits/ 64 <430000000>;
> > -                             opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> > -                             opp-peak-kBps = <5412000>;
> > -                     };
> > -
> > -                     opp-355000000 {
> > -                             opp-hz = /bits/ 64 <355000000>;
> > -                             opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
> > -                             opp-peak-kBps = <3072000>;
> > -                     };
> > -
> > -                     opp-267000000 {
> > -                             opp-hz = /bits/ 64 <267000000>;
> > -                             opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> > -                             opp-peak-kBps = <3072000>;
> > -                     };
> > -
> > -                     opp-180000000 {
> > -                             opp-hz = /bits/ 64 <180000000>;
> > -                             opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> > -                             opp-peak-kBps = <1804000>;
> > -                     };
> > -             };
> > -
> > -             qcom,gmu = <&gmu>;
> > -
> > -             zap-shader {
> > -                     memory-region = <&zap_shader_region>;
> > -                     firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn"
> > -             };
> > -     };
> > -};
> > diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> > new file mode 100644
> > index 000000000000..4315482e0b12
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> > @@ -0,0 +1,256 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +$id: "http://devicetree.org/schemas/display/msm/gpu.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Devicetree bindings for the Adreno or Snapdragon GPUs
> > +
> > +maintainers:
> > +  - Rob Clark <robdclark@gmail.com>
> > +
> > +description: |
> > +  These bindings describe the GPUs
>
> Describe what this h/w is/does. The 'title' tells me more than this
> sentence.
>
> > +
> > +properties:
> > +  compatible:
> > +    anyOf:
>
> How can both be true? Use 'oneOf'.
>
> > +      - items:
> > +          - pattern: '^qcom,adreno-[3-6][0-9][0-9].[0-9]$'
> > +          - const: qcom,adreno
> > +      - items:
> > +          - pattern: '^amd,imageon-200.[0-1]$'
> > +          - const: amd,imageon
> > +
> > +  clocks:
> > +    maxItems: 3
> > +
> > +  clock-names:
> > +    maxItems: 3
> > +
> > +  reg:
> > +    minItems: 1
> > +    maxItems: 2
> > +    description: Physical base address and length of the controller's registers.
>
> Drop description. That's every 'reg'.
>
> > +
> > +  reg-names:
> > +    minItems: 1
> > +    maxItems: 2
>
> Need defined names.
>
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-names:
> > +    maxItems: 1
> > +
> > +  interconnects:
> > +    minItems: 1
> > +    maxItems: 2
> > +    description: |
> > +      optional phandle to an interconnect provider. See
> > +      ../interconnect/interconnect.txt for details.
> > +      Some A3xx and all A4xx platforms will have two paths;
> > +      all others will have one path.
> > +
> > +  interconnect-names:
>
> minItems: 1
>
> to fix your warning.
>
> > +    items:
> > +      - const: gfx-mem
> > +      - const: ocmem
> > +    description: |
> > +      the names of the interconnect paths that correspond to
> > +      the interconnects property
> > +
> > +  iommus:
> > +    maxItems: 1
> > +
> > +  sram:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    minItems: 1
> > +    maxItems: 4
> > +    description: |
> > +      phandles to one or more reserved on-chip SRAM regions.
> > +      phandle to the On Chip Memory (OCMEM) that's present on some a3xx and
> > +      a4xx Snapdragon SoCs. See
> > +      Documentation/devicetree/bindings/sram/qcom,ocmem.yaml
> > +
> > +  operating-points-v2: true
> > +  opp-table: true
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  zap-shader:
> > +    description: |
> > +      For a5xx and a6xx devices this node contains a memory-region that
> > +      points to reserved memory to store the zap shader that can be used to
> > +      help bring the GPU out of secure mode.
>
> Needs a type.
>
> > +
> > +  "#cooling-cells":
> > +    const: 2
> > +    description: |
> > +      For details, please refer
> > +      Documentation/devicetree/bindings/thermal/thermal-cooling-devices.yaml
>
> Drop this.
>
> > +
> > +  qcom,gmu:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: |
> > +      for GMU attached devices a phandle to the GMU device that will
> > +      control the power for the GPU
> > +
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            pattern: '^qcom,adreno-[3-5][0-9][0-9].[0-9]$'
>
> Would be simpler to use just 'qcom,adreno' here.

I just noticed that this pattern is incomplete (misses a6xx), which
might be an argument for just using 'qcom,adreno'..

OTOH, it may be worth pointing out that the driver is parsing the
compat string to figure out the gpu-id and patch level.

BR,
-R

> > +
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: GPU Core clock
> > +            - description: GPU Interface clock
> > +            - description: GPU Memory Interface clock
> > +
> > +        clock-names:
> > +          items:
> > +            - const: core
> > +            - const: iface
> > +            - const: mem_iface
> > +      required:
> > +        - clocks
> > +        - clock-names
>
> What do we have for clocks if this is false?
>
> > +
> > +examples:
> > +  - |
> > +
> > +    // Example a3xx/4xx:
> > +
> > +    #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> > +    #include <dt-bindings/clock/qcom,rpmcc.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    gpu: adreno@fdb00000 {
> > +        compatible = "qcom,adreno-330.2", "qcom,adreno";
> > +
> > +        reg = <0xfdb00000 0x10000>;
> > +        reg-names = "kgsl_3d0_reg_memory";
> > +
> > +        clock-names = "core", "iface", "mem_iface";
> > +        clocks = <&mmcc OXILI_GFX3D_CLK>,
> > +                 <&mmcc OXILICX_AHB_CLK>,
> > +                 <&mmcc OXILICX_AXI_CLK>;
> > +
> > +        interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> > +        interrupt-names = "kgsl_3d0_irq";
> > +
> > +        sram = <&gpu_sram>;
> > +        power-domains = <&mmcc OXILICX_GDSC>;
> > +        operating-points-v2 = <&gpu_opp_table>;
> > +        iommus = <&gpu_iommu 0>;
> > +        #cooling-cells = <2>;
> > +    };
> > +
> > +    ocmem@fdd00000 {
> > +        compatible = "qcom,msm8974-ocmem";
> > +
> > +        reg = <0xfdd00000 0x2000>,
> > +              <0xfec00000 0x180000>;
> > +        reg-names = "ctrl", "mem";
> > +
> > +        clocks = <&rpmcc RPM_SMD_OCMEMGX_CLK>,
> > +                 <&mmcc OCMEMCX_OCMEMNOC_CLK>;
> > +        clock-names = "core", "iface";
> > +
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        ranges = <0 0xfec00000 0x100000>;
> > +
> > +        gpu_sram: gpu-sram@0 {
> > +            reg = <0x0 0x100000>;
> > +        };
> > +    };
> > +  - |
> > +
> > +    // Example a6xx (with GMU):
> > +
> > +    #include <dt-bindings/clock/qcom,gpucc-sdm845.h>
> > +    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> > +    #include <dt-bindings/power/qcom-rpmpd.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interconnect/qcom,sdm845.h>
> > +
> > +    reserved-memory {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        zap_shader_region: gpu@8f200000 {
> > +            compatible = "shared-dma-pool";
> > +            reg = <0x0 0x90b00000 0x0 0xa00000>;
> > +            no-map;
> > +        };
> > +    };
> > +
> > +    gpu@5000000 {
> > +        compatible = "qcom,adreno-630.2", "qcom,adreno";
> > +
> > +        reg = <0x5000000 0x40000>, <0x509e000 0x10>;
> > +        reg-names = "kgsl_3d0_reg_memory", "cx_mem";
> > +
> > +        #cooling-cells = <2>;
> > +
> > +        interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +        iommus = <&adreno_smmu 0>;
> > +
> > +        operating-points-v2 = <&gpu_opp_table>;
> > +
> > +        interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>;
> > +        interconnect-names = "gfx-mem";
> > +
> > +        qcom,gmu = <&gmu>;
> > +
> > +        gpu_opp_table: opp-table {
> > +            compatible = "operating-points-v2";
> > +
> > +            opp-430000000 {
> > +                opp-hz = /bits/ 64 <430000000>;
> > +                opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> > +                opp-peak-kBps = <5412000>;
> > +            };
> > +
> > +            opp-355000000 {
> > +                opp-hz = /bits/ 64 <355000000>;
> > +                opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
> > +                opp-peak-kBps = <3072000>;
> > +            };
> > +
> > +            opp-267000000 {
> > +                opp-hz = /bits/ 64 <267000000>;
> > +                opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> > +                opp-peak-kBps = <3072000>;
> > +            };
> > +
> > +            opp-180000000 {
> > +                opp-hz = /bits/ 64 <180000000>;
> > +                opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> > +                opp-peak-kBps = <1804000>;
> > +            };
> > +        };
> > +
> > +        zap-shader {
> > +            memory-region = <&zap_shader_region>;
> > +            firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn";
> > +        };
> > +    };
> > --
> > 2.30.2
> >
> >

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

* Re: [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML
  2021-07-03 15:18 [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML David Heidelberg
  2021-07-04  8:18 ` Dmitry Baryshkov
  2021-07-12 14:51 ` Rob Herring
@ 2021-07-12 15:18 ` Rob Clark
  2 siblings, 0 replies; 6+ messages in thread
From: Rob Clark @ 2021-07-12 15:18 UTC (permalink / raw)
  To: David Heidelberg
  Cc: Sharat Masetty, Brian Masney, Rob Clark, Jordan Crouse,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm

On Sat, Jul 3, 2021 at 8:20 AM David Heidelberg <david@ixit.cz> wrote:
>
> This warning cannot be fixed by conversion, since this naming is already used.
> Documentation/devicetree/bindings/display/msm/gpu.example.dt.yaml: gpu@5000000: interconnect-names: ['gfx-mem'] is too short
>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  .../devicetree/bindings/display/msm/gpu.txt   | 157 -----------
>  .../devicetree/bindings/display/msm/gpu.yaml  | 256 ++++++++++++++++++
>  2 files changed, 256 insertions(+), 157 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/display/msm/gpu.txt
>  create mode 100644 Documentation/devicetree/bindings/display/msm/gpu.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt
> deleted file mode 100644

[snip]

> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> new file mode 100644
> index 000000000000..4315482e0b12
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml

[snip]

> +  zap-shader:
> +    description: |
> +      For a5xx and a6xx devices this node contains a memory-region that
> +      points to reserved memory to store the zap shader that can be used to
> +      help bring the GPU out of secure mode.
> +

Side note, this node is optional now, we do have some a6xx devices out
there which do not use/require a zap shader (in particular, the
chromebooks).  Not sure if that effects how you want to document it in
the yaml.

Also, new dts for devices that use zap, they should specify a
"firmware-name", since the zap shader is usually signed with a board
specific signing key.

BR,
-R

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

end of thread, other threads:[~2021-07-12 15:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-03 15:18 [PATCH] RFC: dt-bindings: drm/msm/gpu: convert to YAML David Heidelberg
2021-07-04  8:18 ` Dmitry Baryshkov
2021-07-04  9:31   ` David Heidelberg
2021-07-12 14:51 ` Rob Herring
2021-07-12 15:08   ` Rob Clark
2021-07-12 15:18 ` Rob Clark

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