On Wed, Sep 18, 2024 at 12:27:03AM +0300, Dmitry Baryshkov wrote:
> On Wed, Sep 18, 2024 at 02:08:43AM GMT, Akhil P Oommen wrote:
> > From: Puranam V G Tejaswi <quic_pvgtejas@xxxxxxxxxxx>
> >
> > Add gpu and gmu nodes for sa8775p based platforms.
>
> Which platforms? The commit adds nodes to the SoC and the single RIDE
> platform.
>
> >
> > Signed-off-by: Puranam V G Tejaswi <quic_pvgtejas@xxxxxxxxxxx>
> > Signed-off-by: Akhil P Oommen <quic_akhilpo@xxxxxxxxxxx>
> > ---
> > arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 8 ++++
> > arch/arm64/boot/dts/qcom/sa8775p.dtsi | 75 ++++++++++++++++++++++++++++++
> > 2 files changed, 83 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > index 2a6170623ea9..a01e6675c4bb 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
> > @@ -407,6 +407,14 @@ queue3 {
> > };
> > };
> >
> > +&gpu {
> > + status = "okay";
> > +
> > + zap-shader {
>
> It's easier to add gpu_zap_shader_link label in the DTSI file and then
> reference it instead of using the subnode again.
>
> > + firmware-name = "qcom/sa8775p/a663_zap.mbn";
> > + };
> > +};
>
> Separate patch, please.
>
> > +
> > &i2c11 {
> > clock-frequency = <400000>;
> > pinctrl-0 = <&qup_i2c11_default>;
> > diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > index 23f1b2e5e624..12c79135a303 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > @@ -2824,6 +2824,81 @@ tcsr_mutex: hwlock@1f40000 {
> > #hwlock-cells = <1>;
> > };
> >
> > + gpu: gpu@3d00000 {
> > + compatible = "qcom,adreno-663.0", "qcom,adreno";
> > + reg = <0 0x03d00000 0 0x40000>,
> > + <0 0x03d9e000 0 0x1000>,
> > + <0 0x03d61000 0 0x800>;
>
> I think it's suggested to use 0x0 now
>
> > + reg-names = "kgsl_3d0_reg_memory",
> > + "cx_mem",
> > + "cx_dbgc";
> > + interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> > + iommus = <&adreno_smmu 0 0xc00>,
> > + <&adreno_smmu 1 0xc00>;
> > + operating-points-v2 = <&gpu_opp_table>;
> > + qcom,gmu = <&gmu>;
> > + interconnects = <&gem_noc MASTER_GFX3D 0 &mc_virt SLAVE_EBI1 0>;
>
> QCOM_ICC_TAG_ALWAYS instead of 0
>
> > + interconnect-names = "gfx-mem";
> > + #cooling-cells = <2>;
>
> No speed bins?
Thanks for the review. Agree on all comments.
Speedbins were missed because we are sharing these changes early in the
developement cycle, sort of like what we did for chromeos develeopment.
Will try to pick it up in the next patchset.
-Akhil
>
> > +
> > + status = "disabled";
> > +
> > + zap-shader {
>
> gpu_zap_shader: zap-shader
>
> > + memory-region = <&pil_gpu_mem>;
> > + };
> > +
> > + gpu_opp_table: opp-table {
> > + compatible = "operating-points-v2";
> > +
> > + opp-405000000 {
>
> Just a single freq?
>
> > + opp-hz = /bits/ 64 <405000000>;
> > + opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
> > + opp-peak-kBps = <8368000>;
> > + };
> > +
>
> Drop the empty line, please.
>
> > + };
> > + };
> > +
> > + gmu: gmu@3d6a000 {
> > + compatible = "qcom,adreno-gmu-663.0", "qcom,adreno-gmu";
> > + reg = <0 0x03d6a000 0 0x34000>,
> > + <0 0x3de0000 0 0x10000>,
> > + <0 0x0b290000 0 0x10000>;
>
> Wrong indentation, please align to the angle bracket.
> Also I think it's suggested to use 0x0 now
>
> > + reg-names = "gmu", "rscc", "gmu_pdc";
> > + interrupts = <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
>
> And here
>
> > + 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>,
> > + <&gpucc GPU_CC_AHB_CLK>,
> > + <&gpucc GPU_CC_HUB_CX_INT_CLK>,
> > + <&gpucc GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK>;
> > + clock-names = "gmu",
> > + "cxo",
> > + "axi",
> > + "memnoc",
> > + "ahb",
> > + "hub",
> > + "smmu_vote";
> > + power-domains = <&gpucc GPU_CC_CX_GDSC>,
> > + <&gpucc GPU_CC_GX_GDSC>;
> > + power-domain-names = "cx",
> > + "gx";
> > + iommus = <&adreno_smmu 5 0xc00>;
> > + operating-points-v2 = <&gmu_opp_table>;
> > +
> > + gmu_opp_table: opp-table {
> > + compatible = "operating-points-v2";
> > +
> > + opp-200000000 {
> > + opp-hz = /bits/ 64 <200000000>;
> > + opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
> > + };
> > + };
> > + };
> > +
> > gpucc: clock-controller@3d90000 {
> > compatible = "qcom,sa8775p-gpucc";
> > reg = <0x0 0x03d90000 0x0 0xa000>;
> >
> > --
> > 2.45.2
> >
>
> --
> With best wishes
> Dmitry
