Re: [PATCH 2/2] soc: mediatek: mtk-svs: Don't fail init01 on a bank whose buck is not at vboot yet

[Chen-Yu Tsai]

On Tue, Oct 6, 2026 at 4:54 AM Nils Büchner <nils@xxxxxxxxxxx> wrote:
>
> svs_init01() moves every bank to its vboot OPP and then immediately
> requires the bank's buck to read back exactly vboot. For the MT8183 GPU
> banks the buck follows panfrost devfreq, which has not necessarily acted
> on the OPP change yet (an idle GPU stays at its lowest OPP), so the
> check fails and the whole SVS probe aborts with -EPERM, leaving the CPU
> and CCI banks uncalibrated as well:
>
>   SVSB_GPU: buck voltage: 700000uV, expected vboot: 800000uV
>   mtk-svs 1100bc00.svs: error -EPERM: svs start fail
>
> Poll the buck for up to 500ms before giving up. If it still is not at
> vboot, skip only that bank: restore its default OPP voltages, disable it,
> and calibrate the others.
>
> On Acer Chromebook Spin 311 (MT8183) the GPU buck reaches vboot within
> the poll window and all four banks complete init01/init02.

Were both drivers (mtk-svs and panfrost) built-in, or as modules?
This would affect the timing.

> Runtime-tested on an Acer Chromebook Spin 311 (MT8183, juniper) with a
> 7.0-based kernel (the hardware is not set up to boot mainline);
> build-tested on mainline with W=1. On the device together with
> the auxadc_thermal per-sensor zone change: all four banks complete
> init01/init02 at boot (CPU little 1.99 GHz 1050 -> 1000 mV), stable
> under 60 s of full load on all cores at maximum frequency.
>
> The change and this changelog were drafted with an AI coding assistant
> during a debugging session on the hardware; they were reviewed and
> tested by the submitter.

This section could be moved after the "---".

> Fixes: 681a02e95000 ("soc: mediatek: SVS: introduce MTK SVS engine")
> Assisted-by: Claude Opus 5.5
> Signed-off-by: Nils Büchner <nils@xxxxxxxxxxx>
> ---
>  drivers/soc/mediatek/mtk-svs.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c
> index 99edecb20..2598a1ccb 100644
> --- a/drivers/soc/mediatek/mtk-svs.c
> +++ b/drivers/soc/mediatek/mtk-svs.c
> @@ -16,6 +16,7 @@
>  #include <linux/device.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
> +#include <linux/iopoll.h>
>  #include <linux/kernel.h>
>  #include <linux/kthread.h>
>  #include <linux/module.h>
> @@ -1439,6 +1440,7 @@ static int svs_init01(struct svs_platform *svsp)
>         bool search_done;
>         int ret = 0, r;
>         u32 opp_freq, opp_vboot, buck_volt, idx, i;
> +       unsigned long skipped = 0;
>
>         if (!svs_mode_available(svsp, SVSB_MODE_INIT01))
>                 return 0;
> @@ -1542,13 +1544,20 @@ static int svs_init01(struct svs_platform *svsp)
>                                                       bdata->volt_step,
>                                                       bdata->volt_base);
>
> -               buck_volt = regulator_get_voltage(svsb->buck);
> -               if (buck_volt != opp_vboot) {
> -                       dev_err(svsb->dev,
> -                               "buck voltage: %uuV, expected vboot: %uuV\n",
> -                               buck_volt, opp_vboot);
> -                       ret = -EPERM;
> -                       goto svs_init01_finish;
> +               /*
> +                * The consumer's DVFS (e.g. GPU devfreq while the GPU is idle)
> +                * may not have moved to the vboot OPP yet. Give it a moment,
> +                * then calibrate the remaining banks without this one instead
> +                * of failing the whole SVS probe.
> +                */
> +               if (read_poll_timeout(regulator_get_voltage, buck_volt,
> +                                     buck_volt == opp_vboot, 10000, 500000,
> +                                     false, svsb->buck)) {
> +                       dev_warn(svsb->dev,
> +                                "buck voltage: %uuV, expected vboot: %uuV, skipping bank\n",
> +                                buck_volt, opp_vboot);
> +                       __set_bit(idx, &skipped);
> +                       continue;
>                 }
>
>                 spin_lock_irqsave(&svs_lock, flags);
> @@ -1578,6 +1587,14 @@ static int svs_init01(struct svs_platform *svsp)
>                         if (r)
>                                 dev_err(svsb->dev, "opp %uHz enable fail: %d\n",
>                                         svsb->opp_dfreq[i], r);
> +
> +                       /* A skipped bank keeps its default OPP voltages */
> +                       if (test_bit(idx, &skipped))
> +                               dev_pm_opp_adjust_voltage(svsb->opp_dev,
> +                                                         svsb->opp_dfreq[i],
> +                                                         svsb->opp_dvolt[i],
> +                                                         svsb->opp_dvolt[i],
> +                                                         svsb->opp_dvolt[i]);
>                 }
>
>                 if (svsb->volt_flags & SVSB_INIT01_PD_REQ) {
> @@ -1599,6 +1616,9 @@ static int svs_init01(struct svs_platform *svsp)
>                 if (r)
>                         dev_err(svsb->dev, "%s disable fail: %d\n",
>                                 bdata->buck_name, r);
> +
> +               if (test_bit(idx, &skipped))
> +                       svsb->mode_support = SVSB_MODE_ALL_DISABLE;
>         }
>
>  svs_init01_resume_cpuidle:
> --
> 2.53.0
>
>