[PATCH v3 00/10] MT8196 GPU Frequency/Power Control Support

[PATCH v3 00/10] 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 SRAM, 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. There's also
several more powerful OPPs it seemingly refuses to apply. The downstream
chromeos kernel also doesn't reach the frequencies of those OPPs, so we
assume this is expected.

The frequency control driver lives in panthor's subdirectory, as it
needs to pass panthor some data. I've kept the tie-in parts generic
enough however to not make this a complete hack; mediatek_mfg (the
frequency control driver) registers itself as a "devfreq provider" with
panthor, and panthor picks it up during its probe function (or defers if
mediatek_mfg is not ready yet, after adding a device link first).

It's now generic enough to where I'll ponder about moving the devfreq
provider stuff into a header in include/, and moving mediatek_mfg into
the drivers/soc/ subdirectory, but there were enough changes so far to
warrant a v3 without a move or further struct renames added, so that I
can get feedback on this approach.

The mailbox driver is a fairly bog-standard common mailbox framework
driver, just specific to the firmware that runs on the GPUEB.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@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 (10):
      dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant
      dt-bindings: devfreq: add mt8196-gpufreq binding
      dt-bindings: sram: Add compatible for mediatek,mt8196-gpufreq-sram
      dt-bindings: mailbox: Add MT8196 GPUEB Mailbox
      mailbox: add MediaTek GPUEB IPI mailbox
      drm/panthor: call into devfreq for current frequency
      drm/panthor: devfreq: make get_dev_status use get_cur_freq
      drm/panthor: devfreq: add pluggable devfreq providers
      drm/panthor: add no_clocks soc_data member for MT8196
      drm/panthor: add support for MediaTek MFlexGraphics

 .../bindings/devfreq/mediatek,mt8196-gpufreq.yaml  | 112 +++
 .../bindings/gpu/arm,mali-valhall-csf.yaml         |  32 +-
 .../mailbox/mediatek,mt8196-gpueb-mbox.yaml        |  64 ++
 Documentation/devicetree/bindings/sram/sram.yaml   |   1 +
 drivers/gpu/drm/panthor/Kconfig                    |  13 +
 drivers/gpu/drm/panthor/Makefile                   |   2 +
 drivers/gpu/drm/panthor/mediatek_mfg.c             | 946 +++++++++++++++++++++
 drivers/gpu/drm/panthor/panthor_devfreq.c          | 223 ++++-
 drivers/gpu/drm/panthor/panthor_devfreq.h          |  49 ++
 drivers/gpu/drm/panthor/panthor_device.c           |   3 +
 drivers/gpu/drm/panthor/panthor_device.h           |   6 +-
 drivers/gpu/drm/panthor/panthor_drv.c              |   5 +-
 drivers/mailbox/Kconfig                            |  10 +
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/mtk-gpueb-mailbox.c                | 330 +++++++
 15 files changed, 1765 insertions(+), 33 deletions(-)
---
base-commit: 65ca971b838b0fbb45efc48751644105e9d91aa7
change-id: 20250829-mt8196-gpufreq-a7645670d182
prerequisite-message-id: <20250915151947.277983-1-laura.nao@collabora.com>
prerequisite-patch-id: 441c4c2e3d22f83a41241a1ab5c9be1a442f742e
prerequisite-patch-id: 852bfc3d13e2bccc4d6f4813a71c42f329dadb0c
prerequisite-patch-id: 0bc5b7bf268e88a6ef22e46c91db7645e2ce6189
prerequisite-patch-id: 442533e316e46ecd47cd0b7fb410b58fad2b3bf9
prerequisite-patch-id: 04377d5612f4513e44b6d7d6016125d75f9ccd65
prerequisite-patch-id: d61046e2cd2f33024092e96e8a987b9c34c24e73
prerequisite-patch-id: 62f28c3bf605f6df622d71cbe1bc982e289a39cf
prerequisite-patch-id: 27fadb12ce15099a684c08d4f8c785bedc87cef2
prerequisite-patch-id: 7796ec9a0162ae96b708ea513742016657c69e14
prerequisite-patch-id: f7549078f3702acdf1e6dcd36ddebab0e543b5db
prerequisite-patch-id: 3ec0d9470d639154fb347c9561fbe705eee4bd51
prerequisite-patch-id: fa96e18eae90efc14e4b9f13534c013b448a3f84
prerequisite-patch-id: 8c7e63c9ea7863818f7d31291417b90ac3124ee7
prerequisite-patch-id: ffff2977d8f2a3a44c3606f77d38f8745cb3c60a
prerequisite-patch-id: 71d23f4f096e424ae3aa59a23695f5b1e488fab0
prerequisite-patch-id: 3c12631f22a39d6def9340ed840e9e55e1a76304
prerequisite-patch-id: caec6572d5d9a37183601b8e38f50af797df635e
prerequisite-patch-id: d5b3d5719675a1e3be26e028b5596d39e839bc09
prerequisite-patch-id: 9d5359fcf729db5b707772ff3596e72a401473c6
prerequisite-patch-id: f3f789e0d919dd92b7d811a4e11c57bb05f71617
prerequisite-patch-id: 79cca92633ca3d9cc2f1f38b6fc977a8d8543d60
prerequisite-patch-id: f663f8f3bddf198d0cab083bac7ebb88689ffc82
prerequisite-patch-id: a0ffc5b88c5eb88c491f6187672012c621bd520c
prerequisite-patch-id: e6c6d67b034d06b6158a0e0f8299ad28f0e59134
prerequisite-patch-id: 25f658fbd1238bd57e05ff299d0436f942bdcc4d
prerequisite-patch-id: 7be8439e241a320b0eb0a264a8a59a9beef383d6
prerequisite-patch-id: b903714dbe7d6a44fbe18faa02d59862ceadf217
prerequisite-message-id: <20250913002155.1163908-1-olvaffe@gmail.com>
prerequisite-patch-id: a769ebe04bd74f45a3a5b9c1d1396f4b33b7783f
prerequisite-patch-id: 9d71426f40b702e975f2a672509fcd20180ac36c

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

[PATCH v3 01/10] 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.

It lets us omit the OPP tables from the device tree, as those can now be
enumerated at runtime from the MCU. It also means the mali GPU node
described in this binding does not have any clocks in this case, as all
clock control is delegated to the MCU.

Add the mediatek,mt8196-mali compatible, and a performance-domains
property which points to the MCU's device tree node in this case. It's
required on mt8196 devices.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../bindings/gpu/arm,mali-valhall-csf.yaml         | 32 ++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
index 7ad5a3ffc5f5c753322eda9e74cc65de89d11c73..ccab2dd0ea852187e3ab75923e19739622b2b3b8 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
@@ -38,7 +38,6 @@ properties:
       - const: gpu
 
   clocks:
-    minItems: 1
     maxItems: 3
 
   clock-names:
@@ -54,6 +53,9 @@ properties:
   opp-table:
     type: object
 
+  performance-domains:
+    maxItems: 1
+
   power-domains:
     minItems: 1
     maxItems: 5
@@ -92,7 +94,6 @@ required:
   - interrupts
   - interrupt-names
   - clocks
-  - mali-supply
 
 additionalProperties: false
 
@@ -106,9 +107,26 @@ allOf:
       properties:
         clocks:
           minItems: 3
+        performance-domains: false
         power-domains:
           maxItems: 1
         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
+        clocks: false
+        clock-names: false
+      required:
+        - performance-domains
 
 examples:
   - |
@@ -144,5 +162,15 @@ examples:
             };
         };
     };
+  - |
+    gpu@48000000 {
+        compatible = "mediatek,mt8196-mali", "arm,mali-valhall-csf";
+        reg = <0x48000000 0x480000>;
+        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";
+        performance-domains = <&gpufreq>;
+    };
 
 ...

-- 
2.51.0

[PATCH v3 02/10] dt-bindings: devfreq: add mt8196-gpufreq binding

[Nicolas Frattaroli]

On the MediaTek MT8196 SoC, the GPU has its power and frequency
dynamically controlled by an embedded special-purpose MCU. This MCU is
in charge of powering up the GPU silicon. It also provides us with a
list of available OPPs at runtime, and is fully in control of all the
regulator and clock fiddling it takes to reach a certain level of
performance. It's also in charge of enforcing limits on power draw or
temperature.

Add a binding for this device in the devfreq subdirectory, where it
seems to fit in best considering its tasks.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../bindings/devfreq/mediatek,mt8196-gpufreq.yaml  | 112 +++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/Documentation/devicetree/bindings/devfreq/mediatek,mt8196-gpufreq.yaml b/Documentation/devicetree/bindings/devfreq/mediatek,mt8196-gpufreq.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..b1c6751662c085eccdead038a2edc61e4bfc5f5f
--- /dev/null
+++ b/Documentation/devicetree/bindings/devfreq/mediatek,mt8196-gpufreq.yaml
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/devfreq/mediatek,mt8196-gpufreq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek MFlexGraphics Performance 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: '^performance-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
+
+  shmem:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the shared memory region of the GPUEB MCU
+
+  "#performance-domain-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - mboxes
+  - mbox-names
+  - shmem
+  - "#performance-domain-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mediatek,mt8196-clock.h>
+
+    performance-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";
+        shmem = <&gpufreq_shmem>;
+        #performance-domain-cells = <0>;
+    };

-- 
2.51.0

[PATCH v3 03/10] dt-bindings: sram: Add compatible for mediatek,mt8196-gpufreq-sram

[Nicolas Frattaroli]

This compatible is used for an SRAM section that's shared between the
MT8196's application processor cores and the embedded GPUEB MCU that
controls the GPU frequency.

Through this SRAM section, things about the GPU frequency controller
like the OPP table can be read.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 Documentation/devicetree/bindings/sram/sram.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/sram/sram.yaml b/Documentation/devicetree/bindings/sram/sram.yaml
index 7c1337e159f2371401ae99313375656fff014ed4..6ba0dd6a66def11f56a1d5276d7397b655bff11e 100644
--- a/Documentation/devicetree/bindings/sram/sram.yaml
+++ b/Documentation/devicetree/bindings/sram/sram.yaml
@@ -89,6 +89,7 @@ patternProperties:
             - arm,juno-scp-shmem
             - arm,scmi-shmem
             - arm,scp-shmem
+            - mediatek,mt8196-gpufreq-sram
             - renesas,smp-sram
             - rockchip,rk3066-smp-sram
             - samsung,exynos4210-sysram

-- 
2.51.0

[PATCH v3 04/10] dt-bindings: mailbox: Add MT8196 GPUEB Mailbox

[Nicolas Frattaroli]

The MediaTek MT8196 SoC includes an embedded MCU referred to as "GPUEB",
acting as glue logic to control power and frequency of the Mali GPU.
This MCU runs special-purpose firmware for this use, and the main
application processor communicates with it through a mailbox.

Add a binding that describes this mailbox.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../mailbox/mediatek,mt8196-gpueb-mbox.yaml        | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,mt8196-gpueb-mbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,mt8196-gpueb-mbox.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..ab5b780cb83a708a3897ca1a440131d97b56c3a6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/mediatek,mt8196-gpueb-mbox.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/mediatek,mt8196-gpueb-mbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek MFlexGraphics GPUEB Mailbox Controller
+
+maintainers:
+  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
+
+properties:
+  compatible:
+    enum:
+      - mediatek,mt8196-gpueb-mbox
+
+  reg:
+    items:
+      - description: mailbox data registers
+      - description: mailbox control registers
+
+  reg-names:
+    items:
+      - const: data
+      - const: ctl
+
+  clocks:
+    items:
+      - description: main clock of the GPUEB MCU
+
+  interrupts:
+    items:
+      - description: fires when a new message is received
+
+  "#mbox-cells":
+    const: 1
+    description:
+      The number of the mailbox channel.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - interrupts
+  - "#mbox-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mediatek,mt8196-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    mailbox@4b09fd80 {
+        compatible = "mediatek,mt8196-gpueb-mbox";
+        reg = <0x4b09fd80 0x280>,
+              <0x4b170000 0x7c>;
+        reg-names = "data", "ctl";
+        clocks = <&topckgen CLK_TOP_MFG_EB>;
+        interrupts = <GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH 0>;
+        #mbox-cells = <1>;
+    };

