Hi,
On Tue, Dec 18, 2018 at 2:57 PM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> Hi,
>
> On Tue, Dec 18, 2018 at 10:33 AM Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> wrote:
> >
> > Add the nodes to describe the Adreno GPU and GMU devices.
> >
> > Signed-off-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx>
> > ---
> >
> > 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@xxxxxxxxxxxx>
> Tested-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
>
>
> > + 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@xxxxxxxxxxxxxx/
[2] https://lkml.kernel.org/r/20190107100959.14528-2-rnayak@xxxxxxxxxxxxxx
