| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPaKu7QWBShwr+YhFi+nUFo0kJ06k4PK3zggcCefWGjqUmTx5w@mail.gmail.com>
Date: Fri, 3 Oct 2025 14:41:16 -0700
From: Chia-I Wu <olvaffe@...il.com>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Boris Brezillon <boris.brezillon@...labora.com>, Jassi Brar <jassisinghbrar@...il.com>,
Chen-Yu Tsai <wenst@...omium.org>, Steven Price <steven.price@....com>,
Liviu Dudau <liviu.dudau@....com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>, Kees Cook <kees@...nel.org>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>, Ulf Hansson <ulf.hansson@...aro.org>, kernel@...labora.com,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-hardening@...r.kernel.org,
linux-pm@...r.kernel.org
Subject: Re: [PATCH v6 7/7] pmdomain: mediatek: Add support for MFlexGraphics
On Fri, Oct 3, 2025 at 1:16 PM Nicolas Frattaroli
<nicolas.frattaroli@...labora.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.
I like this model a lot. Thanks!
panthor might potentially need to interact with this driver beyond
what pmdomain provides. I am thinking about querying
GF_REG_SHADER_PRESENT. Not sure if we've heard back from the vendor.
Have you considered moving this to drivers/soc/mediatek such that we
can provide include/linux/mtk-mfg.h to panthor?
>
> 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.frattaroli@...labora.com>
> ---
> drivers/pmdomain/mediatek/Kconfig | 16 +
> drivers/pmdomain/mediatek/Makefile | 1 +
> drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c | 1027 ++++++++++++++++++++++++++
> 3 files changed, 1044 insertions(+)
[...]
> +static int mtk_mfg_init_shared_mem(struct mtk_mfg *mfg)
> +{
> + struct device *dev = &mfg->pdev->dev;
> + struct mtk_mfg_ipi_msg msg = {};
> + int ret;
> +
> + dev_dbg(dev, "clearing GPUEB shared memory, 0x%X bytes\n", mfg->shared_mem_size);
> + memset_io(mfg->shared_mem, 0, mfg->shared_mem_size);
> +
> + msg.cmd = CMD_INIT_SHARED_MEM;
> + msg.u.shared_mem.base = mfg->shared_mem_phys;
> + msg.u.shared_mem.size = mfg->shared_mem_size;
> +
> + ret = mtk_mfg_send_ipi(mfg, &msg);
> + if (ret)
> + return ret;
> +
> + if (readl(mfg->shared_mem) != GPUEB_MEM_MAGIC) {
Add the offset GF_REG_MAGIC, even though it is 0.
> + dev_err(dev, "EB did not initialise shared memory correctly\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
[...]
> +static int mtk_mfg_mt8196_init(struct mtk_mfg *mfg)
> +{
> + void __iomem *e2_base;
> +
> + e2_base = devm_platform_ioremap_resource_byname(mfg->pdev, "hw-revision");
> + if (IS_ERR(e2_base))
> + return dev_err_probe(&mfg->pdev->dev, PTR_ERR(e2_base),
> + "Couldn't get hw-revision register\n");
> +
> + if (readl(e2_base) == MFG_MT8196_E2_ID)
> + mfg->ghpm_en_reg = RPC_DUMMY_REG_2;
> + else
> + mfg->ghpm_en_reg = RPC_GHPM_CFG0_CON;
> +
> + return 0;
> +};
Extraneous semicolon.
> +static int mtk_mfg_init_mbox(struct mtk_mfg *mfg)
> +{
> + struct device *dev = &mfg->pdev->dev;
> + struct mtk_mfg_mbox *gf;
> + struct mtk_mfg_mbox *slp;
> +
> + gf = devm_kzalloc(dev, sizeof(*gf), GFP_KERNEL);
> + if (!gf)
> + return -ENOMEM;
> +
> + gf->rx_data = devm_kzalloc(dev, GPUEB_MBOX_MAX_RX_SIZE, GFP_KERNEL);
It looks like gfx->rx_data can simply be "struct mtk_mfg_ipi_msg rx_data;".
> + if (!gf->rx_data)
> + return -ENOMEM;
> +
> + gf->mfg = mfg;
> + init_completion(&gf->rx_done);
> + gf->cl.dev = dev;
> + gf->cl.rx_callback = mtk_mfg_mbox_rx_callback;
> + gf->cl.tx_tout = GPUEB_TIMEOUT_US / USEC_PER_MSEC;
> + gf->ch = mbox_request_channel_byname(&gf->cl, "gpufreq");
> + if (IS_ERR(gf->ch))
> + return PTR_ERR(gf->ch);
> +
> + mfg->gf_mbox = gf;
> +
> + slp = devm_kzalloc(dev, sizeof(*slp), GFP_KERNEL);
> + if (!slp)
> + return -ENOMEM;
> +
> + slp->mfg = mfg;
> + init_completion(&slp->rx_done);
> + slp->cl.dev = dev;
> + slp->cl.tx_tout = GPUEB_TIMEOUT_US / USEC_PER_MSEC;
> + slp->cl.tx_block = true;
> + slp->ch = mbox_request_channel_byname(&slp->cl, "sleep");
> + if (IS_ERR(slp->ch))
> + return PTR_ERR(slp->ch);
Free gf->ch.
> +
> + mfg->slp_mbox = slp;
> +
> + return 0;
> +}
> +
> +static int mtk_mfg_init_clk_provider(struct mtk_mfg *mfg)
> +{
> + struct device *dev = &mfg->pdev->dev;
> + struct clk_hw_onecell_data *clk_data;
> + int ret;
> +
> + clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, 2), GFP_KERNEL);
> + if (!clk_data)
> + return -ENOMEM;
> +
> + clk_data->num = 2;
> +
> + mfg->clk_core_hw.init = &mtk_mfg_clk_gpu_init;
> + mfg->clk_stack_hw.init = &mtk_mfg_clk_stack_init;
> +
> + ret = devm_clk_hw_register(dev, &mfg->clk_core_hw);
> + if (ret)
> + return dev_err_probe(dev, ret, "Couldn't register GPU core clock\n");
> +
> + ret = devm_clk_hw_register(dev, &mfg->clk_stack_hw);
> + if (ret)
> + return dev_err_probe(dev, ret, "Couldn't register GPU stack clock\n");
> +
> + clk_data->hws[0] = &mfg->clk_core_hw;
> + clk_data->hws[1] = &mfg->clk_stack_hw;
> +
> + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
> + if (ret)
> + return dev_err_probe(dev, ret, "Couldn't register clock provider\n");
> +
> + return 0;
> +}
> +
> +static int mtk_mfg_probe(struct platform_device *pdev)
> +{
> + struct device_node *shmem __free(device_node);
> + struct mtk_mfg *mfg;
> + struct device *dev = &pdev->dev;
> + const struct mtk_mfg_variant *data = of_device_get_match_data(dev);
> + struct resource res;
> + int ret, i;
> +
> + mfg = devm_kzalloc(dev, sizeof(*mfg), GFP_KERNEL);
> + if (!mfg)
> + return -ENOMEM;
> +
> + mfg->pdev = pdev;
> + mfg->variant = data;
> +
> + dev_set_drvdata(dev, mfg);
> +
> + mfg->gpr = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(mfg->gpr))
> + return dev_err_probe(dev, PTR_ERR(mfg->gpr),
> + "Couldn't retrieve GPR MMIO registers\n");
> +
> + mfg->rpc = devm_platform_ioremap_resource(pdev, 1);
> + if (IS_ERR(mfg->rpc))
> + return dev_err_probe(dev, PTR_ERR(mfg->rpc),
> + "Couldn't retrieve RPC MMIO registers\n");
> +
> + mfg->clk_eb = devm_clk_get(dev, "eb");
> + if (IS_ERR(mfg->clk_eb))
> + return dev_err_probe(dev, PTR_ERR(mfg->clk_eb),
> + "Couldn't get 'eb' clock\n");
> +
> + mfg->gpu_clks = devm_kcalloc(dev, data->num_clks, sizeof(*mfg->gpu_clks),
> + GFP_KERNEL);
> + if (!mfg->gpu_clks)
> + return -ENOMEM;
> +
> + for (i = 0; i < data->num_clks; i++)
> + mfg->gpu_clks[i].id = data->clk_names[i];
> +
> + ret = devm_clk_bulk_get(dev, data->num_clks, mfg->gpu_clks);
> + if (ret)
> + return dev_err_probe(dev, ret, "Couldn't get GPU clocks\n");
> +
> + mfg->gpu_regs = devm_kcalloc(dev, data->num_regulators,
> + sizeof(*mfg->gpu_regs), GFP_KERNEL);
> + if (!mfg->gpu_regs)
> + return -ENOMEM;
> +
> + for (i = 0; i < data->num_regulators; i++)
> + mfg->gpu_regs[i].supply = data->regulator_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, data->num_regulators, mfg->gpu_regs);
> + if (ret)
> + return dev_err_probe(dev, ret, "Couldn't get GPU regulators\n");
> +
> + ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
> + if (ret)
> + return dev_err_probe(dev, ret, "Couldn't get GPUEB shared memory\n");
> +
> + mfg->shared_mem = devm_ioremap(dev, res.start, resource_size(&res));
> + if (!mfg->shared_mem)
> + return dev_err_probe(dev, -ENOMEM, "Can't ioremap GPUEB shared memory\n");
> + mfg->shared_mem_size = resource_size(&res);
> + mfg->shared_mem_phys = res.start;
> +
> + if (data->init) {
> + ret = data->init(mfg);
> + if (ret)
> + return dev_err_probe(dev, ret, "Variant init failed\n");
> + }
> +
> + mfg->pd.name = dev_name(dev);
> + mfg->pd.attach_dev = mtk_mfg_attach_dev;
> + mfg->pd.detach_dev = mtk_mfg_detach_dev;
> + mfg->pd.power_off = mtk_mfg_power_off;
> + mfg->pd.power_on = mtk_mfg_power_on;
> + mfg->pd.set_performance_state = mtk_mfg_set_performance;
> + mfg->pd.flags = GENPD_FLAG_OPP_TABLE_FW;
> +
> + ret = pm_genpd_init(&mfg->pd, NULL, false);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to initialise power domain\n");
We need to clean up mgf->md on errors from this point on. Maybe we can
move this to a later point?
> +
> + ret = mtk_mfg_init_mbox(mfg);
> + if (ret)
> + return dev_err_probe(dev, ret, "Couldn't initialise mailbox\n");
We need to free the mboxes from this point on.
> + ret = mtk_mfg_power_on(&mfg->pd);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to power on MFG\n");
> +
> + ret = mtk_mfg_init_shared_mem(mfg);
> + if (ret) {
> + dev_err_probe(dev, ret, "Couldn't initialize EB shared memory\n");
> + goto out;
> + }
> +
> + ret = mtk_mfg_read_opp_tables(mfg);
> + if (ret) {
> + dev_err_probe(dev, ret, "Error reading OPP tables from EB\n");
> + goto out;
> + }
> +
> + ret = mtk_mfg_init_clk_provider(mfg);
> + if (ret)
> + goto out;
> +
> + ret = of_genpd_add_provider_simple(dev->of_node, &mfg->pd);
> + if (ret) {
> + dev_err_probe(dev, ret, "Failed to add pmdomain provider\n");
> + goto out;
> + }
> +
> + return 0;
> +
> +out:
> + mtk_mfg_power_off(&mfg->pd);
> + return ret;
> +}
Powered by blists - more mailing lists