-- 
2.51.0

[PATCH v3 05/10] mailbox: add MediaTek GPUEB IPI mailbox

[Nicolas Frattaroli]

The MT8196 SoC uses an embedded MCU to control frequencies and power of
the GPU. This controller is referred to as "GPUEB".

It communicates to the application processor, among other ways, through
a mailbox.

The mailbox exposes one interrupt, which appears to only be fired when a
response is received, rather than a transaction is completed. For us,
this means we unfortunately need to poll for txdone.

The mailbox also requires the EB clock to be on when touching any of the
mailbox registers.

Add a simple driver for it based on the common mailbox framework.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/mailbox/Kconfig             |  10 ++
 drivers/mailbox/Makefile            |   2 +
 drivers/mailbox/mtk-gpueb-mailbox.c | 330 ++++++++++++++++++++++++++++++++++++
 3 files changed, 342 insertions(+)

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 02432d4a5ccd46a16156a09c7f277fb03e4013f5..2016defda1fabb5c0fcc8078f84a52d4e4e00167 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -294,6 +294,16 @@ config MTK_CMDQ_MBOX
 	  critical time limitation, such as updating display configuration
 	  during the vblank.
 
+config MTK_GPUEB_MBOX
+	tristate "MediaTek GPUEB Mailbox Support"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	help
+	  The MediaTek GPUEB mailbox is used to communicate with the embedded
+	  controller in charge of GPU frequency and power management on some
+	  MediaTek SoCs, such as the MT8196.
+	  Say Y or m here if you want to support the MT8196 SoC in your kernel
+	  build.
+
 config ZYNQMP_IPI_MBOX
 	tristate "Xilinx ZynqMP IPI Mailbox"
 	depends on ARCH_ZYNQMP && OF
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 98a68f838486eed117d24296138bf59fda3f92e4..564d06e71313e6d1972e4a6036e1e78c8c7ec450 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -63,6 +63,8 @@ obj-$(CONFIG_MTK_ADSP_MBOX)	+= mtk-adsp-mailbox.o
 
 obj-$(CONFIG_MTK_CMDQ_MBOX)	+= mtk-cmdq-mailbox.o
 
+obj-$(CONFIG_MTK_GPUEB_MBOX)	+= mtk-gpueb-mailbox.o
+
 obj-$(CONFIG_ZYNQMP_IPI_MBOX)	+= zynqmp-ipi-mailbox.o
 
 obj-$(CONFIG_SUN6I_MSGBOX)	+= sun6i-msgbox.o
diff --git a/drivers/mailbox/mtk-gpueb-mailbox.c b/drivers/mailbox/mtk-gpueb-mailbox.c
new file mode 100644
index 0000000000000000000000000000000000000000..b4a8dab5381f67da3783136cd15828f2df281dcf
--- /dev/null
+++ b/drivers/mailbox/mtk-gpueb-mailbox.c
@@ -0,0 +1,330 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MediaTek GPUEB mailbox driver for SoCs such as the MT8196
+ *
+ * Copyright (C) 2025, Collabora Ltd.
+ *
+ * Developers harmed in the making of this driver:
+ *  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
+ */
+
+#include <linux/atomic.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define MBOX_CTL_TX_STS		0x0000
+#define MBOX_CTL_IRQ_SET	0x0004
+#define MBOX_CTL_IRQ_CLR	0x0074
+#define MBOX_CTL_RX_STS		0x0078
+
+#define MBOX_FULL		BIT(0) /* i.e. we've received data */
+#define MBOX_CLOGGED		BIT(1) /* i.e. the channel is shutdown */
+
+#define MBOX_MAX_RX_SIZE	32 /* in bytes */
+
+struct mtk_gpueb_mbox {
+	struct device *dev;
+	struct clk *clk;
+	void __iomem *mbox_mmio;
+	void __iomem *mbox_ctl;
+	struct mbox_controller mbox;
+	struct mtk_gpueb_mbox_chan *ch;
+	int irq;
+	const struct mtk_gpueb_mbox_variant *v;
+};
+
+/**
+ * struct mtk_gpueb_mbox_chan - per-channel runtime data
+ * @ebm: pointer to the parent &struct mtk_gpueb_mbox mailbox
+ * @full_name: descriptive name of channel for IRQ subsystem
+ * @num: channel number, starting at 0
+ * @rx_status: signifies whether channel reception is turned off, or full
+ * @c: pointer to the constant &struct mtk_gpueb_mbox_chan_desc channel data
+ */
+struct mtk_gpueb_mbox_chan {
+	struct mtk_gpueb_mbox *ebm;
+	char *full_name;
+	u8 num;
+	atomic_t rx_status;
+	const struct mtk_gpueb_mbox_chan_desc *c;
+};
+
+/**
+ * struct mtk_gpueb_mbox_chan_desc - per-channel constant data
+ * @name: name of this channel
+ * @num: index of this channel, starting at 0
+ * @tx_offset: byte offset measured from mmio base for outgoing data
+ * @tx_len: size, in bytes, of the outgoing data on this channel
+ * @rx_offset: bytes offset measured from mmio base for incoming data
+ * @rx_len: size, in bytes, of the incoming data on this channel
+ */
+struct mtk_gpueb_mbox_chan_desc {
+	const char *name;
+	const u8 num;
+	const u16 tx_offset;
+	const u8 tx_len;
+	const u16 rx_offset;
+	const u8 rx_len;
+};
+
+struct mtk_gpueb_mbox_variant {
+	const u8 num_channels;
+	const struct mtk_gpueb_mbox_chan_desc channels[] __counted_by(num_channels);
+};
+
+/**
+ * mtk_gpueb_mbox_read_rx - read RX buffer from MMIO into channel's RX buffer
+ * @buf: buffer to read into
+ * @chan: pointer to the channel to read
+ */
+static void mtk_gpueb_mbox_read_rx(void *buf, struct mtk_gpueb_mbox_chan *chan)
+{
+	memcpy_fromio(buf, chan->ebm->mbox_mmio + chan->c->rx_offset, chan->c->rx_len);
+}
+
+static irqreturn_t mtk_gpueb_mbox_isr(int irq, void *data)
+{
+	struct mtk_gpueb_mbox_chan *ch = data;
+	u32 rx_sts;
+
+	rx_sts = readl(ch->ebm->mbox_ctl + MBOX_CTL_RX_STS);
+
+	if (rx_sts & BIT(ch->num)) {
+		if (!atomic_cmpxchg(&ch->rx_status, 0, MBOX_FULL | MBOX_CLOGGED))
+			return IRQ_WAKE_THREAD;
+	}
+
+	return IRQ_NONE;
+}
+
+static irqreturn_t mtk_gpueb_mbox_thread(int irq, void *data)
+{
+	struct mtk_gpueb_mbox_chan *ch = data;
+	u8 buf[MBOX_MAX_RX_SIZE] = {};
+	int status;
+
+	status = atomic_cmpxchg(&ch->rx_status,
+				MBOX_FULL | MBOX_CLOGGED, MBOX_FULL);
+	if (status == (MBOX_FULL | MBOX_CLOGGED)) {
+		mtk_gpueb_mbox_read_rx(buf, ch);
+		writel(BIT(ch->num), ch->ebm->mbox_ctl + MBOX_CTL_IRQ_CLR);
+		mbox_chan_received_data(&ch->ebm->mbox.chans[ch->num], buf);
+		atomic_set(&ch->rx_status, 0);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static int mtk_gpueb_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+	struct mtk_gpueb_mbox_chan *ch = chan->con_priv;
+	int i;
+	u32 *values = data;
+
+	if (atomic_read(&ch->rx_status))
+		return -EBUSY;
+
+	/*
+	 * We don't want any fancy nonsense, just write the 32-bit values in
+	 * order. memcpy_toio/__iowrite32_copy don't work here, because fancy.
+	 */
+	for (i = 0; i < ch->c->tx_len; i += 4)
+		writel(values[i / 4], ch->ebm->mbox_mmio + ch->c->tx_offset + i);
+
+	writel(BIT(ch->num), ch->ebm->mbox_ctl + MBOX_CTL_IRQ_SET);
+
+	return 0;
+}
+
+static int mtk_gpueb_mbox_startup(struct mbox_chan *chan)
+{
+	struct mtk_gpueb_mbox_chan *ch = chan->con_priv;
+	int ret;
+
+	atomic_set(&ch->rx_status, 0);
+
+	ret = clk_enable(ch->ebm->clk);
+	if (ret) {
+		dev_err(ch->ebm->dev, "Failed to enable EB clock: %pe\n",
+			ERR_PTR(ret));
+		goto err_clog;
+	}
+
+	writel(BIT(ch->num), ch->ebm->mbox_ctl + MBOX_CTL_IRQ_CLR);
+
+	ret = devm_request_threaded_irq(ch->ebm->dev, ch->ebm->irq, mtk_gpueb_mbox_isr,
+					mtk_gpueb_mbox_thread, IRQF_SHARED | IRQF_ONESHOT,
+					ch->full_name, ch);
+	if (ret) {
+		dev_err(ch->ebm->dev, "Failed to request IRQ: %pe\n",
+			ERR_PTR(ret));
+		goto err_unclk;
+	}
+
+	return 0;
+
+err_unclk:
+	clk_disable(ch->ebm->clk);
+err_clog:
+	atomic_set(&ch->rx_status, MBOX_CLOGGED);
+
+	return ret;
+}
+
+static void mtk_gpueb_mbox_shutdown(struct mbox_chan *chan)
+{
+	struct mtk_gpueb_mbox_chan *ch = chan->con_priv;
+
+	atomic_set(&ch->rx_status, MBOX_CLOGGED);
+
+	devm_free_irq(ch->ebm->dev, ch->ebm->irq, ch);
+
+	clk_disable(ch->ebm->clk);
+}
+
+static bool mtk_gpueb_mbox_last_tx_done(struct mbox_chan *chan)
+{
+	struct mtk_gpueb_mbox_chan *ch = chan->con_priv;
+
+	return !(readl(ch->ebm->mbox_ctl + MBOX_CTL_TX_STS) & BIT(ch->num));
+}
+
+const struct mbox_chan_ops mtk_gpueb_mbox_ops = {
+	.send_data = mtk_gpueb_mbox_send_data,
+	.startup = mtk_gpueb_mbox_startup,
+	.shutdown = mtk_gpueb_mbox_shutdown,
+	.last_tx_done = mtk_gpueb_mbox_last_tx_done,
+};
+
+static struct mbox_chan *
+mtk_gpueb_mbox_of_xlate(struct mbox_controller *mbox,
+			const struct of_phandle_args *sp)
+{
+	struct mtk_gpueb_mbox *ebm = dev_get_drvdata(mbox->dev);
+
+	if (!sp->args_count)
+		return ERR_PTR(-EINVAL);
+
+	if (sp->args[0] >= ebm->v->num_channels)
+		return ERR_PTR(-ECHRNG);
+
+	return &mbox->chans[sp->args[0]];
+}
+
+static int mtk_gpueb_mbox_probe(struct platform_device *pdev)
+{
+	struct mtk_gpueb_mbox *ebm;
+	unsigned int i;
+
+	ebm = devm_kzalloc(&pdev->dev, sizeof(*ebm), GFP_KERNEL);
+	if (!ebm)
+		return -ENOMEM;
+
+	ebm->dev = &pdev->dev;
+	ebm->v = of_device_get_match_data(ebm->dev);
+
+	ebm->irq = platform_get_irq(pdev, 0);
+	if (ebm->irq < 0)
+		return ebm->irq;
+
+	ebm->clk = devm_clk_get_prepared(ebm->dev, NULL);
+	if (IS_ERR(ebm->clk))
+		return dev_err_probe(ebm->dev, PTR_ERR(ebm->clk),
+				     "Failed to get 'eb' clock\n");
+
+	ebm->mbox_mmio = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ebm->mbox_mmio))
+		return dev_err_probe(ebm->dev, PTR_ERR(ebm->mbox_mmio),
+				     "Couldn't map mailbox data registers\n");
+
+	ebm->mbox_ctl = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(ebm->mbox_ctl))
+		return dev_err_probe(
+			ebm->dev, PTR_ERR(ebm->mbox_ctl),
+			"Couldn't map mailbox control registers\n");
+
+	ebm->ch = devm_kmalloc_array(ebm->dev, ebm->v->num_channels,
+				     sizeof(*ebm->ch), GFP_KERNEL);
+	if (!ebm->ch)
+		return -ENOMEM;
+
+	ebm->mbox.chans = devm_kcalloc(ebm->dev, ebm->v->num_channels,
+				       sizeof(struct mbox_chan), GFP_KERNEL);
+	if (!ebm->mbox.chans)
+		return -ENOMEM;
+
+	for (i = 0; i < ebm->v->num_channels; i++) {
+		ebm->ch[i].c = &ebm->v->channels[i];
+		if (ebm->ch[i].c->rx_len > MBOX_MAX_RX_SIZE) {
+			dev_err(ebm->dev, "Channel %s RX size (%d) too large\n",
+				ebm->ch[i].c->name, ebm->ch[i].c->rx_len);
+			return -EINVAL;
+		}
+		ebm->ch[i].full_name = devm_kasprintf(ebm->dev, GFP_KERNEL, "%s:%s",
+						      dev_name(ebm->dev), ebm->ch[i].c->name);
+		if (!ebm->ch[i].full_name)
+			return -ENOMEM;
+
+		ebm->ch[i].ebm = ebm;
+		ebm->ch[i].num = i;
+		spin_lock_init(&ebm->mbox.chans[i].lock);
+		ebm->mbox.chans[i].con_priv = &ebm->ch[i];
+		atomic_set(&ebm->ch[i].rx_status, MBOX_CLOGGED);
+	}
+
+	ebm->mbox.dev = ebm->dev;
+	ebm->mbox.num_chans = ebm->v->num_channels;
+	ebm->mbox.txdone_poll = true;
+	ebm->mbox.txpoll_period = 0; /* minimum hrtimer interval */
+	ebm->mbox.of_xlate = mtk_gpueb_mbox_of_xlate;
+	ebm->mbox.ops = &mtk_gpueb_mbox_ops;
+
+	dev_set_drvdata(ebm->dev, ebm);
+
+	return devm_mbox_controller_register(ebm->dev, &ebm->mbox);
+}
+
+static const struct mtk_gpueb_mbox_variant mtk_gpueb_mbox_mt8196 = {
+	.num_channels = 12,
+	.channels = {
+		{ "fast-dvfs-event", 0, 0x0000, 16, 0x00e0, 16 },
+		{ "gpufreq",         1, 0x0010, 32, 0x00f0, 32 },
+		{ "sleep",           2, 0x0030, 12, 0x0110,  4 },
+		{ "timer",           3, 0x003c, 24, 0x0114,  4 },
+		{ "fhctl",           4, 0x0054, 36, 0x0118,  4 },
+		{ "ccf",             5, 0x0078, 16, 0x011c, 16 },
+		{ "gpumpu",          6, 0x0088, 24, 0x012c,  4 },
+		{ "fast-dvfs",       7, 0x00a0, 24, 0x0130, 24 },
+		{ "ipir-c-met",      8, 0x00b8,  4, 0x0148, 16 },
+		{ "ipis-c-met",      9, 0x00bc, 16, 0x0158,  4 },
+		{ "brisket",        10, 0x00cc, 16, 0x015c, 16 },
+		{ "ppb",            11, 0x00dc,  4, 0x016c,  4 },
+	},
+};
+
+static const struct of_device_id mtk_gpueb_mbox_of_ids[] = {
+	{ .compatible = "mediatek,mt8196-gpueb-mbox", .data = &mtk_gpueb_mbox_mt8196 },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_gpueb_mbox_of_ids);
+
+static struct platform_driver mtk_gpueb_mbox_drv = {
+	.probe = mtk_gpueb_mbox_probe,
+	.driver = {
+		.name = "mtk-gpueb-mbox",
+		.of_match_table = mtk_gpueb_mbox_of_ids,
+	}
+};
+module_platform_driver(mtk_gpueb_mbox_drv);
+
+MODULE_AUTHOR("Nicolas Frattaroli <nicolas.frattaroli@collabora.com>");
+MODULE_DESCRIPTION("MediaTek GPUEB mailbox driver");
+MODULE_LICENSE("GPL");

