Re: [PATCH] CHROMIUM: arm64: dts: mt8183-elm: Set GPU power regulator to always on

[Anand K. Mistry]

:facepalm: sorry about the subject line. I'll fix it up in the next revision.

On Tue, 25 Aug 2020 at 15:26, Anand K Mistry <amistry@xxxxxxxxxx> wrote:
>
> Keep the da9212 BUCKB always-on. This works around an issue on Elm/Hana
> devices where sometimes, the regulator is disabled before scpsys is
> suspended, causing the suspension of scpsys to fail.
>
> Usually, the GPU and scpsys are suspended by the runtime PM before the
> system is suspended, due to the GPU being idle. In this case, scpsys is
> suspended inline with the GPU suspend, which then disables the
> regulator. However, if the GPU is still active when system is suspended,
> GPU suspend occurs but defers suspending scpsys to the PM's noirq phase.
> Since GPU suspend disables the regulator, scpsys isn't powered and
> suspending it fails with the following error:
> [  523.773227] mtk-scpsys 10006000.scpsys: Failed to power off domain mfg_2d
>
> On resume, scpsys is resumed in the noirq phase. Since scpsys requires
> power from the regulator, which is still disabled at this point,
> attempting to turn it on will hang the CPU. A HW watchdog eventually
> reboots the system.
>
> The obvious solution would be to add a link to the regulator from scpsys
> in the devicetree. This would prevent the regulator from being disabled
> until scpsys is suspended. However, in the case where suspending scpsys
> is deferred to the noirq phase, disabling the regulator will fail since
> it is connected over I2C which requires IRQs to be enabled. Even in the
> usual case where scpsys is suspended inline with the GPU, PM will always
> attempt to resume scpsys in noirq. This will attempt to enable the
> regulator, which will also fail due to being unable to communicate over
> I2C.
>
> Since I2C can't be using with IRQs disabled, a workaround is to never
> turn off the regulator.
>
> Measuring power on the GPU rail on a Elm DVT shows that the change in
> power usage is negligible. The two relavent cases are S0 with an idle
> GPU, and S3.
>
> In S0 with an idle GPU, current behaviour with the regulator off:
> @@           NAME  COUNT  AVERAGE  STDDEV      MAX    MIN
> @@         gpu_mw    600     1.74    1.31     6.75   0.00
> ... and with the regulator on, but no load:
> @@           NAME  COUNT  AVERAGE  STDDEV     MAX    MIN
> @@         gpu_mw    600     1.68    1.25    7.13   0.00
> The difference being well within the margin of error.
>
> In S3, current behaviour with the regulator off:
> @@           NAME  COUNT  AVERAGE  STDDEV     MAX    MIN
> @@         gpu_mw    600     0.94    0.74    3.25   0.00
> ... and with the regulator on:
> @@           NAME  COUNT  AVERAGE  STDDEV     MAX     MIN
> @@         gpu_mw    600     0.83    0.66    3.25    0.00
>
> Signed-off-by: Anand K Mistry <amistry@xxxxxxxxxx>
>
> ---
>
>  arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi b/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi
> index a5a12b2599a4..1294f27b21c1 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173-elm.dtsi
> @@ -304,6 +304,7 @@ da9211_vgpu_reg: BUCKB {
>                                 regulator-min-microamp  = <2000000>;
>                                 regulator-max-microamp  = <3000000>;
>                                 regulator-ramp-delay = <10000>;
> +                               regulator-always-on;
>                         };
>                 };
>         };
> --
> 2.28.0.297.g1956fa8f8d-goog
>


-- 
Anand K. Mistry
Software Engineer
Google Australia