diff mbox series

arm64: dts: mt8183: Add node for the Mali GPU

Message ID 20190905081546.42716-1-drinkcat@chromium.org (mailing list archive)
State New, archived
Headers show
Series arm64: dts: mt8183: Add node for the Mali GPU | expand

Commit Message

Nicolas Boichat Sept. 5, 2019, 8:15 a.m. UTC
Add a basic GPU node and opp table for mt8183.

The binding we use with out-of-tree Mali drivers includes more
clocks, I assume this would be required eventually if we have an
in-tree driver:
clocks =
        <&topckgen CLK_TOP_MFGPLL_CK>,
        <&topckgen CLK_TOP_MUX_MFG>,
        <&clk26m>,
        <&mfgcfg CLK_MFG_BG3D>;
clock-names =
        "clk_main_parent",
        "clk_mux",
        "clk_sub_parent",
        "subsys_mfg_cg";

Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>

---
Upstreaming what matches existing bindings from our Chromium OS tree:
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/arch/arm64/boot/dts/mediatek/mt8183.dtsi#1348

The evb part of this change depends on this patch to add PMIC dtsi:
https://patchwork.kernel.org/patch/10928161/

 arch/arm64/boot/dts/mediatek/mt8183-evb.dts |   7 ++
 arch/arm64/boot/dts/mediatek/mt8183.dtsi    | 103 ++++++++++++++++++++
 2 files changed, 110 insertions(+)

Comments

Rob Herring Sept. 5, 2019, 9:09 a.m. UTC | #1
On Thu, Sep 5, 2019 at 9:16 AM Nicolas Boichat <drinkcat@chromium.org> wrote:
>
> Add a basic GPU node and opp table for mt8183.
>
> The binding we use with out-of-tree Mali drivers includes more
> clocks, I assume this would be required eventually if we have an
> in-tree driver:

We have an in-tree driver...