-- 
2.51.0

[PATCH v3 06/10] 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: 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 | 33 +++++++++++++++++++++++++++----
 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, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
index 3686515d368db5bb329f4858d4a7247a4957cc24..8903f60c0a3f06313ac2008791c210ff32b6bd52 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)
@@ -198,7 +205,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
@@ -296,3 +302,22 @@ 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 || !pdevfreq->devfreq)
+		return 0;
+
+	if (pdevfreq->devfreq->profile->get_cur_freq) {
+		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 b7631de695f7d79456478c87e8af5dc47673cd1d..f8e29e02f66cb3281ed4bb4c75cda9bd4df82b92 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 9f0649ecfc4fc697a21a8b2fc4dd89c8ecf298df..f32c1868bf6d782d99df9dbd0babcea049c917e0 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 ea4a37b566a8b215f2b7a09c333a696f1dcdb58f..4d59d94c353c3ca76f4b98a411c8f8284efafd08 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 v3 07/10] drm/panthor: devfreq: make get_dev_status use get_cur_freq

[Nicolas Frattaroli]

The only device-specific part of panthor's get_dev_status devfreq
callback is getting the clock frequency. All the other logic surrounding
what it does may be useful for other devfreq implementations however.

Make it call into get_cur_freq instead of poking the common clock
framework directly.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_devfreq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
index 8903f60c0a3f06313ac2008791c210ff32b6bd52..118da7cbb3c809e4aabfef7d20914e61c2b62555 100644
--- a/drivers/gpu/drm/panthor/panthor_devfreq.c
+++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
@@ -87,9 +87,13 @@ static int panthor_devfreq_get_dev_status(struct device *dev,
 {
 	struct panthor_device *ptdev = dev_get_drvdata(dev);
 	struct panthor_devfreq *pdevfreq = ptdev->devfreq;
+	struct devfreq_dev_profile *p = pdevfreq->devfreq->profile;
 	unsigned long irqflags;
+	int ret;
 
-	status->current_frequency = clk_get_rate(ptdev->clks.core);
+	ret = p->get_cur_freq(dev, &status->current_frequency);
+	if (ret)
+		return ret;
 
 	spin_lock_irqsave(&pdevfreq->lock, irqflags);
 

-- 
2.51.0

[PATCH v3 08/10] drm/panthor: devfreq: add pluggable devfreq providers

[Nicolas Frattaroli]

On some devices, devfreq is not controlled directly by DT OPPs and the
common clock framework, but through an external devfreq driver. To
permit this type of usage, add the concept of devfreq providers.

Devfreq providers for panthor register themselves with panthor as a
provider. panthor then gets whatever device is pointed at on its
performance-domains property, finds the registered devfreq provider for
it, and uses its registered devfreq ops. It wraps those operations by
passing the provider ops the provider's struct device, as opposed to the
panthor device.

Providers can choose to omit overloading some operations. In that case,
panthor's own implementation is used. The only exception is the exit
operation, which panthor does not use, and the wrapper only gets
registered if a provider needs it.

Should the probe order work out such that panthor probes before the
devfreq provider is finished probing and registering itself, then we
just defer the probe after adding a device link.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_devfreq.c | 190 ++++++++++++++++++++++++++----
 drivers/gpu/drm/panthor/panthor_devfreq.h |  47 ++++++++
 2 files changed, 212 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c
index 118da7cbb3c809e4aabfef7d20914e61c2b62555..1f10724e2f08df09b52fa1a27ffe9cfd49994b09 100644
--- a/drivers/gpu/drm/panthor/panthor_devfreq.c
+++ b/drivers/gpu/drm/panthor/panthor_devfreq.c
@@ -4,6 +4,7 @@
 #include <linux/clk.h>
 #include <linux/devfreq.h>
 #include <linux/devfreq_cooling.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_opp.h>
 
@@ -34,6 +35,12 @@ struct panthor_devfreq {
 	/** @last_busy_state: True if the GPU was busy last time we updated the state. */
 	bool last_busy_state;
 
+	/**
+	 * @panthor_devfreq_provider: the used performance-domain controller
+	 * through which devfreq callbacks are passed onto, or NULL if none.
+	 */
+	struct panthor_devfreq_provider *provider;
+
 	/**
 	 * @lock: Lock used to protect busy_time, idle_time, time_last_update and
 	 * last_busy_state.
@@ -44,6 +51,19 @@ struct panthor_devfreq {
 	spinlock_t lock;
 };
 
+static LIST_HEAD(panthor_devfreq_providers);
+static DEFINE_MUTEX(panthor_devfreq_providers_lock);
+
+int panthor_devfreq_register_provider(struct panthor_devfreq_provider *prov)
+{
+	guard(mutex)(&panthor_devfreq_providers_lock);
+
+	list_add(&prov->node, &panthor_devfreq_providers);
+
+	return 0;
+}
+EXPORT_SYMBOL(panthor_devfreq_register_provider);
+
 static void panthor_devfreq_update_utilization(struct panthor_devfreq *pdevfreq)
 {
 	ktime_t now, last;
@@ -59,12 +79,26 @@ static void panthor_devfreq_update_utilization(struct panthor_devfreq *pdevfreq)
 	pdevfreq->time_last_update = now;
 }
 
+static void panthor_devfreq_exit(struct device *dev)
+{
+	struct panthor_device *ptdev = dev_get_drvdata(dev);
+	struct panthor_devfreq_provider *provider = ptdev->devfreq->provider;
+
+	if (provider && provider->exit)
+		provider->exit(provider->dev);
+}
+
 static int panthor_devfreq_target(struct device *dev, unsigned long *freq,
 				  u32 flags)
 {
+	struct panthor_device *ptdev = dev_get_drvdata(dev);
+	struct panthor_devfreq_provider *provider = ptdev->devfreq->provider;
 	struct dev_pm_opp *opp;
 	int err;
 
+	if (provider && provider->target)
+		return provider->target(provider->dev, freq, flags);
+
 	opp = devfreq_recommended_opp(dev, freq, flags);
 	if (IS_ERR(opp))
 		return PTR_ERR(opp);
@@ -87,6 +121,7 @@ static int panthor_devfreq_get_dev_status(struct device *dev,
 {
 	struct panthor_device *ptdev = dev_get_drvdata(dev);
 	struct panthor_devfreq *pdevfreq = ptdev->devfreq;
+	struct panthor_devfreq_provider *provider = pdevfreq->provider;
 	struct devfreq_dev_profile *p = pdevfreq->devfreq->profile;
 	unsigned long irqflags;
 	int ret;
@@ -97,32 +132,42 @@ static int panthor_devfreq_get_dev_status(struct device *dev,
 
 	spin_lock_irqsave(&pdevfreq->lock, irqflags);
 
-	panthor_devfreq_update_utilization(pdevfreq);
+	if (provider && provider->get_dev_status) {
+		ret = provider->get_dev_status(provider->dev, status);
+	} else {
+		panthor_devfreq_update_utilization(pdevfreq);
 
-	status->total_time = ktime_to_ns(ktime_add(pdevfreq->busy_time,
-						   pdevfreq->idle_time));
+		status->total_time = ktime_to_ns(ktime_add(pdevfreq->busy_time,
+							   pdevfreq->idle_time));
 
-	status->busy_time = ktime_to_ns(pdevfreq->busy_time);
+		status->busy_time = ktime_to_ns(pdevfreq->busy_time);
 
-	panthor_devfreq_reset(pdevfreq);
+		panthor_devfreq_reset(pdevfreq);
+	}
 
 	spin_unlock_irqrestore(&pdevfreq->lock, irqflags);
 
-	drm_dbg(&ptdev->base, "busy %lu total %lu %lu %% freq %lu MHz\n",
-		status->busy_time, status->total_time,
-		status->busy_time / (status->total_time / 100),
-		status->current_frequency / 1000 / 1000);
+	if (!ret)
+		drm_dbg(&ptdev->base, "busy %lu total %lu %lu %% freq %lu MHz\n",
+			status->busy_time, status->total_time,
+			status->busy_time / (status->total_time / 100),
+			status->current_frequency / 1000 / 1000);
 
-	return 0;
+	return ret;
 }
 
 static int panthor_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
 {
 	struct panthor_device *ptdev = dev_get_drvdata(dev);
+	struct panthor_devfreq_provider *provider = ptdev->devfreq->provider;
+	int ret = 0;
 
-	*freq = clk_get_rate(ptdev->clks.core);
+	if (provider && provider->get_cur_freq)
+		ret = provider->get_cur_freq(provider->dev, freq);
+	else
+		*freq = clk_get_rate(ptdev->clks.core);
 
-	return 0;
+	return ret;
 }
 
 static struct devfreq_dev_profile panthor_devfreq_profile = {
@@ -133,7 +178,51 @@ static struct devfreq_dev_profile panthor_devfreq_profile = {
 	.get_cur_freq = panthor_devfreq_get_cur_freq,
 };
 
-int panthor_devfreq_init(struct panthor_device *ptdev)
+static int panthor_devfreq_use_provider(struct panthor_device *ptdev,
+					struct panthor_devfreq_provider *provider)
+{
+	struct devfreq_dev_profile *p = &panthor_devfreq_profile;
+	struct device *dev = ptdev->base.dev;
+	unsigned int i;
+	int ret = 0;
+
+	ptdev->devfreq->provider = provider;
+
+	if (provider->exit)
+		p->exit = panthor_devfreq_exit;
+
+	for (i = 0; i < provider->num_opps; i++) {
+		ret = dev_pm_opp_add_dynamic(dev, provider->opp_table[i]);
+		if (ret) {
+			DRM_DEV_ERROR(dev, "Couldn't add OPP %u: %pe\n", i, ERR_PTR(ret));
+			return ret;
+		}
+	}
+
+	p->max_state = provider->num_opps;
+
+	if (provider->num_opps)
+		ptdev->fast_rate = provider->opp_table[provider->num_opps - 1]->freq;
+
+	return ret;
+}
+
+static int panthor_devfreq_init_provider(struct panthor_device *ptdev,
+					 struct device *provider_dev)
+{
+	struct panthor_devfreq_provider *prov;
+
+	guard(mutex)(&panthor_devfreq_providers_lock);
+
+	list_for_each_entry(prov, &panthor_devfreq_providers, node) {
+		if (prov->dev == provider_dev)
+			return panthor_devfreq_use_provider(ptdev, prov);
+	}
+
+	return -EPROBE_DEFER;
+}
+
+static int panthor_devfreq_init_of(struct panthor_device *ptdev)
 {
 	/* There's actually 2 regulators (mali and sram), but the OPP core only
 	 * supports one.
@@ -142,20 +231,12 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
 	 * the coupling logic deal with voltage updates.
 	 */
 	static const char * const reg_names[] = { "mali", NULL };
-	struct thermal_cooling_device *cooling;
 	struct device *dev = ptdev->base.dev;
-	struct panthor_devfreq *pdevfreq;
 	struct dev_pm_opp *opp;
 	unsigned long cur_freq;
 	unsigned long freq = ULONG_MAX;
 	int ret;
 
-	pdevfreq = drmm_kzalloc(&ptdev->base, sizeof(*ptdev->devfreq), GFP_KERNEL);
-	if (!pdevfreq)
-		return -ENOMEM;
-
-	ptdev->devfreq = pdevfreq;
-
 	ret = devm_pm_opp_set_regulators(dev, reg_names);
 	if (ret) {
 		if (ret != -EPROBE_DEFER)
@@ -168,10 +249,6 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
 	if (ret)
 		return ret;
 
-	spin_lock_init(&pdevfreq->lock);
-
-	panthor_devfreq_reset(pdevfreq);
-
 	cur_freq = clk_get_rate(ptdev->clks.core);
 
 	/* Regulator coupling only takes care of synchronizing/balancing voltage
@@ -229,6 +306,61 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
 
 	dev_pm_opp_put(opp);
 
+	return 0;
+}
+
+static int panthor_devfreq_init_platform(struct panthor_device *ptdev)
+{
+	struct device_node *pcnode;
+	struct platform_device *pdev;
+	struct device_link *link;
+	int ret;
+
+	pcnode = of_parse_phandle(ptdev->base.dev->of_node,
+				  "performance-domains", 0);
+	if (!pcnode)
+		return -EINVAL;
+
+	pdev = of_find_device_by_node(pcnode);
+	of_node_put(pcnode);
+	if (!pdev)
+		return -ENODEV;
+
+	link = device_link_add(ptdev->base.dev, &pdev->dev,
+			       DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
+	if (!link) {
+		dev_err(ptdev->base.dev, "failed to add device link\n");
+		return -ENODEV;
+	}
+
+	ret = panthor_devfreq_init_provider(ptdev, &pdev->dev);
+	if (ret)
+		return dev_err_probe(ptdev->base.dev, ret,
+				     "failed to initialize devfreq provider\n");
+
+	DRM_DEV_INFO(ptdev->base.dev, "initialized devfreq provider %s\n",
+		     dev_name(&pdev->dev));
+
+	return 0;
+}
+
+int panthor_devfreq_init(struct panthor_device *ptdev)
+{
+	struct thermal_cooling_device *cooling;
+	struct device *dev = ptdev->base.dev;
+	struct panthor_devfreq *pdevfreq;
+	int ret;
+
+	pdevfreq = drmm_kzalloc(&ptdev->base, sizeof(*ptdev->devfreq), GFP_KERNEL);
+	if (!pdevfreq)
+		return -ENOMEM;
+
+	ptdev->devfreq = pdevfreq;
+
+	spin_lock_init(&pdevfreq->lock);
+
+	panthor_devfreq_reset(pdevfreq);
+
 	/*
 	 * Setup default thresholds for the simple_ondemand governor.
 	 * The values are chosen based on experiments.
@@ -236,6 +368,14 @@ int panthor_devfreq_init(struct panthor_device *ptdev)
 	pdevfreq->gov_data.upthreshold = 45;
 	pdevfreq->gov_data.downdifferential = 5;
 
+	if (!of_property_present(dev->of_node, "performance-domains"))
+		ret = panthor_devfreq_init_of(ptdev);
+	else
+		ret = panthor_devfreq_init_platform(ptdev);
+
+	if (ret)
+		return ret;
+
 	pdevfreq->devfreq = devm_devfreq_add_device(dev, &panthor_devfreq_profile,
 						    DEVFREQ_GOV_SIMPLE_ONDEMAND,
 						    &pdevfreq->gov_data);
diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.h b/drivers/gpu/drm/panthor/panthor_devfreq.h
index f8e29e02f66cb3281ed4bb4c75cda9bd4df82b92..777045d406242f46dea376bccb999bfc11f92a7a 100644
--- a/drivers/gpu/drm/panthor/panthor_devfreq.h
+++ b/drivers/gpu/drm/panthor/panthor_devfreq.h
@@ -10,6 +10,51 @@ struct thermal_cooling_device;
 struct panthor_device;
 struct panthor_devfreq;
 
+struct panthor_devfreq_provider {
+	/** @dev: device pointer to the provider device */
+	struct device *dev;
+
+	/**
+	 * @opp_table: table of unique OPPs, sorted from lowest frequency to
+	 * highest.
+	 */
+	struct dev_pm_opp_data **opp_table;
+
+	/** @num_opps: number of entries in @opp_table. */
+	unsigned int num_opps;
+
+	/**
+	 * @target: &struct devfreq_dev_profile "target" callback to wrap,
+	 * optional. Falls back to panthor internal implementation if absent.
+	 * Passes @dev as the first parameter, not panthor's device.
+	 */
+	int (*target)(struct device *dev, unsigned long *freq, u32 flags);
+
+	/**
+	 * @get_dev_status: &struct devfreq_dev_profile "get_dev_status" callback
+	 * to wrap, optional. Falls back to panthor internal implementation if
+	 * absent. Passes @dev as the first parameter, not panthor's device.
+	 */
+	int (*get_dev_status)(struct device *dev,
+			      struct devfreq_dev_status *stat);
+
+	/**
+	 * @get_cur_freq: &struct devfreq_dev_profile "get_dev_status" callback
+	 * to wrap, optional. Falls back to panthor internal implementation if
+	 * absent. Passes @dev as the first parameter, not panthor's device.
+	 */
+	int (*get_cur_freq)(struct device *dev, unsigned long *freq);
+
+	/**
+	 * @exit:  &struct devfreq_dev_profile "exit" callback to wrap, optional.
+	 * Falls back to not registering one an exit function at all if absent.
+	 * Passes @dev as the first parameter, not panthor's device.
+	 */
+	void (*exit)(struct device *dev);
+
+	struct list_head node;
+};
+
 int panthor_devfreq_init(struct panthor_device *ptdev);
 
 void panthor_devfreq_resume(struct panthor_device *ptdev);
@@ -20,4 +65,6 @@ void panthor_devfreq_record_idle(struct panthor_device *ptdev);
 
 unsigned long panthor_devfreq_get_freq(struct panthor_device *ptdev);
 
+int panthor_devfreq_register_provider(struct panthor_devfreq_provider *prov);
+
 #endif /* __PANTHOR_DEVFREQ_H__ */

-- 
2.51.0

[PATCH v3 09/10] drm/panthor: add no_clocks soc_data member for MT8196

[Nicolas Frattaroli]

While panthor does not try to do anything untoward with the core clock
outside of increasing its enable/disable count, the spirit of using DT
to describe hardware, not what drivers need, informs us that on the
MT8196, panthor should work without one, as the true owner of the clocks
in this case is the performance-domain.

Add a boolean to the soc_data struct that tells panthor whether on any
given SoC, it can leave even the core clock NULL. Set it to true for the
MT8196 soc_data instance.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_device.c | 3 +++
 drivers/gpu/drm/panthor/panthor_device.h | 3 +++
 drivers/gpu/drm/panthor/panthor_drv.c    | 1 +
 3 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index c7033d82cef55c940adc8434231cac6c5a20c288..581901a8e64fdb0946f20af593489fefde1cc05f 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -43,6 +43,9 @@ static int panthor_gpu_coherency_init(struct panthor_device *ptdev)
 
 static int panthor_clk_init(struct panthor_device *ptdev)
 {
+	if (ptdev->soc_data && ptdev->soc_data->no_clocks)
+		return 0;
+
 	ptdev->clks.core = devm_clk_get(ptdev->base.dev, NULL);
 	if (IS_ERR(ptdev->clks.core))
 		return dev_err_probe(ptdev->base.dev,
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index f32c1868bf6d782d99df9dbd0babcea049c917e0..cc8485afdaf865edc89a36823ba75c588a040e0b 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -40,6 +40,9 @@ struct panthor_soc_data {
 
 	/** @asn_hash: ASN_HASH values when asn_hash_enable is true. */
 	u32 asn_hash[3];
+
+	/** @no_clocks: True if clock control is external, not by panthor. */
+	bool no_clocks;
 };
 
 /**
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 4d59d94c353c3ca76f4b98a411c8f8284efafd08..3583ec955a85fe6e383839ec2bde017e1c6a995c 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1687,6 +1687,7 @@ ATTRIBUTE_GROUPS(panthor);
 static const struct panthor_soc_data soc_data_mediatek_mt8196 = {
 	.asn_hash_enable = true,
 	.asn_hash = { 0xb, 0xe, 0x0, },
+	.no_clocks = true,
 };
 
 static const struct of_device_id dt_match[] = {

-- 
2.51.0

[PATCH v3 10/10] drm/panthor: add support for MediaTek MFlexGraphics

[Nicolas Frattaroli]

MediaTek uses some glue logic to control frequency and power on some of
their GPUs. This is best exposed as a devfreq driver, as it saves us
from having to hardcode OPPs into the device tree, and can be extended
with additional devfreq-y logic like more clever governors that use the
hardware's GPUEB MCU to set frame time targets and power limits.

Add this driver to the panthor subdirectory. It needs to live here as it
needs to call into panthor's devfreq layer to register itself as a
provider.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/Kconfig        |  13 +
 drivers/gpu/drm/panthor/Makefile       |   2 +
 drivers/gpu/drm/panthor/mediatek_mfg.c | 946 +++++++++++++++++++++++++++++++++
 3 files changed, 961 insertions(+)

diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
index 55b40ad07f3b0779e0c434469ddc874ff74fde27..c4d2599c05df9e0e009b8e99b3d29c220269ca0d 100644
--- a/drivers/gpu/drm/panthor/Kconfig
+++ b/drivers/gpu/drm/panthor/Kconfig
@@ -21,3 +21,16 @@ config DRM_PANTHOR
 
 	  Note that the Mali-G68 and Mali-G78, while Valhall architecture, will
 	  be supported with the panfrost driver as they are not CSF GPUs.
+
+config DRM_PANTHOR_MEDIATEK_MFG
+	tristate "MediaTek MFlexGraphics Extensions for Panthor"
+	depends on (DRM_PANTHOR && ARCH_MEDIATEK) || COMPILE_TEST
+	select DEVFREQ_GOV_SIMPLE_ONDEMAND
+	select PM_DEVFREQ
+	select MTK_GPUEB_MBOX
+	help
+	  Support for power and clock control in Panthor for MediaTek
+	  MFlexGraphics devices, such as the GPU on the MT8196 or MT6991 SoCs.
+
+	  These extensions are required for the GPU to work on these platforms,
+	  as they control the glue logic that powers on the GPU.
diff --git a/drivers/gpu/drm/panthor/Makefile b/drivers/gpu/drm/panthor/Makefile
index 02db21748c125688d2ef20ed254b5ebd7ff642e4..e0ebfdfb20bd78e0003c860c86c040746248fb89 100644
--- a/drivers/gpu/drm/panthor/Makefile
+++ b/drivers/gpu/drm/panthor/Makefile
@@ -12,4 +12,6 @@ panthor-y := \
 	panthor_mmu.o \
 	panthor_sched.o
 
+obj-$(CONFIG_DRM_PANTHOR_MEDIATEK_MFG) += mediatek_mfg.o
+
 obj-$(CONFIG_DRM_PANTHOR) += panthor.o
diff --git a/drivers/gpu/drm/panthor/mediatek_mfg.c b/drivers/gpu/drm/panthor/mediatek_mfg.c
new file mode 100644
index 0000000000000000000000000000000000000000..1b3220d721e91334522b4e158ac3e262dea19d49
--- /dev/null
+++ b/drivers/gpu/drm/panthor/mediatek_mfg.c
@@ -0,0 +1,946 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for MediaTek MFlexGraphics Devices
+ *
+ * Copyright (C) 2025, Collabora Ltd.
+ */
+
+#include <linux/atomic.h>
+#include <linux/completion.h>
+#include <linux/clk.h>
+#include <linux/container_of.h>
+#include <linux/devfreq.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/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+
+#include "panthor_devfreq.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_SRAM_MAGIC	0xBABADADAUL
+
+#define GPUEB_TIMEOUT_US	10000UL
+#define GPUEB_POLL_US		50
+
+#define MAX_OPP_NUM             70
+
+#define 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 platform_device *pdev;
+	struct clk *clk_eb;
+	struct clk_bulk_data *gpu_clks;
+	struct regulator_bulk_data *gpu_regs;
+	void __iomem *rpc;
+	void __iomem *gpr;
+	void __iomem *sram;
+	phys_addr_t sram_phys;
+	unsigned int sram_size;
+	unsigned int ghpm_en_reg;
+	u32 ipi_magic;
+	unsigned int num_opps;
+	unsigned int num_unique_gpu_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;
+	int last_opp;
+	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;
+	int (*init)(struct mtk_mfg *mfg);
+};
+
+static inline void mtk_mfg_update_reg_bits(void __iomem *addr, u32 mask, u32 val)
+{
+	writel((readl(addr) & ~mask) | (val & mask), addr);
+}
+
+static inline unsigned long mtk_mfg_read_gpu_freq(struct mtk_mfg *mfg)
+{
+	return readl(mfg->sram + GF_REG_FREQ_CUR_GPU) * 1000UL;
+}
+
+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, we should skip doing
+	 * the power-on sequence, as it wouldn't work without powering it off
+	 * first.
+	 */
+	if ((readl(mfg->rpc + RPC_PWR_CON) & PWR_ACK_M) == PWR_ACK_M)
+		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;
+}
+
+static int mtk_mfg_send_ipi(struct mtk_mfg *mfg, struct mtk_mfg_ipi_msg *msg)
+{
+	struct device *dev = &mfg->pdev->dev;
+	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_for_completion(&mfg->gf_mbox->rx_done);
+
+	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 sram, 0x%X bytes\n", mfg->sram_size);
+	memset_io(mfg->sram, 0, mfg->sram_size);
+
+	msg.cmd = CMD_INIT_SHARED_MEM;
+	msg.u.shared_mem.base = mfg->sram_phys;
+	msg.u.shared_mem.size = mfg->sram_size;
+
+	ret = mtk_mfg_send_ipi(mfg, &msg);
+	if (ret)
+		return ret;
+
+	if (readl(mfg->sram) != GPUEB_SRAM_MAGIC) {
+		dev_err(dev, "EB did not initialise SRAM 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, int opp_idx)
+{
+	struct mtk_mfg_ipi_msg msg = {};
+	int ret;
+
+	if (!mfg->gpu_opps || !mfg->stack_opps)
+		return 0;
+
+	if (opp_idx >= mfg->num_opps)
+		return -EINVAL;
+
+	if (mfg->last_opp == opp_idx)
+		return 0;
+
+	msg.cmd = CMD_FIX_TARGET_OPPIDX;
+	msg.u.oppidx = opp_idx;
+
+	ret = mtk_mfg_send_ipi(mfg, &msg);
+	if (ret)
+		return ret;
+
+	mfg->last_opp = opp_idx;
+
+	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;
+	unsigned long long last_freq;
+
+	mfg->num_opps = readl(mfg->sram + GF_REG_GPU_OPP_NUM);
+	if (mfg->num_opps != readl(mfg->sram + GF_REG_STK_OPP_NUM)) {
+		dev_err(dev, "OPP count of GPU and Stack differ, %u vs. %u\n",
+			mfg->num_opps, readl(mfg->sram + GF_REG_STK_OPP_NUM));
+		return -EINVAL;
+	}
+
+	if (mfg->num_opps > MAX_OPP_NUM || mfg->num_opps == 0) {
+		dev_err(dev, "OPP count (%u) out of range %u >= count > 0\n",
+			mfg->num_opps, MAX_OPP_NUM);
+		return -EINVAL;
+	}
+
+	mfg->gpu_opps = devm_kcalloc(dev, mfg->num_opps,
+				     sizeof(struct dev_pm_opp_data), GFP_KERNEL);
+	if (!mfg->gpu_opps)
+		return -ENOMEM;
+
+	mfg->stack_opps = devm_kcalloc(dev, mfg->num_opps,
+				       sizeof(struct dev_pm_opp_data), GFP_KERNEL);
+	if (!mfg->stack_opps)
+		return -ENOMEM;
+
+	for (i = 0; i < mfg->num_opps; i++) {
+		memcpy_fromio(&e, mfg->sram + 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 * 1000ULL;
+		mfg->gpu_opps[i].u_volt = e.voltage_core * 10;
+
+		if (!last_freq || mfg->gpu_opps[i].freq != last_freq)
+			mfg->num_unique_gpu_opps++;
+
+		last_freq = mfg->gpu_opps[i].freq;
+	}
+
+	/*
+	 * Unused at present; we currently use the same OPP index for both GPU
+	 * core and GPU stack, and the way they contain duplicates seems to
+	 * indicate that this is the way to go.
+	 *
+	 * Might still be useful if we want to expose finer-grained adjustments
+	 * or better information about expected power draw.
+	 */
+	for (i = 0; i < mfg->num_opps; i++) {
+		memcpy_fromio(&e, mfg->sram + GF_REG_OPP_TABLE_STK + 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->stack_opps[i].freq = e.freq_khz * 1000ULL;
+		mfg->stack_opps[i].u_volt = e.voltage_core * 10;
+	}
+
+	return 0;
+}
+
+static inline bool mtk_mfg_opp_idx_match(struct mtk_mfg *mfg, int idx,
+					 unsigned long rate)
+{
+	if ((idx == mfg->num_opps - 1) && mfg->gpu_opps[idx].freq >= rate)
+		return true;
+
+	if (mfg->gpu_opps[idx].freq >= rate && mfg->gpu_opps[idx + 1].freq < rate)
+		return true;
+
+	return false;
+}
+
+/**
+ * mtk_mfg_get_closest_opp_idx - find OPP index for desired GPU frequency
+ * @mfg: pointer to a &struct mtk_mfg driver instance
+ * @gpu_rate: desired rate of the GPU core in Hz
+ *
+ * Given a desired target frequency for the GPU core, find the index of a
+ * matching OPP, or the next higher OPP if no exact match is found, or the
+ * maximum OPP for frequencies exceeding the maximum OPP's frequency.
+ *
+ * For duplicate OPP entries, chooses the highest OPP index, as in, the one
+ * closest to the next lower frequency OPP.
+ *
+ * Returns -EINVAL on error, or the OPP index on success.
+ */
+static int mtk_mfg_get_closest_opp_idx(struct mtk_mfg *mfg, unsigned long gpu_rate)
+{
+	int r = mfg->num_opps - 1;
+	int l = 0;
+	int m;
+
+	if (!mfg->gpu_opps)
+		return -EINVAL;
+
+	if (mfg->gpu_opps[0].freq <= gpu_rate)
+		return 0;
+
+	while (l <= r) {
+		m = l + (r - l) / 2;
+		if (mtk_mfg_opp_idx_match(mfg, m, gpu_rate))
+			return m;
+		if (mfg->gpu_opps[m].freq >= gpu_rate) /* >= for dupes */
+			l = m + 1;
+		else
+			r = m - 1;
+	}
+
+	return -EINVAL;
+}
+
+static int mtk_mfg_devfreq_target(struct device *dev, unsigned long *freq,
+				  u32 flags)
+{
+	struct mtk_mfg *mfg = dev_get_drvdata(dev);
+	int ret, opp;
+
+	opp = mtk_mfg_get_closest_opp_idx(mfg, *freq);
+	if (opp < 0) {
+		dev_err(dev, "Couldn't get closest OPP: %pe\n", ERR_PTR(opp));
+		return opp;
+	}
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	ret =  mtk_mfg_set_oppidx(mfg, opp);
+	if (!ret)
+		*freq = mtk_mfg_read_gpu_freq(mfg);
+	else
+		dev_err(dev, "Couldn't set OPP: %pe\n", ERR_PTR(ret));
+
+	pm_runtime_put(dev);
+
+	return ret;
+}
+
+static int mtk_mfg_devfreq_get_cur_freq(struct device *dev, unsigned long *freq)
+{
+	struct mtk_mfg *mfg = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	*freq = mtk_mfg_read_gpu_freq(mfg);
+
+	pm_runtime_put(dev);
+
+	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");
+
+	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;
+
+	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),
+	.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, MBOX_MAX_RX_SIZE);
+	complete(&mb->rx_done);
+}
+
+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;
+
+	slp = devm_kzalloc(dev, sizeof(*slp), GFP_KERNEL);
+	if (!slp)
+		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->rx_data = devm_kzalloc(dev, MBOX_MAX_RX_SIZE, GFP_KERNEL);
+	if (!gf->rx_data)
+		return -ENOMEM;
+	gf->ch = mbox_request_channel_byname(&gf->cl, "gpufreq");
+	if (IS_ERR(gf->ch))
+		return PTR_ERR(gf->ch);
+
+	mfg->gf_mbox = gf;
+
+	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))
+		return PTR_ERR(slp->ch);
+
+	mfg->slp_mbox = slp;
+
+	return 0;
+}
+
+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 int mtk_mfg_probe(struct platform_device *pdev)
+{
+	struct device_node *shmem __free(device_node);
+	struct panthor_devfreq_provider *prov;
+	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, j;
+
+	mfg = devm_kzalloc(dev, sizeof(*mfg), GFP_KERNEL);
+	if (!mfg)
+		return -ENOMEM;
+
+	prov = devm_kzalloc(dev, sizeof(*prov), GFP_KERNEL);
+	if (!prov)
+		return -ENOMEM;
+
+	prov->dev = dev;
+	prov->get_cur_freq = mtk_mfg_devfreq_get_cur_freq;
+	prov->target = mtk_mfg_devfreq_target;
+
+	mfg->pdev = pdev;
+	mfg->variant = data;
+
+	dev_set_drvdata(dev, mfg);
+
+	mfg->gpr = devm_platform_ioremap_resource_byname(pdev, "gpr");
+	if (IS_ERR(mfg->gpr))
+		return dev_err_probe(dev, PTR_ERR(mfg->gpr),
+				     "Could not retrieve GPR MMIO registers\n");
+
+	mfg->rpc = devm_platform_ioremap_resource_byname(pdev, "rpc");
+	if (IS_ERR(mfg->rpc))
+		return dev_err_probe(dev, PTR_ERR(mfg->rpc),
+				     "Could not 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),
+				     "Could not 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");
+
+	shmem = of_parse_phandle(dev->of_node, "shmem", 0);
+	if (!shmem)
+		return dev_err_probe(dev, -ENODEV, "Could not get 'shmem'\n");
+
+	ret = of_address_to_resource(shmem, 0, &res);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get GPUEB shared memory\n");
+
+	mfg->sram = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!mfg->sram)
+		return dev_err_probe(dev, -EADDRNOTAVAIL,
+				     "failed to ioremap GPUEB sram\n");
+	mfg->sram_size = resource_size(&res);
+	mfg->sram_phys = res.start;
+
+	if (data->init) {
+		ret = data->init(mfg);
+		if (ret)
+			return dev_err_probe(dev, ret, "Variant init failed\n");
+	}
+
+	ret = clk_prepare_enable(mfg->clk_eb);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to turn on EB clock\n");
+	mfg->ipi_magic = readl(mfg->gpr + GPR_IPI_MAGIC);
+	/* Downstream does this, don't know why. */
+	writel(0x0, mfg->gpr + GPR_IPI_MAGIC);
+
+	ret = mtk_mfg_init_mbox(mfg);
+	if (ret) {
+		ret = dev_err_probe(dev, ret, "Couldn't initialise mailbox\n");
+		goto out;
+	}
+
+	clk_disable_unprepare(mfg->clk_eb);
+
+	mfg->last_opp = -1;
+
+	devm_pm_runtime_enable(dev);
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to power on MFG\n");
+
+	ret = mtk_mfg_init_shared_mem(mfg);
+	if (ret) {
+		dev_err(dev, "Couldn't initialize EB SRAM: %pe\n", ERR_PTR(ret));
+		goto out;
+	}
+
+	ret = mtk_mfg_read_opp_tables(mfg);
+	if (ret) {
+		dev_err(dev, "Error reading OPP tables from EB: %pe\n",
+			ERR_PTR(ret));
+		goto out;
+	}
+
+	/* gpu_opps may contain duplicates and is sorted the other way */
+	prov->opp_table = devm_kmalloc_array(dev, mfg->num_unique_gpu_opps,
+					     sizeof(*prov->opp_table), GFP_KERNEL);
+	if (!prov->opp_table) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	prov->num_opps = mfg->num_unique_gpu_opps;
+
+	j = 0;
+	for (i = mfg->num_opps - 1; i >= 0 && j < mfg->num_unique_gpu_opps; i--)
+		if ((j == 0) || (mfg->gpu_opps[i].freq != prov->opp_table[j - 1]->freq))
+			prov->opp_table[j++] = &mfg->gpu_opps[i];
+
+	ret = panthor_devfreq_register_provider(prov);
+
+out:
+	pm_runtime_put(dev);
+	return ret;
+}
+
+static int mtk_mfg_suspend(struct device *dev)
+{
+	struct mtk_mfg *mfg = dev_get_drvdata(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);
+
+	return ret;
+}
+
+static int mtk_mfg_resume(struct device *dev)
+{
+	struct mtk_mfg *mfg = dev_get_drvdata(dev);
+	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;
+
+	ret = mtk_mfg_power_control(mfg, true);
+	if (ret)
+		goto err_eb_off;
+
+	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 DEFINE_RUNTIME_DEV_PM_OPS(mtk_mfg_pm_ops,
+				 mtk_mfg_suspend,
+				 mtk_mfg_resume,
+				 NULL);
+
+static struct platform_driver mtk_mfg_driver = {
+	.driver = {
+		.name = "panthor-mtk-mfg",
+		.of_match_table = mtk_mfg_of_match,
+		.pm = pm_ptr(&mtk_mfg_pm_ops),
+	},
+	.probe = mtk_mfg_probe,
+};
+module_platform_driver(mtk_mfg_driver);
+
+MODULE_SOFTDEP("pre: panthor");
+MODULE_AUTHOR("Nicolas Frattaroli <nicolas.frattaroli@collabora.com>");
+MODULE_DESCRIPTION("MediaTek MFlexGraphics Extension for Panthor");
+MODULE_LICENSE("GPL");

-- 
2.51.0

Re: [PATCH v3 09/10] drm/panthor: add no_clocks soc_data member for MT8196

[AngeloGioacchino Del Regno]

Il 17/09/25 14:22, Nicolas Frattaroli ha scritto:
> While panthor does not try to do anything untoward with the core clock
> outside of increasing its enable/disable count, the spirit of using DT
> to describe hardware, not what drivers need, informs us that on the
> MT8196, panthor should work without one, as the true owner of the clocks
> in this case is the performance-domain.
> 
> Add a boolean to the soc_data struct that tells panthor whether on any
> given SoC, it can leave even the core clock NULL. Set it to true for the
> MT8196 soc_data instance.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

The only doubt that I have here is about the "no_clocks" name, as it may be read
as "number of clocks" ("no" is often used as "number").

I don't really have any better name to suggest though, and the execution is right,
so.....

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Cheers,
Angelo

Re: [PATCH v3 03/10] dt-bindings: sram: Add compatible for mediatek,mt8196-gpufreq-sram

[AngeloGioacchino Del Regno]

Il 17/09/25 14:22, Nicolas Frattaroli ha scritto:
> This compatible is used for an SRAM section that's shared between the
> MT8196's application processor cores and the embedded GPUEB MCU that
> controls the GPU frequency.
> 
> Through this SRAM section, things about the GPU frequency controller
> like the OPP table can be read.
> 
> Acked-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 04/10] dt-bindings: mailbox: Add MT8196 GPUEB Mailbox

[AngeloGioacchino Del Regno]

Il 17/09/25 14:22, Nicolas Frattaroli ha scritto:
> The MediaTek MT8196 SoC includes an embedded MCU referred to as "GPUEB",
> acting as glue logic to control power and frequency of the Mali GPU.
> This MCU runs special-purpose firmware for this use, and the main
> application processor communicates with it through a mailbox.
> 
> Add a binding that describes this mailbox.
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>   .../mailbox/mediatek,mt8196-gpueb-mbox.yaml        | 64 ++++++++++++++++++++++
>   1 file changed, 64 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,mt8196-gpueb-mbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,mt8196-gpueb-mbox.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..ab5b780cb83a708a3897ca1a440131d97b56c3a6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/mediatek,mt8196-gpueb-mbox.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/mediatek,mt8196-gpueb-mbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek MFlexGraphics GPUEB Mailbox Controller
> +
> +maintainers:
> +  - Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mediatek,mt8196-gpueb-mbox

Before anyone asks - yes, it is 100% sure that SoCs will be added here sooner or
later.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 05/10] mailbox: add MediaTek GPUEB IPI mailbox

[AngeloGioacchino Del Regno]

Il 17/09/25 14:22, Nicolas Frattaroli ha scritto:
> The MT8196 SoC uses an embedded MCU to control frequencies and power of
> the GPU. This controller is referred to as "GPUEB".
> 
> It communicates to the application processor, among other ways, through
> a mailbox.
> 
> The mailbox exposes one interrupt, which appears to only be fired when a
> response is received, rather than a transaction is completed. For us,
> this means we unfortunately need to poll for txdone.
> 
> The mailbox also requires the EB clock to be on when touching any of the
> mailbox registers.
> 
> Add a simple driver for it based on the common mailbox framework.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 00/10] MT8196 GPU Frequency/Power Control Support

