[PATCH v8 0/5] MT8196 GPU Frequency/Power Control Support

[PATCH v8 0/5] MT8196 GPU Frequency/Power Control Support

[Nicolas Frattaroli]

This series introduces two new drivers to accomplish controlling the
frequency and power of the Mali GPU on MediaTek MT8196 SoCs.

The reason why it's not as straightforward as with other SoCs is that
the MT8196 has quite complex glue logic in order to squeeze the maximum
amount of performance possible out of the silicon. There's an additional
MCU running a specialised firmware, which communicates with the
application processor through a mailbox and some reserved memory, and is
in charge of controlling the regulators, the PLL clocks, and the power
gating of the GPU, all while also being in charge of any DVFS control.

This set of drivers is enough to communicate desired OPP index limits to
the aforementioned MCU, referred to as "GPUEB" from here on out. The
GPUEB is still free to lower the effective frequency if the GPU has no
jobs going on at all, even when a higher OPP is set.

The power- and frequency control driver, mtk-mfg-pmdomain, is now
implemented as a power domain driver, with a set_performance_state
operation. It also exposes itself as a clock provider, so that panthor
can read the actual achieved DVFS clock rate as per the GPUEB firmware.

This power domain approach means that panthor does not need to know
about how the frequency control works on this SoC, as the OPP core
framework already takes care of it. The only exception is that panthor
needs to not register OPPs from DT itself if there already is an OPP
table present.

The mailbox driver is a fairly bog-standard common mailbox framework
driver, just specific to the firmware that runs on the GPUEB. It was
merged in v6.18 already.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v8:
- mtk-mfg-pmdomain: remove unused shmem variable that caused a warning
  on GCC, but not clang
- Link to v7: https://lore.kernel.org/r/20251015-mt8196-gpufreq-v7-0-0a6435da2080@collabora.com

Changes in v7:
- panthor: rename "t" to "table"
- panthor: add code comment explaining why an existing OPP table is
  being checked for
- mtk-mfg-pmdomain: use GF_REG_MAGIC offset for sake of consistency
- mtk-mfg-pmdomain: remove redundant semicolon after mtk_mfg_mt8196_init
- mtk-mfg-pmdomain: fix resource leaks on probe failure
- mtk-mfg-pmdomain: enable/disable EB clock during MT8196 init, which is
  needed for the register read
- Rebase onto next-20251014, which drops already merged patches, namely
  mailbox driver+bindings, and drops the ASN_HASH patch series
  dependency, which was also merged
- Link to v6: https://lore.kernel.org/r/20251003-mt8196-gpufreq-v6-0-76498ad61d9e@collabora.com

Changes in v6:
- mailbox: move buf definition into if condition, as per Chia-I Wu
- panthor: remove the redundant NULL checks in panthor_devfreq_get_freq
- mtk-mfg-pmdomain: adjust return style consistency
- mtk-mfg-pmdomain: add docstring for mtk_mfg_send_ipi to explain it's
  blocking
- mtk-mfg-pmdomain: use CMD_FIX_DUAL_TARGET_OPPIDX instead of
  CMD_FIX_TARGET_OPPIDX.
- mtk-mfg-pmdomain: reword code comments to not be in the "we" style
- mtk-mfg-pmdomain: shuffle around mbox allocations as per Angelo
- mtk-mfg-pmdomain: don't pointlessly turn on EB clock in probe,
  reducing the need for a comment explaining the bookkeeping
- mtk-mfg-pmdomain: consistently use dev_err_probe and Capitalise first
  letter of error string
- mtk-mfg-pmdomain: get rid of redundant ret = dev_err_probe assignment
- mtk-mfg-pmdomain: reintroduce stack OPP table, choose min(gpu, stack)
  when adding frequencies. Fixes gaps in OPP levels where only stack
  changed, but gpu had duplicates, which resulted in choosing a too slow
  OPP
- mtk-mfg-pmdomain: stub round_rate clk op to opt out of CCF always
  "rounding" a devfreq rate request to the current rate
- Link to v5: https://lore.kernel.org/r/20250929-mt8196-gpufreq-v5-0-3056e5ecf765@collabora.com

Changes in v5:
- mtk-mfg-pmdomain binding: add memory-regions property, remove shmem
  property, as we now correctly describe the shared memory as a regular
  memory region
- mtk-mfg-pmdomain binding: get rid of redundant |
- drop "dt-bindings: sram: Add compatible for
  mediatek,mt8196-gpufreq-sram" as part of the move to reserved memory
- mtk-mfg-pmdomain: move to using reserved-memory for GPUEB shared
  memory
- mtk-mfg-pmdomain: demote some types to smaller sizes in struct
  mtk_mfg, as per Angelo's suggestions
- mtk-mfg-pmdomain: use units.h for Hz-to-KHz
- mtk-mfg-pmdomain: change for loop in attach_dev to reduce indentation
- mtk-mfg-pmdomain: simplify return in mtk_mfg_power_off
- mtk-mfg-pmdomain: move of_device_id after probe
- mtk_mfg_pmdomain: map mmio by index
- mtk_mfg_pmdomain: add error checking to pm_genpd_init()
- mtk_mfg_pmdomain: add remove function
- mtk_mfg_pmdomain: remove last_opp member and logic, since OPP core
  already does that for us
- mtk_mfg_pmdomain: adjust comment in mtk_mfg_set_performance to explain
  why we're doing what we're doing
- mtk_mfg_pmdomain: call mtk_mfg_set_oppidx in mtk_mfg_power_on with
  the performance_state we deferred setting while it was powered off
- mtk_mfg_pmdomain: add inline function for PWR_ACK checking, as it's
  now used twice with the added remove function
- mtk-mfg-pmdomain: add suppress_bind_attrs so people don't play with
  that
- mtk-mfg-pmdomain: change KConfig from tristate to bool, as module
  unloading results in strange likely firmware-induced hardware state
  woes in the mali GPU
- mtk-mfg-pmdomain: read IPI magic in power_on, don't zero it after
  confirming that seemingly had no purpose
- mtk-mfg-pmdomain: misc style changes
- Link to v4: https://lore.kernel.org/r/20250923-mt8196-gpufreq-v4-0-6cd63ade73d6@collabora.com

Changes in v4:
- rebase onto next-20250922, which includes Laura Nao's clock patches
- refactor mediatek_mfg into a pmdomain driver called "mtk-mfg-pmdomain"
- move mt8196-gpufreq binding to the power subdirectory
- mali-valhall-csf binding: adjust for power-domains usage
- mali-valhall-csf binding: use clocks on mt8196
- mailbox: prefix defines with "GPUEB_"
- mailbox: get rid of custom of_xlate
- mailbox: rename "CLOGGED" to "BLOCKED"
- mailbox: adjust send_data comment to include more technical info
- mailbox: misc style improvements
- panthor: drop "drm/panthor: devfreq: make get_dev_status use
  get_cur_freq", as it is now not necessary and makes the code worse
- panthor: drop "drm/panthor: devfreq: add pluggable devfreq providers"
- panthor: drop "drm/panthor: add no_clocks soc_data member for MT8196",
  as we now have clocks courtesy of gpufreq
- panthor: check for existing opp table before registering a new one
- mtk-mfg-pmdomain: add turbo_below variant data, which marks OPPs below
  a certain index as turbo for the OPP subsystem
- mtk-mfg-pmdomain: no longer read stack OPPs, as they weren't used
- mtk-mfg-pmdomain: get rid of num gpu opp != num stack opp check.
  That's the firmware's problem should it ever happen, not ours
- mtk-mfg-pmdomain: some small name and whitespace changes on the defines
- Link to v3: https://lore.kernel.org/r/20250917-mt8196-gpufreq-v3-0-c4ede4b4399e@collabora.com

Changes in v3:
- mali-valhall-csf binding: get rid of clocks for MT8196, rebase onto
  Chia-I Wu's patch
- mt8196-gpufreq binding: rename hw_revision to hw-revision
- mt8196-gpufreq binding: rename clocks
- mt8196-gpufreq binding: drop pointless label in example
- mailbox binding: drop pointless label in example
- mailbox: whitespace changes on defines
- mailbox: remove rx_buf member from channel struct, use stack buffer
- mailbox: check in probe that no rx_len exceeds MBOX_MAX_RX_SIZE
- panthor: add no_clocks SoC data patch, also rebase onto Chia-I Wu's
  series
- panthor: refactor devfreq provider functionality to do allocation and
  initialisation of panthor_devfreq struct in panthor in all cases
- panthor: drop the patch that moves struct panthor_devfreq to a header
  file, as it no longer needs to be exposed to devfreq providers
- mediatek_mfg: refactor devfreq provider functionality to decouple it
  more from panthor itself
- mediatek_mfg: move SRAM magic to a #define
- mediatek_mfg: begrudgingly rename member "padding_lol" to "reserved"
- mediatek_mfg: use local struct device pointer var in more places
- mediatek_mfg: change wording of sleep command failure error message,
  but keep the format specifier because I don't want to throw bare
  errnos at users
- mediatek_mfg: remove unnecessary braces around dev_err EB power off
  timeout message
- mediatek_mfg: allocate rx_data for channels that expect a response
- mediatek_mfg: memcpy the rx buffer from the common mailbox framework
  in the rx callback to rx_data, as mssg now points to stack memory
- mediatek_mfg: make SRAM clearing message dev_dbg
- mediatek_mfg: no longer print physical address of SRAM
- mediatek_mfg: expand on the GF_REG_OPP_TABLE_STK comment, toning down
  its defeatist attitude in the process
- mediatek_mfg: style fixes in mtk_mfg_get_closest_opp_idx
- mediatek_mfg: rename clocks and hw-revision reg as per binding
- Link to v2: https://lore.kernel.org/r/20250912-mt8196-gpufreq-v2-0-779a8a3729d9@collabora.com

Changes in v2:
- mali-valhall-csf binding: move from performance-controller to
  performance-domains property
- mali-valhall-csf binding: fix vendor name oopsie in compatible of if
  condition
- mt8196-gpufreq binding: move from performance-controller to
  performance-domains by adding the cells property
- mt8196-gpufreq binding: rename e2_id to hw_revision
- mt8196-gpufreq binding: add description that mentions "MediaTek
  Flexible Graphics"
- mt8196-gpufreq binding: get rid of mailbox channels we're unlikely to
  use any time soon, if ever
- mt8196-gpufreq binding: change name of mailbox channels to use -
  instead of _
- mailbox binding: change reg-names to "data" and "ctl"
- drm/panthor: mediatek_mfg: rename e2_id to hw_revision
- drm/panthor: devfreq: switch from performance-controller to
  performance-domains
- drm/panthor: devfreq: get rid of the accidental get_cur_freq function
  move
- mailbox: rename mtk_gpueb_mbox_ch to mtk_gpueb_mbox_chan_desc
- mailbox: use smaller types in mtk_gpueb_mbox_chan_desc where possible
- mailbox: add per-channel runtime data struct
- mailbox: request one threaded IRQ per channel, pass channel struct as
  data
- mailbox: make num_channels in variant struct u8
- mailbox: get rid of no_response, as it was redundant
- mailbox: enable and disable clock in mailbox startup/shutdown
- mailbox: point con_priv of mailbox framework channel struct to this
  driver's channel struct
- mailbox: request and free the threaded IRQ in startup/shutdown
- mailbox: only clear IRQ bit flag once RX data has been read from MMIO
- mailbox: reduce needlessly large receive buffer size
- mailbox: handle allocation errors wherever they could pop up
- mailbox: style cleanups in mtk_gpueb_mbox_read_rx
- mailbox: call platform_get_irq earlier on in probe
- mailbox: set drvdata later on in probe
- mailbox: ioremap resources by index, not name
- mailbox: handle devm_mbox_controller_register errors
- mailbox: rename channels to correspond to bindings
- mailbox: document a few of the private driver structs to be kind to
  the next person who will look at this code
- Link to v1: https://lore.kernel.org/r/20250905-mt8196-gpufreq-v1-0-7b6c2d6be221@collabora.com

---
Nicolas Frattaroli (5):
      dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant
      dt-bindings: power: Add MT8196 GPU frequency control binding
      drm/panthor: call into devfreq for current frequency
      drm/panthor: Use existing OPP table if present
      pmdomain: mediatek: Add support for MFlexGraphics

 .../bindings/gpu/arm,mali-valhall-csf.yaml         |   37 +-
 .../bindings/power/mediatek,mt8196-gpufreq.yaml    |  117 +++
 drivers/gpu/drm/panthor/panthor_devfreq.c          |   62 +-
 drivers/gpu/drm/panthor/panthor_devfreq.h          |    2 +
 drivers/gpu/drm/panthor/panthor_device.h           |    3 -
 drivers/gpu/drm/panthor/panthor_drv.c              |    4 +-
 drivers/pmdomain/mediatek/Kconfig                  |   16 +
 drivers/pmdomain/mediatek/Makefile                 |    1 +
 drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c       | 1044 ++++++++++++++++++++
 9 files changed, 1268 insertions(+), 18 deletions(-)
---
base-commit: 3477f49ff0433a241da12ec9cecf6c9b2bd1c6f8
change-id: 20250829-mt8196-gpufreq-a7645670d182

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v8 1/5] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Nicolas Frattaroli]

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.0

[PATCH v8 2/5] dt-bindings: power: Add MT8196 GPU frequency control binding

[Nicolas Frattaroli]

On the MT8196 and MT6991 SoCs, the GPU power and frequency is controlled
by some integration logic, referred to as "MFlexGraphics" by MediaTek,
which comes in the form of an embedded controller running
special-purpose firmware.

This controller takes care of the regulators and PLL clock frequencies
to squeeze the maximum amount of power out of the silicon.

Add a binding which models it as a power domain.

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/power/mediatek,mt8196-gpufreq.yaml    | 117 +++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/mediatek,mt8196-gpufreq.yaml b/Documentation/devicetree/bindings/power/mediatek,mt8196-gpufreq.yaml
new file mode 100644
index 000000000000..b9e43abaf8a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/mediatek,mt8196-gpufreq.yaml
@@ -0,0 +1,117 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/mediatek,mt8196-gpufreq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek MFlexGraphics Power and Frequency Controller
+
+maintainers:
+  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
+
+description:
+  A special-purpose embedded MCU to control power and frequency of GPU devices
+  using MediaTek Flexible Graphics integration hardware.
+
+properties:
+  $nodename:
+    pattern: '^power-controller@[a-f0-9]+$'
+
+  compatible:
+    enum:
+      - mediatek,mt8196-gpufreq
+
+  reg:
+    items:
+      - description: GPR memory area
+      - description: RPC memory area
+      - description: SoC variant ID register
+
+  reg-names:
+    items:
+      - const: gpr
+      - const: rpc
+      - const: hw-revision
+
+  clocks:
+    items:
+      - description: main clock of the embedded controller (EB)
+      - description: core PLL
+      - description: stack 0 PLL
+      - description: stack 1 PLL
+
+  clock-names:
+    items:
+      - const: eb
+      - const: core
+      - const: stack0
+      - const: stack1
+
+  mboxes:
+    items:
+      - description: FastDVFS events
+      - description: frequency control
+      - description: sleep control
+      - description: timer control
+      - description: frequency hopping control
+      - description: hardware voter control
+      - description: FastDVFS control
+
+  mbox-names:
+    items:
+      - const: fast-dvfs-event
+      - const: gpufreq
+      - const: sleep
+      - const: timer
+      - const: fhctl
+      - const: ccf
+      - const: fast-dvfs
+
+  memory-region:
+    items:
+      - description: phandle to the GPUEB shared memory
+
+  "#clock-cells":
+    const: 1
+
+  "#power-domain-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - mboxes
+  - mbox-names
+  - memory-region
+  - "#clock-cells"
+  - "#power-domain-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mediatek,mt8196-clock.h>
+
+    power-controller@4b09fd00 {
+        compatible = "mediatek,mt8196-gpufreq";
+        reg = <0x4b09fd00 0x80>,
+              <0x4b800000 0x1000>,
+              <0x4b860128 0x4>;
+        reg-names = "gpr", "rpc", "hw-revision";
+        clocks = <&topckgen CLK_TOP_MFG_EB>,
+                 <&mfgpll CLK_MFG_AO_MFGPLL>,
+                 <&mfgpll_sc0 CLK_MFGSC0_AO_MFGPLL_SC0>,
+                 <&mfgpll_sc1 CLK_MFGSC1_AO_MFGPLL_SC1>;
+        clock-names = "eb", "core", "stack0", "stack1";
+        mboxes = <&gpueb_mbox 0>, <&gpueb_mbox 1>, <&gpueb_mbox 2>,
+                 <&gpueb_mbox 3>, <&gpueb_mbox 4>, <&gpueb_mbox 5>,
+                 <&gpueb_mbox 7>;
+        mbox-names = "fast-dvfs-event", "gpufreq", "sleep", "timer", "fhctl",
+                     "ccf", "fast-dvfs";
+        memory-region = <&gpueb_shared_memory>;
+        #clock-cells = <1>;
+        #power-domain-cells = <0>;
+    };

-- 
2.51.0

[PATCH v8 3/5] drm/panthor: call into devfreq for current frequency

[Nicolas Frattaroli]

As it stands, panthor keeps a cached current frequency value for when it
wants to retrieve it. This doesn't work well for when things might
switch frequency without panthor's knowledge.

Instead, implement the get_cur_freq operation, and expose it through a
helper function to the rest of panthor.

Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_devfreq.c | 30 ++++++++++++++++++++++++++----
 drivers/gpu/drm/panthor/panthor_devfreq.h |  2 ++
 drivers/gpu/drm/panthor/panthor_device.h  |  3 ---
 drivers/gpu/drm/panthor/panthor_drv.c     |  4 +++-
 4 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
