Message-ID: <2162077.CQOukoFCf9@workhorse>
Date: Wed, 17 Sep 2025 17:44:43 +0200
From: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 Boris Brezillon <boris.brezillon@...labora.com>,
 Steven Price <steven.price@....com>, Liviu Dudau <liviu.dudau@....com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Matthias Brugger <matthias.bgg@...il.com>,
 MyungJoo Ham <myungjoo.ham@...sung.com>,
 Kyungmin Park <kyungmin.park@...sung.com>,
 Chanwoo Choi <cw00.choi@...sung.com>, Jassi Brar <jassisinghbrar@...il.com>,
 Kees Cook <kees@...nel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 Chia-I Wu <olvaffe@...il.com>, Chen-Yu Tsai <wenst@...omium.org>,
 kernel@...labora.com, dri-devel@...ts.freedesktop.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 linux-pm@...r.kernel.org, linux-hardening@...r.kernel.org,
 Conor Dooley <conor.dooley@...rochip.com>
Subject: Re: [PATCH v3 00/10] MT8196 GPU Frequency/Power Control Support

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@...labora.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
> 

Powered by blists - more mailing lists