The Mali-based GPU on the MediaTek MT8196 SoC uses a separate MCU to
control the power and frequency of the GPU. This is modelled as a power
domain and clock provider.
It lets us omit the OPP tables from the device tree, as those can now be
enumerated at runtime from the MCU.
Add the necessary schema logic to handle what this SoC expects in terms
of clocks and power-domains.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
.../bindings/gpu/arm,mali-valhall-csf.yaml | 37 +++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
index 613040fdb444..860691ce985e 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
@@ -45,7 +45,9 @@ properties:
minItems: 1
items:
- const: core
- - const: coregroup
+ - enum:
+ - coregroup
+ - stacks
- const: stacks
mali-supply: true
@@ -110,6 +112,27 @@ allOf:
power-domain-names: false
required:
- mali-supply
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt8196-mali
+ then:
+ properties:
+ mali-supply: false
+ sram-supply: false
+ operating-points-v2: false
+ power-domains:
+ maxItems: 1
+ power-domain-names: false
+ clocks:
+ maxItems: 2
+ clock-names:
+ items:
+ - const: core
+ - const: stacks
+ required:
+ - power-domains
examples:
- |
@@ -145,5 +168,17 @@ examples:
};
};
};
+ - |
+ gpu@48000000 {
+ compatible = "mediatek,mt8196-mali", "arm,mali-valhall-csf";
+ reg = <0x48000000 0x480000>;
+ clocks = <&gpufreq 0>, <&gpufreq 1>;
+ clock-names = "core", "stacks";
+ interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH 0>;
+ interrupt-names = "job", "mmu", "gpu";
+ power-domains = <&gpufreq>;
+ };
...
--
2.51.0On Fri, Oct 17, 2025 at 05:31:08PM +0200, Nicolas Frattaroli wrote:
> The Mali-based GPU on the MediaTek MT8196 SoC uses a separate MCU to
> control the power and frequency of the GPU. This is modelled as a power
> domain and clock provider.
>
> It lets us omit the OPP tables from the device tree, as those can now be
> enumerated at runtime from the MCU.
>
> Add the necessary schema logic to handle what this SoC expects in terms
> of clocks and power-domains.
>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
> .../bindings/gpu/arm,mali-valhall-csf.yaml | 37 +++++++++++++++++++++-
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> index 613040fdb444..860691ce985e 100644
> --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> @@ -45,7 +45,9 @@ properties:
> minItems: 1
> items:
> - const: core
> - - const: coregroup
> + - enum:
> + - coregroup
> + - stacks
> - const: stacks
I'm not sure how to parse this part of the change. We're overwriting the property
for mt8196-mali anyway so why do we need this? And if we do, should 'stacks'
still remain as a const?
Best regards,
Liviu
>
> mali-supply: true
> @@ -110,6 +112,27 @@ allOf:
> power-domain-names: false
> required:
> - mali-supply
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mediatek,mt8196-mali
> + then:
> + properties:
> + mali-supply: false
> + sram-supply: false
> + operating-points-v2: false
> + power-domains:
> + maxItems: 1
> + power-domain-names: false
> + clocks:
> + maxItems: 2
> + clock-names:
> + items:
> + - const: core
> + - const: stacks
> + required:
> + - power-domains
>
> examples:
> - |
> @@ -145,5 +168,17 @@ examples:
> };
> };
> };
> + - |
> + gpu@48000000 {
> + compatible = "mediatek,mt8196-mali", "arm,mali-valhall-csf";
> + reg = <0x48000000 0x480000>;
> + clocks = <&gpufreq 0>, <&gpufreq 1>;
> + clock-names = "core", "stacks";
> + interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH 0>;
> + interrupt-names = "job", "mmu", "gpu";
> + power-domains = <&gpufreq>;
> + };
>
> ...
>
> --
> 2.51.0
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
On Tuesday, 28 October 2025 18:12:35 Central European Standard Time Liviu Dudau wrote:
> On Fri, Oct 17, 2025 at 05:31:08PM +0200, Nicolas Frattaroli wrote:
> > The Mali-based GPU on the MediaTek MT8196 SoC uses a separate MCU to
> > control the power and frequency of the GPU. This is modelled as a power
> > domain and clock provider.
> >
> > It lets us omit the OPP tables from the device tree, as those can now be
> > enumerated at runtime from the MCU.
> >
> > Add the necessary schema logic to handle what this SoC expects in terms
> > of clocks and power-domains.
> >
> > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> > .../bindings/gpu/arm,mali-valhall-csf.yaml | 37 +++++++++++++++++++++-
> > 1 file changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > index 613040fdb444..860691ce985e 100644
> > --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > @@ -45,7 +45,9 @@ properties:
> > minItems: 1
> > items:
> > - const: core
> > - - const: coregroup
> > + - enum:
> > + - coregroup
> > + - stacks
> > - const: stacks
>
> I'm not sure how to parse this part of the change. We're overwriting the property
> for mt8196-mali anyway so why do we need this? And if we do, should 'stacks'
> still remain as a const?
The properties section outside of the if branches outside here
specifies a pattern of properties that matches for all devices.
In this case, I changed it so that the second clock-names item
may either be "coregroup" or "stacks". Yes, the third "stacks"
remains, though if you wanted to be extra precise you could
then specify in the non-MT8196 cases that we should not have
stacks followed by stacks, but I'd wager some checker for
duplicate names may already catch that.
However, I don't think it's a big enough deal to reroll this
series again.
Kind regards,
Nicolas Frattaroli
>
> Best regards,
> Liviu
>
> >
> > mali-supply: true
> > @@ -110,6 +112,27 @@ allOf:
> > power-domain-names: false
> > required:
> > - mali-supply
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: mediatek,mt8196-mali
> > + then:
> > + properties:
> > + mali-supply: false
> > + sram-supply: false
> > + operating-points-v2: false
> > + power-domains:
> > + maxItems: 1
> > + power-domain-names: false
> > + clocks:
> > + maxItems: 2
> > + clock-names:
> > + items:
> > + - const: core
> > + - const: stacks
> > + required:
> > + - power-domains
> >
> > examples:
> > - |
> > @@ -145,5 +168,17 @@ examples:
> > };
> > };
> > };
> > + - |
> > + gpu@48000000 {
> > + compatible = "mediatek,mt8196-mali", "arm,mali-valhall-csf";
> > + reg = <0x48000000 0x480000>;
> > + clocks = <&gpufreq 0>, <&gpufreq 1>;
> > + clock-names = "core", "stacks";
> > + interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH 0>,
> > + <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH 0>,
> > + <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH 0>;
> > + interrupt-names = "job", "mmu", "gpu";
> > + power-domains = <&gpufreq>;
> > + };
> >
> > ...
> >
>
>
On Tue, Oct 28, 2025 at 09:51:43PM +0100, Nicolas Frattaroli wrote:
> On Tuesday, 28 October 2025 18:12:35 Central European Standard Time Liviu Dudau wrote:
> > On Fri, Oct 17, 2025 at 05:31:08PM +0200, Nicolas Frattaroli wrote:
> > > The Mali-based GPU on the MediaTek MT8196 SoC uses a separate MCU to
> > > control the power and frequency of the GPU. This is modelled as a power
> > > domain and clock provider.
> > >
> > > It lets us omit the OPP tables from the device tree, as those can now be
> > > enumerated at runtime from the MCU.
> > >
> > > Add the necessary schema logic to handle what this SoC expects in terms
> > > of clocks and power-domains.
> > >
> > > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > ---
> > > .../bindings/gpu/arm,mali-valhall-csf.yaml | 37 +++++++++++++++++++++-
> > > 1 file changed, 36 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > index 613040fdb444..860691ce985e 100644
> > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > @@ -45,7 +45,9 @@ properties:
> > > minItems: 1
> > > items:
> > > - const: core
> > > - - const: coregroup
> > > + - enum:
> > > + - coregroup
> > > + - stacks
> > > - const: stacks
> >
> > I'm not sure how to parse this part of the change. We're overwriting the property
> > for mt8196-mali anyway so why do we need this? And if we do, should 'stacks'
> > still remain as a const?
>
> The properties section outside of the if branches outside here
> specifies a pattern of properties that matches for all devices.
>
> In this case, I changed it so that the second clock-names item
> may either be "coregroup" or "stacks".
Why would we want to do that for non-MT8196 devices? It doesn't make sense to me.
The overwrite in the if branch should be enough to give you want you want (i.e.
core followed by stacks and only that).
> Yes, the third "stacks"
> remains, though if you wanted to be extra precise you could
> then specify in the non-MT8196 cases that we should not have
> stacks followed by stacks, but I'd wager some checker for
> duplicate names may already catch that.
>
> However, I don't think it's a big enough deal to reroll this
> series again.
I'm not asking you to re-roll the series but if you agree to drop that
part I can make the edit when merging it.
Best regards,
Liviu
>
> Kind regards,
> Nicolas Frattaroli
>
> >
> > Best regards,
> > Liviu
> >
> > >
> > > mali-supply: true
> > > @@ -110,6 +112,27 @@ allOf:
> > > power-domain-names: false
> > > required:
> > > - mali-supply
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: mediatek,mt8196-mali
> > > + then:
> > > + properties:
> > > + mali-supply: false
> > > + sram-supply: false
> > > + operating-points-v2: false
> > > + power-domains:
> > > + maxItems: 1
> > > + power-domain-names: false
> > > + clocks:
> > > + maxItems: 2
> > > + clock-names:
> > > + items:
> > > + - const: core
> > > + - const: stacks
> > > + required:
> > > + - power-domains
> > >
> > > examples:
> > > - |
> > > @@ -145,5 +168,17 @@ examples:
> > > };
> > > };
> > > };
> > > + - |
> > > + gpu@48000000 {
> > > + compatible = "mediatek,mt8196-mali", "arm,mali-valhall-csf";
> > > + reg = <0x48000000 0x480000>;
> > > + clocks = <&gpufreq 0>, <&gpufreq 1>;
> > > + clock-names = "core", "stacks";
> > > + interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH 0>,
> > > + <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH 0>,
> > > + <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH 0>;
> > > + interrupt-names = "job", "mmu", "gpu";
> > > + power-domains = <&gpufreq>;
> > > + };
> > >
> > > ...
> > >
> >
> >
>
>
>
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
On Wednesday, 29 October 2025 02:04:42 Central European Standard Time Liviu Dudau wrote:
> On Tue, Oct 28, 2025 at 09:51:43PM +0100, Nicolas Frattaroli wrote:
> > On Tuesday, 28 October 2025 18:12:35 Central European Standard Time Liviu Dudau wrote:
> > > On Fri, Oct 17, 2025 at 05:31:08PM +0200, Nicolas Frattaroli wrote:
> > > > The Mali-based GPU on the MediaTek MT8196 SoC uses a separate MCU to
> > > > control the power and frequency of the GPU. This is modelled as a power
> > > > domain and clock provider.
> > > >
> > > > It lets us omit the OPP tables from the device tree, as those can now be
> > > > enumerated at runtime from the MCU.
> > > >
> > > > Add the necessary schema logic to handle what this SoC expects in terms
> > > > of clocks and power-domains.
> > > >
> > > > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> > > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > > ---
> > > > .../bindings/gpu/arm,mali-valhall-csf.yaml | 37 +++++++++++++++++++++-
> > > > 1 file changed, 36 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > > index 613040fdb444..860691ce985e 100644
> > > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > > @@ -45,7 +45,9 @@ properties:
> > > > minItems: 1
> > > > items:
> > > > - const: core
> > > > - - const: coregroup
> > > > + - enum:
> > > > + - coregroup
> > > > + - stacks
> > > > - const: stacks
> > >
> > > I'm not sure how to parse this part of the change. We're overwriting the property
> > > for mt8196-mali anyway so why do we need this? And if we do, should 'stacks'
> > > still remain as a const?
> >
> > The properties section outside of the if branches outside here
> > specifies a pattern of properties that matches for all devices.
> >
> > In this case, I changed it so that the second clock-names item
> > may either be "coregroup" or "stacks".
>
> Why would we want to do that for non-MT8196 devices? It doesn't make sense to me.
> The overwrite in the if branch should be enough to give you want you want (i.e.
> core followed by stacks and only that).
I built my understanding of why on the same reason of why we specify
a minItems of 1 but require it to be 3 in the if branch of the only
other compatible (rk3588): it describes what may be found in those
properties, not what is required by the specific compatible preceding
the generic valhall compatible. arm,mali-valhall-csf is currently
not described as a compatible that's allowed to appear stand-alone
without some other compatible before it to specify further which SoC
it's on, so it really just is whatever RK3588 needs vs. whatever
MT8196 needs at the moment.
Arguably though, there's no functional difference here, and I'm not
aware on any rules regarding this. My change may be problematic
however, because of the whole double stacks thing.
> > Yes, the third "stacks"
> > remains, though if you wanted to be extra precise you could
> > then specify in the non-MT8196 cases that we should not have
> > stacks followed by stacks, but I'd wager some checker for
> > duplicate names may already catch that.
> >
> > However, I don't think it's a big enough deal to reroll this
> > series again.
>
> I'm not asking you to re-roll the series but if you agree to drop that
> part I can make the edit when merging it.
If the other DT maintainers (especially Rob who gave it his R-b)
are okay with dropping it, then yes please do.
Kind regards,
Nicolas Frattaroli
>
> Best regards,
> Liviu
>
> >
> > Kind regards,
> > Nicolas Frattaroli
> >
> > >
> > > Best regards,
> > > Liviu
> > >
> > > >
> > > > mali-supply: true
> > > > @@ -110,6 +112,27 @@ allOf:
> > > > power-domain-names: false
> > > > required:
> > > > - mali-supply
> > > > + - if:
> > > > + properties:
> > > > + compatible:
> > > > + contains:
> > > > + const: mediatek,mt8196-mali
> > > > + then:
> > > > + properties:
> > > > + mali-supply: false
> > > > + sram-supply: false
> > > > + operating-points-v2: false
> > > > + power-domains:
> > > > + maxItems: 1
> > > > + power-domain-names: false
> > > > + clocks:
> > > > + maxItems: 2
> > > > + clock-names:
> > > > + items:
> > > > + - const: core
> > > > + - const: stacks
> > > > + required:
> > > > + - power-domains
> > > >
> > > > examples:
> > > > - |
> > > > @@ -145,5 +168,17 @@ examples:
> > > > };
> > > > };
> > > > };
> > > > + - |
> > > > + gpu@48000000 {
> > > > + compatible = "mediatek,mt8196-mali", "arm,mali-valhall-csf";
> > > > + reg = <0x48000000 0x480000>;
> > > > + clocks = <&gpufreq 0>, <&gpufreq 1>;
> > > > + clock-names = "core", "stacks";
> > > > + interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > + <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > + <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH 0>;
> > > > + interrupt-names = "job", "mmu", "gpu";
> > > > + power-domains = <&gpufreq>;
> > > > + };
> > > >
> > > > ...
> > > >
> > >
> > >
> >
> >
> >
> >
>
>
On Wed, Oct 29, 2025 at 02:42:35PM +0100, Nicolas Frattaroli wrote:
> On Wednesday, 29 October 2025 02:04:42 Central European Standard Time Liviu Dudau wrote:
> > On Tue, Oct 28, 2025 at 09:51:43PM +0100, Nicolas Frattaroli wrote:
> > > On Tuesday, 28 October 2025 18:12:35 Central European Standard Time Liviu Dudau wrote:
> > > > On Fri, Oct 17, 2025 at 05:31:08PM +0200, Nicolas Frattaroli wrote:
> > > > > The Mali-based GPU on the MediaTek MT8196 SoC uses a separate MCU to
> > > > > control the power and frequency of the GPU. This is modelled as a power
> > > > > domain and clock provider.
> > > > >
> > > > > It lets us omit the OPP tables from the device tree, as those can now be
> > > > > enumerated at runtime from the MCU.
> > > > >
> > > > > Add the necessary schema logic to handle what this SoC expects in terms
> > > > > of clocks and power-domains.
> > > > >
> > > > > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> > > > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > > > ---
> > > > > .../bindings/gpu/arm,mali-valhall-csf.yaml | 37 +++++++++++++++++++++-
> > > > > 1 file changed, 36 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > > > index 613040fdb444..860691ce985e 100644
> > > > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > > > @@ -45,7 +45,9 @@ properties:
> > > > > minItems: 1
> > > > > items:
> > > > > - const: core
> > > > > - - const: coregroup
> > > > > + - enum:
> > > > > + - coregroup
> > > > > + - stacks
> > > > > - const: stacks
> > > >
> > > > I'm not sure how to parse this part of the change. We're overwriting the property
> > > > for mt8196-mali anyway so why do we need this? And if we do, should 'stacks'
> > > > still remain as a const?
> > >
> > > The properties section outside of the if branches outside here
> > > specifies a pattern of properties that matches for all devices.
> > >
> > > In this case, I changed it so that the second clock-names item
> > > may either be "coregroup" or "stacks".
> >
> > Why would we want to do that for non-MT8196 devices? It doesn't make sense to me.
> > The overwrite in the if branch should be enough to give you want you want (i.e.
> > core followed by stacks and only that).
>
> I built my understanding of why on the same reason of why we specify
> a minItems of 1 but require it to be 3 in the if branch of the only
> other compatible (rk3588): it describes what may be found in those
> properties, not what is required by the specific compatible preceding
> the generic valhall compatible. arm,mali-valhall-csf is currently
> not described as a compatible that's allowed to appear stand-alone
> without some other compatible before it to specify further which SoC
> it's on, so it really just is whatever RK3588 needs vs. whatever
> MT8196 needs at the moment.
>
> Arguably though, there's no functional difference here, and I'm not
> aware on any rules regarding this. My change may be problematic
> however, because of the whole double stacks thing.
I think I'm saying the same thing. The "arm,mali-valhall-csf" is the most general
compatible string and defines the common denominator if not overwritten. I'm
not expecting anyone to use just that string for a compatible, but downstream
we have additional compatible strings that don't have to update the schema at all.
rk3588 has a specific setup that requires 3 clocks so you cannot have any optional,
that's why it is overwriting the minItems. Your whole double stack thing is
actually not needed if all you do is overwrite in the MT8196 case the clock
names and maxItems to only need two clocks.
>
> > > Yes, the third "stacks"
> > > remains, though if you wanted to be extra precise you could
> > > then specify in the non-MT8196 cases that we should not have
> > > stacks followed by stacks, but I'd wager some checker for
> > > duplicate names may already catch that.
> > >
> > > However, I don't think it's a big enough deal to reroll this
> > > series again.
> >
> > I'm not asking you to re-roll the series but if you agree to drop that
> > part I can make the edit when merging it.
>
> If the other DT maintainers (especially Rob who gave it his R-b)
> are okay with dropping it, then yes please do.
Rob, do you agree with dropping the change in the generic bindings?
Best regards,
Liviu
>
> Kind regards,
> Nicolas Frattaroli
>
> >
> > Best regards,
> > Liviu
> >
> > >
> > > Kind regards,
> > > Nicolas Frattaroli
> > >
> > > >
> > > > Best regards,
> > > > Liviu
> > > >
> > > > >
> > > > > mali-supply: true
> > > > > @@ -110,6 +112,27 @@ allOf:
> > > > > power-domain-names: false
> > > > > required:
> > > > > - mali-supply
> > > > > + - if:
> > > > > + properties:
> > > > > + compatible:
> > > > > + contains:
> > > > > + const: mediatek,mt8196-mali
> > > > > + then:
> > > > > + properties:
> > > > > + mali-supply: false
> > > > > + sram-supply: false
> > > > > + operating-points-v2: false
> > > > > + power-domains:
> > > > > + maxItems: 1
> > > > > + power-domain-names: false
> > > > > + clocks:
> > > > > + maxItems: 2
> > > > > + clock-names:
> > > > > + items:
> > > > > + - const: core
> > > > > + - const: stacks
> > > > > + required:
> > > > > + - power-domains
> > > > >
> > > > > examples:
> > > > > - |
> > > > > @@ -145,5 +168,17 @@ examples:
> > > > > };
> > > > > };
> > > > > };
> > > > > + - |
> > > > > + gpu@48000000 {
> > > > > + compatible = "mediatek,mt8196-mali", "arm,mali-valhall-csf";
> > > > > + reg = <0x48000000 0x480000>;
> > > > > + clocks = <&gpufreq 0>, <&gpufreq 1>;
> > > > > + clock-names = "core", "stacks";
> > > > > + interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > > + <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > > + <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH 0>;
> > > > > + interrupt-names = "job", "mmu", "gpu";
> > > > > + power-domains = <&gpufreq>;
> > > > > + };
> > > > >
> > > > > ...
> > > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> >
> >
>
>
>
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
On Wed, Oct 29, 2025 at 01:56:14PM +0000, Liviu Dudau wrote:
> On Wed, Oct 29, 2025 at 02:42:35PM +0100, Nicolas Frattaroli wrote:
> > On Wednesday, 29 October 2025 02:04:42 Central European Standard Time Liviu Dudau wrote:
> > > On Tue, Oct 28, 2025 at 09:51:43PM +0100, Nicolas Frattaroli wrote:
> > > > On Tuesday, 28 October 2025 18:12:35 Central European Standard Time Liviu Dudau wrote:
> > > > > On Fri, Oct 17, 2025 at 05:31:08PM +0200, Nicolas Frattaroli wrote:
> > > > > > The Mali-based GPU on the MediaTek MT8196 SoC uses a separate MCU to
> > > > > > control the power and frequency of the GPU. This is modelled as a power
> > > > > > domain and clock provider.
> > > > > >
> > > > > > It lets us omit the OPP tables from the device tree, as those can now be
> > > > > > enumerated at runtime from the MCU.
> > > > > >
> > > > > > Add the necessary schema logic to handle what this SoC expects in terms
> > > > > > of clocks and power-domains.
> > > > > >
> > > > > > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> > > > > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > > > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > > > > ---
> > > > > > .../bindings/gpu/arm,mali-valhall-csf.yaml | 37 +++++++++++++++++++++-
> > > > > > 1 file changed, 36 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > > > > index 613040fdb444..860691ce985e 100644
> > > > > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > > > > @@ -45,7 +45,9 @@ properties:
> > > > > > minItems: 1
> > > > > > items:
> > > > > > - const: core
> > > > > > - - const: coregroup
> > > > > > + - enum:
> > > > > > + - coregroup
> > > > > > + - stacks
> > > > > > - const: stacks
> > > > >
> > > > > I'm not sure how to parse this part of the change. We're overwriting the property
> > > > > for mt8196-mali anyway so why do we need this? And if we do, should 'stacks'
> > > > > still remain as a const?
> > > >
> > > > The properties section outside of the if branches outside here
> > > > specifies a pattern of properties that matches for all devices.
> > > >
> > > > In this case, I changed it so that the second clock-names item
> > > > may either be "coregroup" or "stacks".
> > >
> > > Why would we want to do that for non-MT8196 devices? It doesn't make sense to me.
> > > The overwrite in the if branch should be enough to give you want you want (i.e.
> > > core followed by stacks and only that).
> >
> > I built my understanding of why on the same reason of why we specify
> > a minItems of 1 but require it to be 3 in the if branch of the only
> > other compatible (rk3588): it describes what may be found in those
> > properties, not what is required by the specific compatible preceding
> > the generic valhall compatible. arm,mali-valhall-csf is currently
> > not described as a compatible that's allowed to appear stand-alone
> > without some other compatible before it to specify further which SoC
> > it's on, so it really just is whatever RK3588 needs vs. whatever
> > MT8196 needs at the moment.
> >
> > Arguably though, there's no functional difference here, and I'm not
> > aware on any rules regarding this. My change may be problematic
> > however, because of the whole double stacks thing.
>
> I think I'm saying the same thing. The "arm,mali-valhall-csf" is the most general
> compatible string and defines the common denominator if not overwritten. I'm
> not expecting anyone to use just that string for a compatible, but downstream
> we have additional compatible strings that don't have to update the schema at all.
> rk3588 has a specific setup that requires 3 clocks so you cannot have any optional,
> that's why it is overwriting the minItems. Your whole double stack thing is
> actually not needed if all you do is overwrite in the MT8196 case the clock
> names and maxItems to only need two clocks.
>
> >
> > > > Yes, the third "stacks"
> > > > remains, though if you wanted to be extra precise you could
> > > > then specify in the non-MT8196 cases that we should not have
> > > > stacks followed by stacks, but I'd wager some checker for
> > > > duplicate names may already catch that.
> > > >
> > > > However, I don't think it's a big enough deal to reroll this
> > > > series again.
> > >
> > > I'm not asking you to re-roll the series but if you agree to drop that
> > > part I can make the edit when merging it.
> >
> > If the other DT maintainers (especially Rob who gave it his R-b)
> > are okay with dropping it, then yes please do.
>
> Rob, do you agree with dropping the change in the generic bindings?
I haven't got any answers, so I'll push the patch as is and send a separate
fix that hopefully catches Rob's attention quicker.
Best regards,
Liviu
> > > > >
> > > > > >
> > > > > > mali-supply: true
> > > > > > @@ -110,6 +112,27 @@ allOf:
> > > > > > power-domain-names: false
> > > > > > required:
> > > > > > - mali-supply
> > > > > > + - if:
> > > > > > + properties:
> > > > > > + compatible:
> > > > > > + contains:
> > > > > > + const: mediatek,mt8196-mali
> > > > > > + then:
> > > > > > + properties:
> > > > > > + mali-supply: false
> > > > > > + sram-supply: false
> > > > > > + operating-points-v2: false
> > > > > > + power-domains:
> > > > > > + maxItems: 1
> > > > > > + power-domain-names: false
> > > > > > + clocks:
> > > > > > + maxItems: 2
> > > > > > + clock-names:
> > > > > > + items:
> > > > > > + - const: core
> > > > > > + - const: stacks
> > > > > > + required:
> > > > > > + - power-domains
> > > > > >
> > > > > > examples:
> > > > > > - |
> > > > > > @@ -145,5 +168,17 @@ examples:
> > > > > > };
> > > > > > };
> > > > > > };
> > > > > > + - |
> > > > > > + gpu@48000000 {
> > > > > > + compatible = "mediatek,mt8196-mali", "arm,mali-valhall-csf";
> > > > > > + reg = <0x48000000 0x480000>;
> > > > > > + clocks = <&gpufreq 0>, <&gpufreq 1>;
> > > > > > + clock-names = "core", "stacks";
> > > > > > + interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > > > + <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH 0>,
> > > > > > + <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH 0>;
> > > > > > + interrupt-names = "job", "mmu", "gpu";
> > > > > > + power-domains = <&gpufreq>;
> > > > > > + };
> > > > > >
> > > > > > ...
> > > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> >
> >
> >
> >
>
> --
> ====================
> | I would like to |
> | fix the world, |
> | but they're not |
> | giving me the |
> \ source code! /
> ---------------
> ¯\_(ツ)_/¯
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
© 2016 - 2025 Red Hat, Inc.