[PATCH 0/2] arm64: dts: Add sdm845 GPU/GMU and SMMU

[PATCH 0/2] arm64: dts: Add sdm845 GPU/GMU and SMMU

[Jordan Crouse]

Building on the sdm845 changes from Rajendra and SMMU changes from Vivek this
is an initial stab at the DT nodes for the sdm845 GPU and GMU (graphics
management unit) which is responsible for the direct power control of the GPU
including the companion arm-smmu-v2 compatible SMMU.

Please refer to https://patchwork.freedesktop.org/series/37428/ for the driver
code.

Jordan Crouse (2):
  dt-bindings: Document qcom,adreno-gmu
  arm64: dts: sdm845: Support GPU/GMU

 .../devicetree/bindings/display/msm/gmu.txt        |  54 ++++++++++
 .../devicetree/bindings/display/msm/gpu.txt        |  10 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi               | 120 +++++++++++++++++++++
 3 files changed, 182 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt

-- 
2.16.1

[PATCH 1/2] dt-bindings: Document qcom,adreno-gmu

[Jordan Crouse]

Document the device tree bindings for the Adreno GMU device
available on Adreno a6xx targets.

Change-Id: I3cfd5fb35ab0045e39905ff12393006e60f1a124
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 .../devicetree/bindings/display/msm/gmu.txt        | 54 ++++++++++++++++++++++
 .../devicetree/bindings/display/msm/gpu.txt        | 10 +++-
 2 files changed, 62 insertions(+), 2 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 000000000000..f65bb49fff36
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
@@ -0,0 +1,54 @@
+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"
+- 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 at 506a000 {
+		compatible="qcom,adreno-gmu";
+
+		reg = <0x506a000 0x30000>,
+			<0xb200000 0x300000>;
+		reg-names = "gmu", "gmu_pdc";
+
+		interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
+			<GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "hfi", "gmu";
+
+		clocks = <&clock_gpucc GPU_CC_CX_GMU_CLK>,
+			<&clock_gpucc GPU_CC_CXO_CLK>,
+			<&clock_gcc GCC_DDRSS_GPU_AXI_CLK>,
+			<&clock_gcc GCC_GPU_MEMNOC_GFX_CLK>;
+		clock-names = "gmu", "cxo", "axi", "memnoc";
+
+		power-domains = <&clock_gpucc GPU_CX_GDSC>;
+		iommus = <&adreno_smmu 5>;
+
+	i	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..0e207498edd3 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -8,12 +8,18 @@ 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
+
+Optional properties.
+- clocks: device clocks. Required for a3xx, a4xx and a5xx targets. a6xx and
+  newer with a GMU attached do not have direct clock control from the CPU and
+  do not need to provide clock properties.
   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"
+- gmu: For a6xx and newer targets a phandle to the GMU device that will
+  control the power for the GPU
 
 Example:
 
-- 
2.16.1

[PATCH 1/2] dt-bindings: Document qcom,adreno-gmu

[Rob Herring]

On Fri, Mar 2, 2018 at 3:56 PM, Jordan Crouse <jcrouse@codeaurora.org> wrote:
> Document the device tree bindings for the Adreno GMU device
> available on Adreno a6xx targets.
>
> Change-Id: I3cfd5fb35ab0045e39905ff12393006e60f1a124

Gerrit!

> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  .../devicetree/bindings/display/msm/gmu.txt        | 54 ++++++++++++++++++++++
>  .../devicetree/bindings/display/msm/gpu.txt        | 10 +++-
>  2 files changed, 62 insertions(+), 2 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 000000000000..f65bb49fff36
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
> @@ -0,0 +1,54 @@
> +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"

Kind of generic. All the features are discoverable?

> +- reg: Physical base address and length of the GMU registers.
> +- reg-names: Matching names for the register regions
> +  * "gmu"
> +  * "gmu_pdc"
> +- 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 at 506a000 {
> +               compatible="qcom,adreno-gmu";
> +
> +               reg = <0x506a000 0x30000>,
> +                       <0xb200000 0x300000>;
> +               reg-names = "gmu", "gmu_pdc";
> +
> +               interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
> +                       <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
> +               interrupt-names = "hfi", "gmu";
> +
> +               clocks = <&clock_gpucc GPU_CC_CX_GMU_CLK>,
> +                       <&clock_gpucc GPU_CC_CXO_CLK>,
> +                       <&clock_gcc GCC_DDRSS_GPU_AXI_CLK>,
> +                       <&clock_gcc GCC_GPU_MEMNOC_GFX_CLK>;
> +               clock-names = "gmu", "cxo", "axi", "memnoc";
> +
> +               power-domains = <&clock_gpucc GPU_CX_GDSC>;
> +               iommus = <&adreno_smmu 5>;
> +
> +       i       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..0e207498edd3 100644
> --- a/Documentation/devicetree/bindings/display/msm/gpu.txt
> +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
> @@ -8,12 +8,18 @@ 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
> +
> +Optional properties.
> +- clocks: device clocks. Required for a3xx, a4xx and a5xx targets. a6xx and
> +  newer with a GMU attached do not have direct clock control from the CPU and
> +  do not need to provide clock properties.
>    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"
> +- gmu: For a6xx and newer targets a phandle to the GMU device that will

qcom,gmu

> +  control the power for the GPU
>
>  Example:
>
> --
> 2.16.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] dt-bindings: Document qcom,adreno-gmu

[Jordan Crouse]

On Mon, Mar 05, 2018 at 01:38:59PM -0600, Rob Herring wrote:
> On Fri, Mar 2, 2018 at 3:56 PM, Jordan Crouse <jcrouse@codeaurora.org> wrote:
> > Document the device tree bindings for the Adreno GMU device
> > available on Adreno a6xx targets.
> >
> > Change-Id: I3cfd5fb35ab0045e39905ff12393006e60f1a124
> 
> Gerrit!
> 
> > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > ---
> >  .../devicetree/bindings/display/msm/gmu.txt        | 54 ++++++++++++++++++++++
> >  .../devicetree/bindings/display/msm/gpu.txt        | 10 +++-
> >  2 files changed, 62 insertions(+), 2 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 000000000000..f65bb49fff36
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
> > @@ -0,0 +1,54 @@
> > +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"
> 
> Kind of generic. All the features are discoverable?

I was just prepping a new version and I realized I never responded to this.
Sorry.

Yes, all the features are discoverable. Since the GMU is on the same die as the
GPU we would use the GPU revision as the criteria for applying hardware
workarounds and whatnot. I can't think of any current situation that would
require us to uniquely identify the GMU from DT.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Jordan Crouse]

