diff mbox series

[v3,3/3] arm64: dts: mediatek: mt8370: Enable gpu support

Message ID 20250207-mt8370-enable-gpu-v3-3-75e9b902f9c1@collabora.com (mailing list archive)
State New, archived
Headers show
Series Add Mali GPU support for Mediatek MT8370 SoC | expand

Commit Message

Louis-Alexis Eyraud Feb. 7, 2025, 3:18 p.m. UTC
Add a new gpu node in mt8370.dtsi to enable support for the
ARM Mali G57 MC2 GPU (Valhall-JM) found on the MT8370 SoC, using the
Panfrost driver.

On a Mediatek Genio 510 EVK board, the panfrost driver probed with the
following message:
```
panfrost 13000000.gpu: clock rate = 390000000
panfrost 13000000.gpu: mali-g57 id 0x9093 major 0x0 minor 0x0 status 0x0
panfrost 13000000.gpu: features: 00000000,000019f7, issues: 00000003,
   80000400
panfrost 13000000.gpu: Features: L2:0x08130206 Shader:0x00000000
   Tiler:0x00000809 Mem:0x1 MMU:0x00002830 AS:0xff JS:0x7
panfrost 13000000.gpu: shader_present=0x5 l2_present=0x1
[drm] Initialized panfrost 1.3.0 for 13000000.gpu on minor 0
```

Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8370.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