[Ulf Hansson]

On Wed, 17 Sept 2025 at 14:23, 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 SRAM, 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. There's also
> several more powerful OPPs it seemingly refuses to apply. The downstream
> chromeos kernel also doesn't reach the frequencies of those OPPs, so we
> assume this is expected.
>
> The frequency control driver lives in panthor's subdirectory, as it
> needs to pass panthor some data. I've kept the tie-in parts generic
> enough however to not make this a complete hack; mediatek_mfg (the
> frequency control driver) registers itself as a "devfreq provider" with
> panthor, and panthor picks it up during its probe function (or defers if
> mediatek_mfg is not ready yet, after adding a device link first).
>
> It's now generic enough to where I'll ponder about moving the devfreq
> provider stuff into a header in include/, and moving mediatek_mfg into
> the drivers/soc/ subdirectory, but there were enough changes so far to
> warrant a v3 without a move or further struct renames added, so that I
> can get feedback on this approach.
>
> The mailbox driver is a fairly bog-standard common mailbox framework
> driver, just specific to the firmware that runs on the GPUEB.

I had a brief look at the series and it seems to me that the devfreq
thing here, may not be the perfect fit.

Rather than using a new binding  (#performance-domain-cells) to model
a performance domain provider using devfreq, I think it could be more
straightforward to model this using the common #power-domain-cells
binding instead. As a power-domain provider then, which would be
capable of scaling performance too. Both genpd and the OPP core
already support this, though via performance-states (as indexes).

In fact, this looks very similar to what we have implemented for the
Arm SCMI performance domain.

If you have a look at the below, I think it should give you an idea of
the pieces.
drivers/pmdomain/arm/scmi_perf_domain.c
drivers/firmware/arm_scmi/perf.c
Documentation/devicetree/bindings/firmware/arm,scmi.yaml (protocol@13
is the performance protocol).

That said, I don't have a strong opinion, but just wanted to share my
thoughts on your approach.

[...]

Kind regards
Uffe

Re: [PATCH v3 01/10] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Rob Herring (Arm)]