Add the nodes and other bits to describe the Adreno GPU and GMU
devices.

Change-Id: Ibf4dc0ebb0ac03d8b6b8e65747e142c440e70b0a
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 120 +++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 7b5c16eb63b7..cc6d367ee55e 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -312,5 +312,125 @@
 				status = "disabled";
 			};
 		};
+
+		adreno_smmu: arm,smmu-adreno at 5040000 {
+			compatible = "qcom,msm8996-smmu-v2";
+			reg = <0x5040000 0x10000>;
+			#iommu-cells = <1>;
+			#global-interrupts = <2>;
+			interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 364 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 365 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 366 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 367 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 368 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 369 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 370 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 371 IRQ_TYPE_EDGE_RISING>;
+			clocks = <&clock_gcc GCC_GPU_MEMNOC_GFX_CLK>,
+				 <&clock_gcc GCC_GPU_CFG_AHB_CLK>;
+			clock-names = "bus", "iface";
+
+			power-domains = <&clock_gpucc GPU_CX_GDSC>;
+		};
+
+		gpu_opp_table: adreno-opp-table {
+			compatible = "operating-points-v2";
+
+			opp-710000000 {
+				opp-hz = /bits/ 64 <710000000>;
+				qcom,arc-level = <416>;
+			};
+
+			opp-675000000 {
+				opp-hz = /bits/ 64 <675000000>;
+				qcom,arc-level = <384>;
+			};
+
+			opp-596000000 {
+				opp-hz = /bits/ 64 <596000000>;
+				qcom,arc-level = <320>;
+			};
+
+			opp-520000000 {
+				opp-hz = /bits/ 64 <520000000>;
+				qcom,arc-level = <256>;
+			};
+
+			opp-414000000 {
+				opp-hz = /bits/ 64 <414000000>;
+				qcom,arc-level = <192>;
+			};
+
+			opp-342000000 {
+				opp-hz = /bits/ 64 <342000000>;
+				qcom,arc-level = <128>;
+			};
+
+			opp-257000000 {
+				opp-hz = /bits/ 64 <257000000>;
+				qcom,arc-level = <64>;
+			};
+		};
+
+		gpu at 5000000 {
+			compatible = "qcom,adreno-630.2", "qcom,adreno";
+			#stream-id-cells = <16>;
+
+			reg = <0x5000000 0x40000>;
+			reg-names = "kgsl_3d0_reg_memory";
+
+			/*
+			 * Look ma, no clocks! The GPU clocks and power are
+			 * controlled entirely by the GMU
+			 */
+
+			interrupts = <GCI_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "kgsl_3d0_irq";
+
+			iommus = <&adreno_smmu 0>;
+
+			operating-points-v2 = <&gpu_opp_table>;
+
+			gmu = <&gmu>;
+		};
+
+		gmu_opp_table: adreno-gmu-opp-table {
+			compatible = "operating-points-v2";
+
+			opp-400000000 {
+				opp-hz = /bits/ 64 <400000000>;
+				qcom,arc-level = <128>;
+			};
+
+			opp-200000000 {
+				opp-hz = /bits/ 64 <200000000>;
+				qcom,arc-level = <48>;
+			};
+		};
+
+		gmu: gmu at 506a000 {
+			compatible="qcom,adreno-gmu";
+
+			reg = <0x506a000 0x30000>,
+			      <0xb200000 0x300000>;
+			reg-names = "gmu", "gmu_pdc";
+
+			interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hfi", "gmu";
+
+			clocks = <&clock_gpucc GPU_CC_CX_GMU_CLK>,
+				 <&clock_gpucc GPU_CC_CXO_CLK>,
+				 <&clock_gcc GCC_DDRSS_GPU_AXI_CLK>,
+				 <&clock_gcc GCC_GPU_MEMNOC_GFX_CLK>;
+			clock-names = "gmu", "cxo", "axi", "memnoc";
+
+			power-domains = <&clock_gpucc GPU_CX_GDSC>;
+			iommus = <&adreno_smmu 5>;
+
+			operating-points-v2 = <&gmu_opp_table>;
+		};
 	};
 };
