On 9/28/26 10:08 AM, Julien Stephan wrote: > The DMA masters of a display pipeline reach DRAM through an SMI local > arbiter, and the larbs themselves through the smi_common block behind > them, whose clocks must be ungated as well. Drive it with a second > driver in the same file, found through the "mediatek,smi" phandle of the > larb, and reference count both: several masters share a larb, and > several larbs share an smi_common. > > U-Boot has no IOMMU driver, so those masters address DRAM directly: add > a helper taking one larb port out of the translated address space, which > the overlay calls once its larb is ungated. > > On some generations, such as on the MT8366, the port mmu_en bits of > SMI_LARB_NONSEC_CON live in the secure world, so clearing them from > U-Boot has no effect, as the Linux driver spells out: > > /* > * When mmu_en bits are in security world, the bank_sel still is > * in the LARB_NONSEC_CON below. And the mmu_en bits of > * LARB_NONSEC_CON have no effect in this case. > */ > if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_CFG_PORT_SEC_CTL)) > arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONTROL, > IOMMU_ATF_CMD_CONFIG_SMI_LARB, ...); > > Ask the secure monitor for an empty mask of IOMMU ports, matching the > physical addresses U-Boot hands to the DMA masters. As in Linux, the call > is only made once the larb clocks are on, from mtk_smi_larb_enable(). > > Four compatibles are added: > > - "mediatek,mt8186-smi-larb", the generic fallback the MT8365 larbs > carry behind their SoC specific string, so a single entry covers > every SoC reusing that larb generation, > - "mediatek,mt8366-smi-larb", which the MT8366 larbs carry on its own > as that SoC is not upstream yet. It is the only entry with driver > data, since it is the only one needing the secure monitor call,
Upstream Linux sets the MTK_SMI_FLAG_CFG_PORT_SEC_CTL flag for mt8188 and mt8189 also. So it is not that these don't have the bit. It is just that apparently these work by default without it (at least HDMI on mt8188, I would have to go back and test DSI on mt8189 without it to be sure - mt8189 is not added in this patch anyway). > - "mediatek,mt8365-smi-common" and "mediatek,mt8366-smi-common" for > the smi_common driver. Neither node declares a generic fallback, so > both SoC specific strings have to be listed. > > Signed-off-by: Julien Stephan <[email protected]> > --- > drivers/memory/Kconfig | 1 + > drivers/memory/mtk-smi.c | 223 > ++++++++++++++++++++++++++++++++++++++++++++--- > include/mtk_smi.h | 19 +++- > 3 files changed, 229 insertions(+), 14 deletions(-) > > diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig > index a3b7c722070..772b315d41b 100644 > --- a/drivers/memory/Kconfig > +++ b/drivers/memory/Kconfig > @@ -23,6 +23,7 @@ config ATMEL_EBI > config MTK_SMI > bool "MediaTek SoC Memory Controller driver" > depends on ARCH_MEDIATEK && DM > + select ARM_SMCCC > select MEMORY > help > This driver is for the Memory Controller module in MediaTek SoCs, > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > index b90f5cc135f..caba8b5c72a 100644 > --- a/drivers/memory/mtk-smi.c > +++ b/drivers/memory/mtk-smi.c > @@ -1,47 +1,204 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* > - * MediaTek SMI local arbiter (larb) driver > + * MediaTek SMI larb and smi_common driver I would leave "local arbiter (larb)" in the comment for those that don't know what larb means. > +static const struct udevice_id mtk_smi_common_ids[] = { > + { .compatible = "mediatek,mt8365-smi-common" }, > + { .compatible = "mediatek,mt8366-smi-common" }, > + { } > +}; > + > +U_BOOT_DRIVER(mtk_smi_common) = { > + .name = "mtk_smi_common", > + .id = UCLASS_MEMORY, > + .of_match = mtk_smi_common_ids, > + .probe = mtk_smi_common_probe, > + .priv_auto = sizeof(struct mtk_smi_common_priv), nit: I would not bother to align these. Alignment tends to break over time. > +}; > + > int mtk_smi_larb_enable(struct udevice *dev) > { > struct mtk_smi_larb_priv *priv = dev_get_priv(dev); > int ret; > > - /* several DMA masters can sit behind the same larb */ > + /* one larb feeds several DMA masters, only the first ungates it */ > if (priv->enable_count++) > return 0; > > + if (priv->smi_common) { > + ret = mtk_smi_common_enable(priv->smi_common); > + if (ret) > + goto err; > + } > + > ret = clk_enable_bulk(&priv->clk_bulk); > if (ret) { > dev_err(dev, "failed to enable clocks: %d\n", ret); > - priv->enable_count--; > + goto err_disable_common; > + } > + > + /* as in Linux, only talk to the secure monitor once the larb is on */ > + if (priv->data && priv->data->port_sec_ctl) { > + ret = mtk_smi_larb_bypass_iommu(dev); > + if (ret) > + goto err_disable_clks; > } > > + return 0; > + > +err_disable_clks: > + clk_disable_bulk(&priv->clk_bulk); > +err_disable_common: > + if (priv->smi_common) > + mtk_smi_common_disable(priv->smi_common); > +err: > + priv->enable_count = 0; > + > return ret; > } > > @@ -60,23 +217,67 @@ int mtk_smi_larb_disable(struct udevice *dev) > if (ret) > dev_err(dev, "failed to disable clocks: %d\n", ret); > > + if (priv->smi_common) > + mtk_smi_common_disable(priv->smi_common); > + > return ret; > } > > +void mtk_smi_larb_port_set_pa(struct udevice *dev, unsigned int port) > +{ > + struct mtk_smi_larb_priv *priv = dev_get_priv(dev); > + > + clrbits_le32(priv->base + SMI_LARB_NONSEC_CON(port), MMU_EN); > +} > + > static int mtk_smi_larb_probe(struct udevice *dev) > { > struct mtk_smi_larb_priv *priv = dev_get_priv(dev); > int ret; > > + priv->base = dev_remap_addr(dev); > + if (!priv->base) > + return -EINVAL; > + > ret = clk_get_bulk(dev, &priv->clk_bulk); > - if (ret) > + if (ret) { > dev_err(dev, "failed to get clocks: %d\n", ret); > + return ret; > + } > > - return ret; > + /* > + * Not every smi_common block is handled by this driver: where it > + * isn't, its clocks are already ungated by the time the DMA masters > + * run, so having no device for it is not an error. > + */ > + ret = uclass_get_device_by_phandle(UCLASS_MEMORY, dev, "mediatek,smi", > + &priv->smi_common); > + if (ret) { This would ignore fail-to-probe errors since it caused the device to be probed if it was not already. Should probabaly only allow -ENODEV and -ENOENT to mean that it is already bound to another driver or there is not such a node. > + dev_dbg(dev, "no smi_common device: %d\n", ret); > + priv->smi_common = NULL; > + } > + > + priv->data = (const struct mtk_smi_larb_data *)dev_get_driver_data(dev); > + > + return 0; > } > > +/* > + * The MT8365 larbs also carry the generic mediatek,mt8186-smi-larb > + * compatible, which is the one matched here. The MT8366 ones only have > + * their own, as that SoC is not upstream yet. > + */ > static const struct udevice_id mtk_smi_larb_ids[] = { > - { .compatible = "mediatek,mt8188-smi-larb" }, > + { > + .compatible = "mediatek,mt8186-smi-larb", > + }, > + { > + .compatible = "mediatek,mt8188-smi-larb", > + }, > + { > + .compatible = "mediatek,mt8366-smi-larb", > + .data = (ulong)&mt8366_larb_data, > + }, > { } > }; > > diff --git a/include/mtk_smi.h b/include/mtk_smi.h > index b0166b37538..4c904d8bfe1 100644 > --- a/include/mtk_smi.h > +++ b/include/mtk_smi.h > @@ -15,8 +15,11 @@ struct udevice; > * mtk_smi_larb_enable() - ungate an SMI local arbiter > * @dev: larb device > * > - * Enable the clocks of the larb so that the DMA master behind it can reach > - * DRAM. Calls are refcounted, so several masters can share one larb. > + * Enable the clocks of the smi_common block behind the larb, if any, then > + * the larb ones, so that the DMA master in front of it can reach DRAM. On > + * the SoCs where the secure world owns the port mmu_en bits, also ask the > + * secure monitor to keep every port of the larb out of the IOMMU. Calls > + * are refcounted, so several masters can share one larb. > * > * Return: 0 on success, or a negative error code > */ > @@ -27,11 +30,21 @@ int mtk_smi_larb_enable(struct udevice *dev); > * @dev: larb device > * > * Drop one reference taken by mtk_smi_larb_enable(), and disable the larb > - * clocks when it was the last one. > + * clocks, then the smi_common ones, when it was the last one. > * > * Return: 0 on success, -EINVAL if the larb is not enabled, or another > * negative error code > */ > int mtk_smi_larb_disable(struct udevice *dev); > > +/** > + * mtk_smi_larb_port_set_pa() - take a larb port out of the IOMMU I assume that pa means "physical address"? Can we put that in the description? > + * @dev: larb device > + * @port: M4U port index within the larb > + * > + * Without an IOMMU driver the DMA masters address DRAM directly, so their > + * ports must be taken out of the translated address space. > + */ > +void mtk_smi_larb_port_set_pa(struct udevice *dev, unsigned int port); > + > #endif /* _MTK_SMI_H */ >