> clocks =
>         <&topckgen CLK_TOP_MFGPLL_CK>,
>         <&topckgen CLK_TOP_MUX_MFG>,
>         <&clk26m>,
>         <&mfgcfg CLK_MFG_BG3D>;
> clock-names =
>         "clk_main_parent",
>         "clk_mux",
>         "clk_sub_parent",
>         "subsys_mfg_cg";
>
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
>
> ---
> Upstreaming what matches existing bindings from our Chromium OS tree:
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/arch/arm64/boot/dts/mediatek/mt8183.dtsi#1348
>
> The evb part of this change depends on this patch to add PMIC dtsi:
> https://patchwork.kernel.org/patch/10928161/
>
>  arch/arm64/boot/dts/mediatek/mt8183-evb.dts |   7 ++
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi    | 103 ++++++++++++++++++++
>  2 files changed, 110 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> index 1fb195c683c3d01..200d8e65a6368a1 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> @@ -7,6 +7,7 @@
>
>  /dts-v1/;
>  #include "mt8183.dtsi"
> +#include "mt6358.dtsi"
>
>  / {
>         model = "MediaTek MT8183 evaluation board";
> @@ -30,6 +31,12 @@
>         status = "okay";
>  };
>
> +&gpu {
> +       supply-names = "mali", "mali_sram";
> +       mali-supply = <&mt6358_vgpu_reg>;
> +       mali_sram-supply = <&mt6358_vsram_gpu_reg>;

Not documented. Just 'sram-supply' is enough.

Note that the binding doc queued up for 5.4 has been converted to DT schema.

> +};
> +
>  &i2c0 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&i2c_pins_0>;
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 97f84aa9fc6e1c1..8ea548a762ea252 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -579,6 +579,109 @@
>                         #clock-cells = <1>;
>                 };
>
> +               gpu: mali@13040000 {

gpu@...

> +                       compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";

You need to add this compatible string too.

> +                       reg = <0 0x13040000 0 0x4000>;
> +                       interrupts =
> +                               <GIC_SPI 280 IRQ_TYPE_LEVEL_LOW>,
> +                               <GIC_SPI 279 IRQ_TYPE_LEVEL_LOW>,
> +                               <GIC_SPI 278 IRQ_TYPE_LEVEL_LOW>;
> +                       interrupt-names = "job", "mmu", "gpu";
> +
> +                       clocks = <&topckgen CLK_TOP_MFGPLL_CK>;
> +                       power-domains =
> +                               <&scpsys MT8183_POWER_DOMAIN_MFG_CORE0>,
> +                               <&scpsys MT8183_POWER_DOMAIN_MFG_CORE1>,
> +                               <&scpsys MT8183_POWER_DOMAIN_MFG_2D>;

This needs to be documented too.

> +
> +                       operating-points-v2 = <&gpu_opp_table>;
> +               };
> +
> +               gpu_opp_table: opp_table0 {
> +                       compatible = "operating-points-v2";
> +                       opp-shared;
> +
> +                       opp-300000000 {
> +                               opp-hz = /bits/ 64 <300000000>;
> +                               opp-microvolt = <625000>, <850000>;
> +                       };
> +
> +                       opp-320000000 {
> +                               opp-hz = /bits/ 64 <320000000>;
> +                               opp-microvolt = <631250>, <850000>;
> +                       };
> +
> +                       opp-340000000 {
> +                               opp-hz = /bits/ 64 <340000000>;
> +                               opp-microvolt = <637500>, <850000>;
> +                       };
> +
> +                       opp-360000000 {
> +                               opp-hz = /bits/ 64 <360000000>;
> +                               opp-microvolt = <643750>, <850000>;
> +                       };
> +
> +                       opp-380000000 {
> +                               opp-hz = /bits/ 64 <380000000>;
> +                               opp-microvolt = <650000>, <850000>;
> +                       };
> +
> +                       opp-400000000 {
> +                               opp-hz = /bits/ 64 <400000000>;
> +                               opp-microvolt = <656250>, <850000>;
> +                       };
> +
> +                       opp-420000000 {
> +                               opp-hz = /bits/ 64 <420000000>;
> +                               opp-microvolt = <662500>, <850000>;
> +                       };
> +
> +                       opp-460000000 {
> +                               opp-hz = /bits/ 64 <460000000>;
> +                               opp-microvolt = <675000>, <850000>;
> +                       };
> +
> +                       opp-500000000 {
> +                               opp-hz = /bits/ 64 <500000000>;
> +                               opp-microvolt = <687500>, <850000>;
> +                       };
> +
> +                       opp-540000000 {
> +                               opp-hz = /bits/ 64 <540000000>;
> +                               opp-microvolt = <700000>, <850000>;
> +                       };
> +
> +                       opp-580000000 {
> +                               opp-hz = /bits/ 64 <580000000>;
> +                               opp-microvolt = <712500>, <850000>;
> +                       };
> +
> +                       opp-620000000 {
> +                               opp-hz = /bits/ 64 <620000000>;
> +                               opp-microvolt = <725000>, <850000>;
> +                       };
> +
> +                       opp-653000000 {
> +                               opp-hz = /bits/ 64 <653000000>;
> +                               opp-microvolt = <743750>, <850000>;
> +                       };
> +
> +                       opp-698000000 {
> +                               opp-hz = /bits/ 64 <698000000>;
> +                               opp-microvolt = <768750>, <868750>;
> +                       };
> +
> +                       opp-743000000 {
> +                               opp-hz = /bits/ 64 <743000000>;
> +                               opp-microvolt = <793750>, <893750>;
> +                       };
> +
> +                       opp-800000000 {
> +                               opp-hz = /bits/ 64 <800000000>;
> +                               opp-microvolt = <825000>, <925000>;
> +                       };

Okay, but I seriously doubt the OPP selection logic is sophisticated
enough or will ever be to use all these levels...

> +               };
> +
>                 mmsys: syscon@14000000 {
>                         compatible = "mediatek,mt8183-mmsys", "syscon";
>                         reg = <0 0x14000000 0 0x1000>;
> --
> 2.23.0.187.g17f5b7556c-goog
>
Nicolas Boichat Sept. 5, 2019, 9:49 a.m. UTC | #2
Thanks for the quick review!

On Thu, Sep 5, 2019 at 5:09 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Sep 5, 2019 at 9:16 AM Nicolas Boichat <drinkcat@chromium.org> wrote:
> >
> > Add a basic GPU node and opp table for mt8183.
> >
> > The binding we use with out-of-tree Mali drivers includes more
> > clocks, I assume this would be required eventually if we have an
> > in-tree driver:
>
> We have an in-tree driver...

Right but AFAICT it does not support Bifrost GPU (yet?).

>
> > clocks =
> >         <&topckgen CLK_TOP_MFGPLL_CK>,
> >         <&topckgen CLK_TOP_MUX_MFG>,
> >         <&clk26m>,
> >         <&mfgcfg CLK_MFG_BG3D>;
> > clock-names =
> >         "clk_main_parent",
> >         "clk_mux",
> >         "clk_sub_parent",
> >         "subsys_mfg_cg";

Do you think we should add those to the binding document? May not be
easy to match what the amlogic binding does (I'm not sure to
understand the details of this device, but I can dig further/ask).

> >
> > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
> >
> > ---
> > Upstreaming what matches existing bindings from our Chromium OS tree:
> > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/arch/arm64/boot/dts/mediatek/mt8183.dtsi#1348
> >
> > The evb part of this change depends on this patch to add PMIC dtsi:
> > https://patchwork.kernel.org/patch/10928161/
> >
> >  arch/arm64/boot/dts/mediatek/mt8183-evb.dts |   7 ++
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi    | 103 ++++++++++++++++++++
> >  2 files changed, 110 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > index 1fb195c683c3d01..200d8e65a6368a1 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > @@ -7,6 +7,7 @@
> >
> >  /dts-v1/;
> >  #include "mt8183.dtsi"
> > +#include "mt6358.dtsi"
> >
> >  / {
> >         model = "MediaTek MT8183 evaluation board";
> > @@ -30,6 +31,12 @@
> >         status = "okay";
> >  };
> >
> > +&gpu {
> > +       supply-names = "mali", "mali_sram";
> > +       mali-supply = <&mt6358_vgpu_reg>;
> > +       mali_sram-supply = <&mt6358_vsram_gpu_reg>;
>
> Not documented. Just 'sram-supply' is enough.

Will fix.

> Note that the binding doc queued up for 5.4 has been converted to DT schema.

Yep I see that in linux-next.

>
> > +};
> > +
> >  &i2c0 {
> >         pinctrl-names = "default";
> >         pinctrl-0 = <&i2c_pins_0>;
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 97f84aa9fc6e1c1..8ea548a762ea252 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -579,6 +579,109 @@
> >                         #clock-cells = <1>;
> >                 };
> >
> > +               gpu: mali@13040000 {
>
> gpu@...
>
> > +                       compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
>
> You need to add this compatible string too.

Will do.

>
> > +                       reg = <0 0x13040000 0 0x4000>;
> > +                       interrupts =
> > +                               <GIC_SPI 280 IRQ_TYPE_LEVEL_LOW>,
> > +                               <GIC_SPI 279 IRQ_TYPE_LEVEL_LOW>,
> > +                               <GIC_SPI 278 IRQ_TYPE_LEVEL_LOW>;
> > +                       interrupt-names = "job", "mmu", "gpu";
> > +
> > +                       clocks = <&topckgen CLK_TOP_MFGPLL_CK>;
> > +                       power-domains =
> > +                               <&scpsys MT8183_POWER_DOMAIN_MFG_CORE0>,
> > +                               <&scpsys MT8183_POWER_DOMAIN_MFG_CORE1>,
> > +                               <&scpsys MT8183_POWER_DOMAIN_MFG_2D>;
>
> This needs to be documented too.

I see that Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
has power-domains in the example both not in the yaml, is that
expected?


>
> > +
> > +                       operating-points-v2 = <&gpu_opp_table>;
> > +               };
> > +
> > +               gpu_opp_table: opp_table0 {
> > +                       compatible = "operating-points-v2";
> > +                       opp-shared;
> > +
> > +                       opp-300000000 {
> > +                               opp-hz = /bits/ 64 <300000000>;
> > +                               opp-microvolt = <625000>, <850000>;
> > +                       };
> > +
> > +                       opp-320000000 {
> > +                               opp-hz = /bits/ 64 <320000000>;
> > +                               opp-microvolt = <631250>, <850000>;
> > +                       };
> > +
> > +                       opp-340000000 {
> > +                               opp-hz = /bits/ 64 <340000000>;
> > +                               opp-microvolt = <637500>, <850000>;
> > +                       };
> > +
> > +                       opp-360000000 {
> > +                               opp-hz = /bits/ 64 <360000000>;
> > +                               opp-microvolt = <643750>, <850000>;
> > +                       };
> > +
> > +                       opp-380000000 {
> > +                               opp-hz = /bits/ 64 <380000000>;
> > +                               opp-microvolt = <650000>, <850000>;
> > +                       };
> > +
> > +                       opp-400000000 {
> > +                               opp-hz = /bits/ 64 <400000000>;
> > +                               opp-microvolt = <656250>, <850000>;
> > +                       };
> > +
> > +                       opp-420000000 {
> > +                               opp-hz = /bits/ 64 <420000000>;
> > +                               opp-microvolt = <662500>, <850000>;
> > +                       };
> > +
> > +                       opp-460000000 {
> > +                               opp-hz = /bits/ 64 <460000000>;
> > +                               opp-microvolt = <675000>, <850000>;
> > +                       };
> > +
> > +                       opp-500000000 {
> > +                               opp-hz = /bits/ 64 <500000000>;
> > +                               opp-microvolt = <687500>, <850000>;
> > +                       };
> > +
> > +                       opp-540000000 {
> > +                               opp-hz = /bits/ 64 <540000000>;
> > +                               opp-microvolt = <700000>, <850000>;
> > +                       };
> > +
> > +                       opp-580000000 {
> > +                               opp-hz = /bits/ 64 <580000000>;
> > +                               opp-microvolt = <712500>, <850000>;
> > +                       };
> > +
> > +                       opp-620000000 {
> > +                               opp-hz = /bits/ 64 <620000000>;
> > +                               opp-microvolt = <725000>, <850000>;
> > +                       };
> > +
> > +                       opp-653000000 {
> > +                               opp-hz = /bits/ 64 <653000000>;
> > +                               opp-microvolt = <743750>, <850000>;
> > +                       };
> > +
> > +                       opp-698000000 {
> > +                               opp-hz = /bits/ 64 <698000000>;
> > +                               opp-microvolt = <768750>, <868750>;
> > +                       };
> > +
> > +                       opp-743000000 {
> > +                               opp-hz = /bits/ 64 <743000000>;
> > +                               opp-microvolt = <793750>, <893750>;
> > +                       };
> > +
> > +                       opp-800000000 {
> > +                               opp-hz = /bits/ 64 <800000000>;
> > +                               opp-microvolt = <825000>, <925000>;
> > +                       };
>
> Okay, but I seriously doubt the OPP selection logic is sophisticated
> enough or will ever be to use all these levels...
>
> > +               };
> > +
> >                 mmsys: syscon@14000000 {
> >                         compatible = "mediatek,mt8183-mmsys", "syscon";
> >                         reg = <0 0x14000000 0 0x1000>;
> > --
> > 2.23.0.187.g17f5b7556c-goog
> >
Rob Herring Sept. 5, 2019, 10:32 a.m. UTC | #3
On Thu, Sep 5, 2019 at 10:49 AM Nicolas Boichat <drinkcat@chromium.org> wrote:
>
> Thanks for the quick review!
>
> On Thu, Sep 5, 2019 at 5:09 PM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Thu, Sep 5, 2019 at 9:16 AM Nicolas Boichat <drinkcat@chromium.org> wrote:
> > >
> > > Add a basic GPU node and opp table for mt8183.
> > >
> > > The binding we use with out-of-tree Mali drivers includes more
> > > clocks, I assume this would be required eventually if we have an
> > > in-tree driver:
> >
> > We have an in-tree driver...
>
> Right but AFAICT it does not support Bifrost GPU (yet?).

It's mostly the mesa userspace side that's missing. The kernel driver
needs the compatible string and page table support[1]. The former
should be enough to access the registers which is typically enough to
sort out an platform specific clock, reset, power issues.

> > > clocks =
> > >         <&topckgen CLK_TOP_MFGPLL_CK>,
> > >         <&topckgen CLK_TOP_MUX_MFG>,
> > >         <&clk26m>,
> > >         <&mfgcfg CLK_MFG_BG3D>;
> > > clock-names =
> > >         "clk_main_parent",
> > >         "clk_mux",
> > >         "clk_sub_parent",
> > >         "subsys_mfg_cg";
>
> Do you think we should add those to the binding document? May not be
> easy to match what the amlogic binding does (I'm not sure to
> understand the details of this device, but I can dig further/ask).

I somewhat expect this needs more investigation. I'm doubtful that
there's a 26MHz clock going to Mali. Ideally, the clocks are what are
actually connected to the h/w, not just a list of all the clocks
needed on some platform because we fail to manage them elsewhere (like
an interconnect driver). Otherwise we end up with a different list for
every platform.

> > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
> > >
> > > ---
> > > Upstreaming what matches existing bindings from our Chromium OS tree:
> > > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/arch/arm64/boot/dts/mediatek/mt8183.dtsi#1348
> > >
> > > The evb part of this change depends on this patch to add PMIC dtsi:
> > > https://patchwork.kernel.org/patch/10928161/
> > >
> > >  arch/arm64/boot/dts/mediatek/mt8183-evb.dts |   7 ++
> > >  arch/arm64/boot/dts/mediatek/mt8183.dtsi    | 103 ++++++++++++++++++++
> > >  2 files changed, 110 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > > index 1fb195c683c3d01..200d8e65a6368a1 100644
> > > --- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > > +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > > @@ -7,6 +7,7 @@
> > >
> > >  /dts-v1/;
> > >  #include "mt8183.dtsi"
> > > +#include "mt6358.dtsi"
> > >
> > >  / {
> > >         model = "MediaTek MT8183 evaluation board";
> > > @@ -30,6 +31,12 @@
> > >         status = "okay";
> > >  };
> > >
> > > +&gpu {
> > > +       supply-names = "mali", "mali_sram";
> > > +       mali-supply = <&mt6358_vgpu_reg>;
> > > +       mali_sram-supply = <&mt6358_vsram_gpu_reg>;
> >
> > Not documented. Just 'sram-supply' is enough.
>
> Will fix.
>
> > Note that the binding doc queued up for 5.4 has been converted to DT schema.
>
> Yep I see that in linux-next.
>
> >
> > > +};
> > > +
> > >  &i2c0 {
> > >         pinctrl-names = "default";
> > >         pinctrl-0 = <&i2c_pins_0>;
> > > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > > index 97f84aa9fc6e1c1..8ea548a762ea252 100644
> > > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > > @@ -579,6 +579,109 @@
> > >                         #clock-cells = <1>;
> > >                 };
> > >
> > > +               gpu: mali@13040000 {
> >
> > gpu@...
> >
> > > +                       compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
> >
> > You need to add this compatible string too.
>
> Will do.
>
> >
> > > +                       reg = <0 0x13040000 0 0x4000>;
> > > +                       interrupts =
> > > +                               <GIC_SPI 280 IRQ_TYPE_LEVEL_LOW>,
> > > +                               <GIC_SPI 279 IRQ_TYPE_LEVEL_LOW>,
> > > +                               <GIC_SPI 278 IRQ_TYPE_LEVEL_LOW>;
> > > +                       interrupt-names = "job", "mmu", "gpu";
> > > +
> > > +                       clocks = <&topckgen CLK_TOP_MFGPLL_CK>;
> > > +                       power-domains =
> > > +                               <&scpsys MT8183_POWER_DOMAIN_MFG_CORE0>,
> > > +                               <&scpsys MT8183_POWER_DOMAIN_MFG_CORE1>,
> > > +                               <&scpsys MT8183_POWER_DOMAIN_MFG_2D>;
> >
> > This needs to be documented too.
>
> I see that Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
> has power-domains in the example both not in the yaml, is that
> expected?

Err, no. Probably some copy-n-paste from utgard.

Rob

[1] https://patchwork.freedesktop.org/patch/304731/
Alyssa Rosenzweig Sept. 13, 2019, 6:17 p.m. UTC | #4
> > > The binding we use with out-of-tree Mali drivers includes more
> > > clocks, I assume this would be required eventually if we have an
> > > in-tree driver:
> >
> > We have an in-tree driver...
> 
> Right but AFAICT it does not support Bifrost GPU (yet?).

By the time MT8183 shows up in more concrete devices, it will, certainly
in kernel-space and likely in userspace as well. At present, the DDK can
be modified to run on top of the in-tree Mali drivers, i.e. "Bifrost on
mainline linux-next (+ page table/compatible patches), with blob
userspace".

While the open userspace isn't ready here quite yet, I would definitely
encourage upstream kernel for ChromeOS, since then there's no need to
maintain the out-of-tree GPU driver.

---

More immediately, per Rob's review, it's important that the bindings
accepted upstream work with the in-tree Bifrost driver. Conceptually,
once Mesa supports Bifrost, if I install Debian on a MT8183 board,
everything should just work. I shouldn't need MT-specific changes / need
to change names for the DT. Regardless of which kernel driver you end up
using, minimally sharing the DT is good for everyone :-)

-Alyssa
Nicolas Boichat Sept. 18, 2019, 10:16 p.m. UTC | #5
Thanks Rob and Alyssa.

+Douglas Anderson +Dominik Behr who may be interested (if not already aware)

On Sat, Sep 14, 2019 at 2:17 AM Alyssa Rosenzweig
<alyssa.rosenzweig@collabora.com> wrote:
>
> > > > The binding we use with out-of-tree Mali drivers includes more
> > > > clocks, I assume this would be required eventually if we have an
> > > > in-tree driver:
> > >
> > > We have an in-tree driver...
> >
> > Right but AFAICT it does not support Bifrost GPU (yet?).
>
> By the time MT8183 shows up in more concrete devices, it will, certainly
> in kernel-space and likely in userspace as well. At present, the DDK can
> be modified to run on top of the in-tree Mali drivers, i.e. "Bifrost on
> mainline linux-next (+ page table/compatible patches), with blob
> userspace".
>
> While the open userspace isn't ready here quite yet, I would definitely
> encourage upstream kernel for ChromeOS, since then there's no need to
> maintain the out-of-tree GPU driver.

That's an interesting idea, I had no idea, thanks for the info!

Would that work with midgard as well? We have released hardware with
RK3288/3399, so it might be nice to experiment with these first.

>
> ---
>
> More immediately, per Rob's review, it's important that the bindings
> accepted upstream work with the in-tree Bifrost driver. Conceptually,
> once Mesa supports Bifrost, if I install Debian on a MT8183 board,
> everything should just work. I shouldn't need MT-specific changes / need
> to change names for the DT. Regardless of which kernel driver you end up
> using, minimally sharing the DT is good for everyone :-)

Yes. I'll try to dig further with MTK, but this may take some time.

>
> -Alyssa
Alyssa Rosenzweig Sept. 19, 2019, 12:32 p.m. UTC | #6
> > By the time MT8183 shows up in more concrete devices, it will, certainly
> > in kernel-space and likely in userspace as well. At present, the DDK can
> > be modified to run on top of the in-tree Mali drivers, i.e. "Bifrost on
> > mainline linux-next (+ page table/compatible patches), with blob
> > userspace".
> >
> > While the open userspace isn't ready here quite yet, I would definitely
> > encourage upstream kernel for ChromeOS, since then there's no need to
> > maintain the out-of-tree GPU driver.
> 
> That's an interesting idea, I had no idea, thanks for the info!
> 
> Would that work with midgard as well? We have released hardware with
> RK3288/3399, so it might be nice to experiment with these first.

Yes, the above would work with Midgard as well with no changes needed.
Ping Steven about thtat (CC'd).

> > More immediately, per Rob's review, it's important that the bindings
> > accepted upstream work with the in-tree Bifrost driver. Conceptually,
> > once Mesa supports Bifrost, if I install Debian on a MT8183 board,
> > everything should just work. I shouldn't need MT-specific changes / need
> > to change names for the DT. Regardless of which kernel driver you end up
> > using, minimally sharing the DT is good for everyone :-)
> 
> Yes. I'll try to dig further with MTK, but this may take some time.

Thank you!
Steven Price Sept. 23, 2019, 2:33 p.m. UTC | #7
On 19/09/2019 13:32, Alyssa Rosenzweig wrote:
>>> By the time MT8183 shows up in more concrete devices, it will, certainly
>>> in kernel-space and likely in userspace as well. At present, the DDK can
>>> be modified to run on top of the in-tree Mali drivers, i.e. "Bifrost on
>>> mainline linux-next (+ page table/compatible patches), with blob
>>> userspace".

Actually most(?) Bifrost GPUs support the "legacy" 'LPAE-ish' page table
format. So the only kernel change *required* is adding the compatible
string (and any SoC-specific quirks).

The support for "blob-on-Panfrost" is currently an experimental internal
investigation. So I can't make any promises about it ever being released
publicly.

>>> While the open userspace isn't ready here quite yet, I would definitely
>>> encourage upstream kernel for ChromeOS, since then there's no need to
>>> maintain the out-of-tree GPU driver.
>>
>> That's an interesting idea, I had no idea, thanks for the info!
>>
>> Would that work with midgard as well? We have released hardware with
>> RK3288/3399, so it might be nice to experiment with these first.
> 
> Yes, the above would work with Midgard as well with no changes needed.
> Ping Steven about thtat (CC'd).

Indeed since it's the same kernel driver the same compatibility layer
can be used to run Midgard DDK blob on Panfrost. Although given the
progress that has already occurred with the Mesa Panfrost user space you
might be able to simply switch to a completely open stack (available now).

Again I'm afraid I'm not in a position to give any guarantees that my
prototype blob-on-Panfrost work will actually be released or timescales
of when. However since the current focus internally is on newer GPUs I'm
less confident that there will be interest for Midgard DDK on Panfrost.

Steve

>>> More immediately, per Rob's review, it's important that the bindings
>>> accepted upstream work with the in-tree Bifrost driver. Conceptually,
>>> once Mesa supports Bifrost, if I install Debian on a MT8183 board,
>>> everything should just work. I shouldn't need MT-specific changes / need
>>> to change names for the DT. Regardless of which kernel driver you end up
>>> using, minimally sharing the DT is good for everyone :-)
>>
>> Yes. I'll try to dig further with MTK, but this may take some time.
> 
> Thank you!
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index 1fb195c683c3d01..200d8e65a6368a1 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -7,6 +7,7 @@ 
 
 /dts-v1/;
 #include "mt8183.dtsi"
+#include "mt6358.dtsi"
 
 / {
 	model = "MediaTek MT8183 evaluation board";
@@ -30,6 +31,12 @@ 
 	status = "okay";
 };
 
+&gpu {
+	supply-names = "mali", "mali_sram";
+	mali-supply = <&mt6358_vgpu_reg>;
+	mali_sram-supply = <&mt6358_vsram_gpu_reg>;
+};
+
 &i2c0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 97f84aa9fc6e1c1..8ea548a762ea252 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -579,6 +579,109 @@ 
 			#clock-cells = <1>;
 		};
 