-- 
2.16.1

[PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Viresh Kumar]

On 02-03-18, 14:56, Jordan Crouse wrote:
> Add the nodes and other bits to describe the Adreno GPU and GMU
> devices.
> 
> Change-Id: Ibf4dc0ebb0ac03d8b6b8e65747e142c440e70b0a

Remove it ?

> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 120 +++++++++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 7b5c16eb63b7..cc6d367ee55e 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -312,5 +312,125 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		adreno_smmu: arm,smmu-adreno at 5040000 {
> +			compatible = "qcom,msm8996-smmu-v2";
> +			reg = <0x5040000 0x10000>;
> +			#iommu-cells = <1>;
> +			#global-interrupts = <2>;
> +			interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 364 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 365 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 366 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 367 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 368 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 369 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 370 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 371 IRQ_TYPE_EDGE_RISING>;
> +			clocks = <&clock_gcc GCC_GPU_MEMNOC_GFX_CLK>,
> +				 <&clock_gcc GCC_GPU_CFG_AHB_CLK>;
> +			clock-names = "bus", "iface";
> +
> +			power-domains = <&clock_gpucc GPU_CX_GDSC>;
> +		};
> +
> +		gpu_opp_table: adreno-opp-table {
> +			compatible = "operating-points-v2";
> +
> +			opp-710000000 {
> +				opp-hz = /bits/ 64 <710000000>;
> +				qcom,arc-level = <416>;

I am not sure if I saw where this is defined ?


-- 
viresh

[PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Jordan Crouse]

On Mon, Mar 05, 2018 at 10:12:21AM +0530, Viresh Kumar wrote:
> On 02-03-18, 14:56, Jordan Crouse wrote:
> > Add the nodes and other bits to describe the Adreno GPU and GMU
> > devices.
> > 
> > Change-Id: Ibf4dc0ebb0ac03d8b6b8e65747e142c440e70b0a
> 
> Remove it ?

* Shakes fist at Gerrit *

> > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 120 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 120 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 7b5c16eb63b7..cc6d367ee55e 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -312,5 +312,125 @@
> >  				status = "disabled";
> >  			};
> >  		};
> > +
> > +		adreno_smmu: arm,smmu-adreno at 5040000 {
> > +			compatible = "qcom,msm8996-smmu-v2";
> > +			reg = <0x5040000 0x10000>;
> > +			#iommu-cells = <1>;
> > +			#global-interrupts = <2>;
> > +			interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 364 IRQ_TYPE_EDGE_RISING>,
> > +				     <GIC_SPI 365 IRQ_TYPE_EDGE_RISING>,
> > +				     <GIC_SPI 366 IRQ_TYPE_EDGE_RISING>,
> > +				     <GIC_SPI 367 IRQ_TYPE_EDGE_RISING>,
> > +				     <GIC_SPI 368 IRQ_TYPE_EDGE_RISING>,
> > +				     <GIC_SPI 369 IRQ_TYPE_EDGE_RISING>,
> > +				     <GIC_SPI 370 IRQ_TYPE_EDGE_RISING>,
> > +				     <GIC_SPI 371 IRQ_TYPE_EDGE_RISING>;
> > +			clocks = <&clock_gcc GCC_GPU_MEMNOC_GFX_CLK>,
> > +				 <&clock_gcc GCC_GPU_CFG_AHB_CLK>;
> > +			clock-names = "bus", "iface";
> > +
> > +			power-domains = <&clock_gpucc GPU_CX_GDSC>;
> > +		};
> > +
> > +		gpu_opp_table: adreno-opp-table {
> > +			compatible = "operating-points-v2";
> > +
> > +			opp-710000000 {
> > +				opp-hz = /bits/ 64 <710000000>;
> > +				qcom,arc-level = <416>;
> 
> I am not sure if I saw where this is defined ?

I'm glad you brought this up - I was trying to find a place in the documentation
to put it, but since target specific nodes would be a new trick for OPP I didn't
quite know how to go about doing it. Do we just list them as Optional: or
should we add a target specific section to the documentation and list them out
there instead?

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Viresh Kumar]

On 05-03-18, 08:28, Jordan Crouse wrote:
> I'm glad you brought this up - I was trying to find a place in the documentation
> to put it, but since target specific nodes would be a new trick for OPP I didn't
> quite know how to go about doing it. Do we just list them as Optional: or
> should we add a target specific section to the documentation and list them out
> there instead?

Something like this and you need to have your own compatible string as
well:

Documentation/devicetree/bindings/opp/ti-omap5-opp-supply.txt

But first, what's the purpose of this field? Just to check if it can
be handled by current bindings.

-- 
viresh

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Jordan Crouse]

