diff mbox series

[v7,6/6] arm64: dts: sdm845: Add gpu and gmu device nodes

Message ID 20181218183241.12830-7-jcrouse@codeaurora.org (mailing list archive)
State Not Applicable, archived
Headers show
Series arm64: dts: Add sdm845 GPU/GMU and SMMU | expand

Commit Message

Jordan Crouse Dec. 18, 2018, 6:32 p.m. UTC
Add the nodes to describe the Adreno GPU and GMU devices.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

v7: Updated the GMU compatible string and removed interrupt-names

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 122 +++++++++++++++++++++++++++
 1 file changed, 122 insertions(+)

Comments

Doug Anderson Dec. 18, 2018, 10:57 p.m. UTC | #1
Hi,

On Tue, Dec 18, 2018 at 10:33 AM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>
> Add the nodes to describe the Adreno GPU and GMU devices.
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>
> v7: Updated the GMU compatible string and removed interrupt-names
>
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 122 +++++++++++++++++++++++++++
>  1 file changed, 122 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 233a5898ebc2..4779014e4a05 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -11,6 +11,7 @@
>  #include <dt-bindings/clock/qcom,rpmh.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/phy/phy-qcom-qusb2.h>
> +#include <dt-bindings/power/qcom-rpmpd.h>
>  #include <dt-bindings/reset/qcom,sdm845-aoss.h>
>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
>  #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> @@ -1349,6 +1350,127 @@
>                         };
>                 };
>
> +
> +               gpu@5000000 {

Repeating my comments from v6:

nit that you're adding an extra blank line here that you don't need.
Given the quantity of outstanding dts patches though, it's almost
certain that Andy will need to manually resolve conflicts when
applying this patch so presumably he can fix up when he lands.

In any case, feel free to add:

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>


> +                       compatible = "qcom,adreno-630.2", "qcom,adreno";
> +                       #stream-id-cells = <16>;
> +
> +                       reg = <0x5000000 0x40000>, <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 = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> +
> +                       iommus = <&adreno_smmu 0>;
> +
> +                       operating-points-v2 = <&gpu_opp_table>;
> +
> +                       qcom,gmu = <&gmu>;
> +
> +                       gpu_opp_table: opp-table {
> +                               compatible = "operating-points-v2-qcom-level";

I believe that the consensus from the v2 comments at
<https://patchwork.kernel.org/patch/10727121/> are that this should
be:

compatible = "operating-points-v2-qcom-level", "operating-points-v2";

...same with the other one below...
Doug Anderson Jan. 9, 2019, 5:20 a.m. UTC | #2
Hi,

On Tue, Dec 18, 2018 at 2:57 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Dec 18, 2018 at 10:33 AM Jordan Crouse <jcrouse@codeaurora.org> wrote:
> >
> > Add the nodes to describe the Adreno GPU and GMU devices.
> >
> > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > ---
> >
> > v7: Updated the GMU compatible string and removed interrupt-names
> >
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 122 +++++++++++++++++++++++++++
> >  1 file changed, 122 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 233a5898ebc2..4779014e4a05 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -11,6 +11,7 @@
> >  #include <dt-bindings/clock/qcom,rpmh.h>
> >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> >  #include <dt-bindings/phy/phy-qcom-qusb2.h>
> > +#include <dt-bindings/power/qcom-rpmpd.h>
> >  #include <dt-bindings/reset/qcom,sdm845-aoss.h>
> >  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> >  #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> > @@ -1349,6 +1350,127 @@
> >                         };
> >                 };
> >
> > +
> > +               gpu@5000000 {
>
> Repeating my comments from v6:
>
> nit that you're adding an extra blank line here that you don't need.
> Given the quantity of outstanding dts patches though, it's almost
> certain that Andy will need to manually resolve conflicts when
> applying this patch so presumably he can fix up when he lands.
>
> In any case, feel free to add:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Tested-by: Douglas Anderson <dianders@chromium.org>
>
>
> > +                       compatible = "qcom,adreno-630.2", "qcom,adreno";
> > +                       #stream-id-cells = <16>;
> > +
> > +                       reg = <0x5000000 0x40000>, <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 = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +                       iommus = <&adreno_smmu 0>;
> > +
> > +                       operating-points-v2 = <&gpu_opp_table>;
> > +
> > +                       qcom,gmu = <&gmu>;
> > +
> > +                       gpu_opp_table: opp-table {
> > +                               compatible = "operating-points-v2-qcom-level";
>
> I believe that the consensus from the v2 comments at
> <https://patchwork.kernel.org/patch/10727121/> are that this should
> be:
>
> compatible = "operating-points-v2-qcom-level", "operating-points-v2";
>
> ...same with the other one below...

OK, so I read and re-read all the discussions about this that have
happened since my last email and also had an offline chat with Stephen
and I believe the decision is _not_ to do the "operating-points-v2"
fallback even if your table happens to have "opp-hz" in it.  So
presumably that's resolved.  One reason this works OK for us is that
we have the "operating-points" as a sub-node of the GPU and thus don't
have to worry about the messing with the skip table to prevent a
device from being created (as discussed in the comments of v6) [1]

...but in the meantime Rajendra has had to change his bindings, so you
still need to spin this to account for Rajendra's v9 bindings [2].
Specifically you need to make changes like:

-                               compatible = "operating-points-v2-qcom-level";
+                               compatible = "operating-points-v2-level";

...and:

-                                       qcom,level =
<RPMH_REGULATOR_LEVEL_TURBO_L1>;
+                                       opp-level =
<RPMH_REGULATOR_LEVEL_TURBO_L1>;

...when you spin for this, please make sure you account for the nit
(blank line) I commented about above.

-Doug

[1] https://lore.kernel.org/linux-arm-kernel/7e310416-78d3-b7e5-5013-0bcc8bfd0351@codeaurora.org/
[2] https://lkml.kernel.org/r/20190107100959.14528-2-rnayak@codeaurora.org
Rajendra Nayak Jan. 9, 2019, 9:32 a.m. UTC | #3
On 1/9/2019 10:50 AM, Doug Anderson wrote:
> ...but in the meantime Rajendra has had to change his bindings, so you
> still need to spin this to account for Rajendra's v9 bindings [2].
> Specifically you need to make changes like:
> 
> -                               compatible = "operating-points-v2-qcom-level";
> +                               compatible = "operating-points-v2-level";

so there's now a v10 [1] and the new compatible is completely dropped and opp-level
is now an optional property using the default "operating-points-v2"

so the change should be
-                               compatible = "operating-points-v2-qcom-level";
+                               compatible = "operating-points-v2";

[1] https://lkml.org/lkml/2019/1/9/152
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 233a5898ebc2..4779014e4a05 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -11,6 +11,7 @@ 
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/phy/phy-qcom-qusb2.h>
+#include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/reset/qcom,sdm845-aoss.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
 #include <dt-bindings/clock/qcom,gcc-sdm845.h>
@@ -1349,6 +1350,127 @@ 
 			};
 		};
 
