On Wed, 17 Sept 2025 at 17:45, Nicolas Frattaroli <nicolas.frattar...@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.frattar...@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