diff mbox series

arm64: dts: mediatek: mt8192-asurada: Update min voltage constraint for Vgpu

Message ID 20240313135157.98989-1-treapking@chromium.org (mailing list archive)
State New, archived
Headers show
Series arm64: dts: mediatek: mt8192-asurada: Update min voltage constraint for Vgpu | expand

Commit Message

Pin-yen Lin March 13, 2024, 1:51 p.m. UTC
Although the minimum voltage listed on the GPU OPP table is 606250 uV,
the actual requested voltage could be even lower when the MTK Smart
Voltage Scaling (SVS) driver is enabled.

Set the minimum voltage to 300000 uV because it's supported by the
regulator.

Fixes: 3183cb62b033 ("arm64: dts: mediatek: asurada: Add SPMI regulators")
Signed-off-by: Pin-yen Lin <treapking@chromium.org>
---

 arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

AngeloGioacchino Del Regno March 13, 2024, 2:36 p.m. UTC | #1
Il 13/03/24 14:51, Pin-yen Lin ha scritto:
> Although the minimum voltage listed on the GPU OPP table is 606250 uV,
> the actual requested voltage could be even lower when the MTK Smart
> Voltage Scaling (SVS) driver is enabled.
> 
> Set the minimum voltage to 300000 uV because it's supported by the
> regulator.
> 
> Fixes: 3183cb62b033 ("arm64: dts: mediatek: asurada: Add SPMI regulators")
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>

Okay, that makes sense, I agree.

...but.

The datasheet never mentions 0.3V as vmin - infact, it does mention that the
vsel is selected as (0V +) 6250 * Vsel, but the brief spec says that for the
standard configuration (in terms of HW), the Vmin is 0.4V and not 0.3.

Reading through makes me think that it's not much about the buck providing an
unstable output, but more about it starting to become inefficient under that
value.

This means that it is sensible to set, instead:

	regulator-min-microvolt = <400000>;

Also, this is repeated on multiple platforms: can you please perform the same
change also on MT8183, MT8186 and MT8195?

P.S.: For MT6358, the Vmin for VGPU is 0.5V :-)

Cheers,
Angelo

> ---
> 
>   arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> index 43d80334610a..5cc5100a7c40 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> @@ -1448,7 +1448,7 @@ regulators {
>   			mt6315_7_vbuck1: vbuck1 {
>   				regulator-compatible = "vbuck1";
>   				regulator-name = "Vgpu";
> -				regulator-min-microvolt = <606250>;
> +				regulator-min-microvolt = <300000>;
>   				regulator-max-microvolt = <800000>;
>   				regulator-enable-ramp-delay = <256>;
>   				regulator-allowed-modes = <0 1 2>;
AngeloGioacchino Del Regno March 14, 2024, 10:02 a.m. UTC | #2
Il 14/03/24 10:59, Pin-yen Lin ha scritto:
> Hi Angelo,
> 
> On Wed, Mar 13, 2024 at 10:36 PM AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com> wrote:
> 
>> Il 13/03/24 14:51, Pin-yen Lin ha scritto:
>>> Although the minimum voltage listed on the GPU OPP table is 606250 uV,
>>> the actual requested voltage could be even lower when the MTK Smart
>>> Voltage Scaling (SVS) driver is enabled.
>>>
>>> Set the minimum voltage to 300000 uV because it's supported by the
>>> regulator.
>>>
>>> Fixes: 3183cb62b033 ("arm64: dts: mediatek: asurada: Add SPMI
>> regulators")
>>> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
>>
>> Okay, that makes sense, I agree.
>>
>> ...but.
>>
>> The datasheet never mentions 0.3V as vmin - infact, it does mention that
>> the
>> vsel is selected as (0V +) 6250 * Vsel, but the brief spec says that for
>> the
>> standard configuration (in terms of HW), the Vmin is 0.4V and not 0.3.
>>
>> Reading through makes me think that it's not much about the buck providing
>> an
>> unstable output, but more about it starting to become inefficient under
>> that
>> value.
>>
>> This means that it is sensible to set, instead:
>>
>>          regulator-min-microvolt = <400000>;
>>
>> Also, this is repeated on multiple platforms: can you please perform the
>> same
>> change also on MT8183, MT8186 and MT8195?
>>
>> P.S.: For MT6358, the Vmin for VGPU is 0.5V :-)
>>
> 
> Thanks for checking! I'll update the values on other platforms as well.
> 
> I assume that we also want to update other mt6315 nodes listing 300000 uV
> as the min voltage, so I'll update them as well.
> 

Yes, didn't notice that, nice catch. Please do!

Cheers,
Angelo

>>
>>> ---
>>>
>>>    arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
>> b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
>>> index 43d80334610a..5cc5100a7c40 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
>>> @@ -1448,7 +1448,7 @@ regulators {
>>>                        mt6315_7_vbuck1: vbuck1 {
>>>                                regulator-compatible = "vbuck1";
>>>                                regulator-name = "Vgpu";
>>> -                             regulator-min-microvolt = <606250>;
>>> +                             regulator-min-microvolt = <300000>;
>>>                                regulator-max-microvolt = <800000>;
>>>                                regulator-enable-ramp-delay = <256>;
>>>                                regulator-allowed-modes = <0 1 2>;
>>
>>
>>
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
index 43d80334610a..5cc5100a7c40 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
@@ -1448,7 +1448,7 @@  regulators {
 			mt6315_7_vbuck1: vbuck1 {
 				regulator-compatible = "vbuck1";
 				regulator-name = "Vgpu";
-				regulator-min-microvolt = <606250>;
+				regulator-min-microvolt = <300000>;
 				regulator-max-microvolt = <800000>;
 				regulator-enable-ramp-delay = <256>;
 				regulator-allowed-modes = <0 1 2>;