Re: [Freedreno] [PATCH v6 1/2] dt-bindings: drm/msm/a6xx: Document GMU and update GPU bindings
On Mon, Dec 17, 2018 at 03:20:10PM -0600, Rob Herring wrote:
> On Wed, Dec 12, 2018 at 02:18:47PM -0700, Jordan Crouse wrote:
> > Update the GPU bindings and document the new bindings for the GMU
> > device found with Adreno a6xx targets.
> >
> > Signed-off-by: Jordan Crouse
> > ---
> > .../devicetree/bindings/display/msm/gmu.txt | 56 +++
> > .../devicetree/bindings/display/msm/gpu.txt | 41 +-
> > 2 files changed, 94 insertions(+), 3 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt
> > b/Documentation/devicetree/bindings/display/msm/gmu.txt
> > new file mode 100644
> > index ..6152cb551d29
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
> > @@ -0,0 +1,56 @@
> > +Qualcomm adreno/snapdragon GMU (Graphics management unit)
> > +
> > +The GMU is a programmable power controller for the GPU. the CPU controls
> > the
> > +GMU which in turn handles power controls for the GPU.
> > +
> > +Required properties:
> > +- compatible:
> > + * "qcom,adreno-gmu"
>
> I probably asked before, but this needs a specific compatible unless you
> have reliable version/capability registers. If you do, please state that
> here.
Most of our decisions are made based on the type of GPU attached but it wouldn't
hurt to make this future proof. I'll do it.
> > +- reg: Physical base address and length of the GMU registers.
> > +- reg-names: Matching names for the register regions
> > + * "gmu"
> > + * "gmu_pdc"
> > + * "gmu_pdc_seg"
> > +- interrupts: The interrupt signals from the GMU.
> > +- interrupt-names: Matching names for the interrupts
> > + * "hfi"
> > + * "gmu"
> > +- clocks: phandles to the device clocks
> > +- clock-names: Matching names for the clocks
> > + * "gmu"
> > + * "cxo"
> > + * "axi"
> > + * "mnoc"
> > +- power-domains: should be <&clock_gpucc GPU_CX_GDSC>
> > +- iommus: phandle to the adreno iommu
> > +- operating-points-v2: phandle to the OPP operating points
> > +
> > +Example:
> > +
> > +/ {
> > + ...
> > +
> > + gmu: gmu@506a000 {
> > + compatible="qcom,adreno-gmu";
> > +
> > + reg = <0x506a000 0x3>,
> > + <0xb28 0x1>,
> > + <0xb48 0x1>;
> > + reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
> > +
> > + interrupts = ,
> > +;
> > + interrupt-names = "hfi", "gmu";
> > +
> > + clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
> > + <&gpucc GPU_CC_CXO_CLK>,
> > + <&gcc GCC_DDRSS_GPU_AXI_CLK>,
> > + <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
> > + clock-names = "gmu", "cxo", "axi", "memnoc";
> > +
> > + power-domains = <&gpucc GPU_CX_GDSC>;
> > + iommus = <&adreno_smmu 5>;
> > +
> > + operating-points-v2 = <&gmu_opp_table>;
> > + };
> > +};
> > diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt
> > b/Documentation/devicetree/bindings/display/msm/gpu.txt
> > index 43fac0fe09bb..8d9415180c22 100644
> > --- a/Documentation/devicetree/bindings/display/msm/gpu.txt
> > +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
> > @@ -8,14 +8,21 @@ Required properties:
> >with the chip-id.
> > - reg: Physical base address and length of the controller's registers.
> > - interrupts: The interrupt signal from the gpu.
> > -- clocks: device clocks
> > +- interrupt-names: List of names for the interrupt signals. The following
> > can be
> > + provided:
> > + * "kgsl_3d0_irq"
>
> I'm pretty sure 'kgsl' is not a hardware thing. You don't need *-names
> when there is only one of something.
This has mainly existed just for compatibility issues. We do only have the one
interrupt so lets zap the downstream name and never look back.
> > +- clocks: device clocks (if applicable)
>
> What does this mean? They are now optional? If so, move to an "Optional"
> section. Likewise for the others.
They are indeed optional now.
> Really, you should add a new compatible so we can validate when clocks
> not being present is valid vs. an error in the DT.
We could use the GPU revision for that, but our approach has been to make all
clocks optional for all targets. Eventually when we go to power up if the GPU
core ends up needing a clock and it isn't defined we fail probe at that point.
I'm not sure if that is resilient enough for DT purposes though.
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v6 1/2] dt-bindings: drm/msm/a6xx: Document GMU and update GPU bindings
On Wed, Dec 12, 2018 at 02:18:47PM -0700, Jordan Crouse wrote:
> Update the GPU bindings and document the new bindings for the GMU
> device found with Adreno a6xx targets.
>
> Signed-off-by: Jordan Crouse
> ---
> .../devicetree/bindings/display/msm/gmu.txt | 56 +++
> .../devicetree/bindings/display/msm/gpu.txt | 41 +-
> 2 files changed, 94 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt
>
> diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt
> b/Documentation/devicetree/bindings/display/msm/gmu.txt
> new file mode 100644
> index ..6152cb551d29
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
> @@ -0,0 +1,56 @@
> +Qualcomm adreno/snapdragon GMU (Graphics management unit)
> +
> +The GMU is a programmable power controller for the GPU. the CPU controls the
> +GMU which in turn handles power controls for the GPU.
> +
> +Required properties:
> +- compatible:
> + * "qcom,adreno-gmu"
I probably asked before, but this needs a specific compatible unless you
have reliable version/capability registers. If you do, please state that
here.
> +- reg: Physical base address and length of the GMU registers.
> +- reg-names: Matching names for the register regions
> + * "gmu"
> + * "gmu_pdc"
> + * "gmu_pdc_seg"
> +- interrupts: The interrupt signals from the GMU.
> +- interrupt-names: Matching names for the interrupts
> + * "hfi"
> + * "gmu"
> +- clocks: phandles to the device clocks
> +- clock-names: Matching names for the clocks
> + * "gmu"
> + * "cxo"
> + * "axi"
> + * "mnoc"
> +- power-domains: should be <&clock_gpucc GPU_CX_GDSC>
> +- iommus: phandle to the adreno iommu
> +- operating-points-v2: phandle to the OPP operating points
> +
> +Example:
> +
> +/ {
> + ...
> +
> + gmu: gmu@506a000 {
> + compatible="qcom,adreno-gmu";
> +
> + reg = <0x506a000 0x3>,
> + <0xb28 0x1>,
> + <0xb48 0x1>;
> + reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
> +
> + interrupts = ,
> + ;
> + interrupt-names = "hfi", "gmu";
> +
> + clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
> + <&gpucc GPU_CC_CXO_CLK>,
> + <&gcc GCC_DDRSS_GPU_AXI_CLK>,
> + <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
> + clock-names = "gmu", "cxo", "axi", "memnoc";
> +
> + power-domains = <&gpucc GPU_CX_GDSC>;
> + iommus = <&adreno_smmu 5>;
> +
> + operating-points-v2 = <&gmu_opp_table>;
> + };
> +};
> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt
> b/Documentation/devicetree/bindings/display/msm/gpu.txt
> index 43fac0fe09bb..8d9415180c22 100644
> --- a/Documentation/devicetree/bindings/display/msm/gpu.txt
> +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
> @@ -8,14 +8,21 @@ Required properties:
>with the chip-id.
> - reg: Physical base address and length of the controller's registers.
> - interrupts: The interrupt signal from the gpu.
> -- clocks: device clocks
> +- interrupt-names: List of names for the interrupt signals. The following
> can be
> + provided:
> + * "kgsl_3d0_irq"
I'm pretty sure 'kgsl' is not a hardware thing. You don't need *-names
when there is only one of something.
> +- clocks: device clocks (if applicable)
What does this mean? They are now optional? If so, move to an "Optional"
section. Likewise for the others.
Really, you should add a new compatible so we can validate when clocks
not being present is valid vs. an error in the DT.
>See ../clocks/clock-bindings.txt for details.
> -- clock-names: the following clocks are required:
> +- clock-names: the following clocks can be provided:
>* "core"
>* "iface"
>* "mem_iface"
> +- iommus: optional phandle to an adreno iommu instance
> +- operating-points-v2: optional phandle to the OPP operating points
> +- qcom,gmu: For a6xx and newer targets a phandle to the GMU device that will
> + control the power for the GPU
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v6 1/2] dt-bindings: drm/msm/a6xx: Document GMU and update GPU bindings
Hi, On Wed, Dec 12, 2018 at 1:18 PM Jordan Crouse wrote: > diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt > b/Documentation/devicetree/bindings/display/msm/gpu.txt > index 43fac0fe09bb..8d9415180c22 100644 > --- a/Documentation/devicetree/bindings/display/msm/gpu.txt > +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt > @@ -8,14 +8,21 @@ Required properties: >with the chip-id. > - reg: Physical base address and length of the controller's registers. > - interrupts: The interrupt signal from the gpu. > -- clocks: device clocks > +- interrupt-names: List of names for the interrupt signals. The following > can be > + provided: > + * "kgsl_3d0_irq" > +- clocks: device clocks (if applicable) >See ../clocks/clock-bindings.txt for details. > -- clock-names: the following clocks are required: > +- clock-names: the following clocks can be provided: >* "core" >* "iface" >* "mem_iface" > +- iommus: optional phandle to an adreno iommu instance > +- operating-points-v2: optional phandle to the OPP operating points > +- qcom,gmu: For a6xx and newer targets a phandle to the GMU device that will > + control the power for the GPU Seems fine to me. If you happen to spin it again for some other reason it might be nice to be more explicit about exactly when clocks are required and when they aren't. IIUC they are always required on systems without a GMU and they are never present on systems with a GMU. ...but I wouldn't spin it just for that. Reviewed-by: Douglas Anderson ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v6 1/2] dt-bindings: drm/msm/a6xx: Document GMU and update GPU bindings
Update the GPU bindings and document the new bindings for the GMU
device found with Adreno a6xx targets.
Signed-off-by: Jordan Crouse
---
.../devicetree/bindings/display/msm/gmu.txt | 56 +++
.../devicetree/bindings/display/msm/gpu.txt | 41 +-
2 files changed, 94 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt
diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt
b/Documentation/devicetree/bindings/display/msm/gmu.txt
new file mode 100644
index ..6152cb551d29
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
@@ -0,0 +1,56 @@
+Qualcomm adreno/snapdragon GMU (Graphics management unit)
+
+The GMU is a programmable power controller for the GPU. the CPU controls the
+GMU which in turn handles power controls for the GPU.
+
+Required properties:
+- compatible:
+ * "qcom,adreno-gmu"
+- reg: Physical base address and length of the GMU registers.
+- reg-names: Matching names for the register regions
+ * "gmu"
+ * "gmu_pdc"
+ * "gmu_pdc_seg"
+- interrupts: The interrupt signals from the GMU.
+- interrupt-names: Matching names for the interrupts
+ * "hfi"
+ * "gmu"
+- clocks: phandles to the device clocks
+- clock-names: Matching names for the clocks
+ * "gmu"
+ * "cxo"
+ * "axi"
+ * "mnoc"
+- power-domains: should be <&clock_gpucc GPU_CX_GDSC>
+- iommus: phandle to the adreno iommu
+- operating-points-v2: phandle to the OPP operating points
+
+Example:
+
+/ {
+ ...
+
+ gmu: gmu@506a000 {
+ compatible="qcom,adreno-gmu";
+
+ reg = <0x506a000 0x3>,
+ <0xb28 0x1>,
+ <0xb48 0x1>;
+ reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
+
+ interrupts = ,
+;
+ interrupt-names = "hfi", "gmu";
+
+ clocks = <&gpucc GPU_CC_CX_GMU_CLK>,
+ <&gpucc GPU_CC_CXO_CLK>,
+ <&gcc GCC_DDRSS_GPU_AXI_CLK>,
+ <&gcc GCC_GPU_MEMNOC_GFX_CLK>;
+ clock-names = "gmu", "cxo", "axi", "memnoc";
+
+ power-domains = <&gpucc GPU_CX_GDSC>;
+ iommus = <&adreno_smmu 5>;
+
+ operating-points-v2 = <&gmu_opp_table>;
+ };
+};
diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt
b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 43fac0fe09bb..8d9415180c22 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -8,14 +8,21 @@ Required properties:
with the chip-id.
- reg: Physical base address and length of the controller's registers.
- interrupts: The interrupt signal from the gpu.
-- clocks: device clocks
+- interrupt-names: List of names for the interrupt signals. The following can
be
+ provided:
+ * "kgsl_3d0_irq"
+- clocks: device clocks (if applicable)
See ../clocks/clock-bindings.txt for details.
-- clock-names: the following clocks are required:
+- clock-names: the following clocks can be provided:
* "core"
* "iface"
* "mem_iface"
+- iommus: optional phandle to an adreno iommu instance
+- operating-points-v2: optional phandle to the OPP operating points
+- qcom,gmu: For a6xx and newer targets a phandle to the GMU device that will
+ control the power for the GPU
-Example:
+Example 3xx/4xx/a5xx:
/ {
...
@@ -36,3 +43,31 @@ Example:
<&mmcc MMSS_IMEM_AHB_CLK>;
};
};
+
+Example a6xx (with GMU):
+
+/ {
+ ...
+
+ gpu@500 {
+ compatible = "qcom,adreno-630.2", "qcom,adreno";
+ #stream-id-cells = <16>;
+
+ reg = <0x500 0x4>, <0x509e000 0x10>;
+ reg-names = "kgsl_3d0_reg_memory", "cx_mem";
+
+ /*
+* Look ma, no clocks! The GPU clocks and power are
+* controlled entirely by the GMU
+*/
+
+ interrupts = ;
+ interrupt-names = "kgsl_3d0_irq";
+
+ iommus = <&adreno_smmu 0>;
+
+ operating-points-v2 = <&gpu_opp_table>;
+
+ qcom,gmu = <&gmu>;
+ };
+};
--
2.18.0
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
