Re: [PATCH v5 2/2] arm64: dts: sdm845: Add gpu and gmu device nodes

Hi,

On Wed, Dec 12, 2018 at 9:31 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

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

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index c27cbd3bcb0a..daa404b05a70 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -7,9 +7,11 @@

#include <dt-bindings/clock/qcom,dispcc-sdm845.h> #include <dt-bindings/clock/qcom,gcc-sdm845.h> +#include <dt-bindings/clock/qcom,gpucc-sdm845.h>

Probably don't need to add this in your patch since (presumably) your patch needs to be based atop https://lore.kernel.org/patchwork/patch/1018365/ and that already adds the #include (and also the node that you depend on).

#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> @@ -1348,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";
  

The second register range isn't in your bindings patch. Can you add it?

•                   /*
  

•                    * Look ma, no clocks! The GPU clocks and power are
  

•                    * controlled entirely by the GMU
  

•                    */
  

•                   interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
  

•                   interrupt-names = "kgsl_3d0_irq";
  

•                   iommus = <&adreno_smmu 0>;
  

It it worth mentioning the iommus and #stream-id-cells in the bindings?

•                   operating-points-v2 = <&gpu_opp_table>;
  

Is it worth mentioning the operating-points-v2 in the bindings patch?

•                   qcom,gmu = <&gmu>;
  

•                   gpu_opp_table: adreno-opp-table {
  

nit: since this is no longer at the top level you can just call it "opp-table" now. That matches what the latest RPMh PD node looks like.

•                           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";
  

•                   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: adreno-gmu-opp-table {
  

Same here that this can just be called "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>;
  

•                           };
  

•                   };
  

•           };
  

•           usb_1_hsphy: phy@88e2000 {
  

Since you won't compile without the gpucc patch maybe pick it before sending out your patch? Then context diff would show gpucc right here and it will avoid merge conflicts.

-Doug