(no subject)

[niklaus.liu]

This series is based on linux-next, tag: next-20251103
V1 patch link:https://lore.kernel.org/linux-mediatek/20251103134241.3773-2-Niklaus.Liu@mediatek.com/

Subject: [V3 PATCH 0/1] soc: mediatek: mtk-regulator-coupler: Add support for MT8189

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 

Niklaus Liu (1):
  soc: mediatek regulator: Add Support for mt8189

 drivers/soc/mediatek/mtk-regulator-coupler.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

-- 
2.46.0

[PATCH V3 1/1] soc: mediatek: mtk-regulator-coupler: Add support for MT8189

[niklaus.liu]

From: Niklaus Liu <niklaus.liu@mediatek.com>

Enhance the regulator coupler driver to support GPU power control on the
MediaTek MT8189 platform. This update ensures proper coordination of
multiple regulators required for GPU operation,improving power management
and system stability.

Signed-off-by: Niklaus Liu <niklaus.liu@mediatek.com>
---
 drivers/soc/mediatek/mtk-regulator-coupler.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-regulator-coupler.c b/drivers/soc/mediatek/mtk-regulator-coupler.c
index 0b6a2884145e..e2a1fb459e42 100644
--- a/drivers/soc/mediatek/mtk-regulator-coupler.c
+++ b/drivers/soc/mediatek/mtk-regulator-coupler.c
@@ -42,6 +42,18 @@ static int mediatek_regulator_balance_voltage(struct regulator_coupler *coupler,
 	int max_uV = INT_MAX;
 	int ret;
 
+	/*
+	 * When vsram_gpu is enabled or disabled and the use_count of the
+	 * vsram_gpu regulator is zero, the regulator coupler driver will
+	 * execute regulator_do_balance_voltage, which adjusts the vsram_gpu
+	 * voltage to the minimum value. This may result in vsram_gpu being
+	 * lower than vgpu. Therefore, when enabling or disabling vsram_gpu,
+	 * the 8189 temporarily skips the regulator coupler driver's modification
+	 * of the vsram_gpu voltage.
+	 */
+	if (of_machine_is_compatible("mediatek,mt8189") && rdev == mrc->vsram_rdev)
+		return 0;
+
 	/*
 	 * If the target device is on, setting the SRAM voltage directly
 	 * is not supported as it scales through its coupled supply voltage.
@@ -148,6 +160,7 @@ static int mediatek_regulator_coupler_init(void)
 	if (!of_machine_is_compatible("mediatek,mt8183") &&
 	    !of_machine_is_compatible("mediatek,mt8186") &&
 	    !of_machine_is_compatible("mediatek,mt8188") &&
+	    !of_machine_is_compatible("mediatek,mt8189") &&
 	    !of_machine_is_compatible("mediatek,mt8192"))
 		return 0;
 
-- 
2.46.0

Re: [PATCH V3 1/1] soc: mediatek: mtk-regulator-coupler: Add support for MT8189

[AngeloGioacchino Del Regno]

Il 04/11/25 08:12, niklaus.liu ha scritto:
> From: Niklaus Liu <niklaus.liu@mediatek.com>
> 
> Enhance the regulator coupler driver to support GPU power control on the
> MediaTek MT8189 platform. This update ensures proper coordination of
> multiple regulators required for GPU operation,improving power management
> and system stability.
> 
> Signed-off-by: Niklaus Liu <niklaus.liu@mediatek.com>
> ---
>   drivers/soc/mediatek/mtk-regulator-coupler.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-regulator-coupler.c b/drivers/soc/mediatek/mtk-regulator-coupler.c
> index 0b6a2884145e..e2a1fb459e42 100644
> --- a/drivers/soc/mediatek/mtk-regulator-coupler.c
> +++ b/drivers/soc/mediatek/mtk-regulator-coupler.c
> @@ -42,6 +42,18 @@ static int mediatek_regulator_balance_voltage(struct regulator_coupler *coupler,
>   	int max_uV = INT_MAX;
>   	int ret;
>   
> +	/*
> +	 * When vsram_gpu is enabled or disabled and the use_count of the
> +	 * vsram_gpu regulator is zero, the regulator coupler driver will
> +	 * execute regulator_do_balance_voltage, which adjusts the vsram_gpu
> +	 * voltage to the minimum value. This may result in vsram_gpu being
> +	 * lower than vgpu. Therefore, when enabling or disabling vsram_gpu,
> +	 * the 8189 temporarily skips the regulator coupler driver's modification
> +	 * of the vsram_gpu voltage.
> +	 */

This will break the rules of your own hardware. Seriously.

The VSRAM_GPU has to be set in a specific range, where the minimum voltage has to
guarantee that the GPU can run in the first quarter of the OPP entries (which does
always correspond to lowest/lower/low or lowest/low) for the GPU.

If you return zero here, and VSRAM_GPU was set to Vmax (scenario: GPU was at Turbo
frequency) before shutting down completely (ex.: system suspend), at device resume
time, the VSRAM_GPU rail will be overvolting the GPU SRAM.

While that won't cause any "magic smoke" because the VSRAM is still in the maximum
operating ranges constraint, this will leak power, hence increase the chip's heat
output, and might as well create possible instabilities.

Remember that the GPU SRAM voltage has to *always be constrained* as:
  - VSRAM <= VGPU when VGPU must be over the VSRAM constraint
  - VSRAM == VGPU if the voltages are in range between each other
  - VSRAM > VGPU *only* for SLEEP (or LOWEST) freq setting

As I already said in my previous review for this patch, your devicetree may be
misconfiguring the regulators, or their hierarchy (as those are MFG0/1+Panfrost).

Besides, are you testing this on Panfrost, even, or on the Mali DDK driver?
That really doesn't look right.

Also - MT8189 is not special. There's nothing different between MT8189 and the
others in regard to how the GPU regulators should be coupled, and in regard to
their constraints relationship.

Regards,
Angelo

> +	if (of_machine_is_compatible("mediatek,mt8189") && rdev == mrc->vsram_rdev)
> +		return 0;
> +
>   	/*
>   	 * If the target device is on, setting the SRAM voltage directly
>   	 * is not supported as it scales through its coupled supply voltage.
> @@ -148,6 +160,7 @@ static int mediatek_regulator_coupler_init(void)
>   	if (!of_machine_is_compatible("mediatek,mt8183") &&
>   	    !of_machine_is_compatible("mediatek,mt8186") &&
>   	    !of_machine_is_compatible("mediatek,mt8188") &&
> +	    !of_machine_is_compatible("mediatek,mt8189") &&

(but this is ok here)

>   	    !of_machine_is_compatible("mediatek,mt8192"))
>   		return 0;
>