index 2df1d76d84a0..a6dca599f0a5 100644
--- a/drivers/gpu/drm/panthor/panthor_devfreq.c
+++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
@@ -62,7 +62,6 @@ static void panthor_devfreq_update_utilization(struct panthor_devfreq *pdevfreq)
 static int panthor_devfreq_target(struct device *dev, unsigned long *freq,
 				  u32 flags)
 {
-	struct panthor_device *ptdev = dev_get_drvdata(dev);
 	struct dev_pm_opp *opp;
 	int err;
 
@@ -72,8 +71,6 @@ static int panthor_devfreq_target(struct device *dev, unsigned long *freq,
 	dev_pm_opp_put(opp);
 
 	err = dev_pm_opp_set_rate(dev, *freq);
-	if (!err)
-		ptdev->current_frequency = *freq;
 
 	return err;
 }
@@ -115,11 +112,21 @@ static int panthor_devfreq_get_dev_status(struct device *dev,
 	return 0;
 }
 
+static int panthor_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
+{
+	struct panthor_device *ptdev = dev_get_drvdata(dev);
+
+	*freq = clk_get_rate(ptdev->clks.core);
+
+	return 0;
+}
+
 static struct devfreq_dev_profile panthor_devfreq_profile = {
 	.timer = DEVFREQ_TIMER_DELAYED,
 	.polling_ms = 50, /* ~3 frames */
 	.target = panthor_devfreq_target,
 	.get_dev_status = panthor_devfreq_get_dev_status,
+	.get_cur_freq = panthor_devfreq_get_cur_freq,
 };
 
 int panthor_devfreq_init(struct panthor_device *ptdev)
@@ -197,7 +204,6 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
 		return PTR_ERR(opp);
 
 	panthor_devfreq_profile.initial_freq = cur_freq;
-	ptdev->current_frequency = cur_freq;
 
 	/*
 	 * Set the recommend OPP this will enable and configure the regulator
@@ -295,3 +301,19 @@ void panthor_devfreq_record_idle(struct panthor_device *ptdev)
 
 	spin_unlock_irqrestore(&pdevfreq->lock, irqflags);
 }
+
+unsigned long panthor_devfreq_get_freq(struct panthor_device *ptdev)
+{
+	struct panthor_devfreq *pdevfreq = ptdev->devfreq;
+	unsigned long freq = 0;
+	int ret;
+
+	if (!pdevfreq->devfreq)
+		return 0;
+
+	ret = pdevfreq->devfreq->profile->get_cur_freq(ptdev->base.dev, &freq);
+	if (ret)
+		return 0;
+
+	return freq;
+}
diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.h b/drivers/gpu/drm/panthor/panthor_devfreq.h
index b7631de695f7..f8e29e02f66c 100644
--- a/drivers/gpu/drm/panthor/panthor_devfreq.h
+++ b/drivers/gpu/drm/panthor/panthor_devfreq.h
@@ -18,4 +18,6 @@ void panthor_devfreq_suspend(struct panthor_device *ptdev);
 void panthor_devfreq_record_busy(struct panthor_device *ptdev);
 void panthor_devfreq_record_idle(struct panthor_device *ptdev);
 
+unsigned long panthor_devfreq_get_freq(struct panthor_device *ptdev);
+
 #endif /* __PANTHOR_DEVFREQ_H__ */
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index 9f0649ecfc4f..f32c1868bf6d 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -214,9 +214,6 @@ struct panthor_device {
 	/** @profile_mask: User-set profiling flags for job accounting. */
 	u32 profile_mask;
 
-	/** @current_frequency: Device clock frequency at present. Set by DVFS*/
-	unsigned long current_frequency;
-
 	/** @fast_rate: Maximum device clock frequency. Set by DVFS */
 	unsigned long fast_rate;
 
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index fb4b293f17f0..75898d83a207 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -25,6 +25,7 @@
 #include <drm/gpu_scheduler.h>
 #include <drm/panthor_drm.h>
 
+#include "panthor_devfreq.h"
 #include "panthor_device.h"
 #include "panthor_fw.h"
 #include "panthor_gem.h"
@@ -1519,7 +1520,8 @@ static void panthor_gpu_show_fdinfo(struct panthor_device *ptdev,
 		drm_printf(p, "drm-cycles-panthor:\t%llu\n", pfile->stats.cycles);
 
 	drm_printf(p, "drm-maxfreq-panthor:\t%lu Hz\n", ptdev->fast_rate);
-	drm_printf(p, "drm-curfreq-panthor:\t%lu Hz\n", ptdev->current_frequency);
+	drm_printf(p, "drm-curfreq-panthor:\t%lu Hz\n",
+		   panthor_devfreq_get_freq(ptdev));
 }
 
 static void panthor_show_internal_memory_stats(struct drm_printer *p, struct drm_file *file)

-- 
2.51.0

[PATCH v8 4/5] drm/panthor: Use existing OPP table if present

[Nicolas Frattaroli]

On SoCs where the GPU's power-domain is in charge of setting performance
levels, the OPP table of the GPU node will have already been populated
during said power-domain's attach_dev operation.

To avoid initialising an OPP table twice, only set the OPP regulator and
the OPPs from DT if there's no OPP table present.

Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_devfreq.c | 32 ++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
index a6dca599f0a5..ec63e27f4883 100644
--- a/drivers/gpu/drm/panthor/panthor_devfreq.c
+++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
@@ -141,6 +141,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
 	struct thermal_cooling_device *cooling;
 	struct device *dev = ptdev->base.dev;
 	struct panthor_devfreq *pdevfreq;
+	struct opp_table *table;
 	struct dev_pm_opp *opp;
 	unsigned long cur_freq;
 	unsigned long freq = ULONG_MAX;
@@ -152,17 +153,30 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
 
 	ptdev->devfreq = pdevfreq;
 
-	ret = devm_pm_opp_set_regulators(dev, reg_names);
-	if (ret && ret != -ENODEV) {
-		if (ret != -EPROBE_DEFER)
-			DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
-		return ret;
+	/*
+	 * The power domain associated with the GPU may have already added an
+	 * OPP table, complete with OPPs, as part of the platform bus
+	 * initialization. If this is the case, the power domain is in charge of
+	 * also controlling the performance, with a set_performance callback.
+	 * Only add a new OPP table from DT if there isn't such a table present
+	 * already.
+	 */
+	table = dev_pm_opp_get_opp_table(dev);
+	if (IS_ERR_OR_NULL(table)) {
+		ret = devm_pm_opp_set_regulators(dev, reg_names);
+		if (ret && ret != -ENODEV) {
+			if (ret != -EPROBE_DEFER)
+				DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
+			return ret;
+		}
+
+		ret = devm_pm_opp_of_add_table(dev);
+		if (ret)
+			return ret;
+	} else {
+		dev_pm_opp_put_opp_table(table);
 	}
 
-	ret = devm_pm_opp_of_add_table(dev);
-	if (ret)
-		return ret;
-
 	spin_lock_init(&pdevfreq->lock);
 
 	panthor_devfreq_reset(pdevfreq);

-- 
2.51.0

[PATCH v8 5/5] pmdomain: mediatek: Add support for MFlexGraphics

[Nicolas Frattaroli]

Various MediaTek SoCs use GPU integration silicon named "MFlexGraphics"
by MediaTek. On the MT8196 and MT6991 SoCs, interacting with this
integration silicon is required to power on the GPU.

This glue silicon is in the form of an embedded microcontroller running
special-purpose firmware, which autonomously adjusts clocks and
regulators.

Implement a driver, modelled as a pmdomain driver with a
set_performance_state operation, to support these SoCs.

The driver also exposes the actual achieved clock rate, as read back
from the MCU, as common clock framework clocks, by acting as a clock
provider as well.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/pmdomain/mediatek/Kconfig            |   16 +
 drivers/pmdomain/mediatek/Makefile           |    1 +
 drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c | 1044 ++++++++++++++++++++++++++
 3 files changed, 1061 insertions(+)

diff --git a/drivers/pmdomain/mediatek/Kconfig b/drivers/pmdomain/mediatek/Kconfig
index 0e34a517ab7d..b06aaa9690f0 100644
--- a/drivers/pmdomain/mediatek/Kconfig
+++ b/drivers/pmdomain/mediatek/Kconfig
@@ -26,6 +26,22 @@ config MTK_SCPSYS_PM_DOMAINS
 	  Control Processor System (SCPSYS) has several power management related
 	  tasks in the system.
 
+config MTK_MFG_PM_DOMAIN
+	bool "MediaTek MFlexGraphics power domain"
+	default ARCH_MEDIATEK
+	depends on PM
+	depends on OF
+	depends on COMMON_CLK
+	select PM_GENERIC_DOMAINS
+	imply MTK_GPUEB_MBOX
+	help
+	  Say y or m here to enable the power domains driver for MediaTek
+	  MFlexGraphics. This driver allows for power and frequency control of
+	  GPUs on MediaTek SoCs such as the MT8196 or MT6991.
+
+	  This driver is required for the Mali GPU to work at all on MT8196 and
+	  MT6991.
+
 config AIROHA_CPU_PM_DOMAIN
 	tristate "Airoha CPU power domain"
 	default ARCH_AIROHA
diff --git a/drivers/pmdomain/mediatek/Makefile b/drivers/pmdomain/mediatek/Makefile
index 18ba92e3c418..b424f1ed8676 100644
--- a/drivers/pmdomain/mediatek/Makefile
+++ b/drivers/pmdomain/mediatek/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_MTK_MFG_PM_DOMAIN)		+= mtk-mfg-pmdomain.o
 obj-$(CONFIG_MTK_SCPSYS)		+= mtk-scpsys.o
 obj-$(CONFIG_MTK_SCPSYS_PM_DOMAINS) 	+= mtk-pm-domains.o
 obj-$(CONFIG_AIROHA_CPU_PM_DOMAIN) 	+= airoha-cpu-pmdomain.o
diff --git a/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c b/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c
new file mode 100644
index 000000000000..af20111067c0
--- /dev/null
+++ b/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c
@@ -0,0 +1,1044 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for MediaTek MFlexGraphics Devices
+ *
+ * Copyright (C) 2025, Collabora Ltd.
+ */
+
+#include <linux/completion.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/container_of.h>
+#include <linux/iopoll.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/overflow.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_opp.h>
+#include <linux/regulator/consumer.h>
+#include <linux/units.h>
+
+#define GPR_LP_STATE		0x0028
+#define   EB_ON_SUSPEND		0x0
+#define   EB_ON_RESUME		0x1
+#define GPR_IPI_MAGIC		0x34
+
+#define RPC_PWR_CON		0x0504
+#define   PWR_ACK_M		GENMASK(31, 30)
+#define RPC_DUMMY_REG_2		0x0658
+#define RPC_GHPM_CFG0_CON	0x0800
+#define   GHPM_ENABLE_M		BIT(0)
+#define   GHPM_ON_SEQ_M		BIT(2)
+#define RPC_GHPM_RO0_CON	0x09A4
+#define   GHPM_STATE_M		GENMASK(7, 0)
+#define   GHPM_PWR_STATE_M	BIT(16)
+
+#define GF_REG_MAGIC			0x0000
+#define GF_REG_GPU_OPP_IDX		0x0004
+#define GF_REG_STK_OPP_IDX		0x0008
+#define GF_REG_GPU_OPP_NUM		0x000c
+#define GF_REG_STK_OPP_NUM		0x0010
+#define GF_REG_GPU_OPP_SNUM		0x0014
+#define GF_REG_STK_OPP_SNUM		0x0018
+#define GF_REG_POWER_COUNT		0x001c
+#define GF_REG_BUCK_COUNT		0x0020
+#define GF_REG_MTCMOS_COUNT		0x0024
+#define GF_REG_CG_COUNT			0x0028 /* CG = Clock Gate? */
+#define GF_REG_ACTIVE_COUNT		0x002C
+#define GF_REG_TEMP_RAW			0x0030
+#define GF_REG_TEMP_NORM_GPU		0x0034
+#define GF_REG_TEMP_HIGH_GPU		0x0038
+#define GF_REG_TEMP_NORM_STK		0x003C
+#define GF_REG_TEMP_HIGH_STK		0x0040
+#define GF_REG_FREQ_CUR_GPU		0x0044
+#define GF_REG_FREQ_CUR_STK		0x0048
+#define GF_REG_FREQ_OUT_GPU		0x004C /* Guess: actual achieved freq */
+#define GF_REG_FREQ_OUT_STK		0x0050 /* Guess: actual achieved freq */
+#define GF_REG_FREQ_METER_GPU		0x0054 /* Seems unused, always 0 */
+#define GF_REG_FREQ_METER_STK		0x0058 /* Seems unused, always 0 */
+#define GF_REG_VOLT_CUR_GPU		0x005C /* in tens of microvolts */
+#define GF_REG_VOLT_CUR_STK		0x0060 /* in tens of microvolts */
+#define GF_REG_VOLT_CUR_GPU_SRAM	0x0064
+#define GF_REG_VOLT_CUR_STK_SRAM	0x0068
+#define GF_REG_VOLT_CUR_GPU_REG		0x006C /* Seems unused, always 0 */
+#define GF_REG_VOLT_CUR_STK_REG		0x0070 /* Seems unused, always 0 */
+#define GF_REG_VOLT_CUR_GPU_REG_SRAM	0x0074
+#define GF_REG_VOLT_CUR_STK_REG_SRAM	0x0078
+#define GF_REG_PWR_CUR_GPU		0x007C /* in milliwatts */
+#define GF_REG_PWR_CUR_STK		0x0080 /* in milliwatts */
+#define GF_REG_PWR_MAX_GPU		0x0084 /* in milliwatts */
+#define GF_REG_PWR_MAX_STK		0x0088 /* in milliwatts */
+#define GF_REG_PWR_MIN_GPU		0x008C /* in milliwatts */
+#define GF_REG_PWR_MIN_STK		0x0090 /* in milliwatts */
+#define GF_REG_LEAKAGE_RT_GPU		0x0094 /* Unknown */
+#define GF_REG_LEAKAGE_RT_STK		0x0098 /* Unknown */
+#define GF_REG_LEAKAGE_RT_SRAM		0x009C /* Unknown */
+#define GF_REG_LEAKAGE_HT_GPU		0x00A0 /* Unknown */
+#define GF_REG_LEAKAGE_HT_STK		0x00A4 /* Unknown */
+#define GF_REG_LEAKAGE_HT_SRAM		0x00A8 /* Unknown */
+#define GF_REG_VOLT_DAC_LOW_GPU		0x00AC /* Seems unused, always 0 */
+#define GF_REG_VOLT_DAC_LOW_STK		0x00B0 /* Seems unused, always 0 */
+#define GF_REG_OPP_CUR_CEIL		0x00B4
+#define GF_REG_OPP_CUR_FLOOR		0x00B8
+#define GF_REG_OPP_CUR_LIMITER_CEIL	0x00BC
+#define GF_REG_OPP_CUR_LIMITER_FLOOR	0x00C0
+#define GF_REG_OPP_PRIORITY_CEIL	0x00C4
+#define GF_REG_OPP_PRIORITY_FLOOR	0x00C8
+#define GF_REG_PWR_CTL			0x00CC
+#define GF_REG_ACTIVE_SLEEP_CTL		0x00D0
+#define GF_REG_DVFS_STATE		0x00D4
+#define GF_REG_SHADER_PRESENT		0x00D8
+#define GF_REG_ASENSOR_ENABLE		0x00DC
+#define GF_REG_AGING_LOAD		0x00E0
+#define GF_REG_AGING_MARGIN		0x00E4
+#define GF_REG_AVS_ENABLE		0x00E8
+#define GF_REG_AVS_MARGIN		0x00EC
+#define GF_REG_CHIP_TYPE		0x00F0
+#define GF_REG_SB_VERSION		0x00F4
+#define GF_REG_PTP_VERSION		0x00F8
+#define GF_REG_DBG_VERSION		0x00FC
+#define GF_REG_KDBG_VERSION		0x0100
+#define GF_REG_GPM1_MODE		0x0104
+#define GF_REG_GPM3_MODE		0x0108
+#define GF_REG_DFD_MODE			0x010C
+#define GF_REG_DUAL_BUCK		0x0110
+#define GF_REG_SEGMENT_ID		0x0114
+#define GF_REG_POWER_TIME_H		0x0118
+#define GF_REG_POWER_TIME_L		0x011C
+#define GF_REG_PWR_STATUS		0x0120
+#define GF_REG_STRESS_TEST		0x0124
+#define GF_REG_TEST_MODE		0x0128
+#define GF_REG_IPS_MODE			0x012C
+#define GF_REG_TEMP_COMP_MODE		0x0130
+#define GF_REG_HT_TEMP_COMP_MODE	0x0134
+#define GF_REG_PWR_TRACKER_MODE		0x0138
+#define GF_REG_OPP_TABLE_GPU		0x0314
+#define GF_REG_OPP_TABLE_STK		0x09A4
+#define GF_REG_OPP_TABLE_GPU_S		0x1034
+#define GF_REG_OPP_TABLE_STK_S		0x16c4
+#define GF_REG_LIMIT_TABLE		0x1d54
+#define GF_REG_GPM3_TABLE		0x223C
+
+#define MFG_MT8196_E2_ID		0x101
+#define GPUEB_SLEEP_MAGIC		0x55667788UL
+#define GPUEB_MEM_MAGIC			0xBABADADAUL
+
+#define GPUEB_TIMEOUT_US		10000UL
+#define GPUEB_POLL_US			50
+
+#define MAX_OPP_NUM			70
+
+#define GPUEB_MBOX_MAX_RX_SIZE		32 /* in bytes */
+
+/*
+ * This enum is part of the ABI of the GPUEB firmware. Don't change the
+ * numbering, as you would wreak havoc.
+ */
+enum mtk_mfg_ipi_cmd {
+	CMD_INIT_SHARED_MEM		= 0,
+	CMD_GET_FREQ_BY_IDX		= 1,
+	CMD_GET_POWER_BY_IDX		= 2,
+	CMD_GET_OPPIDX_BY_FREQ		= 3,
+	CMD_GET_LEAKAGE_POWER		= 4,
+	CMD_SET_LIMIT			= 5,
+	CMD_POWER_CONTROL		= 6,
+	CMD_ACTIVE_SLEEP_CONTROL	= 7,
+	CMD_COMMIT			= 8,
+	CMD_DUAL_COMMIT			= 9,
+	CMD_PDCA_CONFIG			= 10,
+	CMD_UPDATE_DEBUG_OPP_INFO	= 11,
+	CMD_SWITCH_LIMIT		= 12,
+	CMD_FIX_TARGET_OPPIDX		= 13,
+	CMD_FIX_DUAL_TARGET_OPPIDX	= 14,
+	CMD_FIX_CUSTOM_FREQ_VOLT	= 15,
+	CMD_FIX_DUAL_CUSTOM_FREQ_VOLT	= 16,
+	CMD_SET_MFGSYS_CONFIG		= 17,
+	CMD_MSSV_COMMIT			= 18,
+	CMD_NUM				= 19,
+};
+
+/*
+ * This struct is part of the ABI of the GPUEB firmware. Changing it, or
+ * reordering fields in it, will break things, so don't do it. Thank you.
+ */
+struct __packed mtk_mfg_ipi_msg {
+	__le32 magic;
+	__le32 cmd;
+	__le32 target;
+	/*
+	 * Downstream relies on the compiler to implicitly add the following
+	 * padding, as it declares the struct as non-packed.
+	 */
+	__le32 reserved;
+	union {
+		s32 __bitwise oppidx;
+		s32 __bitwise return_value;
+		__le32 freq;
+		__le32 volt;
+		__le32 power;
+		__le32 power_state;
+		__le32 mode;
+		__le32 value;
+		struct {
+			__le64 base;
+			__le32 size;
+		} shared_mem;
+		struct {
+			__le32 freq;
+			__le32 volt;
+		} custom;
+		struct {
+			__le32 limiter;
+			s32 __bitwise ceiling_info;
+			s32 __bitwise floor_info;
+		} set_limit;
+		struct {
+			__le32 target;
+			__le32 val;
+		} mfg_cfg;
+		struct {
+			__le32 target;
+			__le32 val;
+		} mssv;
+		struct {
+			s32 __bitwise gpu_oppidx;
+			s32 __bitwise stack_oppidx;
+		} dual_commit;
+		struct {
+			__le32 fgpu;
+			__le32 vgpu;
+			__le32 fstack;
+			__le32 vstack;
+		} dual_custom;
+	} u;
+};
+
+struct __packed mtk_mfg_ipi_sleep_msg {
+	__le32 event;
+	__le32 state;
+	__le32 magic;
+};
+
+/**
+ * struct mtk_mfg_opp_entry - OPP table entry from firmware
+ * @freq_khz: The operating point's frequency in kilohertz
+ * @voltage_core: The operating point's core voltage in tens of microvolts
+ * @voltage_sram: The operating point's SRAM voltage in tens of microvolts
+ * @posdiv: exponent of base 2 for PLL frequency divisor used for this OPP
+ * @voltage_margin: Number of tens of microvolts the voltage can be undershot
+ * @power_mw: estimate of power usage at this operating point, in milliwatts
+ *
+ * This struct is part of the ABI with the EB firmware. Do not change it.
+ */
+struct __packed mtk_mfg_opp_entry {
+	__le32 freq_khz;
+	__le32 voltage_core;
+	__le32 voltage_sram;
+	__le32 posdiv;
+	__le32 voltage_margin;
+	__le32 power_mw;
+};
+
+struct mtk_mfg_mbox {
+	struct mbox_client cl;
+	struct completion rx_done;
+	struct mtk_mfg *mfg;
+	struct mbox_chan *ch;
+	void *rx_data;
+};
+
+struct mtk_mfg {
+	struct generic_pm_domain pd;
+	struct platform_device *pdev;
+	struct clk *clk_eb;
+	struct clk_bulk_data *gpu_clks;
+	struct clk_hw clk_core_hw;
+	struct clk_hw clk_stack_hw;
+	struct regulator_bulk_data *gpu_regs;
+	void __iomem *rpc;
+	void __iomem *gpr;
+	void __iomem *shared_mem;
+	phys_addr_t shared_mem_phys;
+	unsigned int shared_mem_size;
+	u16 ghpm_en_reg;
+	u32 ipi_magic;
+	unsigned short num_gpu_opps;
+	unsigned short num_stack_opps;
+	struct dev_pm_opp_data *gpu_opps;
+	struct dev_pm_opp_data *stack_opps;
+	struct mtk_mfg_mbox *gf_mbox;
+	struct mtk_mfg_mbox *slp_mbox;
+	const struct mtk_mfg_variant *variant;
+};
+
+struct mtk_mfg_variant {
+	const char *const *clk_names;
+	unsigned int num_clks;
+	const char *const *regulator_names;
+	unsigned int num_regulators;
+	/** @turbo_below: opp indices below this value are considered turbo */
+	unsigned int turbo_below;
+	int (*init)(struct mtk_mfg *mfg);
+};
+
+static inline struct mtk_mfg *mtk_mfg_from_genpd(struct generic_pm_domain *pd)
+{
+	return container_of(pd, struct mtk_mfg, pd);
+}
+
+static inline void mtk_mfg_update_reg_bits(void __iomem *addr, u32 mask, u32 val)
+{
+	writel((readl(addr) & ~mask) | (val & mask), addr);
+}
+
+static inline bool mtk_mfg_is_powered_on(struct mtk_mfg *mfg)
+{
+	return (readl(mfg->rpc + RPC_PWR_CON) & PWR_ACK_M) == PWR_ACK_M;
+}
+
+static unsigned long mtk_mfg_recalc_rate_gpu(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	struct mtk_mfg *mfg = container_of(hw, struct mtk_mfg, clk_core_hw);
+
+	return readl(mfg->shared_mem + GF_REG_FREQ_OUT_GPU) * HZ_PER_KHZ;
+}
+
+static long mtk_mfg_round_rate(struct clk_hw *hw, unsigned long rate,
+			       unsigned long *parent_rate)
+{
+	/*
+	 * The round_rate callback needs to be implemented to avoid returning
+	 * the current clock frequency, rather than something even remotely
+	 * close to the frequency that was asked for.
+	 *
+	 * Instead of writing considerable amounts of possibly slow code just to
+	 * somehow figure out which of the three PLLs to round for, or even to
+	 * do a search through one of two OPP tables in order to find the closest
+	 * OPP of a frequency, just return the rate as-is. This avoids devfreq
+	 * "rounding" a request for the lowest frequency to the possibly very
+	 * high current frequency, breaking the powersave governor in the process.
+	 */
+
+	return rate;
+}
+
+static unsigned long mtk_mfg_recalc_rate_stack(struct clk_hw *hw,
+					       unsigned long parent_rate)
+{
+	struct mtk_mfg *mfg = container_of(hw, struct mtk_mfg, clk_stack_hw);
+
+	return readl(mfg->shared_mem + GF_REG_FREQ_OUT_STK) * HZ_PER_KHZ;
+}
+
+static const struct clk_ops mtk_mfg_clk_gpu_ops = {
+	.recalc_rate = mtk_mfg_recalc_rate_gpu,
+	.round_rate = mtk_mfg_round_rate,
+};
+
+static const struct clk_ops mtk_mfg_clk_stack_ops = {
+	.recalc_rate = mtk_mfg_recalc_rate_stack,
+	.round_rate = mtk_mfg_round_rate,
+};
+
+static const struct clk_init_data mtk_mfg_clk_gpu_init = {
+	.name = "gpu-core",
+	.ops = &mtk_mfg_clk_gpu_ops,
+	.flags = CLK_GET_RATE_NOCACHE,
+};
+
+static const struct clk_init_data mtk_mfg_clk_stack_init = {
+	.name = "gpu-stack",
+	.ops = &mtk_mfg_clk_stack_ops,
+	.flags = CLK_GET_RATE_NOCACHE,
+};
+
+static int mtk_mfg_eb_on(struct mtk_mfg *mfg)
+{
+	struct device *dev = &mfg->pdev->dev;
+	u32 val;
+	int ret;
+
+	/*
+	 * If MFG is already on from e.g. the bootloader, skip doing the
+	 * power-on sequence, as it wouldn't work without powering it off first.
+	 */
+	if (mtk_mfg_is_powered_on(mfg))
+		return 0;
+
+	ret = readl_poll_timeout(mfg->rpc + RPC_GHPM_RO0_CON, val,
+				 !(val & (GHPM_PWR_STATE_M | GHPM_STATE_M)),
+				 GPUEB_POLL_US, GPUEB_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "timed out waiting for EB to power on\n");
+		return ret;
+	}
+
+	mtk_mfg_update_reg_bits(mfg->rpc + mfg->ghpm_en_reg, GHPM_ENABLE_M,
+				GHPM_ENABLE_M);
+
+	mtk_mfg_update_reg_bits(mfg->rpc + RPC_GHPM_CFG0_CON, GHPM_ON_SEQ_M, 0);
+	mtk_mfg_update_reg_bits(mfg->rpc + RPC_GHPM_CFG0_CON, GHPM_ON_SEQ_M,
+				GHPM_ON_SEQ_M);
+
+	mtk_mfg_update_reg_bits(mfg->rpc + mfg->ghpm_en_reg, GHPM_ENABLE_M, 0);
+
+
+	ret = readl_poll_timeout(mfg->rpc + RPC_PWR_CON, val,
+				 (val & PWR_ACK_M) == PWR_ACK_M,
+				 GPUEB_POLL_US, GPUEB_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "timed out waiting for EB power ack, val = 0x%X\n",
+			val);
+		return ret;
+	}
+
+	ret = readl_poll_timeout(mfg->gpr + GPR_LP_STATE, val,
+				 (val == EB_ON_RESUME),
+				 GPUEB_POLL_US, GPUEB_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "timed out waiting for EB to resume, status = 0x%X\n", val);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mtk_mfg_eb_off(struct mtk_mfg *mfg)
+{
+	struct device *dev = &mfg->pdev->dev;
+	struct mtk_mfg_ipi_sleep_msg msg = {
+		.event = 0,
+		.state = 0,
+		.magic = GPUEB_SLEEP_MAGIC
+	};
+	u32 val;
+	int ret;
+
+	ret = mbox_send_message(mfg->slp_mbox->ch, &msg);
+	if (ret < 0) {
+		dev_err(dev, "Cannot send sleep command: %pe\n", ERR_PTR(ret));
+		return ret;
+	}
+
+	ret = readl_poll_timeout(mfg->rpc + RPC_PWR_CON, val,
+				 !(val & PWR_ACK_M), GPUEB_POLL_US,
+				 GPUEB_TIMEOUT_US);
+
+	if (ret) {
+		dev_err(dev, "Timed out waiting for EB to power off, val=0x%08X\n", val);
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * mtk_mfg_send_ipi - synchronously send an IPI message on the gpufreq channel
+ * @mfg: pointer to this driver instance's private &struct mtk_mfg
+ * @msg: pointer to a message to send; will have magic filled and response assigned
+ *
+ * Send an IPI message on the gpufreq channel, and wait for a response. Once a
+ * response is received, assign a pointer to the response buffer (valid until
+ * next response is received) to @msg.
+ *
+ * Returns 0 on success, negative errno on failure.
+ */
+static int mtk_mfg_send_ipi(struct mtk_mfg *mfg, struct mtk_mfg_ipi_msg *msg)
+{
+	struct device *dev = &mfg->pdev->dev;
+	unsigned long wait;
+	int ret;
+
+	msg->magic = mfg->ipi_magic;
+
+	ret = mbox_send_message(mfg->gf_mbox->ch, msg);
+	if (ret < 0) {
+		dev_err(dev, "Cannot send GPUFreq IPI command: %pe\n", ERR_PTR(ret));
+		return ret;
+	}
+
+	wait = wait_for_completion_timeout(&mfg->gf_mbox->rx_done, msecs_to_jiffies(500));
+	if (!wait)
+		return -ETIMEDOUT;
+
+	msg = mfg->gf_mbox->rx_data;
+
+	if (msg->u.return_value < 0) {
+		dev_err(dev, "IPI return: %d\n", msg->u.return_value);
+		return -EPROTO;
+	}
+
+	return 0;
+}
+
+static int mtk_mfg_init_shared_mem(struct mtk_mfg *mfg)
+{
+	struct device *dev = &mfg->pdev->dev;
+	struct mtk_mfg_ipi_msg msg = {};
+	int ret;
+
+	dev_dbg(dev, "clearing GPUEB shared memory, 0x%X bytes\n", mfg->shared_mem_size);
+	memset_io(mfg->shared_mem, 0, mfg->shared_mem_size);
+
+	msg.cmd = CMD_INIT_SHARED_MEM;
+	msg.u.shared_mem.base = mfg->shared_mem_phys;
+	msg.u.shared_mem.size = mfg->shared_mem_size;
+
+	ret = mtk_mfg_send_ipi(mfg, &msg);
+	if (ret)
+		return ret;
+
+	if (readl(mfg->shared_mem + GF_REG_MAGIC) != GPUEB_MEM_MAGIC) {
+		dev_err(dev, "EB did not initialise shared memory correctly\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int mtk_mfg_power_control(struct mtk_mfg *mfg, bool enabled)
+{
+	struct mtk_mfg_ipi_msg msg = {};
+
+	msg.cmd = CMD_POWER_CONTROL;
+	msg.u.power_state = enabled ? 1 : 0;
+
+	return mtk_mfg_send_ipi(mfg, &msg);
+}
+
+static int mtk_mfg_set_oppidx(struct mtk_mfg *mfg, unsigned int opp_idx)
+{
+	struct mtk_mfg_ipi_msg msg = {};
+	int ret;
+
+	if (opp_idx >= mfg->num_gpu_opps)
+		return -EINVAL;
+
+	msg.cmd = CMD_FIX_DUAL_TARGET_OPPIDX;
+	msg.u.dual_commit.gpu_oppidx = opp_idx;
+	msg.u.dual_commit.stack_oppidx = opp_idx;
+
+	ret = mtk_mfg_send_ipi(mfg, &msg);
+	if (ret) {
+		dev_err(&mfg->pdev->dev, "Failed to set OPP %u: %pe\n",
+			opp_idx, ERR_PTR(ret));
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mtk_mfg_read_opp_tables(struct mtk_mfg *mfg)
+{
+	struct device *dev = &mfg->pdev->dev;
+	struct mtk_mfg_opp_entry e = {};
+	unsigned int i;
+
+	mfg->num_gpu_opps = readl(mfg->shared_mem + GF_REG_GPU_OPP_NUM);
+	mfg->num_stack_opps = readl(mfg->shared_mem + GF_REG_STK_OPP_NUM);
+
+	if (mfg->num_gpu_opps > MAX_OPP_NUM || mfg->num_gpu_opps == 0) {
+		dev_err(dev, "GPU OPP count (%u) out of range %u >= count > 0\n",
+			mfg->num_gpu_opps, MAX_OPP_NUM);
+		return -EINVAL;
+	}
+
+	if (mfg->num_stack_opps && mfg->num_stack_opps > MAX_OPP_NUM) {
+		dev_err(dev, "Stack OPP count (%u) out of range %u >= count >= 0\n",
+			mfg->num_stack_opps, MAX_OPP_NUM);
+		return -EINVAL;
+	}
+
+	mfg->gpu_opps = devm_kcalloc(dev, mfg->num_gpu_opps,
+				     sizeof(struct dev_pm_opp_data), GFP_KERNEL);
+	if (!mfg->gpu_opps)
+		return -ENOMEM;
+
+	if (mfg->num_stack_opps) {
+		mfg->stack_opps = devm_kcalloc(dev, mfg->num_stack_opps,
+					       sizeof(struct dev_pm_opp_data), GFP_KERNEL);
+		if (!mfg->stack_opps)
+			return -ENOMEM;
+	}
+
+	for (i = 0; i < mfg->num_gpu_opps; i++) {
+		memcpy_fromio(&e, mfg->shared_mem + GF_REG_OPP_TABLE_GPU + i * sizeof(e),
+			      sizeof(e));
+		if (mem_is_zero(&e, sizeof(e))) {
+			dev_err(dev, "ran into an empty GPU OPP at index %u\n",
+				i);
+			return -EINVAL;
+		}
+		mfg->gpu_opps[i].freq = e.freq_khz * HZ_PER_KHZ;
+		mfg->gpu_opps[i].u_volt = e.voltage_core * 10;
+		mfg->gpu_opps[i].level = i;
+		if (i < mfg->variant->turbo_below)
+			mfg->gpu_opps[i].turbo = true;
+	}
+
+	for (i = 0; i < mfg->num_stack_opps; i++) {
+		memcpy_fromio(&e, mfg->shared_mem + GF_REG_OPP_TABLE_STK + i * sizeof(e),
+			      sizeof(e));
+		if (mem_is_zero(&e, sizeof(e))) {
+			dev_err(dev, "ran into an empty Stack OPP at index %u\n",
+				i);
+			return -EINVAL;
+		}
+		mfg->stack_opps[i].freq = e.freq_khz * HZ_PER_KHZ;
+		mfg->stack_opps[i].u_volt = e.voltage_core * 10;
+		mfg->stack_opps[i].level = i;
+		if (i < mfg->variant->turbo_below)
+			mfg->stack_opps[i].turbo = true;
+	}
+
+	return 0;
+}
+
+static const char *const mtk_mfg_mt8196_clk_names[] = {
+	"core",
+	"stack0",
+	"stack1",
+};
+
+static const char *const mtk_mfg_mt8196_regulators[] = {
+	"core",
+	"stack",
+	"sram",
+};
+
+static int mtk_mfg_mt8196_init(struct mtk_mfg *mfg)
+{
+	void __iomem *e2_base;
+
+	e2_base = devm_platform_ioremap_resource_byname(mfg->pdev, "hw-revision");
+	if (IS_ERR(e2_base))
+		return dev_err_probe(&mfg->pdev->dev, PTR_ERR(e2_base),
+				     "Couldn't get hw-revision register\n");
+
+	clk_prepare_enable(mfg->clk_eb);
+
+	if (readl(e2_base) == MFG_MT8196_E2_ID)
+		mfg->ghpm_en_reg = RPC_DUMMY_REG_2;
+	else
+		mfg->ghpm_en_reg = RPC_GHPM_CFG0_CON;
+
+	clk_disable_unprepare(mfg->clk_eb);
+
+	return 0;
+}
+
+static const struct mtk_mfg_variant mtk_mfg_mt8196_variant = {
+	.clk_names = mtk_mfg_mt8196_clk_names,
+	.num_clks = ARRAY_SIZE(mtk_mfg_mt8196_clk_names),
+	.regulator_names = mtk_mfg_mt8196_regulators,
+	.num_regulators = ARRAY_SIZE(mtk_mfg_mt8196_regulators),
+	.turbo_below = 7,
+	.init = mtk_mfg_mt8196_init,
+};
+
+static void mtk_mfg_mbox_rx_callback(struct mbox_client *cl, void *mssg)
+{
+	struct mtk_mfg_mbox *mb = container_of(cl, struct mtk_mfg_mbox, cl);
+
+	if (mb->rx_data)
+		mb->rx_data = memcpy(mb->rx_data, mssg, GPUEB_MBOX_MAX_RX_SIZE);
+	complete(&mb->rx_done);
+}
+
+static int mtk_mfg_attach_dev(struct generic_pm_domain *pd, struct device *dev)
+{
+	struct mtk_mfg *mfg = mtk_mfg_from_genpd(pd);
+	struct dev_pm_opp_data *so = mfg->stack_opps;
+	struct dev_pm_opp_data *go = mfg->gpu_opps;
+	struct dev_pm_opp_data *prev_o;
+	struct dev_pm_opp_data *o;
+	int i, ret;
+
+	for (i = mfg->num_gpu_opps - 1; i >= 0; i--) {
+		/*
+		 * Adding the lower of the two OPPs avoids gaps of indices in
+		 * situations where the GPU OPPs are duplicated a couple of
+		 * times when only the Stack OPP is being lowered at that index.
+		 */
+		if (i >= mfg->num_stack_opps || go[i].freq < so[i].freq)
+			o = &go[i];
+		else
+			o = &so[i];
+
+		/*
+		 * Skip indices where both GPU and Stack OPPs are equal. Nominally,
+		 * OPP core shouldn't care about dupes, but not doing so will cause
+		 * dev_pm_opp_find_freq_ceil_indexed to -ERANGE later down the line.
+		 */
+		if (prev_o && prev_o->freq == o->freq)
+			continue;
+
+		ret = dev_pm_opp_add_dynamic(dev, o);
+		if (ret) {
+			dev_err(dev, "Failed to add OPP level %u from PD %s: %pe\n",
+				o->level, pd->name, ERR_PTR(ret));
+			dev_pm_opp_remove_all_dynamic(dev);
+			return ret;
+		}
+		prev_o = o;
+	}
+
+	return 0;
+}
+
+static void mtk_mfg_detach_dev(struct generic_pm_domain *pd, struct device *dev)
+{
+	dev_pm_opp_remove_all_dynamic(dev);
+}
+
+static int mtk_mfg_set_performance(struct generic_pm_domain *pd,
+				   unsigned int state)
+{
+	struct mtk_mfg *mfg = mtk_mfg_from_genpd(pd);
+
+	/*
+	 * pmdomain core intentionally sets a performance state before turning
+	 * a domain on, and after turning it off. For the GPUEB however, it's
+	 * only possible to act on performance requests when the GPUEB is
+	 * powered on. To do this, return cleanly without taking action, and
+	 * defer setting what pmdomain core set in mtk_mfg_power_on.
+	 */
+	if (mfg->pd.status != GENPD_STATE_ON)
+		return 0;
+
+	return mtk_mfg_set_oppidx(mfg, state);
+}
+
+static int mtk_mfg_power_on(struct generic_pm_domain *pd)
+{
+	struct mtk_mfg *mfg = mtk_mfg_from_genpd(pd);
+	int ret;
+
+	ret = regulator_bulk_enable(mfg->variant->num_regulators,
+				    mfg->gpu_regs);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(mfg->clk_eb);
+	if (ret)
+		goto err_disable_regulators;
+
+	ret = clk_bulk_prepare_enable(mfg->variant->num_clks, mfg->gpu_clks);
+	if (ret)
+		goto err_disable_eb_clk;
+
+	ret = mtk_mfg_eb_on(mfg);
+	if (ret)
+		goto err_disable_clks;
+
+	mfg->ipi_magic = readl(mfg->gpr + GPR_IPI_MAGIC);
+
+	ret = mtk_mfg_power_control(mfg, true);
+	if (ret)
+		goto err_eb_off;
+
+	/* Don't try to set a OPP in probe before OPPs have been read from EB */
+	if (mfg->gpu_opps) {
+		/* The aforementioned deferred setting of pmdomain's state */
+		ret = mtk_mfg_set_oppidx(mfg, pd->performance_state);
+		if (ret)
+			dev_warn(&mfg->pdev->dev, "Failed to set oppidx in %s\n", __func__);
+	}
+
+	return 0;
+
+err_eb_off:
+	mtk_mfg_eb_off(mfg);
+err_disable_clks:
+	clk_bulk_disable_unprepare(mfg->variant->num_clks, mfg->gpu_clks);
+err_disable_eb_clk:
+	clk_disable_unprepare(mfg->clk_eb);
+err_disable_regulators:
+	regulator_bulk_disable(mfg->variant->num_regulators, mfg->gpu_regs);
+
+	return ret;
+}
+
+static int mtk_mfg_power_off(struct generic_pm_domain *pd)
+{
+	struct mtk_mfg *mfg = mtk_mfg_from_genpd(pd);
+	struct device *dev = &mfg->pdev->dev;
+	int ret;
+
+	ret = mtk_mfg_power_control(mfg, false);
+	if (ret) {
+		dev_err(dev, "power_control failed: %pe\n", ERR_PTR(ret));
+		return ret;
+	}
+
+	ret = mtk_mfg_eb_off(mfg);
+	if (ret) {
+		dev_err(dev, "eb_off failed: %pe\n", ERR_PTR(ret));
+		return ret;
+	}
+
+	clk_bulk_disable_unprepare(mfg->variant->num_clks, mfg->gpu_clks);
+	clk_disable_unprepare(mfg->clk_eb);
+	ret = regulator_bulk_disable(mfg->variant->num_regulators, mfg->gpu_regs);
+	if (ret) {
+		dev_err(dev, "Disabling regulators failed: %pe\n", ERR_PTR(ret));
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mtk_mfg_init_mbox(struct mtk_mfg *mfg)
+{
+	struct device *dev = &mfg->pdev->dev;
+	struct mtk_mfg_mbox *gf;
+	struct mtk_mfg_mbox *slp;
+
+	gf = devm_kzalloc(dev, sizeof(*gf), GFP_KERNEL);
+	if (!gf)
+		return -ENOMEM;
+
+	gf->rx_data = devm_kzalloc(dev, GPUEB_MBOX_MAX_RX_SIZE, GFP_KERNEL);
+	if (!gf->rx_data)
+		return -ENOMEM;
+
+	gf->mfg = mfg;
+	init_completion(&gf->rx_done);
+	gf->cl.dev = dev;
+	gf->cl.rx_callback = mtk_mfg_mbox_rx_callback;
+	gf->cl.tx_tout = GPUEB_TIMEOUT_US / USEC_PER_MSEC;
+	gf->ch = mbox_request_channel_byname(&gf->cl, "gpufreq");
+	if (IS_ERR(gf->ch))
+		return PTR_ERR(gf->ch);
+
+	mfg->gf_mbox = gf;
+
+	slp = devm_kzalloc(dev, sizeof(*slp), GFP_KERNEL);
+	if (!slp)
+		return -ENOMEM;
+
+	slp->mfg = mfg;
+	init_completion(&slp->rx_done);
+	slp->cl.dev = dev;
+	slp->cl.tx_tout = GPUEB_TIMEOUT_US / USEC_PER_MSEC;
+	slp->cl.tx_block = true;
+	slp->ch = mbox_request_channel_byname(&slp->cl, "sleep");
+	if (IS_ERR(slp->ch)) {
+		mbox_free_channel(gf->ch);
+		return PTR_ERR(slp->ch);
+	}
+
+	mfg->slp_mbox = slp;
+
+	return 0;
+}
+
+static int mtk_mfg_init_clk_provider(struct mtk_mfg *mfg)
+{
+	struct device *dev = &mfg->pdev->dev;
+	struct clk_hw_onecell_data *clk_data;
+	int ret;
+
+	clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, 2), GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	clk_data->num = 2;
+
+	mfg->clk_core_hw.init = &mtk_mfg_clk_gpu_init;
+	mfg->clk_stack_hw.init = &mtk_mfg_clk_stack_init;
+
+	ret = devm_clk_hw_register(dev, &mfg->clk_core_hw);
+	if (ret)
+		return dev_err_probe(dev, ret, "Couldn't register GPU core clock\n");
+
+	ret = devm_clk_hw_register(dev, &mfg->clk_stack_hw);
+	if (ret)
+		return dev_err_probe(dev, ret, "Couldn't register GPU stack clock\n");
+
+	clk_data->hws[0] = &mfg->clk_core_hw;
+	clk_data->hws[1] = &mfg->clk_stack_hw;
+
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
+	if (ret)
+		return dev_err_probe(dev, ret, "Couldn't register clock provider\n");
+
+	return 0;
+}
+
+static int mtk_mfg_probe(struct platform_device *pdev)
+{
+	struct mtk_mfg *mfg;
+	struct device *dev = &pdev->dev;
+	const struct mtk_mfg_variant *data = of_device_get_match_data(dev);
+	struct resource res;
+	int ret, i;
+
+	mfg = devm_kzalloc(dev, sizeof(*mfg), GFP_KERNEL);
+	if (!mfg)
+		return -ENOMEM;
+
+	mfg->pdev = pdev;
+	mfg->variant = data;
+
+	dev_set_drvdata(dev, mfg);
+
+	mfg->gpr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(mfg->gpr))
+		return dev_err_probe(dev, PTR_ERR(mfg->gpr),
+				     "Couldn't retrieve GPR MMIO registers\n");
+
+	mfg->rpc = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(mfg->rpc))
+		return dev_err_probe(dev, PTR_ERR(mfg->rpc),
+				     "Couldn't retrieve RPC MMIO registers\n");
+
+	mfg->clk_eb = devm_clk_get(dev, "eb");
+	if (IS_ERR(mfg->clk_eb))
+		return dev_err_probe(dev, PTR_ERR(mfg->clk_eb),
+				     "Couldn't get 'eb' clock\n");
+
+	mfg->gpu_clks = devm_kcalloc(dev, data->num_clks, sizeof(*mfg->gpu_clks),
+				     GFP_KERNEL);
+	if (!mfg->gpu_clks)
+		return -ENOMEM;
+
+	for (i = 0; i < data->num_clks; i++)
+		mfg->gpu_clks[i].id = data->clk_names[i];
+
+	ret = devm_clk_bulk_get(dev, data->num_clks, mfg->gpu_clks);
+	if (ret)
+		return dev_err_probe(dev, ret, "Couldn't get GPU clocks\n");
+
+	mfg->gpu_regs = devm_kcalloc(dev, data->num_regulators,
+				     sizeof(*mfg->gpu_regs), GFP_KERNEL);
+	if (!mfg->gpu_regs)
+		return -ENOMEM;
+
+	for (i = 0; i < data->num_regulators; i++)
+		mfg->gpu_regs[i].supply = data->regulator_names[i];
+
+	ret = devm_regulator_bulk_get(dev, data->num_regulators, mfg->gpu_regs);
+	if (ret)
+		return dev_err_probe(dev, ret, "Couldn't get GPU regulators\n");
+
+	ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
+	if (ret)
+		return dev_err_probe(dev, ret, "Couldn't get GPUEB shared memory\n");
+
+	mfg->shared_mem = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!mfg->shared_mem)
+		return dev_err_probe(dev, -ENOMEM, "Can't ioremap GPUEB shared memory\n");
+	mfg->shared_mem_size = resource_size(&res);
+	mfg->shared_mem_phys = res.start;
+
+	if (data->init) {
+		ret = data->init(mfg);
+		if (ret)
+			return dev_err_probe(dev, ret, "Variant init failed\n");
+	}
+
+	mfg->pd.name = dev_name(dev);
+	mfg->pd.attach_dev = mtk_mfg_attach_dev;
+	mfg->pd.detach_dev = mtk_mfg_detach_dev;
+	mfg->pd.power_off = mtk_mfg_power_off;
+	mfg->pd.power_on = mtk_mfg_power_on;
+	mfg->pd.set_performance_state = mtk_mfg_set_performance;
+	mfg->pd.flags = GENPD_FLAG_OPP_TABLE_FW;
+
+	ret = pm_genpd_init(&mfg->pd, NULL, false);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialise power domain\n");
+
+	ret = mtk_mfg_init_mbox(mfg);
+	if (ret) {
+		dev_err_probe(dev, ret, "Couldn't initialise mailbox\n");
+		goto err_remove_genpd;
+	}
+
+	ret = mtk_mfg_power_on(&mfg->pd);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to power on MFG\n");
+		goto err_free_mbox;
+	}
+
+	ret = mtk_mfg_init_shared_mem(mfg);
+	if (ret) {
+		dev_err_probe(dev, ret, "Couldn't initialize EB shared memory\n");
+		goto err_power_off;
+	}
+
+	ret = mtk_mfg_read_opp_tables(mfg);
+	if (ret) {
+		dev_err_probe(dev, ret, "Error reading OPP tables from EB\n");
+		goto err_power_off;
+	}
+
+	ret = mtk_mfg_init_clk_provider(mfg);
+	if (ret)
+		goto err_power_off;
+
+	ret = of_genpd_add_provider_simple(dev->of_node, &mfg->pd);
+	if (ret) {
+		dev_err_probe(dev, ret, "Failed to add pmdomain provider\n");
+		goto err_power_off;
+	}
+
+	return 0;
+
+err_power_off:
+	mtk_mfg_power_off(&mfg->pd);
+err_free_mbox:
+	mbox_free_channel(mfg->slp_mbox->ch);
+	mfg->slp_mbox->ch = NULL;
+	mbox_free_channel(mfg->gf_mbox->ch);
+	mfg->gf_mbox->ch = NULL;
+err_remove_genpd:
+	pm_genpd_remove(&mfg->pd);
+
+	return ret;
+}
+
+static const struct of_device_id mtk_mfg_of_match[] = {
+	{ .compatible = "mediatek,mt8196-gpufreq", .data = &mtk_mfg_mt8196_variant },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mtk_mfg_of_match);
+
+static void mtk_mfg_remove(struct platform_device *pdev)
+{
+	struct mtk_mfg *mfg = dev_get_drvdata(&pdev->dev);
+
+	if (mtk_mfg_is_powered_on(mfg))
+		mtk_mfg_power_off(&mfg->pd);
+
+	of_genpd_del_provider(pdev->dev.of_node);
+	pm_genpd_remove(&mfg->pd);
+
+	mbox_free_channel(mfg->gf_mbox->ch);
+	mfg->gf_mbox->ch = NULL;
+
+	mbox_free_channel(mfg->slp_mbox->ch);
+	mfg->slp_mbox->ch = NULL;
+}
+
+static struct platform_driver mtk_mfg_driver = {
+	.driver = {
+		.name = "mtk-mfg-pmdomain",
+		.of_match_table = mtk_mfg_of_match,
+		.suppress_bind_attrs = true,
+	},
+	.probe = mtk_mfg_probe,
+	.remove = mtk_mfg_remove,
+};
+module_platform_driver(mtk_mfg_driver);
+
+MODULE_AUTHOR("Nicolas Frattaroli <nicolas.frattaroli@collabora.com>");
+MODULE_DESCRIPTION("MediaTek MFlexGraphics Power Domain Driver");
+MODULE_LICENSE("GPL");

-- 
2.51.0

Re: [PATCH v8 3/5] drm/panthor: call into devfreq for current frequency

[Karunika Choo]

On 17/10/2025 16:31, Nicolas Frattaroli wrote:
> As it stands, panthor keeps a cached current frequency value for when it
> wants to retrieve it. This doesn't work well for when things might
> switch frequency without panthor's knowledge.
> 
> Instead, implement the get_cur_freq operation, and expose it through a
> helper function to the rest of panthor.
> 
> Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> Reviewed-by: Steven Price <steven.price@arm.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_devfreq.c | 30 ++++++++++++++++++++++++++----
>  drivers/gpu/drm/panthor/panthor_devfreq.h |  2 ++
>  drivers/gpu/drm/panthor/panthor_device.h  |  3 ---
>  drivers/gpu/drm/panthor/panthor_drv.c     |  4 +++-
>  4 files changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
> index 2df1d76d84a0..a6dca599f0a5 100644
> --- a/drivers/gpu/drm/panthor/panthor_devfreq.c
> +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
> @@ -62,7 +62,6 @@ static void panthor_devfreq_update_utilization(struct panthor_devfreq *pdevfreq)
>  static int panthor_devfreq_target(struct device *dev, unsigned long *freq,
>  				  u32 flags)
>  {
> -	struct panthor_device *ptdev = dev_get_drvdata(dev);
>  	struct dev_pm_opp *opp;
>  	int err;
>  
> @@ -72,8 +71,6 @@ static int panthor_devfreq_target(struct device *dev, unsigned long *freq,
>  	dev_pm_opp_put(opp);
>  
>  	err = dev_pm_opp_set_rate(dev, *freq);
> -	if (!err)
> -		ptdev->current_frequency = *freq;
>  
>  	return err;
>  }
> @@ -115,11 +112,21 @@ static int panthor_devfreq_get_dev_status(struct device *dev,
>  	return 0;
>  }
>  
> +static int panthor_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
> +{
> +	struct panthor_device *ptdev = dev_get_drvdata(dev);
> +
> +	*freq = clk_get_rate(ptdev->clks.core);
> +
> +	return 0;
> +}
> +
>  static struct devfreq_dev_profile panthor_devfreq_profile = {
>  	.timer = DEVFREQ_TIMER_DELAYED,
>  	.polling_ms = 50, /* ~3 frames */
>  	.target = panthor_devfreq_target,
>  	.get_dev_status = panthor_devfreq_get_dev_status,
> +	.get_cur_freq = panthor_devfreq_get_cur_freq,
>  };
>  
>  int panthor_devfreq_init(struct panthor_device *ptdev)
> @@ -197,7 +204,6 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
>  		return PTR_ERR(opp);
>  
>  	panthor_devfreq_profile.initial_freq = cur_freq;
> -	ptdev->current_frequency = cur_freq;
>  
>  	/*
>  	 * Set the recommend OPP this will enable and configure the regulator
> @@ -295,3 +301,19 @@ void panthor_devfreq_record_idle(struct panthor_device *ptdev)
>  
>  	spin_unlock_irqrestore(&pdevfreq->lock, irqflags);
>  }
> +
> +unsigned long panthor_devfreq_get_freq(struct panthor_device *ptdev)
> +{
> +	struct panthor_devfreq *pdevfreq = ptdev->devfreq;
> +	unsigned long freq = 0;
> +	int ret;
> +
> +	if (!pdevfreq->devfreq)
> +		return 0;
> +
> +	ret = pdevfreq->devfreq->profile->get_cur_freq(ptdev->base.dev, &freq);
> +	if (ret)
> +		return 0;
> +
> +	return freq;
> +}
> diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.h b/drivers/gpu/drm/panthor/panthor_devfreq.h
> index b7631de695f7..f8e29e02f66c 100644
> --- a/drivers/gpu/drm/panthor/panthor_devfreq.h
> +++ b/drivers/gpu/drm/panthor/panthor_devfreq.h
> @@ -18,4 +18,6 @@ void panthor_devfreq_suspend(struct panthor_device *ptdev);
>  void panthor_devfreq_record_busy(struct panthor_device *ptdev);
>  void panthor_devfreq_record_idle(struct panthor_device *ptdev);
>  
> +unsigned long panthor_devfreq_get_freq(struct panthor_device *ptdev);
> +
>  #endif /* __PANTHOR_DEVFREQ_H__ */
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index 9f0649ecfc4f..f32c1868bf6d 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -214,9 +214,6 @@ struct panthor_device {
>  	/** @profile_mask: User-set profiling flags for job accounting. */
>  	u32 profile_mask;
>  
> -	/** @current_frequency: Device clock frequency at present. Set by DVFS*/
> -	unsigned long current_frequency;
> -
>  	/** @fast_rate: Maximum device clock frequency. Set by DVFS */
>  	unsigned long fast_rate;
>  
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index fb4b293f17f0..75898d83a207 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -25,6 +25,7 @@
>  #include <drm/gpu_scheduler.h>
>  #include <drm/panthor_drm.h>
>  
> +#include "panthor_devfreq.h"
>  #include "panthor_device.h"
>  #include "panthor_fw.h"
>  #include "panthor_gem.h"
> @@ -1519,7 +1520,8 @@ static void panthor_gpu_show_fdinfo(struct panthor_device *ptdev,
>  		drm_printf(p, "drm-cycles-panthor:\t%llu\n", pfile->stats.cycles);
>  
>  	drm_printf(p, "drm-maxfreq-panthor:\t%lu Hz\n", ptdev->fast_rate);
> -	drm_printf(p, "drm-curfreq-panthor:\t%lu Hz\n", ptdev->current_frequency);
> +	drm_printf(p, "drm-curfreq-panthor:\t%lu Hz\n",
> +		   panthor_devfreq_get_freq(ptdev));
>  }
>  
>  static void panthor_show_internal_memory_stats(struct drm_printer *p, struct drm_file *file)
> 

Reviewed-by: Karunika Choo <karunika.choo@arm.com>

Re: [PATCH v8 4/5] drm/panthor: Use existing OPP table if present

[Karunika Choo]

On 17/10/2025 16:31, Nicolas Frattaroli wrote:
> On SoCs where the GPU's power-domain is in charge of setting performance
> levels, the OPP table of the GPU node will have already been populated
> during said power-domain's attach_dev operation.
> 
> To avoid initialising an OPP table twice, only set the OPP regulator and
> the OPPs from DT if there's no OPP table present.
> 
> Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_devfreq.c | 32 ++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
> index a6dca599f0a5..ec63e27f4883 100644
> --- a/drivers/gpu/drm/panthor/panthor_devfreq.c
> +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
> @@ -141,6 +141,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
>  	struct thermal_cooling_device *cooling;
>  	struct device *dev = ptdev->base.dev;
>  	struct panthor_devfreq *pdevfreq;
> +	struct opp_table *table;
>  	struct dev_pm_opp *opp;
>  	unsigned long cur_freq;
>  	unsigned long freq = ULONG_MAX;
> @@ -152,17 +153,30 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
>  
>  	ptdev->devfreq = pdevfreq;
>  
> -	ret = devm_pm_opp_set_regulators(dev, reg_names);
> -	if (ret && ret != -ENODEV) {
> -		if (ret != -EPROBE_DEFER)
> -			DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
> -		return ret;
> +	/*
> +	 * The power domain associated with the GPU may have already added an
> +	 * OPP table, complete with OPPs, as part of the platform bus
> +	 * initialization. If this is the case, the power domain is in charge of
> +	 * also controlling the performance, with a set_performance callback.
> +	 * Only add a new OPP table from DT if there isn't such a table present
> +	 * already.
> +	 */
> +	table = dev_pm_opp_get_opp_table(dev);
> +	if (IS_ERR_OR_NULL(table)) {
> +		ret = devm_pm_opp_set_regulators(dev, reg_names);
> +		if (ret && ret != -ENODEV) {

Is there a reason to not fail on -ENODEV? I would assume it is a valid 
failure path. 

Kind regards,
Karunika Choo

> +			if (ret != -EPROBE_DEFER)
> +				DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
> +			return ret;
> +		}
> +
> +		ret = devm_pm_opp_of_add_table(dev);
> +		if (ret)
> +			return ret;
> +	} else {
> +		dev_pm_opp_put_opp_table(table);
>  	}
>  
> -	ret = devm_pm_opp_of_add_table(dev);
> -	if (ret)
> -		return ret;
> -
>  	spin_lock_init(&pdevfreq->lock);
>  
>  	panthor_devfreq_reset(pdevfreq);
> 

Re: [PATCH v8 4/5] drm/panthor: Use existing OPP table if present

[Nicolas Frattaroli]

On Monday, 20 October 2025 10:35:04 Central European Summer Time Karunika Choo wrote:
> On 17/10/2025 16:31, Nicolas Frattaroli wrote:
> > On SoCs where the GPU's power-domain is in charge of setting performance
> > levels, the OPP table of the GPU node will have already been populated
> > during said power-domain's attach_dev operation.
> > 
> > To avoid initialising an OPP table twice, only set the OPP regulator and
> > the OPPs from DT if there's no OPP table present.
> > 
> > Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  drivers/gpu/drm/panthor/panthor_devfreq.c | 32 ++++++++++++++++++++++---------
> >  1 file changed, 23 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
> > index a6dca599f0a5..ec63e27f4883 100644
> > --- a/drivers/gpu/drm/panthor/panthor_devfreq.c
> > +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
> > @@ -141,6 +141,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
> >  	struct thermal_cooling_device *cooling;
> >  	struct device *dev = ptdev->base.dev;
> >  	struct panthor_devfreq *pdevfreq;
> > +	struct opp_table *table;
> >  	struct dev_pm_opp *opp;
> >  	unsigned long cur_freq;
> >  	unsigned long freq = ULONG_MAX;
> > @@ -152,17 +153,30 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
> >  
> >  	ptdev->devfreq = pdevfreq;
> >  
> > -	ret = devm_pm_opp_set_regulators(dev, reg_names);
> > -	if (ret && ret != -ENODEV) {
> > -		if (ret != -EPROBE_DEFER)
> > -			DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
> > -		return ret;
> > +	/*
> > +	 * The power domain associated with the GPU may have already added an
> > +	 * OPP table, complete with OPPs, as part of the platform bus
> > +	 * initialization. If this is the case, the power domain is in charge of
> > +	 * also controlling the performance, with a set_performance callback.
> > +	 * Only add a new OPP table from DT if there isn't such a table present
> > +	 * already.
> > +	 */
> > +	table = dev_pm_opp_get_opp_table(dev);
> > +	if (IS_ERR_OR_NULL(table)) {
> > +		ret = devm_pm_opp_set_regulators(dev, reg_names);
> > +		if (ret && ret != -ENODEV) {
> 
> Is there a reason to not fail on -ENODEV? I would assume it is a valid 
> failure path. 

Hi,

the -ENODEV logic wasn't added by me, it was added in
Commit: a8cb5ca53690 ("drm/panthor: skip regulator setup if no such prop")

with the justification

  The regulator is optional, skip the setup instead of returning an
  error if it is not present

I will not be changing anything about this logic in this patch set,
as it is not in scope for MT8196 enablement, since MT8196 does not
use this code path at all.

Kind regards,
Nicolas Frattaroli

> 
> Kind regards,
> Karunika Choo
> 
> > +			if (ret != -EPROBE_DEFER)
> > +				DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
> > +			return ret;
> > +		}
> > +
> > +		ret = devm_pm_opp_of_add_table(dev);
> > +		if (ret)
> > +			return ret;
> > +	} else {
> > +		dev_pm_opp_put_opp_table(table);
> >  	}
> >  
> > -	ret = devm_pm_opp_of_add_table(dev);
> > -	if (ret)
> > -		return ret;
> > -
> >  	spin_lock_init(&pdevfreq->lock);
> >  
> >  	panthor_devfreq_reset(pdevfreq);
> > 
> 
> 

Re: [PATCH v8 4/5] drm/panthor: Use existing OPP table if present

[Steven Price]

On 17/10/2025 16:31, Nicolas Frattaroli wrote:
> On SoCs where the GPU's power-domain is in charge of setting performance
> levels, the OPP table of the GPU node will have already been populated
> during said power-domain's attach_dev operation.
> 
> To avoid initialising an OPP table twice, only set the OPP regulator and
> the OPPs from DT if there's no OPP table present.
> 
> Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>  drivers/gpu/drm/panthor/panthor_devfreq.c | 32 ++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
> index a6dca599f0a5..ec63e27f4883 100644
> --- a/drivers/gpu/drm/panthor/panthor_devfreq.c
> +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
> @@ -141,6 +141,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
>  	struct thermal_cooling_device *cooling;
>  	struct device *dev = ptdev->base.dev;
>  	struct panthor_devfreq *pdevfreq;
> +	struct opp_table *table;
>  	struct dev_pm_opp *opp;
>  	unsigned long cur_freq;
>  	unsigned long freq = ULONG_MAX;
> @@ -152,17 +153,30 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
>  
>  	ptdev->devfreq = pdevfreq;
>  
> -	ret = devm_pm_opp_set_regulators(dev, reg_names);
> -	if (ret && ret != -ENODEV) {
> -		if (ret != -EPROBE_DEFER)
> -			DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
> -		return ret;
> +	/*
> +	 * The power domain associated with the GPU may have already added an
> +	 * OPP table, complete with OPPs, as part of the platform bus
> +	 * initialization. If this is the case, the power domain is in charge of
> +	 * also controlling the performance, with a set_performance callback.
> +	 * Only add a new OPP table from DT if there isn't such a table present
> +	 * already.
> +	 */
> +	table = dev_pm_opp_get_opp_table(dev);
> +	if (IS_ERR_OR_NULL(table)) {
> +		ret = devm_pm_opp_set_regulators(dev, reg_names);
> +		if (ret && ret != -ENODEV) {
> +			if (ret != -EPROBE_DEFER)
> +				DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n");
> +			return ret;
> +		}
> +
> +		ret = devm_pm_opp_of_add_table(dev);
> +		if (ret)
> +			return ret;
> +	} else {
> +		dev_pm_opp_put_opp_table(table);
>  	}
>  
> -	ret = devm_pm_opp_of_add_table(dev);
> -	if (ret)
> -		return ret;
> -
>  	spin_lock_init(&pdevfreq->lock);
>  
>  	panthor_devfreq_reset(pdevfreq);
> 

Re: [PATCH v8 0/5] MT8196 GPU Frequency/Power Control Support

[Ulf Hansson]

On Fri, 17 Oct 2025 at 17:32, Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
>
> This series introduces two new drivers to accomplish controlling the
> frequency and power of the Mali GPU on MediaTek MT8196 SoCs.
>
> The reason why it's not as straightforward as with other SoCs is that
> the MT8196 has quite complex glue logic in order to squeeze the maximum
> amount of performance possible out of the silicon. There's an additional
> MCU running a specialised firmware, which communicates with the
> application processor through a mailbox and some reserved memory, and is
> in charge of controlling the regulators, the PLL clocks, and the power
> gating of the GPU, all while also being in charge of any DVFS control.
>
> This set of drivers is enough to communicate desired OPP index limits to
> the aforementioned MCU, referred to as "GPUEB" from here on out. The
> GPUEB is still free to lower the effective frequency if the GPU has no
> jobs going on at all, even when a higher OPP is set.
>
> The power- and frequency control driver, mtk-mfg-pmdomain, is now
> implemented as a power domain driver, with a set_performance_state
> operation. It also exposes itself as a clock provider, so that panthor
> can read the actual achieved DVFS clock rate as per the GPUEB firmware.
>
> This power domain approach means that panthor does not need to know
> about how the frequency control works on this SoC, as the OPP core
> framework already takes care of it. The only exception is that panthor
> needs to not register OPPs from DT itself if there already is an OPP
> table present.
>
> The mailbox driver is a fairly bog-standard common mailbox framework
> driver, just specific to the firmware that runs on the GPUEB. It was
> merged in v6.18 already.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

This looks good to me!

I can certainly pick up patch2 and patch5, but before I go ahead I
just wanted to check what is the preferred merging strategy?

The drm/gpu patches can go independently from the pmdomain patches
others, right? In either case, I can pick this complete series too via
my pmdomain tree, if that makes sense for everyone. Please let me
know.

Kind regards
Uffe

> ---
> Changes in v8:
> - mtk-mfg-pmdomain: remove unused shmem variable that caused a warning
>   on GCC, but not clang
> - Link to v7: https://lore.kernel.org/r/20251015-mt8196-gpufreq-v7-0-0a6435da2080@collabora.com
>
> Changes in v7:
> - panthor: rename "t" to "table"
> - panthor: add code comment explaining why an existing OPP table is
>   being checked for
> - mtk-mfg-pmdomain: use GF_REG_MAGIC offset for sake of consistency
> - mtk-mfg-pmdomain: remove redundant semicolon after mtk_mfg_mt8196_init
> - mtk-mfg-pmdomain: fix resource leaks on probe failure
> - mtk-mfg-pmdomain: enable/disable EB clock during MT8196 init, which is
>   needed for the register read
> - Rebase onto next-20251014, which drops already merged patches, namely
>   mailbox driver+bindings, and drops the ASN_HASH patch series
>   dependency, which was also merged
> - Link to v6: https://lore.kernel.org/r/20251003-mt8196-gpufreq-v6-0-76498ad61d9e@collabora.com
>
> Changes in v6:
> - mailbox: move buf definition into if condition, as per Chia-I Wu
> - panthor: remove the redundant NULL checks in panthor_devfreq_get_freq
> - mtk-mfg-pmdomain: adjust return style consistency
> - mtk-mfg-pmdomain: add docstring for mtk_mfg_send_ipi to explain it's
>   blocking
> - mtk-mfg-pmdomain: use CMD_FIX_DUAL_TARGET_OPPIDX instead of
>   CMD_FIX_TARGET_OPPIDX.
> - mtk-mfg-pmdomain: reword code comments to not be in the "we" style
> - mtk-mfg-pmdomain: shuffle around mbox allocations as per Angelo
> - mtk-mfg-pmdomain: don't pointlessly turn on EB clock in probe,
>   reducing the need for a comment explaining the bookkeeping
> - mtk-mfg-pmdomain: consistently use dev_err_probe and Capitalise first
>   letter of error string
> - mtk-mfg-pmdomain: get rid of redundant ret = dev_err_probe assignment
> - mtk-mfg-pmdomain: reintroduce stack OPP table, choose min(gpu, stack)
>   when adding frequencies. Fixes gaps in OPP levels where only stack
>   changed, but gpu had duplicates, which resulted in choosing a too slow
>   OPP
> - mtk-mfg-pmdomain: stub round_rate clk op to opt out of CCF always
>   "rounding" a devfreq rate request to the current rate
> - Link to v5: https://lore.kernel.org/r/20250929-mt8196-gpufreq-v5-0-3056e5ecf765@collabora.com
>
> Changes in v5:
> - mtk-mfg-pmdomain binding: add memory-regions property, remove shmem
>   property, as we now correctly describe the shared memory as a regular
>   memory region
> - mtk-mfg-pmdomain binding: get rid of redundant |
> - drop "dt-bindings: sram: Add compatible for
>   mediatek,mt8196-gpufreq-sram" as part of the move to reserved memory
> - mtk-mfg-pmdomain: move to using reserved-memory for GPUEB shared
>   memory
> - mtk-mfg-pmdomain: demote some types to smaller sizes in struct
>   mtk_mfg, as per Angelo's suggestions
> - mtk-mfg-pmdomain: use units.h for Hz-to-KHz
> - mtk-mfg-pmdomain: change for loop in attach_dev to reduce indentation
> - mtk-mfg-pmdomain: simplify return in mtk_mfg_power_off
> - mtk-mfg-pmdomain: move of_device_id after probe
> - mtk_mfg_pmdomain: map mmio by index
> - mtk_mfg_pmdomain: add error checking to pm_genpd_init()
> - mtk_mfg_pmdomain: add remove function
> - mtk_mfg_pmdomain: remove last_opp member and logic, since OPP core
>   already does that for us
> - mtk_mfg_pmdomain: adjust comment in mtk_mfg_set_performance to explain
>   why we're doing what we're doing
> - mtk_mfg_pmdomain: call mtk_mfg_set_oppidx in mtk_mfg_power_on with
>   the performance_state we deferred setting while it was powered off
> - mtk_mfg_pmdomain: add inline function for PWR_ACK checking, as it's
>   now used twice with the added remove function
> - mtk-mfg-pmdomain: add suppress_bind_attrs so people don't play with
>   that
> - mtk-mfg-pmdomain: change KConfig from tristate to bool, as module
>   unloading results in strange likely firmware-induced hardware state
>   woes in the mali GPU
> - mtk-mfg-pmdomain: read IPI magic in power_on, don't zero it after
>   confirming that seemingly had no purpose
> - mtk-mfg-pmdomain: misc style changes
> - Link to v4: https://lore.kernel.org/r/20250923-mt8196-gpufreq-v4-0-6cd63ade73d6@collabora.com
>
> Changes in v4:
> - rebase onto next-20250922, which includes Laura Nao's clock patches
> - refactor mediatek_mfg into a pmdomain driver called "mtk-mfg-pmdomain"
> - move mt8196-gpufreq binding to the power subdirectory
> - mali-valhall-csf binding: adjust for power-domains usage
> - mali-valhall-csf binding: use clocks on mt8196
> - mailbox: prefix defines with "GPUEB_"
> - mailbox: get rid of custom of_xlate
> - mailbox: rename "CLOGGED" to "BLOCKED"
> - mailbox: adjust send_data comment to include more technical info
> - mailbox: misc style improvements
> - panthor: drop "drm/panthor: devfreq: make get_dev_status use
>   get_cur_freq", as it is now not necessary and makes the code worse
> - panthor: drop "drm/panthor: devfreq: add pluggable devfreq providers"
> - panthor: drop "drm/panthor: add no_clocks soc_data member for MT8196",
>   as we now have clocks courtesy of gpufreq
> - panthor: check for existing opp table before registering a new one
> - mtk-mfg-pmdomain: add turbo_below variant data, which marks OPPs below
>   a certain index as turbo for the OPP subsystem
> - mtk-mfg-pmdomain: no longer read stack OPPs, as they weren't used
> - mtk-mfg-pmdomain: get rid of num gpu opp != num stack opp check.
>   That's the firmware's problem should it ever happen, not ours
> - mtk-mfg-pmdomain: some small name and whitespace changes on the defines
> - Link to v3: https://lore.kernel.org/r/20250917-mt8196-gpufreq-v3-0-c4ede4b4399e@collabora.com
>
> Changes in v3:
> - mali-valhall-csf binding: get rid of clocks for MT8196, rebase onto
>   Chia-I Wu's patch
> - mt8196-gpufreq binding: rename hw_revision to hw-revision
> - mt8196-gpufreq binding: rename clocks
> - mt8196-gpufreq binding: drop pointless label in example
> - mailbox binding: drop pointless label in example
> - mailbox: whitespace changes on defines
> - mailbox: remove rx_buf member from channel struct, use stack buffer
> - mailbox: check in probe that no rx_len exceeds MBOX_MAX_RX_SIZE
> - panthor: add no_clocks SoC data patch, also rebase onto Chia-I Wu's
>   series
> - panthor: refactor devfreq provider functionality to do allocation and
>   initialisation of panthor_devfreq struct in panthor in all cases
> - panthor: drop the patch that moves struct panthor_devfreq to a header
>   file, as it no longer needs to be exposed to devfreq providers
> - mediatek_mfg: refactor devfreq provider functionality to decouple it
>   more from panthor itself
> - mediatek_mfg: move SRAM magic to a #define
> - mediatek_mfg: begrudgingly rename member "padding_lol" to "reserved"
> - mediatek_mfg: use local struct device pointer var in more places
> - mediatek_mfg: change wording of sleep command failure error message,
>   but keep the format specifier because I don't want to throw bare
>   errnos at users
> - mediatek_mfg: remove unnecessary braces around dev_err EB power off
>   timeout message
> - mediatek_mfg: allocate rx_data for channels that expect a response
> - mediatek_mfg: memcpy the rx buffer from the common mailbox framework
>   in the rx callback to rx_data, as mssg now points to stack memory
> - mediatek_mfg: make SRAM clearing message dev_dbg
> - mediatek_mfg: no longer print physical address of SRAM
> - mediatek_mfg: expand on the GF_REG_OPP_TABLE_STK comment, toning down
>   its defeatist attitude in the process
> - mediatek_mfg: style fixes in mtk_mfg_get_closest_opp_idx
> - mediatek_mfg: rename clocks and hw-revision reg as per binding
> - Link to v2: https://lore.kernel.org/r/20250912-mt8196-gpufreq-v2-0-779a8a3729d9@collabora.com
>
> Changes in v2:
> - mali-valhall-csf binding: move from performance-controller to
>   performance-domains property
> - mali-valhall-csf binding: fix vendor name oopsie in compatible of if
>   condition
> - mt8196-gpufreq binding: move from performance-controller to
>   performance-domains by adding the cells property
> - mt8196-gpufreq binding: rename e2_id to hw_revision
> - mt8196-gpufreq binding: add description that mentions "MediaTek
>   Flexible Graphics"
> - mt8196-gpufreq binding: get rid of mailbox channels we're unlikely to
>   use any time soon, if ever
> - mt8196-gpufreq binding: change name of mailbox channels to use -
>   instead of _
> - mailbox binding: change reg-names to "data" and "ctl"
> - drm/panthor: mediatek_mfg: rename e2_id to hw_revision
> - drm/panthor: devfreq: switch from performance-controller to
>   performance-domains
> - drm/panthor: devfreq: get rid of the accidental get_cur_freq function
>   move
> - mailbox: rename mtk_gpueb_mbox_ch to mtk_gpueb_mbox_chan_desc
> - mailbox: use smaller types in mtk_gpueb_mbox_chan_desc where possible
> - mailbox: add per-channel runtime data struct
> - mailbox: request one threaded IRQ per channel, pass channel struct as
>   data
> - mailbox: make num_channels in variant struct u8
> - mailbox: get rid of no_response, as it was redundant
> - mailbox: enable and disable clock in mailbox startup/shutdown
> - mailbox: point con_priv of mailbox framework channel struct to this
>   driver's channel struct
> - mailbox: request and free the threaded IRQ in startup/shutdown
> - mailbox: only clear IRQ bit flag once RX data has been read from MMIO
> - mailbox: reduce needlessly large receive buffer size
> - mailbox: handle allocation errors wherever they could pop up
> - mailbox: style cleanups in mtk_gpueb_mbox_read_rx
> - mailbox: call platform_get_irq earlier on in probe
> - mailbox: set drvdata later on in probe
> - mailbox: ioremap resources by index, not name
> - mailbox: handle devm_mbox_controller_register errors
> - mailbox: rename channels to correspond to bindings
> - mailbox: document a few of the private driver structs to be kind to
>   the next person who will look at this code
> - Link to v1: https://lore.kernel.org/r/20250905-mt8196-gpufreq-v1-0-7b6c2d6be221@collabora.com
>
> ---
> Nicolas Frattaroli (5):
>       dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant
>       dt-bindings: power: Add MT8196 GPU frequency control binding
>       drm/panthor: call into devfreq for current frequency
>       drm/panthor: Use existing OPP table if present
>       pmdomain: mediatek: Add support for MFlexGraphics
>
>  .../bindings/gpu/arm,mali-valhall-csf.yaml         |   37 +-
>  .../bindings/power/mediatek,mt8196-gpufreq.yaml    |  117 +++
>  drivers/gpu/drm/panthor/panthor_devfreq.c          |   62 +-
>  drivers/gpu/drm/panthor/panthor_devfreq.h          |    2 +
>  drivers/gpu/drm/panthor/panthor_device.h           |    3 -
>  drivers/gpu/drm/panthor/panthor_drv.c              |    4 +-
>  drivers/pmdomain/mediatek/Kconfig                  |   16 +
>  drivers/pmdomain/mediatek/Makefile                 |    1 +
>  drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c       | 1044 ++++++++++++++++++++
>  9 files changed, 1268 insertions(+), 18 deletions(-)
> ---
> base-commit: 3477f49ff0433a241da12ec9cecf6c9b2bd1c6f8
> change-id: 20250829-mt8196-gpufreq-a7645670d182
>
> Best regards,
> --
> Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>

Re: [PATCH v8 0/5] MT8196 GPU Frequency/Power Control Support

[Steven Price]

On 22/10/2025 14:52, Ulf Hansson wrote:
> On Fri, 17 Oct 2025 at 17:32, Nicolas Frattaroli
> <nicolas.frattaroli@collabora.com> wrote:
>>
>> This series introduces two new drivers to accomplish controlling the
>> frequency and power of the Mali GPU on MediaTek MT8196 SoCs.
>>
>> The reason why it's not as straightforward as with other SoCs is that
>> the MT8196 has quite complex glue logic in order to squeeze the maximum
>> amount of performance possible out of the silicon. There's an additional
>> MCU running a specialised firmware, which communicates with the
>> application processor through a mailbox and some reserved memory, and is
>> in charge of controlling the regulators, the PLL clocks, and the power
>> gating of the GPU, all while also being in charge of any DVFS control.
>>
>> This set of drivers is enough to communicate desired OPP index limits to
>> the aforementioned MCU, referred to as "GPUEB" from here on out. The
>> GPUEB is still free to lower the effective frequency if the GPU has no
>> jobs going on at all, even when a higher OPP is set.
>>
>> The power- and frequency control driver, mtk-mfg-pmdomain, is now
>> implemented as a power domain driver, with a set_performance_state
>> operation. It also exposes itself as a clock provider, so that panthor
>> can read the actual achieved DVFS clock rate as per the GPUEB firmware.
>>
>> This power domain approach means that panthor does not need to know
>> about how the frequency control works on this SoC, as the OPP core
>> framework already takes care of it. The only exception is that panthor
>> needs to not register OPPs from DT itself if there already is an OPP
>> table present.
>>
>> The mailbox driver is a fairly bog-standard common mailbox framework
>> driver, just specific to the firmware that runs on the GPUEB. It was
>> merged in v6.18 already.
>>
>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> 
> This looks good to me!
> 
> I can certainly pick up patch2 and patch5, but before I go ahead I
> just wanted to check what is the preferred merging strategy?
> 
> The drm/gpu patches can go independently from the pmdomain patches
> others, right? In either case, I can pick this complete series too via
> my pmdomain tree, if that makes sense for everyone. Please let me
> know.

I'm about to go on holiday, so I'm not about to merge and run ;)
But I'm happy if you want to take the complete series through your tree.

Thanks,
Steve

> Kind regards
> Uffe
> 
>> ---
>> Changes in v8:
>> - mtk-mfg-pmdomain: remove unused shmem variable that caused a warning
>>   on GCC, but not clang
>> - Link to v7: https://lore.kernel.org/r/20251015-mt8196-gpufreq-v7-0-0a6435da2080@collabora.com
>>
>> Changes in v7:
>> - panthor: rename "t" to "table"
>> - panthor: add code comment explaining why an existing OPP table is
>>   being checked for
>> - mtk-mfg-pmdomain: use GF_REG_MAGIC offset for sake of consistency
>> - mtk-mfg-pmdomain: remove redundant semicolon after mtk_mfg_mt8196_init
>> - mtk-mfg-pmdomain: fix resource leaks on probe failure
>> - mtk-mfg-pmdomain: enable/disable EB clock during MT8196 init, which is
>>   needed for the register read
>> - Rebase onto next-20251014, which drops already merged patches, namely
>>   mailbox driver+bindings, and drops the ASN_HASH patch series
>>   dependency, which was also merged
>> - Link to v6: https://lore.kernel.org/r/20251003-mt8196-gpufreq-v6-0-76498ad61d9e@collabora.com
>>
>> Changes in v6:
>> - mailbox: move buf definition into if condition, as per Chia-I Wu
>> - panthor: remove the redundant NULL checks in panthor_devfreq_get_freq
>> - mtk-mfg-pmdomain: adjust return style consistency
>> - mtk-mfg-pmdomain: add docstring for mtk_mfg_send_ipi to explain it's
>>   blocking
>> - mtk-mfg-pmdomain: use CMD_FIX_DUAL_TARGET_OPPIDX instead of
>>   CMD_FIX_TARGET_OPPIDX.
>> - mtk-mfg-pmdomain: reword code comments to not be in the "we" style
>> - mtk-mfg-pmdomain: shuffle around mbox allocations as per Angelo
>> - mtk-mfg-pmdomain: don't pointlessly turn on EB clock in probe,
>>   reducing the need for a comment explaining the bookkeeping
>> - mtk-mfg-pmdomain: consistently use dev_err_probe and Capitalise first
>>   letter of error string
>> - mtk-mfg-pmdomain: get rid of redundant ret = dev_err_probe assignment
>> - mtk-mfg-pmdomain: reintroduce stack OPP table, choose min(gpu, stack)
>>   when adding frequencies. Fixes gaps in OPP levels where only stack
>>   changed, but gpu had duplicates, which resulted in choosing a too slow
>>   OPP
>> - mtk-mfg-pmdomain: stub round_rate clk op to opt out of CCF always
>>   "rounding" a devfreq rate request to the current rate
>> - Link to v5: https://lore.kernel.org/r/20250929-mt8196-gpufreq-v5-0-3056e5ecf765@collabora.com
>>
>> Changes in v5:
>> - mtk-mfg-pmdomain binding: add memory-regions property, remove shmem
>>   property, as we now correctly describe the shared memory as a regular
>>   memory region
>> - mtk-mfg-pmdomain binding: get rid of redundant |
>> - drop "dt-bindings: sram: Add compatible for
>>   mediatek,mt8196-gpufreq-sram" as part of the move to reserved memory
>> - mtk-mfg-pmdomain: move to using reserved-memory for GPUEB shared
>>   memory
>> - mtk-mfg-pmdomain: demote some types to smaller sizes in struct
>>   mtk_mfg, as per Angelo's suggestions
>> - mtk-mfg-pmdomain: use units.h for Hz-to-KHz
>> - mtk-mfg-pmdomain: change for loop in attach_dev to reduce indentation
>> - mtk-mfg-pmdomain: simplify return in mtk_mfg_power_off
>> - mtk-mfg-pmdomain: move of_device_id after probe
>> - mtk_mfg_pmdomain: map mmio by index
>> - mtk_mfg_pmdomain: add error checking to pm_genpd_init()
>> - mtk_mfg_pmdomain: add remove function
>> - mtk_mfg_pmdomain: remove last_opp member and logic, since OPP core
>>   already does that for us
>> - mtk_mfg_pmdomain: adjust comment in mtk_mfg_set_performance to explain
>>   why we're doing what we're doing
>> - mtk_mfg_pmdomain: call mtk_mfg_set_oppidx in mtk_mfg_power_on with
>>   the performance_state we deferred setting while it was powered off
>> - mtk_mfg_pmdomain: add inline function for PWR_ACK checking, as it's
>>   now used twice with the added remove function
>> - mtk-mfg-pmdomain: add suppress_bind_attrs so people don't play with
>>   that
>> - mtk-mfg-pmdomain: change KConfig from tristate to bool, as module
>>   unloading results in strange likely firmware-induced hardware state
>>   woes in the mali GPU
>> - mtk-mfg-pmdomain: read IPI magic in power_on, don't zero it after
>>   confirming that seemingly had no purpose
>> - mtk-mfg-pmdomain: misc style changes
>> - Link to v4: https://lore.kernel.org/r/20250923-mt8196-gpufreq-v4-0-6cd63ade73d6@collabora.com
>>
>> Changes in v4:
>> - rebase onto next-20250922, which includes Laura Nao's clock patches
>> - refactor mediatek_mfg into a pmdomain driver called "mtk-mfg-pmdomain"
>> - move mt8196-gpufreq binding to the power subdirectory
>> - mali-valhall-csf binding: adjust for power-domains usage
>> - mali-valhall-csf binding: use clocks on mt8196
>> - mailbox: prefix defines with "GPUEB_"
>> - mailbox: get rid of custom of_xlate
>> - mailbox: rename "CLOGGED" to "BLOCKED"
>> - mailbox: adjust send_data comment to include more technical info
>> - mailbox: misc style improvements
>> - panthor: drop "drm/panthor: devfreq: make get_dev_status use
>>   get_cur_freq", as it is now not necessary and makes the code worse
>> - panthor: drop "drm/panthor: devfreq: add pluggable devfreq providers"
>> - panthor: drop "drm/panthor: add no_clocks soc_data member for MT8196",
>>   as we now have clocks courtesy of gpufreq
>> - panthor: check for existing opp table before registering a new one
>> - mtk-mfg-pmdomain: add turbo_below variant data, which marks OPPs below
>>   a certain index as turbo for the OPP subsystem
>> - mtk-mfg-pmdomain: no longer read stack OPPs, as they weren't used
>> - mtk-mfg-pmdomain: get rid of num gpu opp != num stack opp check.
>>   That's the firmware's problem should it ever happen, not ours
>> - mtk-mfg-pmdomain: some small name and whitespace changes on the defines
>> - Link to v3: https://lore.kernel.org/r/20250917-mt8196-gpufreq-v3-0-c4ede4b4399e@collabora.com
>>
>> Changes in v3:
>> - mali-valhall-csf binding: get rid of clocks for MT8196, rebase onto
>>   Chia-I Wu's patch
>> - mt8196-gpufreq binding: rename hw_revision to hw-revision
>> - mt8196-gpufreq binding: rename clocks
>> - mt8196-gpufreq binding: drop pointless label in example
>> - mailbox binding: drop pointless label in example
>> - mailbox: whitespace changes on defines
>> - mailbox: remove rx_buf member from channel struct, use stack buffer
>> - mailbox: check in probe that no rx_len exceeds MBOX_MAX_RX_SIZE
>> - panthor: add no_clocks SoC data patch, also rebase onto Chia-I Wu's
>>   series
>> - panthor: refactor devfreq provider functionality to do allocation and
>>   initialisation of panthor_devfreq struct in panthor in all cases
>> - panthor: drop the patch that moves struct panthor_devfreq to a header
>>   file, as it no longer needs to be exposed to devfreq providers
>> - mediatek_mfg: refactor devfreq provider functionality to decouple it
>>   more from panthor itself
>> - mediatek_mfg: move SRAM magic to a #define
>> - mediatek_mfg: begrudgingly rename member "padding_lol" to "reserved"
>> - mediatek_mfg: use local struct device pointer var in more places
>> - mediatek_mfg: change wording of sleep command failure error message,
>>   but keep the format specifier because I don't want to throw bare
>>   errnos at users
>> - mediatek_mfg: remove unnecessary braces around dev_err EB power off
>>   timeout message
>> - mediatek_mfg: allocate rx_data for channels that expect a response
>> - mediatek_mfg: memcpy the rx buffer from the common mailbox framework
>>   in the rx callback to rx_data, as mssg now points to stack memory
>> - mediatek_mfg: make SRAM clearing message dev_dbg
>> - mediatek_mfg: no longer print physical address of SRAM
>> - mediatek_mfg: expand on the GF_REG_OPP_TABLE_STK comment, toning down
>>   its defeatist attitude in the process
>> - mediatek_mfg: style fixes in mtk_mfg_get_closest_opp_idx
>> - mediatek_mfg: rename clocks and hw-revision reg as per binding
>> - Link to v2: https://lore.kernel.org/r/20250912-mt8196-gpufreq-v2-0-779a8a3729d9@collabora.com
>>
>> Changes in v2:
>> - mali-valhall-csf binding: move from performance-controller to
>>   performance-domains property
>> - mali-valhall-csf binding: fix vendor name oopsie in compatible of if
>>   condition
>> - mt8196-gpufreq binding: move from performance-controller to
>>   performance-domains by adding the cells property
>> - mt8196-gpufreq binding: rename e2_id to hw_revision
>> - mt8196-gpufreq binding: add description that mentions "MediaTek
>>   Flexible Graphics"
>> - mt8196-gpufreq binding: get rid of mailbox channels we're unlikely to
>>   use any time soon, if ever
>> - mt8196-gpufreq binding: change name of mailbox channels to use -
>>   instead of _
>> - mailbox binding: change reg-names to "data" and "ctl"
>> - drm/panthor: mediatek_mfg: rename e2_id to hw_revision
>> - drm/panthor: devfreq: switch from performance-controller to
>>   performance-domains
>> - drm/panthor: devfreq: get rid of the accidental get_cur_freq function
>>   move
>> - mailbox: rename mtk_gpueb_mbox_ch to mtk_gpueb_mbox_chan_desc
>> - mailbox: use smaller types in mtk_gpueb_mbox_chan_desc where possible
>> - mailbox: add per-channel runtime data struct
>> - mailbox: request one threaded IRQ per channel, pass channel struct as
>>   data
>> - mailbox: make num_channels in variant struct u8
>> - mailbox: get rid of no_response, as it was redundant
>> - mailbox: enable and disable clock in mailbox startup/shutdown
>> - mailbox: point con_priv of mailbox framework channel struct to this
>>   driver's channel struct
>> - mailbox: request and free the threaded IRQ in startup/shutdown
>> - mailbox: only clear IRQ bit flag once RX data has been read from MMIO
>> - mailbox: reduce needlessly large receive buffer size
>> - mailbox: handle allocation errors wherever they could pop up
>> - mailbox: style cleanups in mtk_gpueb_mbox_read_rx
>> - mailbox: call platform_get_irq earlier on in probe
>> - mailbox: set drvdata later on in probe
>> - mailbox: ioremap resources by index, not name
>> - mailbox: handle devm_mbox_controller_register errors
>> - mailbox: rename channels to correspond to bindings
>> - mailbox: document a few of the private driver structs to be kind to
>>   the next person who will look at this code
>> - Link to v1: https://lore.kernel.org/r/20250905-mt8196-gpufreq-v1-0-7b6c2d6be221@collabora.com
>>
>> ---
>> Nicolas Frattaroli (5):
>>       dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant
>>       dt-bindings: power: Add MT8196 GPU frequency control binding
>>       drm/panthor: call into devfreq for current frequency
>>       drm/panthor: Use existing OPP table if present
>>       pmdomain: mediatek: Add support for MFlexGraphics
>>
>>  .../bindings/gpu/arm,mali-valhall-csf.yaml         |   37 +-
>>  .../bindings/power/mediatek,mt8196-gpufreq.yaml    |  117 +++
>>  drivers/gpu/drm/panthor/panthor_devfreq.c          |   62 +-
>>  drivers/gpu/drm/panthor/panthor_devfreq.h          |    2 +
>>  drivers/gpu/drm/panthor/panthor_device.h           |    3 -
>>  drivers/gpu/drm/panthor/panthor_drv.c              |    4 +-
>>  drivers/pmdomain/mediatek/Kconfig                  |   16 +
>>  drivers/pmdomain/mediatek/Makefile                 |    1 +
>>  drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c       | 1044 ++++++++++++++++++++
>>  9 files changed, 1268 insertions(+), 18 deletions(-)
>> ---
>> base-commit: 3477f49ff0433a241da12ec9cecf6c9b2bd1c6f8
>> change-id: 20250829-mt8196-gpufreq-a7645670d182
>>
>> Best regards,
>> --
>> Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>>

Re: [PATCH v8 0/5] MT8196 GPU Frequency/Power Control Support

[Ulf Hansson]

On Fri, 24 Oct 2025 at 15:09, Steven Price <steven.price@arm.com> wrote:
>
> On 22/10/2025 14:52, Ulf Hansson wrote:
> > On Fri, 17 Oct 2025 at 17:32, Nicolas Frattaroli
> > <nicolas.frattaroli@collabora.com> wrote:
> >>
> >> This series introduces two new drivers to accomplish controlling the
> >> frequency and power of the Mali GPU on MediaTek MT8196 SoCs.
> >>
> >> The reason why it's not as straightforward as with other SoCs is that
> >> the MT8196 has quite complex glue logic in order to squeeze the maximum
> >> amount of performance possible out of the silicon. There's an additional
> >> MCU running a specialised firmware, which communicates with the
> >> application processor through a mailbox and some reserved memory, and is
> >> in charge of controlling the regulators, the PLL clocks, and the power
> >> gating of the GPU, all while also being in charge of any DVFS control.
> >>
> >> This set of drivers is enough to communicate desired OPP index limits to
> >> the aforementioned MCU, referred to as "GPUEB" from here on out. The
> >> GPUEB is still free to lower the effective frequency if the GPU has no
> >> jobs going on at all, even when a higher OPP is set.
> >>
> >> The power- and frequency control driver, mtk-mfg-pmdomain, is now
> >> implemented as a power domain driver, with a set_performance_state
> >> operation. It also exposes itself as a clock provider, so that panthor
> >> can read the actual achieved DVFS clock rate as per the GPUEB firmware.
> >>
> >> This power domain approach means that panthor does not need to know
> >> about how the frequency control works on this SoC, as the OPP core
> >> framework already takes care of it. The only exception is that panthor
> >> needs to not register OPPs from DT itself if there already is an OPP
> >> table present.
> >>
> >> The mailbox driver is a fairly bog-standard common mailbox framework
> >> driver, just specific to the firmware that runs on the GPUEB. It was
> >> merged in v6.18 already.
> >>
> >> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> >
> > This looks good to me!
> >
> > I can certainly pick up patch2 and patch5, but before I go ahead I
> > just wanted to check what is the preferred merging strategy?
> >
> > The drm/gpu patches can go independently from the pmdomain patches
> > others, right? In either case, I can pick this complete series too via
> > my pmdomain tree, if that makes sense for everyone. Please let me
> > know.
>
> I'm about to go on holiday, so I'm not about to merge and run ;)
> But I'm happy if you want to take the complete series through your tree.

Unfortunately the panthor specific changes didn't apply cleanly on my
side. Looks like you have some patches queued already for the panther
driver and I guess Nicolas may have based the changes on top of them.

That said, I decided to pick the pmdomain patches, patch 2 and patch 5, thanks!

Kind regards
Uffe


>
> Thanks,
> Steve
>
> > Kind regards
> > Uffe
> >
> >> ---
> >> Changes in v8:
> >> - mtk-mfg-pmdomain: remove unused shmem variable that caused a warning
> >>   on GCC, but not clang
> >> - Link to v7: https://lore.kernel.org/r/20251015-mt8196-gpufreq-v7-0-0a6435da2080@collabora.com
> >>
> >> Changes in v7:
> >> - panthor: rename "t" to "table"
> >> - panthor: add code comment explaining why an existing OPP table is
> >>   being checked for
> >> - mtk-mfg-pmdomain: use GF_REG_MAGIC offset for sake of consistency
> >> - mtk-mfg-pmdomain: remove redundant semicolon after mtk_mfg_mt8196_init
> >> - mtk-mfg-pmdomain: fix resource leaks on probe failure
> >> - mtk-mfg-pmdomain: enable/disable EB clock during MT8196 init, which is
> >>   needed for the register read
> >> - Rebase onto next-20251014, which drops already merged patches, namely
> >>   mailbox driver+bindings, and drops the ASN_HASH patch series
> >>   dependency, which was also merged
> >> - Link to v6: https://lore.kernel.org/r/20251003-mt8196-gpufreq-v6-0-76498ad61d9e@collabora.com
> >>
> >> Changes in v6:
> >> - mailbox: move buf definition into if condition, as per Chia-I Wu
> >> - panthor: remove the redundant NULL checks in panthor_devfreq_get_freq
> >> - mtk-mfg-pmdomain: adjust return style consistency
> >> - mtk-mfg-pmdomain: add docstring for mtk_mfg_send_ipi to explain it's
> >>   blocking
> >> - mtk-mfg-pmdomain: use CMD_FIX_DUAL_TARGET_OPPIDX instead of
> >>   CMD_FIX_TARGET_OPPIDX.
> >> - mtk-mfg-pmdomain: reword code comments to not be in the "we" style
> >> - mtk-mfg-pmdomain: shuffle around mbox allocations as per Angelo
> >> - mtk-mfg-pmdomain: don't pointlessly turn on EB clock in probe,
> >>   reducing the need for a comment explaining the bookkeeping
> >> - mtk-mfg-pmdomain: consistently use dev_err_probe and Capitalise first
> >>   letter of error string
> >> - mtk-mfg-pmdomain: get rid of redundant ret = dev_err_probe assignment
> >> - mtk-mfg-pmdomain: reintroduce stack OPP table, choose min(gpu, stack)
> >>   when adding frequencies. Fixes gaps in OPP levels where only stack
> >>   changed, but gpu had duplicates, which resulted in choosing a too slow
> >>   OPP
> >> - mtk-mfg-pmdomain: stub round_rate clk op to opt out of CCF always
> >>   "rounding" a devfreq rate request to the current rate
> >> - Link to v5: https://lore.kernel.org/r/20250929-mt8196-gpufreq-v5-0-3056e5ecf765@collabora.com
> >>
> >> Changes in v5:
> >> - mtk-mfg-pmdomain binding: add memory-regions property, remove shmem
> >>   property, as we now correctly describe the shared memory as a regular
> >>   memory region
> >> - mtk-mfg-pmdomain binding: get rid of redundant |
> >> - drop "dt-bindings: sram: Add compatible for
> >>   mediatek,mt8196-gpufreq-sram" as part of the move to reserved memory
> >> - mtk-mfg-pmdomain: move to using reserved-memory for GPUEB shared
> >>   memory
> >> - mtk-mfg-pmdomain: demote some types to smaller sizes in struct
> >>   mtk_mfg, as per Angelo's suggestions
> >> - mtk-mfg-pmdomain: use units.h for Hz-to-KHz
> >> - mtk-mfg-pmdomain: change for loop in attach_dev to reduce indentation
> >> - mtk-mfg-pmdomain: simplify return in mtk_mfg_power_off
> >> - mtk-mfg-pmdomain: move of_device_id after probe
> >> - mtk_mfg_pmdomain: map mmio by index
> >> - mtk_mfg_pmdomain: add error checking to pm_genpd_init()
> >> - mtk_mfg_pmdomain: add remove function
> >> - mtk_mfg_pmdomain: remove last_opp member and logic, since OPP core
> >>   already does that for us
> >> - mtk_mfg_pmdomain: adjust comment in mtk_mfg_set_performance to explain
> >>   why we're doing what we're doing
> >> - mtk_mfg_pmdomain: call mtk_mfg_set_oppidx in mtk_mfg_power_on with
> >>   the performance_state we deferred setting while it was powered off
> >> - mtk_mfg_pmdomain: add inline function for PWR_ACK checking, as it's
> >>   now used twice with the added remove function
> >> - mtk-mfg-pmdomain: add suppress_bind_attrs so people don't play with
> >>   that
> >> - mtk-mfg-pmdomain: change KConfig from tristate to bool, as module
> >>   unloading results in strange likely firmware-induced hardware state
> >>   woes in the mali GPU
> >> - mtk-mfg-pmdomain: read IPI magic in power_on, don't zero it after
> >>   confirming that seemingly had no purpose
> >> - mtk-mfg-pmdomain: misc style changes
> >> - Link to v4: https://lore.kernel.org/r/20250923-mt8196-gpufreq-v4-0-6cd63ade73d6@collabora.com
> >>
> >> Changes in v4:
> >> - rebase onto next-20250922, which includes Laura Nao's clock patches
> >> - refactor mediatek_mfg into a pmdomain driver called "mtk-mfg-pmdomain"
> >> - move mt8196-gpufreq binding to the power subdirectory
> >> - mali-valhall-csf binding: adjust for power-domains usage
> >> - mali-valhall-csf binding: use clocks on mt8196
> >> - mailbox: prefix defines with "GPUEB_"
> >> - mailbox: get rid of custom of_xlate
> >> - mailbox: rename "CLOGGED" to "BLOCKED"
> >> - mailbox: adjust send_data comment to include more technical info
> >> - mailbox: misc style improvements
> >> - panthor: drop "drm/panthor: devfreq: make get_dev_status use
> >>   get_cur_freq", as it is now not necessary and makes the code worse
> >> - panthor: drop "drm/panthor: devfreq: add pluggable devfreq providers"
> >> - panthor: drop "drm/panthor: add no_clocks soc_data member for MT8196",
> >>   as we now have clocks courtesy of gpufreq
> >> - panthor: check for existing opp table before registering a new one
> >> - mtk-mfg-pmdomain: add turbo_below variant data, which marks OPPs below
> >>   a certain index as turbo for the OPP subsystem
> >> - mtk-mfg-pmdomain: no longer read stack OPPs, as they weren't used
> >> - mtk-mfg-pmdomain: get rid of num gpu opp != num stack opp check.
> >>   That's the firmware's problem should it ever happen, not ours
> >> - mtk-mfg-pmdomain: some small name and whitespace changes on the defines
> >> - Link to v3: https://lore.kernel.org/r/20250917-mt8196-gpufreq-v3-0-c4ede4b4399e@collabora.com
> >>
> >> Changes in v3:
> >> - mali-valhall-csf binding: get rid of clocks for MT8196, rebase onto
> >>   Chia-I Wu's patch
> >> - mt8196-gpufreq binding: rename hw_revision to hw-revision
> >> - mt8196-gpufreq binding: rename clocks
> >> - mt8196-gpufreq binding: drop pointless label in example
> >> - mailbox binding: drop pointless label in example
> >> - mailbox: whitespace changes on defines
> >> - mailbox: remove rx_buf member from channel struct, use stack buffer
> >> - mailbox: check in probe that no rx_len exceeds MBOX_MAX_RX_SIZE
> >> - panthor: add no_clocks SoC data patch, also rebase onto Chia-I Wu's
> >>   series
> >> - panthor: refactor devfreq provider functionality to do allocation and
> >>   initialisation of panthor_devfreq struct in panthor in all cases
> >> - panthor: drop the patch that moves struct panthor_devfreq to a header
> >>   file, as it no longer needs to be exposed to devfreq providers
> >> - mediatek_mfg: refactor devfreq provider functionality to decouple it
> >>   more from panthor itself
> >> - mediatek_mfg: move SRAM magic to a #define
> >> - mediatek_mfg: begrudgingly rename member "padding_lol" to "reserved"
> >> - mediatek_mfg: use local struct device pointer var in more places
> >> - mediatek_mfg: change wording of sleep command failure error message,
> >>   but keep the format specifier because I don't want to throw bare
> >>   errnos at users
> >> - mediatek_mfg: remove unnecessary braces around dev_err EB power off
> >>   timeout message
> >> - mediatek_mfg: allocate rx_data for channels that expect a response
> >> - mediatek_mfg: memcpy the rx buffer from the common mailbox framework
> >>   in the rx callback to rx_data, as mssg now points to stack memory
> >> - mediatek_mfg: make SRAM clearing message dev_dbg
> >> - mediatek_mfg: no longer print physical address of SRAM
> >> - mediatek_mfg: expand on the GF_REG_OPP_TABLE_STK comment, toning down
> >>   its defeatist attitude in the process
> >> - mediatek_mfg: style fixes in mtk_mfg_get_closest_opp_idx
> >> - mediatek_mfg: rename clocks and hw-revision reg as per binding
> >> - Link to v2: https://lore.kernel.org/r/20250912-mt8196-gpufreq-v2-0-779a8a3729d9@collabora.com
> >>
> >> Changes in v2:
> >> - mali-valhall-csf binding: move from performance-controller to
> >>   performance-domains property
> >> - mali-valhall-csf binding: fix vendor name oopsie in compatible of if
> >>   condition
> >> - mt8196-gpufreq binding: move from performance-controller to
> >>   performance-domains by adding the cells property
> >> - mt8196-gpufreq binding: rename e2_id to hw_revision
> >> - mt8196-gpufreq binding: add description that mentions "MediaTek
> >>   Flexible Graphics"
> >> - mt8196-gpufreq binding: get rid of mailbox channels we're unlikely to
> >>   use any time soon, if ever
> >> - mt8196-gpufreq binding: change name of mailbox channels to use -
> >>   instead of _
> >> - mailbox binding: change reg-names to "data" and "ctl"
> >> - drm/panthor: mediatek_mfg: rename e2_id to hw_revision
> >> - drm/panthor: devfreq: switch from performance-controller to
> >>   performance-domains
> >> - drm/panthor: devfreq: get rid of the accidental get_cur_freq function
> >>   move
> >> - mailbox: rename mtk_gpueb_mbox_ch to mtk_gpueb_mbox_chan_desc
> >> - mailbox: use smaller types in mtk_gpueb_mbox_chan_desc where possible
> >> - mailbox: add per-channel runtime data struct
> >> - mailbox: request one threaded IRQ per channel, pass channel struct as
> >>   data
> >> - mailbox: make num_channels in variant struct u8
> >> - mailbox: get rid of no_response, as it was redundant
> >> - mailbox: enable and disable clock in mailbox startup/shutdown
> >> - mailbox: point con_priv of mailbox framework channel struct to this
> >>   driver's channel struct
> >> - mailbox: request and free the threaded IRQ in startup/shutdown
> >> - mailbox: only clear IRQ bit flag once RX data has been read from MMIO
> >> - mailbox: reduce needlessly large receive buffer size
> >> - mailbox: handle allocation errors wherever they could pop up
> >> - mailbox: style cleanups in mtk_gpueb_mbox_read_rx
> >> - mailbox: call platform_get_irq earlier on in probe
> >> - mailbox: set drvdata later on in probe
> >> - mailbox: ioremap resources by index, not name
> >> - mailbox: handle devm_mbox_controller_register errors
> >> - mailbox: rename channels to correspond to bindings
> >> - mailbox: document a few of the private driver structs to be kind to
> >>   the next person who will look at this code
> >> - Link to v1: https://lore.kernel.org/r/20250905-mt8196-gpufreq-v1-0-7b6c2d6be221@collabora.com
> >>
> >> ---
> >> Nicolas Frattaroli (5):
> >>       dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant
> >>       dt-bindings: power: Add MT8196 GPU frequency control binding
> >>       drm/panthor: call into devfreq for current frequency
> >>       drm/panthor: Use existing OPP table if present
> >>       pmdomain: mediatek: Add support for MFlexGraphics
> >>
> >>  .../bindings/gpu/arm,mali-valhall-csf.yaml         |   37 +-
> >>  .../bindings/power/mediatek,mt8196-gpufreq.yaml    |  117 +++
> >>  drivers/gpu/drm/panthor/panthor_devfreq.c          |   62 +-
> >>  drivers/gpu/drm/panthor/panthor_devfreq.h          |    2 +
> >>  drivers/gpu/drm/panthor/panthor_device.h           |    3 -
> >>  drivers/gpu/drm/panthor/panthor_drv.c              |    4 +-
> >>  drivers/pmdomain/mediatek/Kconfig                  |   16 +
> >>  drivers/pmdomain/mediatek/Makefile                 |    1 +
> >>  drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c       | 1044 ++++++++++++++++++++
> >>  9 files changed, 1268 insertions(+), 18 deletions(-)
> >> ---
> >> base-commit: 3477f49ff0433a241da12ec9cecf6c9b2bd1c6f8
> >> change-id: 20250829-mt8196-gpufreq-a7645670d182
> >>
> >> Best regards,
> >> --
> >> Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> >>
>

Re: [PATCH v8 1/5] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Liviu Dudau]

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?

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!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v8 1/5] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Nicolas Frattaroli]

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>;
> > +    };
> >  
> >  ...
> > 
> 
> 

Re: [PATCH v8 1/5] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Liviu Dudau]

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!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v8 1/5] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Nicolas Frattaroli]

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>;
> > > > +    };
> > > >  
> > > >  ...
> > > > 
> > > 
> > > 
> > 
> > 
> > 
> > 
> 
> 