+		gpu: mali@13040000 {
+			compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+			reg = <0 0x13040000 0 0x4000>;
+			interrupts =
+				<GIC_SPI 280 IRQ_TYPE_LEVEL_LOW>,
+				<GIC_SPI 279 IRQ_TYPE_LEVEL_LOW>,
+				<GIC_SPI 278 IRQ_TYPE_LEVEL_LOW>;
+			interrupt-names = "job", "mmu", "gpu";
+
+			clocks = <&topckgen CLK_TOP_MFGPLL_CK>;
+			power-domains =
+				<&scpsys MT8183_POWER_DOMAIN_MFG_CORE0>,
+				<&scpsys MT8183_POWER_DOMAIN_MFG_CORE1>,
+				<&scpsys MT8183_POWER_DOMAIN_MFG_2D>;
+
+			operating-points-v2 = <&gpu_opp_table>;
+		};
+
+		gpu_opp_table: opp_table0 {
+			compatible = "operating-points-v2";
+			opp-shared;
+
+			opp-300000000 {
+				opp-hz = /bits/ 64 <300000000>;
+				opp-microvolt = <625000>, <850000>;
+			};
+
+			opp-320000000 {
+				opp-hz = /bits/ 64 <320000000>;
+				opp-microvolt = <631250>, <850000>;
+			};
+
+			opp-340000000 {
+				opp-hz = /bits/ 64 <340000000>;
+				opp-microvolt = <637500>, <850000>;
+			};
+
+			opp-360000000 {
+				opp-hz = /bits/ 64 <360000000>;
+				opp-microvolt = <643750>, <850000>;
+			};
+
+			opp-380000000 {
+				opp-hz = /bits/ 64 <380000000>;
+				opp-microvolt = <650000>, <850000>;
+			};
+
+			opp-400000000 {
+				opp-hz = /bits/ 64 <400000000>;
+				opp-microvolt = <656250>, <850000>;
+			};
+
+			opp-420000000 {
+				opp-hz = /bits/ 64 <420000000>;
+				opp-microvolt = <662500>, <850000>;
+			};
+
+			opp-460000000 {
+				opp-hz = /bits/ 64 <460000000>;
+				opp-microvolt = <675000>, <850000>;
+			};
+
+			opp-500000000 {
+				opp-hz = /bits/ 64 <500000000>;
+				opp-microvolt = <687500>, <850000>;
+			};
+
+			opp-540000000 {
+				opp-hz = /bits/ 64 <540000000>;
+				opp-microvolt = <700000>, <850000>;
+			};
+
+			opp-580000000 {
+				opp-hz = /bits/ 64 <580000000>;
+				opp-microvolt = <712500>, <850000>;
+			};
+
+			opp-620000000 {
+				opp-hz = /bits/ 64 <620000000>;
+				opp-microvolt = <725000>, <850000>;
+			};
+
+			opp-653000000 {
+				opp-hz = /bits/ 64 <653000000>;
+				opp-microvolt = <743750>, <850000>;
+			};
+
+			opp-698000000 {
+				opp-hz = /bits/ 64 <698000000>;
+				opp-microvolt = <768750>, <868750>;
+			};
+
+			opp-743000000 {
+				opp-hz = /bits/ 64 <743000000>;
+				opp-microvolt = <793750>, <893750>;
+			};
+
+			opp-800000000 {
+				opp-hz = /bits/ 64 <800000000>;
+				opp-microvolt = <825000>, <925000>;
+			};
+		};
+
 		mmsys: syscon@14000000 {
 			compatible = "mediatek,mt8183-mmsys", "syscon";
 			reg = <0 0x14000000 0 0x1000>;