On Wed, 17 Sep 2025 14:22:32 +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.
> 
> It lets us omit the OPP tables from the device tree, as those can now be
> enumerated at runtime from the MCU. It also means the mali GPU node
> described in this binding does not have any clocks in this case, as all
> clock control is delegated to the MCU.
> 
> Add the mediatek,mt8196-mali compatible, and a performance-domains
> property which points to the MCU's device tree node in this case. It's
> required on mt8196 devices.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .../bindings/gpu/arm,mali-valhall-csf.yaml         | 32 ++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.example.dtb: gpu@48000000 (mediatek,mt8196-mali): 'clocks' is a required property
	from schema $id: http://devicetree.org/schemas/gpu/arm,mali-valhall-csf.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250917-mt8196-gpufreq-v3-1-c4ede4b4399e@collabora.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v3 00/10] MT8196 GPU Frequency/Power Control Support

[Nicolas Frattaroli]

On Wednesday, 17 September 2025 15:28:59 Central European Summer Time Ulf Hansson wrote:
> On Wed, 17 Sept 2025 at 14:23, 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 SRAM, 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. There's also
> > several more powerful OPPs it seemingly refuses to apply. The downstream
> > chromeos kernel also doesn't reach the frequencies of those OPPs, so we
> > assume this is expected.
> >
> > The frequency control driver lives in panthor's subdirectory, as it
> > needs to pass panthor some data. I've kept the tie-in parts generic
> > enough however to not make this a complete hack; mediatek_mfg (the
> > frequency control driver) registers itself as a "devfreq provider" with
> > panthor, and panthor picks it up during its probe function (or defers if
> > mediatek_mfg is not ready yet, after adding a device link first).
> >
> > It's now generic enough to where I'll ponder about moving the devfreq
> > provider stuff into a header in include/, and moving mediatek_mfg into
> > the drivers/soc/ subdirectory, but there were enough changes so far to
> > warrant a v3 without a move or further struct renames added, so that I
> > can get feedback on this approach.
> >
> > The mailbox driver is a fairly bog-standard common mailbox framework
> > driver, just specific to the firmware that runs on the GPUEB.
> 
> I had a brief look at the series and it seems to me that the devfreq
> thing here, may not be the perfect fit.
> 
> Rather than using a new binding  (#performance-domain-cells) to model
> a performance domain provider using devfreq, I think it could be more
> straightforward to model this using the common #power-domain-cells
> binding instead. As a power-domain provider then, which would be
> capable of scaling performance too. Both genpd and the OPP core
> already support this, though via performance-states (as indexes).
> 
> In fact, this looks very similar to what we have implemented for the
> Arm SCMI performance domain.
> 
> If you have a look at the below, I think it should give you an idea of
> the pieces.
> drivers/pmdomain/arm/scmi_perf_domain.c
> drivers/firmware/arm_scmi/perf.c
> Documentation/devicetree/bindings/firmware/arm,scmi.yaml (protocol@13
> is the performance protocol).
> 
> That said, I don't have a strong opinion, but just wanted to share my
> thoughts on your approach.

Yeah, I found out about the pmdomain set_performance_state callback
a few days ago. I've not looked into it much so far because not
unlike a veterinarian on a cattle ranch, I was elbow-deep in v3's
guts already and didn't want to pivot to something different before
pushing it out, but I'll look into it more seriously now.

Even if it means I have to get rid of my fun array binary search
and rely on the opp core to do its linear time linked list
traversal. :'( (But moving OPP core to use XArrays instead is a
concern for the future.)