Re: [PATCH v8 1/5] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Liviu Dudau]

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!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v8 1/5] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Liviu Dudau]

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!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v8 0/5] MT8196 GPU Frequency/Power Control Support

[Liviu Dudau]

On Fri, Oct 24, 2025 at 04:50:15PM +0200, Ulf Hansson wrote:
> On Fri, 24 Oct 2025 at 15:09, Steven Price <steven.price@arm.com> wrote:
> >
> > On 22/10/2025 14:52, Ulf Hansson wrote:
> > > On Fri, 17 Oct 2025 at 17:32, Nicolas Frattaroli
> > > <nicolas.frattaroli@collabora.com> wrote:
> > >>
> > >> This series introduces two new drivers to accomplish controlling the
> > >> frequency and power of the Mali GPU on MediaTek MT8196 SoCs.
> > >>
> > >> The reason why it's not as straightforward as with other SoCs is that
> > >> the MT8196 has quite complex glue logic in order to squeeze the maximum
> > >> amount of performance possible out of the silicon. There's an additional
> > >> MCU running a specialised firmware, which communicates with the
> > >> application processor through a mailbox and some reserved memory, and is
> > >> in charge of controlling the regulators, the PLL clocks, and the power
> > >> gating of the GPU, all while also being in charge of any DVFS control.
> > >>
> > >> This set of drivers is enough to communicate desired OPP index limits to
> > >> the aforementioned MCU, referred to as "GPUEB" from here on out. The
> > >> GPUEB is still free to lower the effective frequency if the GPU has no
> > >> jobs going on at all, even when a higher OPP is set.
> > >>
> > >> The power- and frequency control driver, mtk-mfg-pmdomain, is now
> > >> implemented as a power domain driver, with a set_performance_state
> > >> operation. It also exposes itself as a clock provider, so that panthor
> > >> can read the actual achieved DVFS clock rate as per the GPUEB firmware.
> > >>
> > >> This power domain approach means that panthor does not need to know
> > >> about how the frequency control works on this SoC, as the OPP core
> > >> framework already takes care of it. The only exception is that panthor
> > >> needs to not register OPPs from DT itself if there already is an OPP
> > >> table present.
> > >>
> > >> The mailbox driver is a fairly bog-standard common mailbox framework
> > >> driver, just specific to the firmware that runs on the GPUEB. It was
> > >> merged in v6.18 already.
> > >>
> > >> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > >
> > > This looks good to me!
> > >
> > > I can certainly pick up patch2 and patch5, but before I go ahead I
> > > just wanted to check what is the preferred merging strategy?
> > >
> > > The drm/gpu patches can go independently from the pmdomain patches
> > > others, right? In either case, I can pick this complete series too via
> > > my pmdomain tree, if that makes sense for everyone. Please let me
> > > know.
> >
> > I'm about to go on holiday, so I'm not about to merge and run ;)
> > But I'm happy if you want to take the complete series through your tree.
> 
> Unfortunately the panthor specific changes didn't apply cleanly on my
> side. Looks like you have some patches queued already for the panther
> driver and I guess Nicolas may have based the changes on top of them.
> 
> That said, I decided to pick the pmdomain patches, patch 2 and patch 5, thanks!

