Re: [PATCH] arm64: dts: rockchip: Fix the DCDC_REG2 minimum voltage on Quartz64 Model B

Heiko Stübner
2024-05-31T18:40:23Z

Am Freitag, 31. Mai 2024, 00:48:45 CEST schrieb Dragan Simic:
> Hello Chen-Yu,
>
> On 2024-05-29 18:27, Chen-Yu Tsai wrote:
> > On Tue, May 21, 2024 at 1:20 AM Dragan Simic <dsimic@manjaro.org>
> > wrote:
> >>
> >> Correct the specified regulator-min-microvolt value for the buck
> >> DCDC_REG2
> >> regulator, which is part of the Rockchip RK809 PMIC, in the Pine64
> >> Quartz64
> >> Model B board dts. According to the RK809 datasheet, version 1.01,
> >> this
> >> regulator is capable of producing voltages as low as 0.5 V on its
> >> output,
> >> instead of going down to 0.9 V only, which is additionally confirmed
> >> by the
> >> regulator-min-microvolt values found in the board dts files for the
> >> other
> >> supported boards that use the same RK809 PMIC.
> >>
> >> This allows the DVFS to clock the GPU on the Quartz64 Model B below
> >> 700 MHz,
> >> all the way down to 200 MHz, which saves some power and reduces the
> >> amount of
> >> generated heat a bit, improving the thermal headroom and possibly
> >> improving
> >> the bursty CPU and GPU performance on this board.
> >>
> >> This also eliminates the following warnings in the kernel log:
> >>
> >> core: _opp_supported_by_regulators: OPP minuV: 825000 maxuV: 825000,
> >> not supported by regulator
> >> panfrost fde60000.gpu: _opp_add: OPP not supported by regulators
> >> (200000000)
> >> core: _opp_supported_by_regulators: OPP minuV: 825000 maxuV: 825000,
> >> not supported by regulator
> >> panfrost fde60000.gpu: _opp_add: OPP not supported by regulators
> >> (300000000)
> >> core: _opp_supported_by_regulators: OPP minuV: 825000 maxuV: 825000,
> >> not supported by regulator
> >> panfrost fde60000.gpu: _opp_add: OPP not supported by regulators
> >> (400000000)
> >> core: _opp_supported_by_regulators: OPP minuV: 825000 maxuV: 825000,
> >> not supported by regulator
> >> panfrost fde60000.gpu: _opp_add: OPP not supported by regulators
> >> (600000000)
> >>
> >> Fixes: dcc8c66bef79 ("arm64: dts: rockchip: add Pine64 Quartz64-B
> >> device tree")
> >> Cc: stable@vger.kernel.org
> >> Reported-By: Diederik de Haas <didi.debian@cknow.org>
> >> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> >> ---
> >> arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
> >> b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
> >> index 26322a358d91..b908ce006c26 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
> >> +++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
> >> @@ -289,7 +289,7 @@ vdd_gpu: DCDC_REG2 {
> >> regulator-name = "vdd_gpu";
> >> regulator-always-on;
> >> regulator-boot-on;
> >> - regulator-min-microvolt = <900000>;
> >> + regulator-min-microvolt = <500000>;
> >
> > The constraints here are supposed to be the constraints of the
> > consumer,
> > not the provider. The latter is already known by the implementation.
> >
> > So if the GPU can go down to 0.825V or 0.81V even (based on the
> > datasheet),
> > this should say the corresponding value. Surely the GPU can't go down
> > to
> > 0.5V?
> >
> > Can you send another fix for it?
>
> I can confirm that the voltage of the power supply of GPU found inside
> the RK3566 can be as low as 0.81 V, according to the datasheet, or as
> low as 0.825 V, according to the GPU OPPs found in rk356x.dtsi.
>
> If we want the regulator-min-microvolt parameter to reflect the
> contraint
> of the GPU as the consumer, which I agree with, we should do that for
> other
> RK3566-based boards as well, and almost surely for the boards based on
> the
> RK3568, too.

Hmm, I'm not so sure about that.

The binding does define:
regulator-min-microvolt:
description: smallest voltage consumers may set

This does not seem to describe it as a constraint solely of the consumer.
At least the wording sounds way more flexible there.

Also any regulator _could_ have multiple consumers, whose value would
it need then.

While true, setting it to the lowest the regulator can do in the original
fix patch, might've been a bit much and a saner value might be better.

> This would ensure consistency, but I'd like to know are all those
> resulting
> patches going to be accepted before starting to prepare them? There
> will
> be a whole bunch of small patches.

Hmm, though I'd say that would be one patch per soc?

I.e. you're setting the min-voltage of _one_ regulator used
on each board to a value to support the defined OPPs.

I.e. in my mind you'd end up with:
arm64: dts: rockchip: set better min voltage for vdd_gpu on rk356x boards

And setting the lower voltage to reach that lower OPP on all affected
rk356x boards.

Heiko

>
> >> regulator-max-microvolt = <1350000>;
> >> regulator-ramp-delay = <6001>;