I've also been avoiding it because I didn't know how much
additional functionality we'll add later, but I've talked with
Angelo about it an hour ago and he agrees that I should go down
the pmdomain route for the current functionality.

Thank you for the hints!

Kind regards,
Nicolas Frattaroli

> 
> [...]
> 
> Kind regards
> Uffe
> 

Re: [PATCH v3 01/10] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Krzysztof Kozlowski]

On Wed, Sep 17, 2025 at 02:22:32PM +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.
> 
> It lets us omit the OPP tables from the device tree, as those can now be
> enumerated at runtime from the MCU. It also means the mali GPU node
> described in this binding does not have any clocks in this case, as all
> clock control is delegated to the MCU.
> 
> Add the mediatek,mt8196-mali compatible, and a performance-domains
> property which points to the MCU's device tree node in this case. It's
> required on mt8196 devices.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .../bindings/gpu/arm,mali-valhall-csf.yaml         | 32 ++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> index 7ad5a3ffc5f5c753322eda9e74cc65de89d11c73..ccab2dd0ea852187e3ab75923e19739622b2b3b8 100644
> --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> @@ -38,7 +38,6 @@ properties:
>        - const: gpu
>  
>    clocks:
> -    minItems: 1

I don't understand why.

Best regards,
Krzysztof