On Tue, Mar 06, 2018 at 09:56:56AM +0530, Viresh Kumar wrote:
> On 05-03-18, 08:28, Jordan Crouse wrote:
> > I'm glad you brought this up - I was trying to find a place in the documentation
> > to put it, but since target specific nodes would be a new trick for OPP I didn't
> > quite know how to go about doing it. Do we just list them as Optional: or
> > should we add a target specific section to the documentation and list them out
> > there instead?
> 
> Something like this and you need to have your own compatible string as
> well:
> 
> Documentation/devicetree/bindings/opp/ti-omap5-opp-supply.txt
> 
> But first, what's the purpose of this field? Just to check if it can
> be handled by current bindings.

I'll try to explain but I might need Stephen or some of the other folks to jump
in and save me.

On sdm845 there are shared power resources controlled by the RPMh which is
programed by a series of commands from the regulator driver; but in the case
of the GPU the votes are passed to the GMU (graphics management unit) which
communicates with the RPMh on behalf of the GPU.

In order to construct the RPMh vote we need first need a voltage level that we
can look up in a database. qcom,arc-level is the voltage level associated with
a specific OPP.

I had previously been abusing this with opp-microvolt but that was wrong for
obvious reasons and then Stephen pointed out that this would be a better way.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Viresh Kumar]

On 06-03-18, 08:37, Jordan Crouse wrote:
> I'll try to explain but I might need Stephen or some of the other folks to jump
> in and save me.

Maybe you should start using his kernel.org address then ? :)

> On sdm845 there are shared power resources controlled by the RPMh which is
> programed by a series of commands from the regulator driver; but in the case
> of the GPU the votes are passed to the GMU (graphics management unit) which
> communicates with the RPMh on behalf of the GPU.
> 
> In order to construct the RPMh vote we need first need a voltage level that we
> can look up in a database. qcom,arc-level is the voltage level associated with
> a specific OPP.
> 
> I had previously been abusing this with opp-microvolt but that was wrong for
> obvious reasons and then Stephen pointed out that this would be a better way.

Glad that you shared that :)

A solution is already in progress for that and is partially merged as
well.

Look for "performance_state" in genpd and OPP cores (already merged),
followed by this series here:

https://lkml.kernel.org/r/cover.1513926033.git.viresh.kumar at linaro.org

