Message-ID: <52b54fa1-7f7a-85ad-a7fa-1c05d52f898b@collabora.com>
Date:   Fri, 24 Feb 2023 14:04:12 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v2 07/16] arm64: dts: mediatek: mt8192: Add GPU nodes
Content-Language: en-US
To:     Chen-Yu Tsai <wenst@chromium.org>
Cc:     matthias.bgg@gmail.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org,
        Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>,
        =?UTF-8?Q?N=c3=adcolas_F_=2e_R_=2e_A_=2e_Prado?= 
        <nfraprado@collabora.com>
References: <20230223134345.82625-1-angelogioacchino.delregno@collabora.com>
 <20230223134345.82625-8-angelogioacchino.delregno@collabora.com>
 <CAGXv+5H4a686nZPWjd7yFXSDCjttg0OPz1VD_8ppGASKgTWc7g@mail.gmail.com>
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
In-Reply-To: <CAGXv+5H4a686nZPWjd7yFXSDCjttg0OPz1VD_8ppGASKgTWc7g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Il 24/02/23 10:55, Chen-Yu Tsai ha scritto:
> On Thu, Feb 23, 2023 at 9:44 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> From: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
>>
>> The MediaTek MT8192 includes a Mali-G57 GPU supported in Panfrost. Add
>> the GPU node to the device tree to enable 3D acceleration.
>>
>> The GPU node is disabled by default. It should be enabled by board with
>> its power supplies correctly assigned.
>>
>> Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
>> [nfraprado: removed sram supply, tweaked opp node name, adjusted commit message]
>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>> [wenst@: disable GPU by default; adjusted prefix; split out board change]
>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   arch/arm64/boot/dts/mediatek/mt8192.dtsi | 109 +++++++++++++++++++++++
>>   1 file changed, 109 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>> index 87b91c8feaf9..2a3606f68ae4 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>> @@ -312,6 +312,91 @@ timer: timer {
>>                  clock-frequency = <13000000>;
>>          };
>>
>> +       gpu_opp_table: opp-table-0 {
>> +               compatible = "operating-points-v2";
>> +               opp-shared;
>> +
>> +               opp-358000000 {
>> +                       opp-hz = /bits/ 64 <358000000>;
>> +                       opp-microvolt = <606250>;
>> +               };
>> +
>> +               opp-399000000 {
>> +                       opp-hz = /bits/ 64 <399000000>;
>> +                       opp-microvolt = <618750>;
>> +               };
>> +
>> +               opp-440000000 {
>> +                       opp-hz = /bits/ 64 <440000000>;
>> +                       opp-microvolt = <631250>;
>> +               };
>> +
>> +               opp-482000000 {
>> +                       opp-hz = /bits/ 64 <482000000>;
>> +                       opp-microvolt = <643750>;
>> +               };
>> +
>> +               opp-523000000 {
>> +                       opp-hz = /bits/ 64 <523000000>;
>> +                       opp-microvolt = <656250>;
>> +               };
>> +
>> +               opp-564000000 {
>> +                       opp-hz = /bits/ 64 <564000000>;
>> +                       opp-microvolt = <668750>;
>> +               };
>> +
>> +               opp-605000000 {
>> +                       opp-hz = /bits/ 64 <605000000>;
>> +                       opp-microvolt = <681250>;
>> +               };
>> +
>> +               opp-647000000 {
>> +                       opp-hz = /bits/ 64 <647000000>;
>> +                       opp-microvolt = <693750>;
>> +               };
>> +
>> +               opp-688000000 {
>> +                       opp-hz = /bits/ 64 <688000000>;
>> +                       opp-microvolt = <706250>;
>> +               };
>> +
>> +               opp-724000000 {
>> +                       opp-hz = /bits/ 64 <724000000>;
>> +                       opp-microvolt = <725000>;
>> +               };
>> +
>> +               opp-748000000 {
>> +                       opp-hz = /bits/ 64 <748000000>;
>> +                       opp-microvolt = <737500>;
>> +               };
>> +
>> +               opp-772000000 {
>> +                       opp-hz = /bits/ 64 <772000000>;
>> +                       opp-microvolt = <750000>;
>> +               };
>> +
>> +               opp-795000000 {
>> +                       opp-hz = /bits/ 64 <795000000>;
>> +                       opp-microvolt = <762500>;
>> +               };
>> +
>> +               opp-819000000 {
>> +                       opp-hz = /bits/ 64 <819000000>;
>> +                       opp-microvolt = <775000>;
>> +               };
>> +
>> +               opp-843000000 {
>> +                       opp-hz = /bits/ 64 <843000000>;
>> +                       opp-microvolt = <787500>;
>> +               };
>> +
>> +               opp-866000000 {
>> +                       opp-hz = /bits/ 64 <866000000>;
>> +                       opp-microvolt = <800000>;
>> +               };
>> +       };
>> +
>>          soc {
>>                  #address-cells = <2>;
>>                  #size-cells = <2>;
>> @@ -1266,6 +1351,30 @@ mmc1: mmc@11f70000 {
>>                          status = "disabled";
>>                  };
>>
>> +               gpu: gpu@13000000 {
>> +                       compatible = "mediatek,mt8192-mali", "arm,mali-valhall-jm";
>> +                       reg = <0 0x13000000 0 0x4000>;
>> +                       interrupts =
>> +                               <GIC_SPI 365 IRQ_TYPE_LEVEL_HIGH 0>,
>> +                               <GIC_SPI 364 IRQ_TYPE_LEVEL_HIGH 0>,
>> +                               <GIC_SPI 363 IRQ_TYPE_LEVEL_HIGH 0>;
> 
> Nit: Move the first entry to the same line as the property, and align
> the following lines' angle brackets?
>  > Same for the power-domains.
> 

Makes sense, yes, I should've paid more attention to that.

Regards,
Angelo

> Otherwise,
> 
> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
> 
>> +                       interrupt-names = "job", "mmu", "gpu";
>> +
>> +                       clocks = <&apmixedsys CLK_APMIXED_MFGPLL>;
>> +
>> +                       power-domains =
>> +                               <&spm MT8192_POWER_DOMAIN_MFG2>,
>> +                               <&spm MT8192_POWER_DOMAIN_MFG3>,
>> +                               <&spm MT8192_POWER_DOMAIN_MFG4>,
>> +                               <&spm MT8192_POWER_DOMAIN_MFG5>,
>> +                               <&spm MT8192_POWER_DOMAIN_MFG6>;
>> +                       power-domain-names = "core0", "core1", "core2", "core3", "core4";
>> +
>> +                       operating-points-v2 = <&gpu_opp_table>;
>> +
>> +                       status = "disabled";
>> +               };
>> +
>>                  mfgcfg: clock-controller@13fbf000 {
>>                          compatible = "mediatek,mt8192-mfgcfg";
>>                          reg = <0 0x13fbf000 0 0x1000>;
>> --
>> 2.39.2
>>