From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CDAB7C4338F for ; Thu, 5 Aug 2021 02:56:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A5FD661073 for ; Thu, 5 Aug 2021 02:56:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230209AbhHEC44 (ORCPT ); Wed, 4 Aug 2021 22:56:56 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:50226 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229609AbhHEC44 (ORCPT ); Wed, 4 Aug 2021 22:56:56 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: ezequiel) with ESMTPSA id C08151F40EF5 Message-ID: Subject: Re: [PATCH 3/4] arm64: dts: rockchip: Add GPU node for rk3568 From: Ezequiel Garcia To: Johan Jonker , devicetree@vger.kernel.org, linux-rockchip@lists.infradead.org Cc: Rob Herring , Heiko Stuebner , Kever Yang , Benjamin Gaignard , Peter Geis Date: Wed, 04 Aug 2021 23:56:30 -0300 In-Reply-To: References: <20210730164515.83044-1-ezequiel@collabora.com> <20210730164515.83044-4-ezequiel@collabora.com> Organization: Collabora Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3-1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Hi Johan, Thanks for the review. On Fri, 2021-07-30 at 21:38 +0200, Johan Jonker wrote: > Hi Ezequiel, > > Some comments. Have a look if it's useful. > > On 7/30/21 6:45 PM, Ezequiel Garcia wrote: > > Rockchip SoCs RK3566 and RK3568 have a Mali Gondul core > > which is based on the Bifrost architecture. It has > > one shader core and two execution engines. > > > > Quoting the datasheet: > > > > Mali-G52 1-Core-2EE > > * Support 1600Mpix/s fill rate when 800MHz clock frequency > > * Support 38.4GLOPs when 800MHz clock frequency > > > > Signed-off-by: Ezequiel Garcia > > --- > >  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 47 ++++++++++++++++++++++++ > >  1 file changed, 47 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > > index bef747fb1fe2..f4f400792659 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi > > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi > > @@ -121,6 +121,35 @@ opp-1800000000 { > >                 }; > >         }; > >   > > > +       gpu_opp_table: opp-table2 { > >         gpu_opp_table: gpu-opp-table { > Makes sense. > > +               compatible = "operating-points-v2"; > > + > > +               opp-200000000 { > > +                       opp-hz = /bits/ 64 <200000000>; > > +                       opp-microvolt = <825000>; > > +               }; > > Similar to cpu0_opp_table keep the same style and add an empty line > between nodes. > Makes sense. > > +               opp-300000000 { > > +                       opp-hz = /bits/ 64 <300000000>; > > +                       opp-microvolt = <825000>; > > +               }; > > +               opp-400000000 { > > +                       opp-hz = /bits/ 64 <400000000>; > > +                       opp-microvolt = <825000>; > > +               }; > > +               opp-600000000 { > > +                       opp-hz = /bits/ 64 <600000000>; > > +                       opp-microvolt = <825000>; > > +               }; > > +               opp-700000000 { > > +                       opp-hz = /bits/ 64 <700000000>; > > +                       opp-microvolt = <900000>; > > +               }; > > +               opp-800000000 { > > +                       opp-hz = /bits/ 64 <800000000>; > > +                       opp-microvolt = <1000000>; > > +               }; > > +       }; > > + > >         firmware { > >                 scmi: scmi { > >                         compatible = "arm,scmi-smc"; > > @@ -332,6 +361,24 @@ power-domain@RK3568_PD_RKVENC { > >                 }; > >         }; > >   > > +       gpu: gpu@fde60000 { > > +               compatible = "rockchip,rk3568-mali", "arm,mali-bifrost"; > > +               reg = <0x0 0xfde60000 0x0 0x4000>; > > > + > > remove empty lines > Makes sense. > > +               interrupts = , > > +                            , > > +                            ; > > +               interrupt-names = "job", "mmu", "gpu"; > > > + > > dito > > > +               clocks = <&scmi_clk 1>, <&cru CLK_GPU>; > > +               clock-names = "core", "bus"; > > Not sure if it's possible, but could you keep them all a little bit in > the same order/style as arm,mali-400? > > From arm,mali-utgard.yaml: > >   clock-names: >     items: >       - const: bus >       - const: core > Don't think we can do that in this case, as the first clock is already expected as the "core" one. These clocks might need some cleaner specification in the driver or the dt-binding (or both), but I'd like to defer that to the driver maintainers. -- Kindly, Ezequiel