-- 
viresh

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Jordan Crouse]

On Wed, Mar 07, 2018 at 10:36:24AM +0530, Viresh Kumar wrote:
> On 06-03-18, 08:37, Jordan Crouse wrote:
> > I'll try to explain but I might need Stephen or some of the other folks to jump
> > in and save me.
> 
> Maybe you should start using his kernel.org address then ? :)
> 
> > On sdm845 there are shared power resources controlled by the RPMh which is
> > programed by a series of commands from the regulator driver; but in the case
> > of the GPU the votes are passed to the GMU (graphics management unit) which
> > communicates with the RPMh on behalf of the GPU.
> > 
> > In order to construct the RPMh vote we need first need a voltage level that we
> > can look up in a database. qcom,arc-level is the voltage level associated with
> > a specific OPP.
> > 
> > I had previously been abusing this with opp-microvolt but that was wrong for
> > obvious reasons and then Stephen pointed out that this would be a better way.
> 
> Glad that you shared that :)
> 
> A solution is already in progress for that and is partially merged as
> well.
> 
> Look for "performance_state" in genpd and OPP cores (already merged),
> followed by this series here:
> 
> https://lkml.kernel.org/r/cover.1513926033.git.viresh.kumar at linaro.org

It seems to me that performance_state has a direct relationship with genpd
which is good for CPU votes but in this case, we're just passing along raw data
to an independent microcontroller. The 'qcom,arc-level' is used to construct
the actual values that the GMU will program into the RPMh. Since these are
informational (from the CPU perspective) rather than functional I feel like
that using performance_state for this would be as hacky as using opp-microvolt
or something else.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Viresh Kumar]

On 08-03-18, 13:14, Jordan Crouse wrote:
> It seems to me that performance_state has a direct relationship with genpd
> which is good for CPU votes but in this case, we're just passing along raw data
> to an independent microcontroller. The 'qcom,arc-level' is used to construct
> the actual values that the GMU will program into the RPMh. Since these are

The "genpd" here is created specially for this RPM. The performance-state thing
is designed to solve this very specific problem of qualcomm SoCs and there is no
way we are going to add another property for this now.

> informational (from the CPU perspective) rather than functional I feel like
> that using performance_state for this would be as hacky as using opp-microvolt
> or something else.

There is some WIP stuff here Rajendra is testing currently.

ssh://git at git.linaro.org/people/viresh.kumar/mylinux.git opp/genpd/qcom

Please have a talk with Rajendra who can help you understand on how this can be
used for GPUs.

-- 
viresh

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Rajendra Nayak]

Hey Jordan/Viresh,

On 03/09/2018 09:13 AM, Viresh Kumar wrote:
> On 08-03-18, 13:14, Jordan Crouse wrote:
>> It seems to me that performance_state has a direct relationship with genpd
>> which is good for CPU votes but in this case, we're just passing along raw data
>> to an independent microcontroller. The 'qcom,arc-level' is used to construct
>> the actual values that the GMU will program into the RPMh. Since these are
> 
> The "genpd" here is created specially for this RPM. The performance-state thing
> is designed to solve this very specific problem of qualcomm SoCs and there is no
> way we are going to add another property for this now.
> 
>> informational (from the CPU perspective) rather than functional I feel like
>> that using performance_state for this would be as hacky as using opp-microvolt
>> or something else.
> 
> There is some WIP stuff here Rajendra is testing currently.

What I am doing is a powerdomain driver to communicate magic values from
CPU to rpmh, looks like we need another one to communicate from CPU to
GMU now :)

A few things,
* someone will need to map these larger magic values into something that
rpmh would understand (between 0 to 15 on the sdm845), thats done by
reading the command db level maps. Is this done by GMU?

* are these communicated just once for every OPP at init/boot, or for
every OPP switch?

* whats the communication mechanism we use between CPU and GMU? 

regards
Rajendra

> 
> ssh://git at git.linaro.org/people/viresh.kumar/mylinux.git opp/genpd/qcom
> 
> Please have a talk with Rajendra who can help you understand on how this can be
> used for GPUs.
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Jordan Crouse]