+
+		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";
+
+			/*
+			 * 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>;
+
+			qcom,gmu = <&gmu>;
+
+			gpu_opp_table: opp-table {
+				compatible = "operating-points-v2-qcom-level";
+
+				opp-710000000 {
+					opp-hz = /bits/ 64 <710000000>;
+					qcom,level = <RPMH_REGULATOR_LEVEL_TURBO_L1>;
+				};
+
+				opp-675000000 {
+					opp-hz = /bits/ 64 <675000000>;
+					qcom,level = <RPMH_REGULATOR_LEVEL_TURBO>;
+				};
+
+				opp-596000000 {
+					opp-hz = /bits/ 64 <596000000>;
+					qcom,level = <RPMH_REGULATOR_LEVEL_NOM_L1>;
+				};
+
+				opp-520000000 {
+					opp-hz = /bits/ 64 <520000000>;
+					qcom,level = <RPMH_REGULATOR_LEVEL_NOM>;
+				};
+
+				opp-414000000 {
+					opp-hz = /bits/ 64 <414000000>;
+					qcom,level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
+				};
+
+				opp-342000000 {
+					opp-hz = /bits/ 64 <342000000>;
+					qcom,level = <RPMH_REGULATOR_LEVEL_SVS>;
+				};
+
+				opp-257000000 {
+					opp-hz = /bits/ 64 <257000000>;
+					qcom,level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+				};
+			};
+		};
+
+		adreno_smmu: iommu@5040000 {
+			compatible = "qcom,sdm845-smmu-v2", "qcom,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 = <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
+				<&gcc GCC_GPU_CFG_AHB_CLK>;
+			clock-names = "bus", "iface";
+
+			power-domains = <&gpucc GPU_CX_GDSC>;
+		};
+
+		gmu: gmu@506a000 {
+			compatible="qcom,adreno-gmu-630.2", "qcom,adreno-gmu";
+
+			reg = <0x506a000 0x30000>,
+				<0xb280000 0x10000>,
+				<0xb480000 0x10000>;
+			reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq";
+
+			interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
+			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>;
+
+			gmu_opp_table: opp-table {
+				compatible = "operating-points-v2-qcom-level";
+
+				opp-400000000 {
+					opp-hz = /bits/ 64 <400000000>;
+					qcom,level = <RPMH_REGULATOR_LEVEL_SVS>;
+				};
+
+				opp-200000000 {
+					opp-hz = /bits/ 64 <200000000>;
+					qcom,level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
+				};
+			};
+		};
+
 		gpucc: clock-controller@5090000 {
 			compatible = "qcom,sdm845-gpucc";
 			reg = <0x5090000 0x9000>;