Re: [PATCH v3 02/10] dt-bindings: devfreq: add mt8196-gpufreq binding

[Krzysztof Kozlowski]

On Wed, Sep 17, 2025 at 02:22:33PM +0200, Nicolas Frattaroli wrote:
 +
> +  shmem:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the shared memory region of the GPUEB MCU
> +
> +  "#performance-domain-cells":
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - mboxes
> +  - mbox-names
> +  - shmem
> +  - "#performance-domain-cells"

Keep same order as in "properties:" block.

> +
> +additionalProperties: false

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v3 01/10] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Nicolas Frattaroli]

On Thursday, 18 September 2025 02:30:09 Central European Summer Time Krzysztof Kozlowski wrote:
> On Wed, Sep 17, 2025 at 02:22:32PM +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.
> > 
> > It lets us omit the OPP tables from the device tree, as those can now be
> > enumerated at runtime from the MCU. It also means the mali GPU node
> > described in this binding does not have any clocks in this case, as all
> > clock control is delegated to the MCU.
> > 
> > Add the mediatek,mt8196-mali compatible, and a performance-domains
> > property which points to the MCU's device tree node in this case. It's
> > required on mt8196 devices.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  .../bindings/gpu/arm,mali-valhall-csf.yaml         | 32 ++++++++++++++++++++--
> >  1 file changed, 30 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > index 7ad5a3ffc5f5c753322eda9e74cc65de89d11c73..ccab2dd0ea852187e3ab75923e19739622b2b3b8 100644
> > --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > @@ -38,7 +38,6 @@ properties:
> >        - const: gpu
> >  
> >    clocks:
> > -    minItems: 1
> 
> I don't understand why.
> 
> Best regards,
> Krzysztof
> 
> 

I am executing a Convex hull algorithm on the 3D space of "dt-bindings
maintainer opinions" to get a convex hull of acceptable dt-bindings
choices where two different choices are functionally equivalent.

With this additional opinion on the krzk axis, I now know that having
the base properties accurate for the general case is not required if
the per-compatible case sets the property to false anyway.

I hope no two opinions are collinear, as this would surely be my
undoing.

You get to pick which axis (X, Y, Z) you are. Right-hand rule, of
course.

Kind regards,
Nicolas Frattaroli

Re: [PATCH v3 00/10] MT8196 GPU Frequency/Power Control Support

[Ulf Hansson]

On Wed, 17 Sept 2025 at 17:45, Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
>
> On Wednesday, 17 September 2025 15:28:59 Central European Summer Time Ulf Hansson wrote:
> > On Wed, 17 Sept 2025 at 14:23, 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 SRAM, 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. There's also
> > > several more powerful OPPs it seemingly refuses to apply. The downstream
> > > chromeos kernel also doesn't reach the frequencies of those OPPs, so we
> > > assume this is expected.
> > >
> > > The frequency control driver lives in panthor's subdirectory, as it
> > > needs to pass panthor some data. I've kept the tie-in parts generic
> > > enough however to not make this a complete hack; mediatek_mfg (the
> > > frequency control driver) registers itself as a "devfreq provider" with
> > > panthor, and panthor picks it up during its probe function (or defers if
> > > mediatek_mfg is not ready yet, after adding a device link first).
> > >
> > > It's now generic enough to where I'll ponder about moving the devfreq
> > > provider stuff into a header in include/, and moving mediatek_mfg into
> > > the drivers/soc/ subdirectory, but there were enough changes so far to
> > > warrant a v3 without a move or further struct renames added, so that I
> > > can get feedback on this approach.
> > >
> > > The mailbox driver is a fairly bog-standard common mailbox framework
> > > driver, just specific to the firmware that runs on the GPUEB.
> >
> > I had a brief look at the series and it seems to me that the devfreq
> > thing here, may not be the perfect fit.
> >
> > Rather than using a new binding  (#performance-domain-cells) to model
> > a performance domain provider using devfreq, I think it could be more
> > straightforward to model this using the common #power-domain-cells
> > binding instead. As a power-domain provider then, which would be
> > capable of scaling performance too. Both genpd and the OPP core
> > already support this, though via performance-states (as indexes).
> >
> > In fact, this looks very similar to what we have implemented for the
> > Arm SCMI performance domain.
> >
> > If you have a look at the below, I think it should give you an idea of
> > the pieces.
> > drivers/pmdomain/arm/scmi_perf_domain.c
> > drivers/firmware/arm_scmi/perf.c
> > Documentation/devicetree/bindings/firmware/arm,scmi.yaml (protocol@13
> > is the performance protocol).
> >
> > That said, I don't have a strong opinion, but just wanted to share my
> > thoughts on your approach.
>
> Yeah, I found out about the pmdomain set_performance_state callback
> a few days ago. I've not looked into it much so far because not
> unlike a veterinarian on a cattle ranch, I was elbow-deep in v3's
> guts already and didn't want to pivot to something different before
> pushing it out, but I'll look into it more seriously now.

:-)

>
> Even if it means I have to get rid of my fun array binary search
> and rely on the opp core to do its linear time linked list
> traversal. :'( (But moving OPP core to use XArrays instead is a
> concern for the future.)

Sure!

>
> I've also been avoiding it because I didn't know how much
> additional functionality we'll add later, but I've talked with
> Angelo about it an hour ago and he agrees that I should go down
> the pmdomain route for the current functionality.
>
> Thank you for the hints!

Np! I am glad to help!

I will try my best to continue to review/comment on these things, if
you need it.

Kind regards
Uffe

Re: [PATCH v3 01/10] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Krzysztof Kozlowski]

On 18/09/2025 23:01, Nicolas Frattaroli wrote:
> On Thursday, 18 September 2025 02:30:09 Central European Summer Time Krzysztof Kozlowski wrote:
>> On Wed, Sep 17, 2025 at 02:22:32PM +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.
>>>
>>> It lets us omit the OPP tables from the device tree, as those can now be
>>> enumerated at runtime from the MCU. It also means the mali GPU node
>>> described in this binding does not have any clocks in this case, as all
>>> clock control is delegated to the MCU.
>>>
>>> Add the mediatek,mt8196-mali compatible, and a performance-domains
>>> property which points to the MCU's device tree node in this case. It's
>>> required on mt8196 devices.
>>>
>>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>>> ---
>>>  .../bindings/gpu/arm,mali-valhall-csf.yaml         | 32 ++++++++++++++++++++--
>>>  1 file changed, 30 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
>>> index 7ad5a3ffc5f5c753322eda9e74cc65de89d11c73..ccab2dd0ea852187e3ab75923e19739622b2b3b8 100644
>>> --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
>>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
>>> @@ -38,7 +38,6 @@ properties:
>>>        - const: gpu
>>>  
>>>    clocks:
>>> -    minItems: 1
>>
>> I don't understand why.
>>
>> Best regards,
>> Krzysztof
>>
>>
> 
> I am executing a Convex hull algorithm on the 3D space of "dt-bindings
> maintainer opinions" to get a convex hull of acceptable dt-bindings
> choices where two different choices are functionally equivalent.
> 
> With this additional opinion on the krzk axis, I now know that having
> the base properties accurate for the general case is not required if
> the per-compatible case sets the property to false anyway.
> 
> I hope no two opinions are collinear, as this would surely be my
> undoing.
> 
> You get to pick which axis (X, Y, Z) you are. Right-hand rule, of
> course.


This piece of code is wrong and I could not deduce the reason. That's
why I asked why you need that change. If you intend to waste my time, I
will don't bother with this, but code is still wrong.

Best regards,
Krzysztof

Re: [PATCH v3 01/10] dt-bindings: gpu: mali-valhall-csf: add mediatek,mt8196-mali variant

[Nicolas Frattaroli]

On Friday, 19 September 2025 06:28:54 Central European Summer Time Krzysztof Kozlowski wrote:
> On 18/09/2025 23:01, Nicolas Frattaroli wrote:
> > On Thursday, 18 September 2025 02:30:09 Central European Summer Time Krzysztof Kozlowski wrote:
> >> On Wed, Sep 17, 2025 at 02:22:32PM +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.
> >>>
> >>> It lets us omit the OPP tables from the device tree, as those can now be
> >>> enumerated at runtime from the MCU. It also means the mali GPU node
> >>> described in this binding does not have any clocks in this case, as all
> >>> clock control is delegated to the MCU.
> >>>
> >>> Add the mediatek,mt8196-mali compatible, and a performance-domains
> >>> property which points to the MCU's device tree node in this case. It's
> >>> required on mt8196 devices.
> >>>
> >>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> >>> ---
> >>>  .../bindings/gpu/arm,mali-valhall-csf.yaml         | 32 ++++++++++++++++++++--
> >>>  1 file changed, 30 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> >>> index 7ad5a3ffc5f5c753322eda9e74cc65de89d11c73..ccab2dd0ea852187e3ab75923e19739622b2b3b8 100644
> >>> --- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> >>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> >>> @@ -38,7 +38,6 @@ properties:
> >>>        - const: gpu
> >>>  
> >>>    clocks:
> >>> -    minItems: 1
> >>
> >> I don't understand why.
> >>
> >> Best regards,
> >> Krzysztof
> >>
> >>
> > 
> > I am executing a Convex hull algorithm on the 3D space of "dt-bindings
> > maintainer opinions" to get a convex hull of acceptable dt-bindings
> > choices where two different choices are functionally equivalent.
> > 
> > With this additional opinion on the krzk axis, I now know that having
> > the base properties accurate for the general case is not required if
> > the per-compatible case sets the property to false anyway.
> > 
> > I hope no two opinions are collinear, as this would surely be my
> > undoing.
> > 
> > You get to pick which axis (X, Y, Z) you are. Right-hand rule, of
> > course.
> 
> 
> This piece of code is wrong and I could not deduce the reason. That's
> why I asked why you need that change. If you intend to waste my time, I
> will don't bother with this, but code is still wrong.
> 
> Best regards,
> Krzysztof
> 