On Fri, Mar 09, 2018 at 03:49:00PM +0530, Rajendra Nayak wrote:
> Hey Jordan/Viresh,
> 
> On 03/09/2018 09:13 AM, Viresh Kumar wrote:
> > On 08-03-18, 13:14, Jordan Crouse wrote:
> >> It seems to me that performance_state has a direct relationship with genpd
> >> which is good for CPU votes but in this case, we're just passing along raw data
> >> to an independent microcontroller. The 'qcom,arc-level' is used to construct
> >> the actual values that the GMU will program into the RPMh. Since these are
> > 
> > The "genpd" here is created specially for this RPM. The performance-state thing
> > is designed to solve this very specific problem of qualcomm SoCs and there is no
> > way we are going to add another property for this now.
> > 
> >> informational (from the CPU perspective) rather than functional I feel like
> >> that using performance_state for this would be as hacky as using opp-microvolt
> >> or something else.
> > 
> > There is some WIP stuff here Rajendra is testing currently.
> 
> What I am doing is a powerdomain driver to communicate magic values from
> CPU to rpmh, looks like we need another one to communicate from CPU to
> GMU now :)
> 
> A few things,
> * someone will need to map these larger magic values into something that
> rpmh would understand (between 0 to 15 on the sdm845), thats done by
> reading the command db level maps. Is this done by GMU?

It is done by the GPU - we take the magic values, construct them into other
magic values and send them to the GMU. As far as I know we are the only other
in-kernel client of cmd-db other than the regulator(s).

> * are these communicated just once for every OPP at init/boot, or for
> every OPP switch?

Just once at init.

> * whats the communication mechanism we use between CPU and GMU?

Shared memory queues inspired by Venus HFI.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Jordan Crouse]

On Fri, Mar 09, 2018 at 09:13:32AM +0530, Viresh Kumar wrote:
> On 08-03-18, 13:14, Jordan Crouse wrote:
> > It seems to me that performance_state has a direct relationship with genpd
> > which is good for CPU votes but in this case, we're just passing along raw data
> > to an independent microcontroller. The 'qcom,arc-level' is used to construct
> > the actual values that the GMU will program into the RPMh. Since these are
> 
> The "genpd" here is created specially for this RPM. The performance-state thing
> is designed to solve this very specific problem of qualcomm SoCs and there is no
> way we are going to add another property for this now.
> 
> > informational (from the CPU perspective) rather than functional I feel like
> > that using performance_state for this would be as hacky as using opp-microvolt
> > or something else.
> 
> There is some WIP stuff here Rajendra is testing currently.
> 
> ssh://git at git.linaro.org/people/viresh.kumar/mylinux.git opp/genpd/qcom
> 
> Please have a talk with Rajendra who can help you understand on how this can be
> used for GPUs.

I don't think we are understanding each other. The GMU is a separate
microcontroller. It is given a magic number (actually a combination of magic
numbers) that it then uses to directly interact with the other hardware to make
the vote. The only responsibility that the CPU has is to construct that magic
number (once, at init) and send it across when asked.

Looking at the sdhc code from the testing tree it makes perfect sense
that you have a device that needs to eventually do a RPMh vote from the CPU and
so the 'required-opp' and performance state come together to do the right thing.
This is good code.

None of this is true for the GPU. The CPU never votes for the GPU so there 
isn't any need to connect it to the power domain drivers. Even if you did
there isn't any current mechanism for the rpmpd driver to pass the right magic
number to the GPU driver which is what it really needs.

I suppose that instead of using 'qcom,arc-level' we could use 'qcom-corner' but
then thats just a naming dispute. We still need a way for the GPU to query the
magic values.

Jordan
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Stephen Boyd]

(I wrote an email that seems to have been lost)

