Re: [PATCH v2 0/4] pmdomain: mediatek: Enable the GPU timestamp counter on MT8188

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

On 10/11/26 20:47, Michał Kopeć wrote:
> On MT8188 the GPU timestamp counter reads zero until a register in the
> MFGCFG block feeds it the SoC system timer, and MFGCFG loses that
> setting whenever the MFG1 power domain is turned off. As a result, the
> SYSTEM_TIMESTAMP query and the timestamps written by GPU jobs are always
> zero, which breaks timestamp queries in userspace drivers.
> 
> v1 restored the register from panfrost through a new mediatek,mfgcfg
> phandle on the GPU node. As Krzysztof pointed out, the register is lost
> by the power domain, so v2 restores it there instead: the power

The register is not lost by the power domain, it's lost by the MFGCFG block
getting unpowered and (all) its registers being reset to the defaults, where
the defaults happen to be zero... and then the MFGCFG has a bus protection
block yes (unused for us), but that's at offset 0x20, so if you define that
as 0x130 it becomes plain wrong :-(

Besides, get this:

"MFG_TIMESTAMP_EN" is a register field that doesn't exist... so here are the
correct definitions ;')

#define MFG_TIMESTAMP		0x130
#  define TOP_TSVALUEB_EN	BIT(0)
#  define TIMER_SEL_SOC_TIMER	BIT(1) /* unset: MFG timer, set: SOC timer */

The issue that you're talking about here is not specific to MT8188, but to all
of the MediaTek SoCs with either Mali or PowerVR that are *not* using the GPUEB
firmware (so all Chromebooks and Genio boards before MT8196/MT6991/MT8894).

This doesn't belong to the power domain driver, as this is definitely not in any
way related to powering on the MediaTek Flexible Graphics (:'D) block, so this
has to be done somewhere else.

That said, let's see what a good solution could be here...

...so, the MFG Block has got a Hardware Timestamp Engine that needs to be kicked
from the MFG Configuration register area, specifically from the the MFG IP
Timestamp register.

Once the timestamp engine's timer source is selected (TIMER_SEL) and you made sure
that the actual timer source is running (eh, first make sure it's running, then
select), you enable the MFG-Top (Top==Global) TimeStampValueB (TOP_TSVALUEB_EN).

When those registers are set, your Hardware Timestamp Engine is fully ready and
will start running when frames/commands/etc come, adding a timer time to each of
the frames/commands/etc (which is why this is a timestamp engine) in a hardware
assisted manner (and this is why it's a *hardware* timestamp engine).

Now that we know what we're dealing with... we're pretty sure that this is a

H T E  .... (drivers/hte hello)

Documentation/devicetree/bindings/timestamp/hardware-timestamps-common.yaml

Panfrost needs a hardware timestamp engine to do timestamp counting, right?
Panthor would need the same.

That's not MediaTek specific, because there's a generic hardware timestamp engine
binding, so I'm sorry but you can go back implementing HTE support in Panfrost and
Panthor - in a very-very-very generic way - using the HTE APIs.

You'd have to write a (very small, really) HTE driver for the hte-mediatek-mfg.c
but that's not a big deal, a first version of that driver, which can be upstreamed
could do just:
  1. Make sure that the MFG-TOP power domain is on (not the cores domain, just the
     top one!), which is easy, it's just one line; and
  2. Set the register fields in MFG_TIMESTAMP to the correct value when Panfrost
     asks to start or initialize the HTE; and
  3. UNset TOP_TSVALUEB_EN when going to sleep, or when Panfrost asks to stop/deinit
     the HTE.

It's a 50 lines driver, nothing more (well, I didn't write anything so I don't know
but trust me it's going to be around that linecount).

That's how this matter can be resolved.

Btw, thanks for bringing this up, I didn't even realize this was an issue.

Cheers!
Angelo

> controller lists MFGCFG in access-controllers and writes the register
> every time MFG1 has been powered on.
> 
> Patch 4 converts mt8188.dtsi from the deprecated per-domain
> mediatek,infracfg phandles to access-controllers. A devicetree with it
> needs the driver changes of patches 2 and 3, because older kernels
> expect exactly one access controller on MT8188 and fail to probe the
> power controller otherwise. Older devicetrees keep working with the new
> driver, without the timestamp.
> 
> MT8370 uses the MT8188 power controller and mt8188.dtsi, so it is
> covered as well.
> 
> Tested on the Lenovo Chromebook Duet (MT8188): the timestamp counts at
> 13 MHz, also after suspend and resume.
> 
> Changes in v2:
> - Restore the register from the MFG1 power domain instead of panfrost,
>    and drop the GPU phandle (Krzysztof).
> - Drop the panfrost patch, which makes the review comments about error
>    propagation and MT8370 moot.
> 
> v1: https://lore.kernel.org/all/20261003200132.866240-1-michal@nozomi.space/
> 
> Michał Kopeć (4):
>    dt-bindings: power: mediatek: Add the MT8188 access controllers
>    pmdomain: mediatek: Restore a register after powering a domain on
>    pmdomain: mediatek: mt8188: Enable the GPU timestamp counter
>    arm64: dts: mediatek: mt8188: Use access-controllers in the power
>      controller
> 
>   .../power/mediatek,power-controller.yaml      | 16 +++++++++++++-
>   arch/arm64/boot/dts/mediatek/mt8188.dtsi      | 22 +------------------
>   drivers/pmdomain/mediatek/mt8188-pm-domains.h | 16 +++++++++++++-
>   drivers/pmdomain/mediatek/mtk-pm-domains.c    | 19 ++++++++++++++++
>   drivers/pmdomain/mediatek/mtk-pm-domains.h    | 15 +++++++++++++
>   5 files changed, 65 insertions(+), 23 deletions(-)
>