AngeloGioacchino Del Regno Feb. 10, 2025, 10:52 a.m. UTC | #1
Il 07/02/25 16:18, Louis-Alexis Eyraud ha scritto:
> Add a new gpu node in mt8370.dtsi to enable support for the
> ARM Mali G57 MC2 GPU (Valhall-JM) found on the MT8370 SoC, using the
> Panfrost driver.
> 
> On a Mediatek Genio 510 EVK board, the panfrost driver probed with the
> following message:
> ```
> panfrost 13000000.gpu: clock rate = 390000000
> panfrost 13000000.gpu: mali-g57 id 0x9093 major 0x0 minor 0x0 status 0x0
> panfrost 13000000.gpu: features: 00000000,000019f7, issues: 00000003,
>     80000400
> panfrost 13000000.gpu: Features: L2:0x08130206 Shader:0x00000000
>     Tiler:0x00000809 Mem:0x1 MMU:0x00002830 AS:0xff JS:0x7
> panfrost 13000000.gpu: shader_present=0x5 l2_present=0x1
> [drm] Initialized panfrost 1.3.0 for 13000000.gpu on minor 0
> ```
> 
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Krzysztof Kozlowski Feb. 11, 2025, 8:31 a.m. UTC | #2
On Fri, Feb 07, 2025 at 04:18:32PM +0100, Louis-Alexis Eyraud wrote:
> Add a new gpu node in mt8370.dtsi to enable support for the
> ARM Mali G57 MC2 GPU (Valhall-JM) found on the MT8370 SoC, using the
> Panfrost driver.
> 
> On a Mediatek Genio 510 EVK board, the panfrost driver probed with the
> following message:
> ```
> panfrost 13000000.gpu: clock rate = 390000000
> panfrost 13000000.gpu: mali-g57 id 0x9093 major 0x0 minor 0x0 status 0x0
> panfrost 13000000.gpu: features: 00000000,000019f7, issues: 00000003,
>    80000400
> panfrost 13000000.gpu: Features: L2:0x08130206 Shader:0x00000000
>    Tiler:0x00000809 Mem:0x1 MMU:0x00002830 AS:0xff JS:0x7
> panfrost 13000000.gpu: shader_present=0x5 l2_present=0x1
> [drm] Initialized panfrost 1.3.0 for 13000000.gpu on minor 0
> ```
> 
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8370.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8370.dtsi b/arch/arm64/boot/dts/mediatek/mt8370.dtsi
> index cf1a3759451ff899ce9e63e5a00f192fb483f6e5..2f27f7e7ab813b97f869297ae360f69854e966e1 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8370.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8370.dtsi
> @@ -59,6 +59,15 @@ &cpu_little3_cooling_map0 {
>  				<&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>  };
>  
> +&gpu {
> +	compatible = "mediatek,mt8370-mali", "arm,mali-valhall-jm";

It's up to platform maintainers, but IMHO this is discouraged practice.
If you ever need to override compatible, this means the node is not
really shared between this and base SoC (base DTSI).

Best regards,
Krzysztof
AngeloGioacchino Del Regno Feb. 11, 2025, 9:28 a.m. UTC | #3
Il 11/02/25 09:31, Krzysztof Kozlowski ha scritto:
> On Fri, Feb 07, 2025 at 04:18:32PM +0100, Louis-Alexis Eyraud wrote:
>> Add a new gpu node in mt8370.dtsi to enable support for the
>> ARM Mali G57 MC2 GPU (Valhall-JM) found on the MT8370 SoC, using the
>> Panfrost driver.
>>
>> On a Mediatek Genio 510 EVK board, the panfrost driver probed with the
>> following message:
>> ```
>> panfrost 13000000.gpu: clock rate = 390000000
>> panfrost 13000000.gpu: mali-g57 id 0x9093 major 0x0 minor 0x0 status 0x0
>> panfrost 13000000.gpu: features: 00000000,000019f7, issues: 00000003,
>>     80000400
>> panfrost 13000000.gpu: Features: L2:0x08130206 Shader:0x00000000
>>     Tiler:0x00000809 Mem:0x1 MMU:0x00002830 AS:0xff JS:0x7
>> panfrost 13000000.gpu: shader_present=0x5 l2_present=0x1
>> [drm] Initialized panfrost 1.3.0 for 13000000.gpu on minor 0
>> ```
>>
>> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
>> ---
>>   arch/arm64/boot/dts/mediatek/mt8370.dtsi | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8370.dtsi b/arch/arm64/boot/dts/mediatek/mt8370.dtsi
>> index cf1a3759451ff899ce9e63e5a00f192fb483f6e5..2f27f7e7ab813b97f869297ae360f69854e966e1 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8370.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8370.dtsi
>> @@ -59,6 +59,15 @@ &cpu_little3_cooling_map0 {
>>   				<&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>   };
>>   
>> +&gpu {
>> +	compatible = "mediatek,mt8370-mali", "arm,mali-valhall-jm";
> 
> It's up to platform maintainers, but IMHO this is discouraged practice.
> If you ever need to override compatible, this means the node is not
> really shared between this and base SoC (base DTSI).
> 

That's true, indeed, but this is a special case, where the GPU actually is really
architecturally and generationally the same, difference being that one core is
lasered off from the lower binned silicon.

I appreciate you pointing that out, and effectively we shall not create any
misunderstanding on this practice, which shall remain discouraged.

Speaking of which!

Louis, since you anyway have to send a v4, please add a comment before that
gpu node override saying:

/*
  * Please note that overriding compatibles is a discouraged practice and is a
  * clear indication of nodes not being, well, compatible!
  *
  * This is a special case, where the GPU is the same as MT8188, but with one
  * of the cores fused out in this lower-binned SoC.
  */
&gpu {
  ....etc


Thanks,
Angelo
Louis-Alexis Eyraud Feb. 11, 2025, 10:41 a.m. UTC | #4
On Tue, 2025-02-11 at 10:28 +0100, AngeloGioacchino Del Regno wrote:
> Il 11/02/25 09:31, Krzysztof Kozlowski ha scritto:
> > On Fri, Feb 07, 2025 at 04:18:32PM +0100, Louis-Alexis Eyraud
> > wrote:
> > > Add a new gpu node in mt8370.dtsi to enable support for the
> > > ARM Mali G57 MC2 GPU (Valhall-JM) found on the MT8370 SoC, using
> > > the
> > > Panfrost driver.
> > > 
> > > On a Mediatek Genio 510 EVK board, the panfrost driver probed
> > > with the
> > > following message:
> > > ```
> > > panfrost 13000000.gpu: clock rate = 390000000
> > > panfrost 13000000.gpu: mali-g57 id 0x9093 major 0x0 minor 0x0
> > > status 0x0
> > > panfrost 13000000.gpu: features: 00000000,000019f7, issues:
> > > 00000003,
> > >     80000400
> > > panfrost 13000000.gpu: Features: L2:0x08130206 Shader:0x00000000
> > >     Tiler:0x00000809 Mem:0x1 MMU:0x00002830 AS:0xff JS:0x7
> > > panfrost 13000000.gpu: shader_present=0x5 l2_present=0x1
> > > [drm] Initialized panfrost 1.3.0 for 13000000.gpu on minor 0
> > > ```
> > > 
> > > Signed-off-by: Louis-Alexis Eyraud
> > > <louisalexis.eyraud@collabora.com>
> > > ---
> > >   arch/arm64/boot/dts/mediatek/mt8370.dtsi | 9 +++++++++
> > >   1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/mediatek/mt8370.dtsi
> > > b/arch/arm64/boot/dts/mediatek/mt8370.dtsi
> > > index
> > > cf1a3759451ff899ce9e63e5a00f192fb483f6e5..2f27f7e7ab813b97f869297
> > > ae360f69854e966e1 100644
> > > --- a/arch/arm64/boot/dts/mediatek/mt8370.dtsi
> > > +++ b/arch/arm64/boot/dts/mediatek/mt8370.dtsi
> > > @@ -59,6 +59,15 @@ &cpu_little3_cooling_map0 {
> > >   				<&cpu3 THERMAL_NO_LIMIT
> > > THERMAL_NO_LIMIT>;
> > >   };
> > >   
> > > +&gpu {
> > > +	compatible = "mediatek,mt8370-mali", "arm,mali-valhall-
> > > jm";
> > 
> > It's up to platform maintainers, but IMHO this is discouraged
> > practice.
> > If you ever need to override compatible, this means the node is not
> > really shared between this and base SoC (base DTSI).
> > 
> 
> That's true, indeed, but this is a special case, where the GPU
> actually is really
> architecturally and generationally the same, difference being that
> one core is
> lasered off from the lower binned silicon.
> 
> I appreciate you pointing that out, and effectively we shall not
> create any
> misunderstanding on this practice, which shall remain discouraged.
> 
> Speaking of which!
> 
> Louis, since you anyway have to send a v4, please add a comment
> before that
> gpu node override saying:
> 
> /*
>   * Please note that overriding compatibles is a discouraged practice
> and is a
>   * clear indication of nodes not being, well, compatible!
>   *
>   * This is a special case, where the GPU is the same as MT8188, but
> with one
>   * of the cores fused out in this lower-binned SoC.
>   */
> &gpu {
>   ....etc
> 
> 
> Thanks,
> Angelo

Hi,

I understand your concerns and I agree with you.

Adding this warning comment in mt8370.dtsi file seems appropriate to
explain why it was done this way, and make more cautious those who
might read this override code and inspire from it.

I'll amend this commit with it in the v4 patchset.

Regards,
Louis-Alexis
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt8370.dtsi b/arch/arm64/boot/dts/mediatek/mt8370.dtsi
index cf1a3759451ff899ce9e63e5a00f192fb483f6e5..2f27f7e7ab813b97f869297ae360f69854e966e1 100644
--- a/arch/arm64/boot/dts/mediatek/mt8370.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8370.dtsi
@@ -59,6 +59,15 @@  &cpu_little3_cooling_map0 {
 				<&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
 };
 
+&gpu {
+	compatible = "mediatek,mt8370-mali", "arm,mali-valhall-jm";
+
+	power-domains = <&spm MT8188_POWER_DOMAIN_MFG2>,
+			<&spm MT8188_POWER_DOMAIN_MFG3>;
+
+	power-domain-names = "core0", "core1";
+};
+
 &ppi_cluster0 {
 	affinity = <&cpu0 &cpu1 &cpu2 &cpu3>;
 };