On Tue, 23 Sept 2025 at 13:41, Nicolas Frattaroli
<nicolas.frattar...@collabora.com> wrote:
>
> Various MediaTek SoCs use GPU integration silicon named "MFlexGraphics"
> by MediaTek. On the MT8196 and MT6991 SoCs, interacting with this
> integration silicon is required to power on the GPU.
>
> This glue silicon is in the form of an embedded microcontroller running
> special-purpose firmware, which autonomously adjusts clocks and
> regulators.
>
> Implement a driver, modelled as a pmdomain driver with a
> set_performance_state operation, to support these SoCs.
>
> The driver also exposes the actual achieved clock rate, as read back
> from the MCU, as common clock framework clocks, by acting as a clock
> provider as well.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattar...@collabora.com>
> ---
> drivers/pmdomain/mediatek/Kconfig | 16 +
> drivers/pmdomain/mediatek/Makefile | 1 +
> drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c | 928
> +++++++++++++++++++++++++++
> 3 files changed, 945 insertions(+)
[...]
> +
> +static int mtk_mfg_set_performance(struct generic_pm_domain *pd,
> + unsigned int state)
> +{
> + struct mtk_mfg *mfg = mtk_mfg_from_genpd(pd);
> +
> + /*
> + * Occasionally, we're asked to set OPPs when we're off. This will
> fail,
> + * so don't do it at all. We do foo != GENPD_STATE_ON instead of !foo
> + * as to not depend on the actual value of the enum.
> + */
> + if (mfg->pd.status != GENPD_STATE_ON)
> + return 0;
Returning 0 here, means that we may end up never restoring the
performance state for a device and its genpd, when the device is
getting runtime resumed. In genpd_runtime_resume() we are calling
genpd_restore_performance_state() before calling genpd_power_on().
This is deliberate, see commit ae8ac19655e0.
That said, I think we need to manage the restore in the ->power_on()
callback. In principle, it means we should call
mtk_mfg_set_oppidx(mfg, genpd->performance_state) from there.
> +
> + return mtk_mfg_set_oppidx(mfg, state);
> +}
> +
> +static int mtk_mfg_power_on(struct generic_pm_domain *pd)
> +{
> + struct mtk_mfg *mfg = mtk_mfg_from_genpd(pd);
> + int ret;
> +
> + ret = regulator_bulk_enable(mfg->variant->num_regulators,
> + mfg->gpu_regs);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(mfg->clk_eb);
> + if (ret)
> + goto err_disable_regulators;
> +
> + ret = clk_bulk_prepare_enable(mfg->variant->num_clks, mfg->gpu_clks);
> + if (ret)
> + goto err_disable_eb_clk;
> +
> + ret = mtk_mfg_eb_on(mfg);
> + if (ret)
> + goto err_disable_clks;
> +
> + ret = mtk_mfg_power_control(mfg, true);
> + if (ret)
> + goto err_eb_off;
> +
> + return 0;
> +
> +err_eb_off:
> + mtk_mfg_eb_off(mfg);
> +err_disable_clks:
> + clk_bulk_disable_unprepare(mfg->variant->num_clks, mfg->gpu_clks);
> +err_disable_eb_clk:
> + clk_disable_unprepare(mfg->clk_eb);
> +err_disable_regulators:
> + regulator_bulk_disable(mfg->variant->num_regulators, mfg->gpu_regs);
> +
> + return ret;
> +}
[...]
Note, I intend to have a bit closer look at this soon, but I just
observed the issue I pointed out above from my first quick look.
Kind regards
Uffe