Quoting Jordan Crouse (2018-03-09 08:03:55)
> On Fri, Mar 09, 2018 at 09:13:32AM +0530, Viresh Kumar wrote:
> > On 08-03-18, 13:14, Jordan Crouse wrote:
> > > It seems to me that performance_state has a direct relationship with genpd
> > > which is good for CPU votes but in this case, we're just passing along raw data
> > > to an independent microcontroller. The 'qcom,arc-level' is used to construct
> > > the actual values that the GMU will program into the RPMh. Since these are
> > 
> > The "genpd" here is created specially for this RPM. The performance-state thing
> > is designed to solve this very specific problem of qualcomm SoCs and there is no
> > way we are going to add another property for this now.
> > 
> > > informational (from the CPU perspective) rather than functional I feel like
> > > that using performance_state for this would be as hacky as using opp-microvolt
> > > or something else.
> > 
> > There is some WIP stuff here Rajendra is testing currently.
> > 
> > ssh://git at git.linaro.org/people/viresh.kumar/mylinux.git opp/genpd/qcom
> > 
> > Please have a talk with Rajendra who can help you understand on how this can be
> > used for GPUs.
> 
> I don't think we are understanding each other. The GMU is a separate
> microcontroller. It is given a magic number (actually a combination of magic
> numbers) that it then uses to directly interact with the other hardware to make
> the vote. The only responsibility that the CPU has is to construct that magic
> number (once, at init) and send it across when asked.
> 
> Looking at the sdhc code from the testing tree it makes perfect sense
> that you have a device that needs to eventually do a RPMh vote from the CPU and
> so the 'required-opp' and performance state come together to do the right thing.
> This is good code.
> 
> None of this is true for the GPU. The CPU never votes for the GPU so there 
> isn't any need to connect it to the power domain drivers. Even if you did
> there isn't any current mechanism for the rpmpd driver to pass the right magic
> number to the GPU driver which is what it really needs.
> 
> I suppose that instead of using 'qcom,arc-level' we could use 'qcom-corner' but
> then thats just a naming dispute. We still need a way for the GPU to query the
> magic values.
> 

Agreed. Forcing genpd and set_performance_state APIs on GMU doesn't make
any sense if the GMU is doing it all itself.

The binding should be the same between sdhc and GMU if they're actually
talking about the same thing though. I think they are, because I suspect
GMU needs to translate the human reasonable value of qcom,corner into
the hardware qcom,arc-level value to write into it's hardware so the GMU
can auto-transition performance states. If I'm wrong, then
qcom,arc-level needs to be created alongside of qcom,corner because
they're different number spaces.

BTW, it's qcom,corner and not qcom-corner right?

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Jordan Crouse]

On Fri, Mar 09, 2018 at 09:18:41AM -0800, Stephen Boyd wrote:
> (I wrote an email that seems to have been lost)
> 
> Quoting Jordan Crouse (2018-03-09 08:03:55)
> > On Fri, Mar 09, 2018 at 09:13:32AM +0530, Viresh Kumar wrote:
> > > On 08-03-18, 13:14, Jordan Crouse wrote:
> > > > It seems to me that performance_state has a direct relationship with genpd
> > > > which is good for CPU votes but in this case, we're just passing along raw data
> > > > to an independent microcontroller. The 'qcom,arc-level' is used to construct
> > > > the actual values that the GMU will program into the RPMh. Since these are
> > > 
> > > The "genpd" here is created specially for this RPM. The performance-state thing
> > > is designed to solve this very specific problem of qualcomm SoCs and there is no
> > > way we are going to add another property for this now.
> > > 
> > > > informational (from the CPU perspective) rather than functional I feel like
> > > > that using performance_state for this would be as hacky as using opp-microvolt
> > > > or something else.
> > > 
> > > There is some WIP stuff here Rajendra is testing currently.
> > > 
> > > ssh://git at git.linaro.org/people/viresh.kumar/mylinux.git opp/genpd/qcom
> > > 
> > > Please have a talk with Rajendra who can help you understand on how this can be
> > > used for GPUs.
> > 
> > I don't think we are understanding each other. The GMU is a separate
> > microcontroller. It is given a magic number (actually a combination of magic
> > numbers) that it then uses to directly interact with the other hardware to make
> > the vote. The only responsibility that the CPU has is to construct that magic
> > number (once, at init) and send it across when asked.
> > 
> > Looking at the sdhc code from the testing tree it makes perfect sense
> > that you have a device that needs to eventually do a RPMh vote from the CPU and
> > so the 'required-opp' and performance state come together to do the right thing.
> > This is good code.
> > 
> > None of this is true for the GPU. The CPU never votes for the GPU so there 
> > isn't any need to connect it to the power domain drivers. Even if you did
> > there isn't any current mechanism for the rpmpd driver to pass the right magic
> > number to the GPU driver which is what it really needs.
> > 
> > I suppose that instead of using 'qcom,arc-level' we could use 'qcom-corner' but
> > then thats just a naming dispute. We still need a way for the GPU to query the
> > magic values.
> > 
> 
> Agreed. Forcing genpd and set_performance_state APIs on GMU doesn't make
> any sense if the GMU is doing it all itself.
> 
> The binding should be the same between sdhc and GMU if they're actually
> talking about the same thing though. I think they are, because I suspect
> GMU needs to translate the human reasonable value of qcom,corner into
> the hardware qcom,arc-level value to write into it's hardware so the GMU
> can auto-transition performance states. If I'm wrong, then
> qcom,arc-level needs to be created alongside of qcom,corner because
> they're different number spaces.

