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

[Doug Anderson]

Hi,

On Tue, Dec 18, 2018 at 2:57 PM Doug Anderson <dianders at chromium.org> wrote:
>
> Hi,
>
> On Tue, Dec 18, 2018 at 10:33 AM Jordan Crouse <jcrouse at codeaurora.org> wrote:
> >
> > Add the nodes to describe the Adreno GPU and GMU devices.
> >
> > Signed-off-by: Jordan Crouse <jcrouse at 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 at 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 at chromium.org>
> Tested-by: Douglas Anderson <dianders at 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