Re: [PATCH] arm64: dts: mt8183: Add node for the Mali GPU
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-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
Re: [PATCH] arm64: dts: mt8183: Add node for the Mali GPU
> > 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! signature.asc Description: PGP signature
Re: [PATCH] arm64: dts: mt8183: Add node for the Mali GPU
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 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
Re: [PATCH] arm64: dts: mt8183: Add node for the Mali GPU
> > > 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 signature.asc Description: PGP signature
Re: [PATCH] arm64: dts: mt8183: Add node for the Mali GPU
On Thu, Sep 5, 2019 at 10:49 AM Nicolas Boichat wrote:
>
> Thanks for the quick review!
>
> On Thu, Sep 5, 2019 at 5:09 PM Rob Herring wrote:
> >
> > On Thu, Sep 5, 2019 at 9:16 AM Nicolas Boichat
> > 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 =
> > > < CLK_TOP_MFGPLL_CK>,
> > > < CLK_TOP_MUX_MFG>,
> > > <>,
> > > < 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
> > >
> > > ---
> > > 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";
> > > };
> > >
> > > + {
> > > + supply-names = "mali", "mali_sram";
> > > + mali-supply = <_vgpu_reg>;
> > > + mali_sram-supply = <_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.
>
> >
> > > +};
> > > +
> > > {
> > > pinctrl-names = "default";
> > > pinctrl-0 = <_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@1304 {
> >
> > gpu@...
> >
> > > + compatible = "mediatek,mt8183-mali",
> > > "arm,mali-bifrost";
> >
> > You need to add this compatible string too.
>
> Will do.
>
> >
> > > + reg = <0 0x1304 0 0x4000>;
> > > + interrupts =
> > > + ,
> > > + ,
> > > + ;
> > > + interrupt-names = "job", "mmu", "gpu";
> > > +
> > > + clocks = < CLK_TOP_MFGPLL_CK>;
> > > + power-domains =
> > > + < MT8183_POWER_DOMAIN_MFG_CORE0>,
> > > + < MT8183_POWER_DOMAIN_MFG_CORE1>,
> > > + < 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/
Re: [PATCH] arm64: dts: mt8183: Add node for the Mali GPU
Thanks for the quick review!
On Thu, Sep 5, 2019 at 5:09 PM Rob Herring wrote:
>
> On Thu, Sep 5, 2019 at 9:16 AM Nicolas Boichat 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 =
> > < CLK_TOP_MFGPLL_CK>,
> > < CLK_TOP_MUX_MFG>,
> > <>,
> > < 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
> >
> > ---
> > 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";
> > };
> >
> > + {
> > + supply-names = "mali", "mali_sram";
> > + mali-supply = <_vgpu_reg>;
> > + mali_sram-supply = <_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.
>
> > +};
> > +
> > {
> > pinctrl-names = "default";
> > pinctrl-0 = <_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@1304 {
>
> gpu@...
>
> > + compatible = "mediatek,mt8183-mali",
> > "arm,mali-bifrost";
>
> You need to add this compatible string too.
Will do.
>
> > + reg = <0 0x1304 0 0x4000>;
> > + interrupts =
> > + ,
> > + ,
> > + ;
> > + interrupt-names = "job", "mmu", "gpu";
> > +
> > + clocks = < CLK_TOP_MFGPLL_CK>;
> > + power-domains =
> > + < MT8183_POWER_DOMAIN_MFG_CORE0>,
> > + < MT8183_POWER_DOMAIN_MFG_CORE1>,
> > + < 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 = <_opp_table>;
> > + };
> > +
> > + gpu_opp_table: opp_table0 {
> > + compatible = "operating-points-v2";
> > + opp-shared;
> > +
> > + opp-3 {
> > + opp-hz = /bits/ 64 <3>;
> > + opp-microvolt = <625000>, <85>;
> > + };
> > +
> > + opp-32000 {
> > + opp-hz = /bits/ 64 <32000>;
> > + opp-microvolt = <631250>, <85>;
> > + };
> > +
> > + opp-34000 {
> > + opp-hz = /bits/ 64 <34000>;
> > + opp-microvolt = <637500>, <85>;
> > + };
> > +
> > + opp-36000 {
> > + opp-hz = /bits/ 64 <36000>;
> > + opp-microvolt = <643750>, <85>;
> > + };
> > +
> > + opp-38000 {
> > + opp-hz = /bits/ 64 <38000>;
> > +
Re: [PATCH] arm64: dts: mt8183: Add node for the Mali GPU
On Thu, Sep 5, 2019 at 9:16 AM Nicolas Boichat 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 =
> < CLK_TOP_MFGPLL_CK>,
> < CLK_TOP_MUX_MFG>,
> <>,
> < CLK_MFG_BG3D>;
> clock-names =
> "clk_main_parent",
> "clk_mux",
> "clk_sub_parent",
> "subsys_mfg_cg";
>
> Signed-off-by: Nicolas Boichat
>
> ---
> 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";
> };
>
> + {
> + supply-names = "mali", "mali_sram";
> + mali-supply = <_vgpu_reg>;
> + mali_sram-supply = <_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.
> +};
> +
> {
> pinctrl-names = "default";
> pinctrl-0 = <_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@1304 {
gpu@...
> + compatible = "mediatek,mt8183-mali",
> "arm,mali-bifrost";
You need to add this compatible string too.
> + reg = <0 0x1304 0 0x4000>;
> + interrupts =
> + ,
> + ,
> + ;
> + interrupt-names = "job", "mmu", "gpu";
> +
> + clocks = < CLK_TOP_MFGPLL_CK>;
> + power-domains =
> + < MT8183_POWER_DOMAIN_MFG_CORE0>,
> + < MT8183_POWER_DOMAIN_MFG_CORE1>,
> + < MT8183_POWER_DOMAIN_MFG_2D>;
This needs to be documented too.
> +
> + operating-points-v2 = <_opp_table>;
> + };
> +
> + gpu_opp_table: opp_table0 {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-3 {
> + opp-hz = /bits/ 64 <3>;
> + opp-microvolt = <625000>, <85>;
> + };
> +
> + opp-32000 {
> + opp-hz = /bits/ 64 <32000>;
> + opp-microvolt = <631250>, <85>;
> + };
> +
> + opp-34000 {
> + opp-hz = /bits/ 64 <34000>;
> + opp-microvolt = <637500>, <85>;
> + };
> +
> + opp-36000 {
> + opp-hz = /bits/ 64 <36000>;
> + opp-microvolt = <643750>, <85>;
> + };
> +
> + opp-38000 {
> + opp-hz = /bits/ 64 <38000>;
> + opp-microvolt = <65>, <85>;
> + };
> +
> + opp-4 {
> + opp-hz = /bits/ 64 <4>;
> + opp-microvolt = <656250>, <85>;
> + };
> +
> + opp-42000 {
> + opp-hz = /bits/ 64 <42000>;
> + opp-microvolt = <662500>, <85>;
> + };
> +
> + opp-46000 {
> + opp-hz = /bits/ 64 <46000>;
> + opp-microvolt = <675000>, <85>;
> + };
> +
> + opp-5 {
> +