I think the value that we're discussing here is the RPMh voltage level which we
translate into the qcom,arc-level(s). I'm not sure how that matches up with
qcom,corner.

> BTW, it's qcom,corner and not qcom-corner right?

http://git.linaro.org/people/viresh.kumar/mylinux.git/commit/?h=opp/genpd/qcom&id=7586600b3bf3f8e79ce9198922fad7d4aa5b3f8d

+					qcom-corner = <1>;

shrug?

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Viresh Kumar]

On 09-03-18, 10:42, Jordan Crouse wrote:
> On Fri, Mar 09, 2018 at 09:18:41AM -0800, Stephen Boyd wrote:
> > BTW, it's qcom,corner and not qcom-corner right?
> 
> http://git.linaro.org/people/viresh.kumar/mylinux.git/commit/?h=opp/genpd/qcom&id=7586600b3bf3f8e79ce9198922fad7d4aa5b3f8d
> 
> +					qcom-corner = <1>;
> 
> shrug?

My branch isn't final and this naming has to come from Qcom (Rajendra). So it
can be qcom,corner eventually :)

-- 
viresh

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Viresh Kumar]

On 09-03-18, 09:03, Jordan Crouse wrote:
> I don't think we are understanding each other. The GMU is a separate
> microcontroller. It is given a magic number (actually a combination of magic
> numbers) that it then uses to directly interact with the other hardware to make
> the vote. The only responsibility that the CPU has is to construct that magic
> number (once, at init) and send it across when asked.
> 
> Looking at the sdhc code from the testing tree it makes perfect sense
> that you have a device that needs to eventually do a RPMh vote from the CPU and
> so the 'required-opp' and performance state come together to do the right thing.
> This is good code.
> 
> None of this is true for the GPU. The CPU never votes for the GPU so there 
> isn't any need to connect it to the power domain drivers. Even if you did
> there isn't any current mechanism for the rpmpd driver to pass the right magic
> number to the GPU driver which is what it really needs.
> 
> I suppose that instead of using 'qcom,arc-level' we could use 'qcom-corner' but
> then thats just a naming dispute. We still need a way for the GPU to query the
> magic values.

Okay, I get it. You can't (shouldn't) use genpd stuff here. I would still like
you guys (You/Rajendra/Stephen) to conclude if qcom-corner and qcom,arc-level
are eventually same values and we should use same property for them.

-- 
viresh

[Freedreno] [PATCH 2/2] arm64: dts: sdm845: Support GPU/GMU

[Stephen Boyd]

On Sun, Mar 11, 2018 at 10:52 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 09-03-18, 09:03, Jordan Crouse wrote:
>> I don't think we are understanding each other. The GMU is a separate
>> microcontroller. It is given a magic number (actually a combination of magic
>> numbers) that it then uses to directly interact with the other hardware to make
>> the vote. The only responsibility that the CPU has is to construct that magic
>> number (once, at init) and send it across when asked.
>>
>> Looking at the sdhc code from the testing tree it makes perfect sense
>> that you have a device that needs to eventually do a RPMh vote from the CPU and
>> so the 'required-opp' and performance state come together to do the right thing.
>> This is good code.
>>
>> None of this is true for the GPU. The CPU never votes for the GPU so there
>> isn't any need to connect it to the power domain drivers. Even if you did
>> there isn't any current mechanism for the rpmpd driver to pass the right magic
>> number to the GPU driver which is what it really needs.
>>
>> I suppose that instead of using 'qcom,arc-level' we could use 'qcom-corner' but
>> then thats just a naming dispute. We still need a way for the GPU to query the
>> magic values.
>
> Okay, I get it. You can't (shouldn't) use genpd stuff here. I would still like
> you guys (You/Rajendra/Stephen) to conclude if qcom-corner and qcom,arc-level
> are eventually same values and we should use same property for them.
>

It sounds like it's qcom,{corner,level} from Jordan's description. In
my mind 'level' and 'corner' are the same but they got a name change
with the new RPM interface. Then another number space was introduced
with the new RPM interface, 'arc-level', which represents the numbers
that the hardware deals with. It may be that DT doesn't ever care to
express the 'arc-level', because cmd db hides the numberspace by
requiring software to translate the software 'level' to the hardware
'arc-level'. So the whole thing may be a moot point and we can decide
to use qcom,level everywhere because it's the future.