I've pushed the rest of the series through drm-misc-next.

Best regards,
Liviu

> 
> Kind regards
> Uffe
> 
> 
> >
> > Thanks,
> > Steve
> >
> > > Kind regards
> > > Uffe
> > >
> > >> ---
> > >> Changes in v8:
> > >> - mtk-mfg-pmdomain: remove unused shmem variable that caused a warning
> > >>   on GCC, but not clang
> > >> - Link to v7: https://lore.kernel.org/r/20251015-mt8196-gpufreq-v7-0-0a6435da2080@collabora.com
> > >>
> > >> Changes in v7:
> > >> - panthor: rename "t" to "table"
> > >> - panthor: add code comment explaining why an existing OPP table is
> > >>   being checked for
> > >> - mtk-mfg-pmdomain: use GF_REG_MAGIC offset for sake of consistency
> > >> - mtk-mfg-pmdomain: remove redundant semicolon after mtk_mfg_mt8196_init
> > >> - mtk-mfg-pmdomain: fix resource leaks on probe failure
> > >> - mtk-mfg-pmdomain: enable/disable EB clock during MT8196 init, which is
> > >>   needed for the register read
> > >> - Rebase onto next-20251014, which drops already merged patches, namely
> > >>   mailbox driver+bindings, and drops the ASN_HASH patch series
> > >>   dependency, which was also merged
> > >> - Link to v6: https://lore.kernel.org/r/20251003-mt8196-gpufreq-v6-0-76498ad61d9e@collabora.com
> > >>
> > >> Changes in v6:
> > >> - mailbox: move buf definition into if condition, as per Chia-I Wu
> > >> - panthor: remove the redundant NULL checks in panthor_devfreq_get_freq
> > >> - mtk-mfg-pmdomain: adjust return style consistency
> > >> - mtk-mfg-pmdomain: add docstring for mtk_mfg_send_ipi to explain it's
> > >>   blocking
> > >> - mtk-mfg-pmdomain: use CMD_FIX_DUAL_TARGET_OPPIDX instead of
> > >>   CMD_FIX_TARGET_OPPIDX.
> > >> - mtk-mfg-pmdomain: reword code comments to not be in the "we" style
> > >> - mtk-mfg-pmdomain: shuffle around mbox allocations as per Angelo
> > >> - mtk-mfg-pmdomain: don't pointlessly turn on EB clock in probe,
> > >>   reducing the need for a comment explaining the bookkeeping
> > >> - mtk-mfg-pmdomain: consistently use dev_err_probe and Capitalise first
> > >>   letter of error string
> > >> - mtk-mfg-pmdomain: get rid of redundant ret = dev_err_probe assignment
> > >> - mtk-mfg-pmdomain: reintroduce stack OPP table, choose min(gpu, stack)
> > >>   when adding frequencies. Fixes gaps in OPP levels where only stack
> > >>   changed, but gpu had duplicates, which resulted in choosing a too slow
> > >>   OPP
> > >> - mtk-mfg-pmdomain: stub round_rate clk op to opt out of CCF always
> > >>   "rounding" a devfreq rate request to the current rate
> > >> - Link to v5: https://lore.kernel.org/r/20250929-mt8196-gpufreq-v5-0-3056e5ecf765@collabora.com
> > >>
> > >> Changes in v5:
> > >> - mtk-mfg-pmdomain binding: add memory-regions property, remove shmem
> > >>   property, as we now correctly describe the shared memory as a regular
> > >>   memory region
> > >> - mtk-mfg-pmdomain binding: get rid of redundant |
> > >> - drop "dt-bindings: sram: Add compatible for
> > >>   mediatek,mt8196-gpufreq-sram" as part of the move to reserved memory
> > >> - mtk-mfg-pmdomain: move to using reserved-memory for GPUEB shared
> > >>   memory
> > >> - mtk-mfg-pmdomain: demote some types to smaller sizes in struct
> > >>   mtk_mfg, as per Angelo's suggestions
> > >> - mtk-mfg-pmdomain: use units.h for Hz-to-KHz
> > >> - mtk-mfg-pmdomain: change for loop in attach_dev to reduce indentation
> > >> - mtk-mfg-pmdomain: simplify return in mtk_mfg_power_off
> > >> - mtk-mfg-pmdomain: move of_device_id after probe
> > >> - mtk_mfg_pmdomain: map mmio by index
> > >> - mtk_mfg_pmdomain: add error checking to pm_genpd_init()
> > >> - mtk_mfg_pmdomain: add remove function
> > >> - mtk_mfg_pmdomain: remove last_opp member and logic, since OPP core
> > >>   already does that for us
> > >> - mtk_mfg_pmdomain: adjust comment in mtk_mfg_set_performance to explain
> > >>   why we're doing what we're doing
> > >> - mtk_mfg_pmdomain: call mtk_mfg_set_oppidx in mtk_mfg_power_on with
> > >>   the performance_state we deferred setting while it was powered off
> > >> - mtk_mfg_pmdomain: add inline function for PWR_ACK checking, as it's
> > >>   now used twice with the added remove function
> > >> - mtk-mfg-pmdomain: add suppress_bind_attrs so people don't play with
> > >>   that
> > >> - mtk-mfg-pmdomain: change KConfig from tristate to bool, as module
> > >>   unloading results in strange likely firmware-induced hardware state
> > >>   woes in the mali GPU
> > >> - mtk-mfg-pmdomain: read IPI magic in power_on, don't zero it after
> > >>   confirming that seemingly had no purpose
> > >> - mtk-mfg-pmdomain: misc style changes
> > >> - Link to v4: https://lore.kernel.org/r/20250923-mt8196-gpufreq-v4-0-6cd63ade73d6@collabora.com
> > >>
> > >> Changes in v4:
> > >> - rebase onto next-20250922, which includes Laura Nao's clock patches
> > >> - refactor mediatek_mfg into a pmdomain driver called "mtk-mfg-pmdomain"
> > >> - move mt8196-gpufreq binding to the power subdirectory
> > >> - mali-valhall-csf binding: adjust for power-domains usage
> > >> - mali-valhall-csf binding: use clocks on mt8196
> > >> - mailbox: prefix defines with "GPUEB_"
> > >> - mailbox: get rid of custom of_xlate
> > >> - mailbox: rename "CLOGGED" to "BLOCKED"
> > >> - mailbox: adjust send_data comment to include more technical info
> > >> - mailbox: misc style improvements
> > >> - panthor: drop "drm/panthor: devfreq: make get_dev_status use
> > >>   get_cur_freq", as it is now not necessary and makes the code worse
> > >> - panthor: drop "drm/panthor: devfreq: add pluggable devfreq providers"
> > >> - panthor: drop "drm/panthor: add no_clocks soc_data member for MT8196",
> > >>   as we now have clocks courtesy of gpufreq
> > >> - panthor: check for existing opp table before registering a new one
> > >> - mtk-mfg-pmdomain: add turbo_below variant data, which marks OPPs below
> > >>   a certain index as turbo for the OPP subsystem
> > >> - mtk-mfg-pmdomain: no longer read stack OPPs, as they weren't used
> > >> - mtk-mfg-pmdomain: get rid of num gpu opp != num stack opp check.
> > >>   That's the firmware's problem should it ever happen, not ours
> > >> - mtk-mfg-pmdomain: some small name and whitespace changes on the defines
> > >> - Link to v3: https://lore.kernel.org/r/20250917-mt8196-gpufreq-v3-0-c4ede4b4399e@collabora.com
> > >>
> > >> Changes in v3:
> > >> - mali-valhall-csf binding: get rid of clocks for MT8196, rebase onto
> > >>   Chia-I Wu's patch
> > >> - mt8196-gpufreq binding: rename hw_revision to hw-revision
> > >> - mt8196-gpufreq binding: rename clocks
> > >> - mt8196-gpufreq binding: drop pointless label in example
> > >> - mailbox binding: drop pointless label in example
> > >> - mailbox: whitespace changes on defines
> > >> - mailbox: remove rx_buf member from channel struct, use stack buffer
> > >> - mailbox: check in probe that no rx_len exceeds MBOX_MAX_RX_SIZE
> > >> - panthor: add no_clocks SoC data patch, also rebase onto Chia-I Wu's
> > >>   series
> > >> - panthor: refactor devfreq provider functionality to do allocation and
> > >>   initialisation of panthor_devfreq struct in panthor in all cases
> > >> - panthor: drop the patch that moves struct panthor_devfreq to a header
> > >>   file, as it no longer needs to be exposed to devfreq providers
> > >> - mediatek_mfg: refactor devfreq provider functionality to decouple it
> > >>   more from panthor itself
> > >> - mediatek_mfg: move SRAM magic to a #define
> > >> - mediatek_mfg: begrudgingly rename member "padding_lol" to "reserved"
> > >> - mediatek_mfg: use local struct device pointer var in more places
> > >> - mediatek_mfg: change wording of sleep command failure error message,
> > >>   but keep the format specifier because I don't want to throw bare
> > >>   errnos at users
> > >> - mediatek_mfg: remove unnecessary braces around dev_err EB power off
> > >>   timeout message
> > >> - mediatek_mfg: allocate rx_data for channels that expect a response
> > >> - mediatek_mfg: memcpy the rx buffer from the common mailbox framework
> > >>   in the rx callback to rx_data, as mssg now points to stack memory
> > >> - mediatek_mfg: make SRAM clearing message dev_dbg
> > >> - mediatek_mfg: no longer print physical address of SRAM
> > >> - mediatek_mfg: expand on the GF_REG_OPP_TABLE_STK comment, toning down
> > >>   its defeatist attitude in the process
> > >> - mediatek_mfg: style fixes in mtk_mfg_get_closest_opp_idx
> > >> - mediatek_mfg: rename clocks and hw-revision reg as per binding
> > >> - Link to v2: https://lore.kernel.org/r/20250912-mt8196-gpufreq-v2-0-779a8a3729d9@collabora.com
> > >>
> > >> Changes in v2:
> > >> - mali-valhall-csf binding: move from performance-controller to
> > >>   performance-domains property
> > >> - mali-valhall-csf binding: fix vendor name oopsie in compatible of if
> > >>   condition
> > >> - mt8196-gpufreq binding: move from performance-controller to
> > >>   performance-domains by adding the cells property
> > >> - mt8196-gpufreq binding: rename e2_id to hw_revision
> > >> - mt8196-gpufreq binding: add description that mentions "MediaTek
> > >>   Flexible Graphics"
> > >> - mt8196-gpufreq binding: get rid of mailbox channels we're unlikely to
> > >>   use any time soon, if ever
> > >> - mt8196-gpufreq binding: change name of mailbox channels to use -
> > >>   instead of _
> > >> - mailbox binding: change reg-names to "data" and "ctl"
> > >> - drm/panthor: mediatek_mfg: rename e2_id to hw_revision
> > >> - drm/panthor: devfreq: switch from performance-controller to
> > >>   performance-domains
> > >> - drm/panthor: devfreq: get rid of the accidental get_cur_freq function
> > >>   move
> > >> - mailbox: rename mtk_gpueb_mbox_ch to mtk_gpueb_mbox_chan_desc
> > >> - mailbox: use smaller types in mtk_gpueb_mbox_chan_desc where possible
> > >> - mailbox: add per-channel runtime data struct
> > >> - mailbox: request one threaded IRQ per channel, pass channel struct as
> > >>   data
> > >> - mailbox: make num_channels in variant struct u8
> > >> - mailbox: get rid of no_response, as it was redundant
> > >> - mailbox: enable and disable clock in mailbox startup/shutdown
> > >> - mailbox: point con_priv of mailbox framework channel struct to this
> > >>   driver's channel struct
> > >> - mailbox: request and free the threaded IRQ in startup/shutdown
> > >> - mailbox: only clear IRQ bit flag once RX data has been read from MMIO
> > >> - mailbox: reduce needlessly large receive buffer size
> > >> - mailbox: handle allocation errors wherever they could pop up
> > >> - mailbox: style cleanups in mtk_gpueb_mbox_read_rx
> > >> - mailbox: call platform_get_irq earlier on in probe
> > >> - mailbox: set drvdata later on in probe
> > >> - mailbox: ioremap resources by index, not name
> > >> - mailbox: handle devm_mbox_controller_register errors
> > >> - mailbox: rename channels to correspond to bindings
> > >> - mailbox: document a few of the private driver structs to be kind to
> > >>   the next person who will look at this code
> > >> - Link to v1: https://lore.kernel.org/r/20250905-mt8196-gpufreq-v1-0-7b6c2d6be221@collabora.com
> > >>
> > >> ---
> > >> Nicolas Frattaroli (5):
> > >>       dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant
> > >>       dt-bindings: power: Add MT8196 GPU frequency control binding
> > >>       drm/panthor: call into devfreq for current frequency
> > >>       drm/panthor: Use existing OPP table if present
> > >>       pmdomain: mediatek: Add support for MFlexGraphics
> > >>
> > >>  .../bindings/gpu/arm,mali-valhall-csf.yaml         |   37 +-
> > >>  .../bindings/power/mediatek,mt8196-gpufreq.yaml    |  117 +++
> > >>  drivers/gpu/drm/panthor/panthor_devfreq.c          |   62 +-
> > >>  drivers/gpu/drm/panthor/panthor_devfreq.h          |    2 +
> > >>  drivers/gpu/drm/panthor/panthor_device.h           |    3 -
> > >>  drivers/gpu/drm/panthor/panthor_drv.c              |    4 +-
> > >>  drivers/pmdomain/mediatek/Kconfig                  |   16 +
> > >>  drivers/pmdomain/mediatek/Makefile                 |    1 +
> > >>  drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c       | 1044 ++++++++++++++++++++
> > >>  9 files changed, 1268 insertions(+), 18 deletions(-)
> > >> ---
> > >> base-commit: 3477f49ff0433a241da12ec9cecf6c9b2bd1c6f8
> > >> change-id: 20250829-mt8196-gpufreq-a7645670d182
> > >>
> > >> Best regards,
> > >> --
> > >> Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > >>
> >

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