The previous reply was in jest, I did not know that this was incorrect.
That's because it has no effect; there is no option for a bare
`arm,mali-valhall-csf` compatible with no other compatible preceding
it right now as far as I can parse the binding (which in of itself
seems wrong.) So minItems: 1 didn't do anything, because the RK3588
compatible would override it anyway, and the mt8196 compatible would
disable the property entirely.

Hence, this piece of code is not wrong, as it's functionally
equivalent to what was already there. Unless that is my reasoning
is erroneous, in which case I'd appreciate an explanation for how
it is wrong, rather than having to argue why I think you think
wrongly that it is wrong.

Kind regards,
Nicolas Frattaroli

Re: [PATCH v3 05/10] mailbox: add MediaTek GPUEB IPI mailbox

[Jassi Brar]

On Wed, Sep 17, 2025 at 7:23 AM Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
....

> +#define MBOX_CTL_TX_STS                0x0000
> +#define MBOX_CTL_IRQ_SET       0x0004
> +#define MBOX_CTL_IRQ_CLR       0x0074
> +#define MBOX_CTL_RX_STS                0x0078
> +
1) Please don't pollute the global namespace. Make these something
like MBOX_MTK_GPUEB_xxx. Here and elsewhere.
2) You don't write short values, so maybe just 0x04, 0x04 0x74 and 0x78.


> +#define MBOX_FULL              BIT(0) /* i.e. we've received data */
> +#define MBOX_CLOGGED           BIT(1) /* i.e. the channel is shutdown */
> +
This is confusing. CLOGGED usually means malfunction, but it seems you
want to call it STOPPED or UNINIT?


> +#define MBOX_MAX_RX_SIZE       32 /* in bytes */
> +
> +struct mtk_gpueb_mbox {
> +       struct device *dev;
> +       struct clk *clk;
> +       void __iomem *mbox_mmio;
> +       void __iomem *mbox_ctl;
> +       struct mbox_controller mbox;
> +       struct mtk_gpueb_mbox_chan *ch;
> +       int irq;
> +       const struct mtk_gpueb_mbox_variant *v;
> +};
Other structures have kernel-doc, so why not here too?
...

> +
> +static int mtk_gpueb_mbox_send_data(struct mbox_chan *chan, void *data)
> +{
> +       struct mtk_gpueb_mbox_chan *ch = chan->con_priv;
> +       int i;
> +       u32 *values = data;
> +
maybe order in decreasing lengths ?


> +
> +       /*
> +        * We don't want any fancy nonsense, just write the 32-bit values in
> +        * order. memcpy_toio/__iowrite32_copy don't work here, because fancy.
> +        */
>
Please make the comment technical. Currently it just expresses your
distaste for fancy :)

> +       for (i = 0; i < ch->c->tx_len; i += 4)
> +               writel(values[i / 4], ch->ebm->mbox_mmio + ch->c->tx_offset + i);
> +

...
> +
> +static struct mbox_chan *
> +mtk_gpueb_mbox_of_xlate(struct mbox_controller *mbox,
> +                       const struct of_phandle_args *sp)
> +{
> +       struct mtk_gpueb_mbox *ebm = dev_get_drvdata(mbox->dev);
> +
> +       if (!sp->args_count)
> +               return ERR_PTR(-EINVAL);
> +
> +       if (sp->args[0] >= ebm->v->num_channels)
> +               return ERR_PTR(-ECHRNG);
> +
> +       return &mbox->chans[sp->args[0]];
> +}
>
Just use the default of_mbox_index_xlate()

....
> +
> +       for (i = 0; i < ebm->v->num_channels; i++) {

You make this block a bit cleaner by using a temporary variable
        echan = &ebm->ch[i];
and using echan instead of ebm->ch[i] a dozen times below.

> +               ebm->ch[i].c = &ebm->v->channels[i];
> +               if (ebm->ch[i].c->rx_len > MBOX_MAX_RX_SIZE) {
> +                       dev_err(ebm->dev, "Channel %s RX size (%d) too large\n",
> +                               ebm->ch[i].c->name, ebm->ch[i].c->rx_len);
> +                       return -EINVAL;
> +               }
> +               ebm->ch[i].full_name = devm_kasprintf(ebm->dev, GFP_KERNEL, "%s:%s",
> +                                                     dev_name(ebm->dev), ebm->ch[i].c->name);
> +               if (!ebm->ch[i].full_name)
> +                       return -ENOMEM;
> +
> +               ebm->ch[i].ebm = ebm;
> +               ebm->ch[i].num = i;
> +               spin_lock_init(&ebm->mbox.chans[i].lock);
> +               ebm->mbox.chans[i].con_priv = &ebm->ch[i];
> +               atomic_set(&ebm->ch[i].rx_status, MBOX_CLOGGED);
> +       }
> +


-j

Re: [PATCH v3 05/10] mailbox: add MediaTek GPUEB IPI mailbox

[Nicolas Frattaroli]

On Sunday, 21 September 2025 07:00:59 Central European Summer Time Jassi Brar wrote:
> On Wed, Sep 17, 2025 at 7:23 AM Nicolas Frattaroli
> <nicolas.frattaroli@collabora.com> wrote:
> ....
> 
> > +#define MBOX_CTL_TX_STS                0x0000
> > +#define MBOX_CTL_IRQ_SET       0x0004
> > +#define MBOX_CTL_IRQ_CLR       0x0074
> > +#define MBOX_CTL_RX_STS                0x0078
> > +
> 1) Please don't pollute the global namespace. Make these something
> like MBOX_MTK_GPUEB_xxx. Here and elsewhere.

I tend to disagree. These don't pollute the global namespace, they're
defined as part of the file, so only pollute its local scope. I'm not
going to make 25 character long symbols just to work around an issue
that doesn't exist, but may exist in the unlikely future where
mailbox.h gets its own symbol named precisely the same way but
whoever adds it doesn't try to compile test every single mailbox
driver to make sure they didn't break anything.

> 2) You don't write short values, so maybe just 0x04, 0x04 0x74 and 0x78.
> 
> 
> > +#define MBOX_FULL              BIT(0) /* i.e. we've received data */
> > +#define MBOX_CLOGGED           BIT(1) /* i.e. the channel is shutdown */
> > +
> This is confusing. CLOGGED usually means malfunction, but it seems you
> want to call it STOPPED or UNINIT?

I don't agree that "CLOGGED usually means malfunction". To clog something
is to impede its flow, which in this case is the correct terminology to
refer to what's happened to the channel. "UNINIT" is wrong, it's initialised
properly. "STOPPED" is also wrong, it's not stopped, it still sends, it just
won't pass it on through.

> 
> 
> > +#define MBOX_MAX_RX_SIZE       32 /* in bytes */
> > +
> > +struct mtk_gpueb_mbox {
> > +       struct device *dev;
> > +       struct clk *clk;
> > +       void __iomem *mbox_mmio;
> > +       void __iomem *mbox_ctl;
> > +       struct mbox_controller mbox;
> > +       struct mtk_gpueb_mbox_chan *ch;
> > +       int irq;
> > +       const struct mtk_gpueb_mbox_variant *v;
> > +};
> Other structures have kernel-doc, so why not here too?
> ...
>

Because fully documenting all internal structures is not required
for acceptance and writing redundant explanations for members that
can be understood from name and context is redundant.

> > +
> > +static int mtk_gpueb_mbox_send_data(struct mbox_chan *chan, void *data)
> > +{
> > +       struct mtk_gpueb_mbox_chan *ch = chan->con_priv;
> > +       int i;
> > +       u32 *values = data;
> > +
> maybe order in decreasing lengths ?
> 
> 
> > +
> > +       /*
> > +        * We don't want any fancy nonsense, just write the 32-bit values in
> > +        * order. memcpy_toio/__iowrite32_copy don't work here, because fancy.
> > +        */
> >
> Please make the comment technical. Currently it just expresses your
> distaste for fancy :)
> 

Then I will have to make an assertive statement about memory semantics
of those two calls and how they differ from writel, which I don't want
to do, because it would likely be inaccurate or not the full picture
as those two calls can do a variety of things depending on the platform.

Saying that I want 32-bit writes in order is much simpler than explaining
how the two mentioned calls some well-meaning future developer may wish
to replace this with don't do that.

> > +       for (i = 0; i < ch->c->tx_len; i += 4)
> > +               writel(values[i / 4], ch->ebm->mbox_mmio + ch->c->tx_offset + i);
> > +
> 
> ...
> > +
> > +static struct mbox_chan *
> > +mtk_gpueb_mbox_of_xlate(struct mbox_controller *mbox,
> > +                       const struct of_phandle_args *sp)
> > +{
> > +       struct mtk_gpueb_mbox *ebm = dev_get_drvdata(mbox->dev);
> > +
> > +       if (!sp->args_count)
> > +               return ERR_PTR(-EINVAL);
> > +
> > +       if (sp->args[0] >= ebm->v->num_channels)
> > +               return ERR_PTR(-ECHRNG);
> > +
> > +       return &mbox->chans[sp->args[0]];
> > +}
> >
> Just use the default of_mbox_index_xlate()
> 
> ....
> > +
> > +       for (i = 0; i < ebm->v->num_channels; i++) {
> 
> You make this block a bit cleaner by using a temporary variable
>         echan = &ebm->ch[i];
> and using echan instead of ebm->ch[i] a dozen times below.
> 
> > +               ebm->ch[i].c = &ebm->v->channels[i];
> > +               if (ebm->ch[i].c->rx_len > MBOX_MAX_RX_SIZE) {
> > +                       dev_err(ebm->dev, "Channel %s RX size (%d) too large\n",
> > +                               ebm->ch[i].c->name, ebm->ch[i].c->rx_len);
> > +                       return -EINVAL;
> > +               }
> > +               ebm->ch[i].full_name = devm_kasprintf(ebm->dev, GFP_KERNEL, "%s:%s",
> > +                                                     dev_name(ebm->dev), ebm->ch[i].c->name);
> > +               if (!ebm->ch[i].full_name)
> > +                       return -ENOMEM;
> > +
> > +               ebm->ch[i].ebm = ebm;
> > +               ebm->ch[i].num = i;
> > +               spin_lock_init(&ebm->mbox.chans[i].lock);
> > +               ebm->mbox.chans[i].con_priv = &ebm->ch[i];
> > +               atomic_set(&ebm->ch[i].rx_status, MBOX_CLOGGED);
> > +       }
> > +
> 
> 
> -j
> 

Re: [PATCH v3 05/10] mailbox: add MediaTek GPUEB IPI mailbox

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

On Mon, Sep 22, 2025 at 02:59:43PM +0200, Nicolas Frattaroli wrote:
> On Sunday, 21 September 2025 07:00:59 Central European Summer Time Jassi Brar wrote:

> > > +#define MBOX_FULL              BIT(0) /* i.e. we've received data */
> > > +#define MBOX_CLOGGED           BIT(1) /* i.e. the channel is shutdown */

> > This is confusing. CLOGGED usually means malfunction, but it seems you
> > want to call it STOPPED or UNINIT?

> I don't agree that "CLOGGED usually means malfunction". To clog something
> is to impede its flow, which in this case is the correct terminology to
> refer to what's happened to the channel. "UNINIT" is wrong, it's initialised
> properly. "STOPPED" is also wrong, it's not stopped, it still sends, it just
> won't pass it on through.

As a native English speaker I'd say that clogged has heavy overtones of
broken and malfunction, like Jassi says it'd usually describe a fault
condition.  Something like "blocked" might be more neutral.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]