Message-ID: <145da72b-b209-4e8d-9dcd-b82a412b800a@collabora.com>
Date: Thu, 6 Nov 2025 11:47:53 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Niklaus Liu <Niklaus.Liu@...iatek.com>,
 Matthias Brugger <matthias.bgg@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-mediatek@...ts.infradead.org,
 Project_Global_Chrome_Upstream_Group@...iatek.com, sirius.wang@...iatek.com,
 vince-wl.liu@...iatek.com, jh.hsu@...iatek.com, zhigang.qin@...iatek.com,
 sen.chu@...iatek.com, Louis-Alexis Eyraud <louisalexis.eyraud@...labora.com>
Subject: Re: [PATCH v4 0/1] soc: mediatek: mtk-regulator-coupler: Add support
 for MT8189

Il 06/11/25 06:48, Niklaus Liu ha scritto:
> Refer to the discussion in the link:
> v3: https://patchwork.kernel.org/project/linux-mediatek/patch/20251104071252.12539-2-Niklaus.Liu@mediatek.com/
> 
> - reply comment:
> Hi Angelo:
> We have already confirmed with the GPU Owner/HW team that for the MT8189 SoC without AOC 2.0, the GPU power sequence requires vgpu to be enabled before vsram. This is also the case for the MT8188. In addition, we have already implemented MFG0/1 for GPU/GPU_VSRAM. Is there anything else that needs clarification from our side?
> 

I have just rechecked MT8188. You're right, there, the sequence is inverted - but
on that SoC the coupler works just fine, and the first setting of power rail
PP0850_DVDD_SRAM_GPU (mt6359_vsram_others) doesn't produce the side effect that
you are describing in this commit.

I'm still not convinced about that, and if this commit is *really* required, it
logically has to be required for at least MT8188 as well as MT8189, but then I
don't understand why MT8188 works.

This needs some more research efforts because, if there's anything wrong, it's
more than what you're fixing here. Let me check a bit.

As Collabora is also bringing up MT8189 (Genio variant, same SoC anyway), I'll wait
until GPU is up on our Evaluation Kit and then I will check this regulator coupler
situation more carefully, directly on the hardware - because what is happening is
a bit too fishy to just accept a fast patch and let it go.

If I simply accept this patch and let go, some other SoC (newer than, or older than
8189) which is currently not upstream will probably fail as much as this one and
that is not acceptable, because this driver has to work with them all without any
quirk that is strictly SoC-specific (unless one of those has broken HW, which is
not the case here).

Hope you understand.

Regards,
Angelo

>>
>> changes in v4:
>>    - reply comment:
> 
> Niklaus, please just reply inline to the emails instead of sending an entirely new
> version just for a reply: it's easier for everyone to follow, and it's also easier
> for me to read, and for you to send a reply by clicking one button :-)
> 
>> 1. MTK hardware requires that vsram_gpu must be higher than vgpu; this rule must be satisfied.
>>
>> 2. When the GPU powers on, the mtcmos driver first calls regulator_enable to turn on vgpu, then calls regulator_enable to
>> turn on vsram_gpu. When enabling vgpu, mediatek_regulator_balance_voltage sets the voltages for both vgpu and vsram_gpu.
>> However, when enabling vsram_gpu, mediatek_regulator_balance_voltage is also executed, and at this time, the vsram_gpu voltage
>> is set to the minimum voltage specified in the DTS, which does not comply with the requirement that vsram_gpu must be higher than vgpu.
>>
> 
> 2. -> There's your problem! VSRAM_GPU has to be turned on *first*, VGPU has to be
> turned on *last* instead.
> 
> Logically, you need SRAM up before the GPU is up as if the GPU tries to use SRAM
> it'll produce unpowered access issues: even though it's *very* unlikely for that
> to happen on older Mali, it's still a logical mistake that might, one day, come
> back at us and create instabilities.
> 
> Now, the easy fix is to just invert the regulators in MFG nodes. As I explained
> *multiple* times, you have a misconfiguration in your DT.
> 
> GPU subsystem main MFG -> VSRAM
> GPU core complex MFG -> VGPU
> GPU per-core MFG -> nothing
> 
>> 3.During suspend, the voltages of vgpu and vsram_gpu should remain unchanged, and when resuming, vgpu and vsram_gpu should be
>> restored to their previous voltages. When the vgpu voltage is adjusted, mediatek_regulator_balance_voltage already synchronizes the
>> adjustment of vsram_gpu voltage. Therefore, adjusting the vsram_gpu voltage again in mediatek_regulator_balance_voltage is redundant.
> 
> If you fix your DT, N.3 won't happen.
> 
> Regards,
> Angelo
> 
>>
>>
>> changes in v3:
>>    - modify for comment[add the new entry by alphabetical order]
>>
>> changes in v2:
>>    - change title for patch
>>    - reply comment: This is a software regulator coupler mechanism, and the regulator-coupled-with
>> configuration has been added in the MT8189 device tree. This patchaddresses an issue reported by a
>> Chromebook customer. When the GPU regulator is turned on, mediatek_regulator_balance_voltage already
>> sets both the GPU and GPU_SRAM voltages at the same time, so there is no need to adjust the GPU_SRAM
>> voltage again in a second round. Therefore, a return is set for MT8189.
>> If the user calls mediatek_regulator_balance_voltage again for GPU_SRAM, it may cause abnormal behavior of GPU_SRAM.
>>
>>
>> changes in v1:
>>    - mediatek-regulator-coupler mechanism for platform MT8189
>>
>> *** BLURB HERE ***
>>
>> Niklaus Liu (1):
>>     soc: mediatek: mtk-regulator-coupler: Add support for MT8189
>>
>>    drivers/soc/mediatek/mtk-regulator-coupler.c | 13 +++++++++++++
>>    1 file changed, 13 insertions(+)
>>
> 
> Niklaus Liu (1):
>    soc: mediatek: mtk-regulator-coupler: Add support for MT8189
> 
>   drivers/soc/mediatek/mtk-regulator-coupler.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 

Powered by blists - more mailing lists