* [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
@ 2021-08-19 15:13 ` Alexandre Bailon
0 siblings, 0 replies; 27+ messages in thread
From: Alexandre Bailon @ 2021-08-19 15:13 UTC (permalink / raw)
To: ohad, bjorn.andersson, mathieu.poirier, robh+dt
Cc: matthias.bgg, linux-remoteproc, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel, stephane.leprovost, gpain, khilman,
Alexandre Bailon
This adds a driver to control the APU present in the MT8183.
This loads the firmware and start the DSP.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
drivers/remoteproc/Kconfig | 10 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/mtk_apu.c | 440 +++++++++++++++++++++++++++++++++++
3 files changed, 451 insertions(+)
create mode 100644 drivers/remoteproc/mtk_apu.c
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 9a6eedc3994a5..6c106a6c3ad5d 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -53,6 +53,16 @@ config MTK_SCP
It's safe to say N here.
+config MTK_APU
+ tristate "Mediatek APU remoteproc support"
+ depends on ARCH_MEDIATEK
+ depends on MTK_IOMMU
+ help
+ Say y to support the Mediatek's Accelerated Processing Unit (APU) via
+ the remote processor framework.
+
+ It's safe to say N here.
+
config OMAP_REMOTEPROC
tristate "OMAP remoteproc support"
depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index bb26c9e4ef9cf..e395af86c17b0 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o
obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
obj-$(CONFIG_INGENIC_VPU_RPROC) += ingenic_rproc.o
obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
+obj-$(CONFIG_MTK_APU) += mtk_apu.o
obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
new file mode 100644
index 0000000000000..0e3f63987bd85
--- /dev/null
+++ b/drivers/remoteproc/mtk_apu.c
@@ -0,0 +1,440 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 BayLibre SAS
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/iommu.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+#define SW_RST (0x0000000C)
+#define SW_RST_OCD_HALT_ON_RST BIT(12)
+#define SW_RST_IPU_D_RST BIT(8)
+#define SW_RST_IPU_B_RST BIT(4)
+#define CORE_CTRL (0x00000110)
+#define CORE_CTRL_PDEBUG_ENABLE BIT(31)
+#define CORE_CTRL_SRAM_64K_iMEM (0x00 << 27)
+#define CORE_CTRL_SRAM_96K_iMEM (0x01 << 27)
+#define CORE_CTRL_SRAM_128K_iMEM (0x02 << 27)
+#define CORE_CTRL_SRAM_192K_iMEM (0x03 << 27)
+#define CORE_CTRL_SRAM_256K_iMEM (0x04 << 27)
+#define CORE_CTRL_PBCLK_ENABLE BIT(26)
+#define CORE_CTRL_RUN_STALL BIT(23)
+#define CORE_CTRL_STATE_VECTOR_SELECT BIT(19)
+#define CORE_CTRL_PIF_GATED BIT(17)
+#define CORE_CTRL_NMI BIT(0)
+#define CORE_XTENSA_INT (0x00000114)
+#define CORE_CTL_XTENSA_INT (0x00000118)
+#define CORE_DEFAULT0 (0x0000013C)
+#define CORE_DEFAULT0_QOS_SWAP_0 (0x00 << 28)
+#define CORE_DEFAULT0_QOS_SWAP_1 (0x01 << 28)
+#define CORE_DEFAULT0_QOS_SWAP_2 (0x02 << 28)
+#define CORE_DEFAULT0_QOS_SWAP_3 (0x03 << 28)
+#define CORE_DEFAULT0_ARUSER_USE_IOMMU (0x10 << 23)
+#define CORE_DEFAULT0_AWUSER_USE_IOMMU (0x10 << 18)
+#define CORE_DEFAULT1 (0x00000140)
+#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU (0x10 << 0)
+#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU (0x10 << 5)
+#define CORE_XTENSA_ALTRESETVEC (0x000001F8)
+
+#define RSC_VENDOR_CARVEOUT (RSC_VENDOR_START + 1)
+
+#define APU_RESET_DELAY (27)
+
+struct mtk_apu_rproc {
+ struct device *dev;
+ void __iomem *base;
+ int irq;
+ struct clk_bulk_data clks[3];
+ struct list_head mappings;
+};
+
+static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ struct device *dev = rproc->dev.parent;
+ struct rproc_mem_entry *mapping;
+ struct iommu_domain *domain;
+ int ret;
+ u64 pa;
+
+ mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
+ if (!mapping)
+ return -ENOMEM;
+
+ if (!entry->va)
+ pa = entry->dma;
+ else
+ pa = rproc_va_to_pa(entry->va);
+
+ if ((strcmp(entry->name, "vdev0vring0") == 0 ||
+ strcmp(entry->name, "vdev0vring1") == 0)) {
+ entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
+ if (IS_ERR_OR_NULL(entry->va)) {
+ dev_err(dev, "Unable to map memory region: %pa+%lx\n",
+ &entry->dma, entry->len);
+ ret = PTR_ERR(mapping->va);
+ goto free_mapping;
+ }
+ mapping->va = entry->va;
+ }
+
+ domain = iommu_get_domain_for_dev(dev);
+ ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
+ if (ret) {
+ dev_err(dev, "iommu_map failed: %d\n", ret);
+ goto err_memunmap;
+ }
+
+ mapping->da = entry->da;
+ mapping->len = entry->len;
+ list_add_tail(&mapping->node, &apu_rproc->mappings);
+
+ return 0;
+
+err_memunmap:
+ memunmap(mapping->va);
+free_mapping:
+ kfree(mapping);
+
+ return ret;
+}
+
+static void mtk_apu_iommu_unmap_all(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ struct device *dev = rproc->dev.parent;
+ struct rproc_mem_entry *entry, *tmp;
+ struct iommu_domain *domain;
+
+ /* clean up iommu mapping entries */
+ list_for_each_entry_safe(entry, tmp, &apu_rproc->mappings, node) {
+ size_t unmapped;
+
+ domain = iommu_get_domain_for_dev(dev);
+ unmapped = iommu_unmap(domain, entry->da, entry->len);
+ if (unmapped != entry->len) {
+ /* nothing much to do besides complaining */
+ dev_err(dev, "failed to unmap %zx/%zu\n", entry->len,
+ unmapped);
+ }
+ memunmap(entry->va);
+
+ list_del(&entry->node);
+ kfree(entry);
+ }
+}
+
+static int mtk_apu_rproc_prepare(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ int ret;
+
+ ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
+ apu_rproc->clks);
+ if (ret)
+ dev_err(apu_rproc->dev, "Failed to enable clocks\n");
+
+ return ret;
+}
+
+static int mtk_apu_rproc_unprepare(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+
+ clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
+ apu_rproc->clks);
+
+ return 0;
+}
+
+static int mtk_apu_rproc_start(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ u32 core_ctrl;
+
+ /* Set reset vector of APU firmware boot address */
+ writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
+
+ /* Turn on the clocks and stall the APU */
+ core_ctrl = readl(apu_rproc->base + CORE_CTRL);
+ core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
+ CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
+ CORE_CTRL_PIF_GATED;
+ writel(core_ctrl, apu_rproc->base + CORE_CTRL);
+
+ /* Reset the APU: this requires 27 ns to be effective on any platform */
+ writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
+ apu_rproc->base + SW_RST);
+ ndelay(APU_RESET_DELAY);
+ writel(0, apu_rproc->base + SW_RST);
+
+ core_ctrl &= ~CORE_CTRL_PIF_GATED;
+ writel(core_ctrl, apu_rproc->base + CORE_CTRL);
+
+ /* Configure memory accesses to go through the IOMMU */
+ writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
+ CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
+ writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
+ CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
+ apu_rproc->base + CORE_DEFAULT1);
+
+ /* Release the APU */
+ core_ctrl &= ~CORE_CTRL_RUN_STALL;
+ writel(core_ctrl, apu_rproc->base + CORE_CTRL);
+
+ return 0;
+}
+
+static int mtk_apu_rproc_stop(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ u32 core_ctrl;
+
+ core_ctrl = readl(apu_rproc->base + CORE_CTRL);
+ writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
+
+ mtk_apu_iommu_unmap_all(rproc);
+
+ return 0;
+}
+
+static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+
+ writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
+}
+
+static int mtk_apu_load(struct rproc *rproc, const struct firmware *fw)
+
+{
+ struct rproc_mem_entry *entry, *tmp;
+ int ret;
+
+ ret = rproc_elf_load_segments(rproc, fw);
+ if (ret)
+ return ret;
+
+ list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
+ ret = mtk_apu_iommu_map(rproc, entry);
+ if (ret)
+ goto err_unmap_all;
+ }
+
+ return 0;
+
+err_unmap_all:
+ mtk_apu_iommu_unmap_all(rproc);
+
+ return ret;
+}
+
+static struct reserved_mem *of_reserved_mem_by_name(struct rproc *rproc,
+ const char *name)
+{
+ struct device *dev = rproc->dev.parent;
+ struct device_node *np = dev->of_node;
+ struct device_node *target;
+ struct reserved_mem *rmem;
+ int idx;
+
+ idx = of_property_match_string(np, "memory-region-names", name);
+ if (idx < 0) {
+ dev_err(dev, "failed to find %s memory\n", name);
+ return NULL;
+ }
+
+ target = of_parse_phandle(np, "memory-region", idx);
+ if (!target)
+ return NULL;
+
+ rmem = of_reserved_mem_lookup(target);
+ if (!rmem)
+ dev_err(dev, "unable to acquire memory-region\n");
+ of_node_put(target);
+
+ return rmem;
+}
+
+static int mtk_apu_handle_rsc(struct rproc *rproc, u32 rsc_type, void *rsc,
+ int offset, int avail)
+{
+ struct device *dev = rproc->dev.parent;
+
+ if (rsc_type == RSC_VENDOR_CARVEOUT) {
+ struct fw_rsc_carveout *rsc_carveout = rsc;
+ struct rproc_mem_entry *mem;
+ struct reserved_mem *rmem;
+
+ rmem = of_reserved_mem_by_name(rproc, rsc_carveout->name);
+ if (!rmem)
+ return -ENOMEM;
+
+ if (rmem->size < rsc_carveout->len) {
+ dev_err(dev, "The reserved memory is too small\n");
+ return -ENOMEM;
+ }
+
+ mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base,
+ rsc_carveout->len, rsc_carveout->da,
+ NULL, NULL, rsc_carveout->name);
+ if (!mem)
+ return -ENOMEM;
+
+ mem->flags = rsc_carveout->flags;
+ rsc_carveout->pa = rmem->base;
+ rproc_add_carveout(rproc, mem);
+ }
+
+ return RSC_HANDLED;
+}
+
+static const struct rproc_ops mtk_apu_rproc_ops = {
+ .prepare = mtk_apu_rproc_prepare,
+ .unprepare = mtk_apu_rproc_unprepare,
+ .start = mtk_apu_rproc_start,
+ .stop = mtk_apu_rproc_stop,
+ .kick = mtk_apu_rproc_kick,
+ .load = mtk_apu_load,
+ .parse_fw = rproc_elf_load_rsc_table,
+ .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+ .sanity_check = rproc_elf_sanity_check,
+ .get_boot_addr = rproc_elf_get_boot_addr,
+ .handle_rsc = mtk_apu_handle_rsc,
+};
+
+static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
+{
+ struct rproc *rproc = data;
+ struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
+
+ writel(1, apu_rproc->base + CORE_XTENSA_INT);
+
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t handle_event(int irq, void *data)
+{
+ struct rproc *rproc = data;
+
+ rproc_vq_interrupt(rproc, 0);
+ rproc_vq_interrupt(rproc, 1);
+
+ return IRQ_HANDLED;
+}
+
+static int mtk_apu_rproc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mtk_apu_rproc *apu_rproc;
+ struct rproc *rproc;
+ struct resource *res;
+ int ret;
+
+ rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
+ sizeof(*apu_rproc));
+ if (!rproc)
+ return -ENOMEM;
+
+ rproc->recovery_disabled = true;
+ rproc->has_iommu = false;
+ rproc->auto_boot = false;
+
+ apu_rproc = rproc->priv;
+ apu_rproc->dev = dev;
+ INIT_LIST_HEAD(&apu_rproc->mappings);
+
+ platform_set_drvdata(pdev, rproc);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ apu_rproc->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(apu_rproc->base)) {
+ dev_err(dev, "Failed to map mmio\n");
+ ret = PTR_ERR(apu_rproc->base);
+ goto free_rproc;
+ }
+
+ apu_rproc->irq = platform_get_irq(pdev, 0);
+ if (apu_rproc->irq < 0) {
+ ret = apu_rproc->irq;
+ goto free_rproc;
+ }
+
+ ret = devm_request_threaded_irq(dev, apu_rproc->irq,
+ mtk_apu_rproc_callback, handle_event,
+ IRQF_SHARED | IRQF_ONESHOT,
+ NULL, rproc);
+ if (ret) {
+ dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
+ goto free_rproc;
+ }
+
+ apu_rproc->clks[0].id = "ipu";
+ apu_rproc->clks[1].id = "axi";
+ apu_rproc->clks[2].id = "jtag";
+
+ ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
+ apu_rproc->clks);
+ if (ret) {
+ dev_err(dev, "Failed to get clocks\n");
+ goto free_rproc;
+ }
+
+ ret = rproc_add(rproc);
+ if (ret) {
+ dev_err(dev, "rproc_add failed: %d\n", ret);
+ goto free_rproc;
+ }
+
+ return 0;
+
+free_rproc:
+ rproc_free(rproc);
+
+ return ret;
+}
+
+static int mtk_apu_rproc_remove(struct platform_device *pdev)
+{
+ struct rproc *rproc = platform_get_drvdata(pdev);
+ struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
+ struct device *dev = &pdev->dev;
+
+ disable_irq(apu_rproc->irq);
+
+ rproc_del(rproc);
+ of_reserved_mem_device_release(dev);
+ rproc_free(rproc);
+
+ return 0;
+}
+
+static const struct of_device_id mtk_apu_rproc_of_match[] = {
+ { .compatible = "mediatek,mt8183-apu", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
+
+static struct platform_driver mtk_apu_rproc_driver = {
+ .probe = mtk_apu_rproc_probe,
+ .remove = mtk_apu_rproc_remove,
+ .driver = {
+ .name = "mtk_apu-rproc",
+ .of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
+ },
+};
+module_platform_driver(mtk_apu_rproc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Alexandre Bailon");
+MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
@ 2021-08-19 15:13 ` Alexandre Bailon
0 siblings, 0 replies; 27+ messages in thread
From: Alexandre Bailon @ 2021-08-19 15:13 UTC (permalink / raw)
To: ohad, bjorn.andersson, mathieu.poirier, robh+dt
Cc: matthias.bgg, linux-remoteproc, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel, stephane.leprovost, gpain, khilman,
Alexandre Bailon
This adds a driver to control the APU present in the MT8183.
This loads the firmware and start the DSP.
Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
drivers/remoteproc/Kconfig | 10 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/mtk_apu.c | 440 +++++++++++++++++++++++++++++++++++
3 files changed, 451 insertions(+)
create mode 100644 drivers/remoteproc/mtk_apu.c
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 9a6eedc3994a5..6c106a6c3ad5d 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -53,6 +53,16 @@ config MTK_SCP
It's safe to say N here.
+config MTK_APU
+ tristate "Mediatek APU remoteproc support"
+ depends on ARCH_MEDIATEK
+ depends on MTK_IOMMU
+ help
+ Say y to support the Mediatek's Accelerated Processing Unit (APU) via
+ the remote processor framework.
+
+ It's safe to say N here.
+
config OMAP_REMOTEPROC
tristate "OMAP remoteproc support"
depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index bb26c9e4ef9cf..e395af86c17b0 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o
obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
obj-$(CONFIG_INGENIC_VPU_RPROC) += ingenic_rproc.o
obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
+obj-$(CONFIG_MTK_APU) += mtk_apu.o
obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
new file mode 100644
index 0000000000000..0e3f63987bd85
--- /dev/null
+++ b/drivers/remoteproc/mtk_apu.c
@@ -0,0 +1,440 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 BayLibre SAS
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/iommu.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+#define SW_RST (0x0000000C)
+#define SW_RST_OCD_HALT_ON_RST BIT(12)
+#define SW_RST_IPU_D_RST BIT(8)
+#define SW_RST_IPU_B_RST BIT(4)
+#define CORE_CTRL (0x00000110)
+#define CORE_CTRL_PDEBUG_ENABLE BIT(31)
+#define CORE_CTRL_SRAM_64K_iMEM (0x00 << 27)
+#define CORE_CTRL_SRAM_96K_iMEM (0x01 << 27)
+#define CORE_CTRL_SRAM_128K_iMEM (0x02 << 27)
+#define CORE_CTRL_SRAM_192K_iMEM (0x03 << 27)
+#define CORE_CTRL_SRAM_256K_iMEM (0x04 << 27)
+#define CORE_CTRL_PBCLK_ENABLE BIT(26)
+#define CORE_CTRL_RUN_STALL BIT(23)
+#define CORE_CTRL_STATE_VECTOR_SELECT BIT(19)
+#define CORE_CTRL_PIF_GATED BIT(17)
+#define CORE_CTRL_NMI BIT(0)
+#define CORE_XTENSA_INT (0x00000114)
+#define CORE_CTL_XTENSA_INT (0x00000118)
+#define CORE_DEFAULT0 (0x0000013C)
+#define CORE_DEFAULT0_QOS_SWAP_0 (0x00 << 28)
+#define CORE_DEFAULT0_QOS_SWAP_1 (0x01 << 28)
+#define CORE_DEFAULT0_QOS_SWAP_2 (0x02 << 28)
+#define CORE_DEFAULT0_QOS_SWAP_3 (0x03 << 28)
+#define CORE_DEFAULT0_ARUSER_USE_IOMMU (0x10 << 23)
+#define CORE_DEFAULT0_AWUSER_USE_IOMMU (0x10 << 18)
+#define CORE_DEFAULT1 (0x00000140)
+#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU (0x10 << 0)
+#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU (0x10 << 5)
+#define CORE_XTENSA_ALTRESETVEC (0x000001F8)
+
+#define RSC_VENDOR_CARVEOUT (RSC_VENDOR_START + 1)
+
+#define APU_RESET_DELAY (27)
+
+struct mtk_apu_rproc {
+ struct device *dev;
+ void __iomem *base;
+ int irq;
+ struct clk_bulk_data clks[3];
+ struct list_head mappings;
+};
+
+static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ struct device *dev = rproc->dev.parent;
+ struct rproc_mem_entry *mapping;
+ struct iommu_domain *domain;
+ int ret;
+ u64 pa;
+
+ mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
+ if (!mapping)
+ return -ENOMEM;
+
+ if (!entry->va)
+ pa = entry->dma;
+ else
+ pa = rproc_va_to_pa(entry->va);
+
+ if ((strcmp(entry->name, "vdev0vring0") == 0 ||
+ strcmp(entry->name, "vdev0vring1") == 0)) {
+ entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
+ if (IS_ERR_OR_NULL(entry->va)) {
+ dev_err(dev, "Unable to map memory region: %pa+%lx\n",
+ &entry->dma, entry->len);
+ ret = PTR_ERR(mapping->va);
+ goto free_mapping;
+ }
+ mapping->va = entry->va;
+ }
+
+ domain = iommu_get_domain_for_dev(dev);
+ ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
+ if (ret) {
+ dev_err(dev, "iommu_map failed: %d\n", ret);
+ goto err_memunmap;
+ }
+
+ mapping->da = entry->da;
+ mapping->len = entry->len;
+ list_add_tail(&mapping->node, &apu_rproc->mappings);
+
+ return 0;
+
+err_memunmap:
+ memunmap(mapping->va);
+free_mapping:
+ kfree(mapping);
+
+ return ret;
+}
+
+static void mtk_apu_iommu_unmap_all(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ struct device *dev = rproc->dev.parent;
+ struct rproc_mem_entry *entry, *tmp;
+ struct iommu_domain *domain;
+
+ /* clean up iommu mapping entries */
+ list_for_each_entry_safe(entry, tmp, &apu_rproc->mappings, node) {
+ size_t unmapped;
+
+ domain = iommu_get_domain_for_dev(dev);
+ unmapped = iommu_unmap(domain, entry->da, entry->len);
+ if (unmapped != entry->len) {
+ /* nothing much to do besides complaining */
+ dev_err(dev, "failed to unmap %zx/%zu\n", entry->len,
+ unmapped);
+ }
+ memunmap(entry->va);
+
+ list_del(&entry->node);
+ kfree(entry);
+ }
+}
+
+static int mtk_apu_rproc_prepare(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ int ret;
+
+ ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
+ apu_rproc->clks);
+ if (ret)
+ dev_err(apu_rproc->dev, "Failed to enable clocks\n");
+
+ return ret;
+}
+
+static int mtk_apu_rproc_unprepare(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+
+ clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
+ apu_rproc->clks);
+
+ return 0;
+}
+
+static int mtk_apu_rproc_start(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ u32 core_ctrl;
+
+ /* Set reset vector of APU firmware boot address */
+ writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
+
+ /* Turn on the clocks and stall the APU */
+ core_ctrl = readl(apu_rproc->base + CORE_CTRL);
+ core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
+ CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
+ CORE_CTRL_PIF_GATED;
+ writel(core_ctrl, apu_rproc->base + CORE_CTRL);
+
+ /* Reset the APU: this requires 27 ns to be effective on any platform */
+ writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
+ apu_rproc->base + SW_RST);
+ ndelay(APU_RESET_DELAY);
+ writel(0, apu_rproc->base + SW_RST);
+
+ core_ctrl &= ~CORE_CTRL_PIF_GATED;
+ writel(core_ctrl, apu_rproc->base + CORE_CTRL);
+
+ /* Configure memory accesses to go through the IOMMU */
+ writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
+ CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
+ writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
+ CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
+ apu_rproc->base + CORE_DEFAULT1);
+
+ /* Release the APU */
+ core_ctrl &= ~CORE_CTRL_RUN_STALL;
+ writel(core_ctrl, apu_rproc->base + CORE_CTRL);
+
+ return 0;
+}
+
+static int mtk_apu_rproc_stop(struct rproc *rproc)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+ u32 core_ctrl;
+
+ core_ctrl = readl(apu_rproc->base + CORE_CTRL);
+ writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
+
+ mtk_apu_iommu_unmap_all(rproc);
+
+ return 0;
+}
+
+static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
+{
+ struct mtk_apu_rproc *apu_rproc = rproc->priv;
+
+ writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
+}
+
+static int mtk_apu_load(struct rproc *rproc, const struct firmware *fw)
+
+{
+ struct rproc_mem_entry *entry, *tmp;
+ int ret;
+
+ ret = rproc_elf_load_segments(rproc, fw);
+ if (ret)
+ return ret;
+
+ list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
+ ret = mtk_apu_iommu_map(rproc, entry);
+ if (ret)
+ goto err_unmap_all;
+ }
+
+ return 0;
+
+err_unmap_all:
+ mtk_apu_iommu_unmap_all(rproc);
+
+ return ret;
+}
+
+static struct reserved_mem *of_reserved_mem_by_name(struct rproc *rproc,
+ const char *name)
+{
+ struct device *dev = rproc->dev.parent;
+ struct device_node *np = dev->of_node;
+ struct device_node *target;
+ struct reserved_mem *rmem;
+ int idx;
+
+ idx = of_property_match_string(np, "memory-region-names", name);
+ if (idx < 0) {
+ dev_err(dev, "failed to find %s memory\n", name);
+ return NULL;
+ }
+
+ target = of_parse_phandle(np, "memory-region", idx);
+ if (!target)
+ return NULL;
+
+ rmem = of_reserved_mem_lookup(target);
+ if (!rmem)
+ dev_err(dev, "unable to acquire memory-region\n");
+ of_node_put(target);
+
+ return rmem;
+}
+
+static int mtk_apu_handle_rsc(struct rproc *rproc, u32 rsc_type, void *rsc,
+ int offset, int avail)
+{
+ struct device *dev = rproc->dev.parent;
+
+ if (rsc_type == RSC_VENDOR_CARVEOUT) {
+ struct fw_rsc_carveout *rsc_carveout = rsc;
+ struct rproc_mem_entry *mem;
+ struct reserved_mem *rmem;
+
+ rmem = of_reserved_mem_by_name(rproc, rsc_carveout->name);
+ if (!rmem)
+ return -ENOMEM;
+
+ if (rmem->size < rsc_carveout->len) {
+ dev_err(dev, "The reserved memory is too small\n");
+ return -ENOMEM;
+ }
+
+ mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base,
+ rsc_carveout->len, rsc_carveout->da,
+ NULL, NULL, rsc_carveout->name);
+ if (!mem)
+ return -ENOMEM;
+
+ mem->flags = rsc_carveout->flags;
+ rsc_carveout->pa = rmem->base;
+ rproc_add_carveout(rproc, mem);
+ }
+
+ return RSC_HANDLED;
+}
+
+static const struct rproc_ops mtk_apu_rproc_ops = {
+ .prepare = mtk_apu_rproc_prepare,
+ .unprepare = mtk_apu_rproc_unprepare,
+ .start = mtk_apu_rproc_start,
+ .stop = mtk_apu_rproc_stop,
+ .kick = mtk_apu_rproc_kick,
+ .load = mtk_apu_load,
+ .parse_fw = rproc_elf_load_rsc_table,
+ .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+ .sanity_check = rproc_elf_sanity_check,
+ .get_boot_addr = rproc_elf_get_boot_addr,
+ .handle_rsc = mtk_apu_handle_rsc,
+};
+
+static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
+{
+ struct rproc *rproc = data;
+ struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
+
+ writel(1, apu_rproc->base + CORE_XTENSA_INT);
+
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t handle_event(int irq, void *data)
+{
+ struct rproc *rproc = data;
+
+ rproc_vq_interrupt(rproc, 0);
+ rproc_vq_interrupt(rproc, 1);
+
+ return IRQ_HANDLED;
+}
+
+static int mtk_apu_rproc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mtk_apu_rproc *apu_rproc;
+ struct rproc *rproc;
+ struct resource *res;
+ int ret;
+
+ rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
+ sizeof(*apu_rproc));
+ if (!rproc)
+ return -ENOMEM;
+
+ rproc->recovery_disabled = true;
+ rproc->has_iommu = false;
+ rproc->auto_boot = false;
+
+ apu_rproc = rproc->priv;
+ apu_rproc->dev = dev;
+ INIT_LIST_HEAD(&apu_rproc->mappings);
+
+ platform_set_drvdata(pdev, rproc);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ apu_rproc->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(apu_rproc->base)) {
+ dev_err(dev, "Failed to map mmio\n");
+ ret = PTR_ERR(apu_rproc->base);
+ goto free_rproc;
+ }
+
+ apu_rproc->irq = platform_get_irq(pdev, 0);
+ if (apu_rproc->irq < 0) {
+ ret = apu_rproc->irq;
+ goto free_rproc;
+ }
+
+ ret = devm_request_threaded_irq(dev, apu_rproc->irq,
+ mtk_apu_rproc_callback, handle_event,
+ IRQF_SHARED | IRQF_ONESHOT,
+ NULL, rproc);
+ if (ret) {
+ dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
+ goto free_rproc;
+ }
+
+ apu_rproc->clks[0].id = "ipu";
+ apu_rproc->clks[1].id = "axi";
+ apu_rproc->clks[2].id = "jtag";
+
+ ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
+ apu_rproc->clks);
+ if (ret) {
+ dev_err(dev, "Failed to get clocks\n");
+ goto free_rproc;
+ }
+
+ ret = rproc_add(rproc);
+ if (ret) {
+ dev_err(dev, "rproc_add failed: %d\n", ret);
+ goto free_rproc;
+ }
+
+ return 0;
+
+free_rproc:
+ rproc_free(rproc);
+
+ return ret;
+}
+
+static int mtk_apu_rproc_remove(struct platform_device *pdev)
+{
+ struct rproc *rproc = platform_get_drvdata(pdev);
+ struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
+ struct device *dev = &pdev->dev;
+
+ disable_irq(apu_rproc->irq);
+
+ rproc_del(rproc);
+ of_reserved_mem_device_release(dev);
+ rproc_free(rproc);
+
+ return 0;
+}
+
+static const struct of_device_id mtk_apu_rproc_of_match[] = {
+ { .compatible = "mediatek,mt8183-apu", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
+
+static struct platform_driver mtk_apu_rproc_driver = {
+ .probe = mtk_apu_rproc_probe,
+ .remove = mtk_apu_rproc_remove,
+ .driver = {
+ .name = "mtk_apu-rproc",
+ .of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
+ },
+};
+module_platform_driver(mtk_apu_rproc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Alexandre Bailon");
+MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
--
2.31.1
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
2021-08-19 15:13 ` Alexandre Bailon
(?)
@ 2021-08-19 23:14 ` kernel test robot
-1 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2021-08-19 23:14 UTC (permalink / raw)
To: Alexandre Bailon, ohad, bjorn.andersson, mathieu.poirier, robh+dt
Cc: kbuild-all, matthias.bgg, linux-remoteproc, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4648 bytes --]
Hi Alexandre,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v5.14-rc6 next-20210819]
[cannot apply to remoteproc/for-next rpmsg/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Alexandre-Bailon/Add-support-of-mt8183-APU/20210819-231419
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c161c7d11a0fdf701085657c15f949f397ade5be
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alexandre-Bailon/Add-support-of-mt8183-APU/20210819-231419
git checkout c161c7d11a0fdf701085657c15f949f397ade5be
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/iommu.h:11,
from drivers/remoteproc/mtk_apu.c:10:
drivers/remoteproc/mtk_apu.c: In function 'mtk_apu_iommu_map':
>> drivers/remoteproc/mtk_apu.c:83:38: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
83 | dev_err(dev, "Unable to map memory region: %pa+%lx\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
drivers/remoteproc/mtk_apu.c:83:25: note: in expansion of macro 'dev_err'
83 | dev_err(dev, "Unable to map memory region: %pa+%lx\n",
| ^~~~~~~
drivers/remoteproc/mtk_apu.c:83:74: note: format string is defined here
83 | dev_err(dev, "Unable to map memory region: %pa+%lx\n",
| ~~^
| |
| long unsigned int
| %x
vim +83 drivers/remoteproc/mtk_apu.c
60
61 static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
62 {
63 struct mtk_apu_rproc *apu_rproc = rproc->priv;
64 struct device *dev = rproc->dev.parent;
65 struct rproc_mem_entry *mapping;
66 struct iommu_domain *domain;
67 int ret;
68 u64 pa;
69
70 mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
71 if (!mapping)
72 return -ENOMEM;
73
74 if (!entry->va)
75 pa = entry->dma;
76 else
77 pa = rproc_va_to_pa(entry->va);
78
79 if ((strcmp(entry->name, "vdev0vring0") == 0 ||
80 strcmp(entry->name, "vdev0vring1") == 0)) {
81 entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
82 if (IS_ERR_OR_NULL(entry->va)) {
> 83 dev_err(dev, "Unable to map memory region: %pa+%lx\n",
84 &entry->dma, entry->len);
85 ret = PTR_ERR(mapping->va);
86 goto free_mapping;
87 }
88 mapping->va = entry->va;
89 }
90
91 domain = iommu_get_domain_for_dev(dev);
92 ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
93 if (ret) {
94 dev_err(dev, "iommu_map failed: %d\n", ret);
95 goto err_memunmap;
96 }
97
98 mapping->da = entry->da;
99 mapping->len = entry->len;
100 list_add_tail(&mapping->node, &apu_rproc->mappings);
101
102 return 0;
103
104 err_memunmap:
105 memunmap(mapping->va);
106 free_mapping:
107 kfree(mapping);
108
109 return ret;
110 }
111
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 78470 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
@ 2021-08-19 23:14 ` kernel test robot
0 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2021-08-19 23:14 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4759 bytes --]
Hi Alexandre,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v5.14-rc6 next-20210819]
[cannot apply to remoteproc/for-next rpmsg/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Alexandre-Bailon/Add-support-of-mt8183-APU/20210819-231419
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c161c7d11a0fdf701085657c15f949f397ade5be
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alexandre-Bailon/Add-support-of-mt8183-APU/20210819-231419
git checkout c161c7d11a0fdf701085657c15f949f397ade5be
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/iommu.h:11,
from drivers/remoteproc/mtk_apu.c:10:
drivers/remoteproc/mtk_apu.c: In function 'mtk_apu_iommu_map':
>> drivers/remoteproc/mtk_apu.c:83:38: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
83 | dev_err(dev, "Unable to map memory region: %pa+%lx\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
drivers/remoteproc/mtk_apu.c:83:25: note: in expansion of macro 'dev_err'
83 | dev_err(dev, "Unable to map memory region: %pa+%lx\n",
| ^~~~~~~
drivers/remoteproc/mtk_apu.c:83:74: note: format string is defined here
83 | dev_err(dev, "Unable to map memory region: %pa+%lx\n",
| ~~^
| |
| long unsigned int
| %x
vim +83 drivers/remoteproc/mtk_apu.c
60
61 static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
62 {
63 struct mtk_apu_rproc *apu_rproc = rproc->priv;
64 struct device *dev = rproc->dev.parent;
65 struct rproc_mem_entry *mapping;
66 struct iommu_domain *domain;
67 int ret;
68 u64 pa;
69
70 mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
71 if (!mapping)
72 return -ENOMEM;
73
74 if (!entry->va)
75 pa = entry->dma;
76 else
77 pa = rproc_va_to_pa(entry->va);
78
79 if ((strcmp(entry->name, "vdev0vring0") == 0 ||
80 strcmp(entry->name, "vdev0vring1") == 0)) {
81 entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
82 if (IS_ERR_OR_NULL(entry->va)) {
> 83 dev_err(dev, "Unable to map memory region: %pa+%lx\n",
84 &entry->dma, entry->len);
85 ret = PTR_ERR(mapping->va);
86 goto free_mapping;
87 }
88 mapping->va = entry->va;
89 }
90
91 domain = iommu_get_domain_for_dev(dev);
92 ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
93 if (ret) {
94 dev_err(dev, "iommu_map failed: %d\n", ret);
95 goto err_memunmap;
96 }
97
98 mapping->da = entry->da;
99 mapping->len = entry->len;
100 list_add_tail(&mapping->node, &apu_rproc->mappings);
101
102 return 0;
103
104 err_memunmap:
105 memunmap(mapping->va);
106 free_mapping:
107 kfree(mapping);
108
109 return ret;
110 }
111
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 78470 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
@ 2021-08-19 23:14 ` kernel test robot
0 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2021-08-19 23:14 UTC (permalink / raw)
To: Alexandre Bailon, ohad, bjorn.andersson, mathieu.poirier, robh+dt
Cc: kbuild-all, matthias.bgg, linux-remoteproc, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4648 bytes --]
Hi Alexandre,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v5.14-rc6 next-20210819]
[cannot apply to remoteproc/for-next rpmsg/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Alexandre-Bailon/Add-support-of-mt8183-APU/20210819-231419
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c161c7d11a0fdf701085657c15f949f397ade5be
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alexandre-Bailon/Add-support-of-mt8183-APU/20210819-231419
git checkout c161c7d11a0fdf701085657c15f949f397ade5be
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/iommu.h:11,
from drivers/remoteproc/mtk_apu.c:10:
drivers/remoteproc/mtk_apu.c: In function 'mtk_apu_iommu_map':
>> drivers/remoteproc/mtk_apu.c:83:38: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
83 | dev_err(dev, "Unable to map memory region: %pa+%lx\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
drivers/remoteproc/mtk_apu.c:83:25: note: in expansion of macro 'dev_err'
83 | dev_err(dev, "Unable to map memory region: %pa+%lx\n",
| ^~~~~~~
drivers/remoteproc/mtk_apu.c:83:74: note: format string is defined here
83 | dev_err(dev, "Unable to map memory region: %pa+%lx\n",
| ~~^
| |
| long unsigned int
| %x
vim +83 drivers/remoteproc/mtk_apu.c
60
61 static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
62 {
63 struct mtk_apu_rproc *apu_rproc = rproc->priv;
64 struct device *dev = rproc->dev.parent;
65 struct rproc_mem_entry *mapping;
66 struct iommu_domain *domain;
67 int ret;
68 u64 pa;
69
70 mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
71 if (!mapping)
72 return -ENOMEM;
73
74 if (!entry->va)
75 pa = entry->dma;
76 else
77 pa = rproc_va_to_pa(entry->va);
78
79 if ((strcmp(entry->name, "vdev0vring0") == 0 ||
80 strcmp(entry->name, "vdev0vring1") == 0)) {
81 entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
82 if (IS_ERR_OR_NULL(entry->va)) {
> 83 dev_err(dev, "Unable to map memory region: %pa+%lx\n",
84 &entry->dma, entry->len);
85 ret = PTR_ERR(mapping->va);
86 goto free_mapping;
87 }
88 mapping->va = entry->va;
89 }
90
91 domain = iommu_get_domain_for_dev(dev);
92 ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
93 if (ret) {
94 dev_err(dev, "iommu_map failed: %d\n", ret);
95 goto err_memunmap;
96 }
97
98 mapping->da = entry->da;
99 mapping->len = entry->len;
100 list_add_tail(&mapping->node, &apu_rproc->mappings);
101
102 return 0;
103
104 err_memunmap:
105 memunmap(mapping->va);
106 free_mapping:
107 kfree(mapping);
108
109 return ret;
110 }
111
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 78470 bytes --]
[-- Attachment #3: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
2021-08-19 15:13 ` Alexandre Bailon
(?)
@ 2021-08-30 18:19 ` Mathieu Poirier
-1 siblings, 0 replies; 27+ messages in thread
From: Mathieu Poirier @ 2021-08-30 18:19 UTC (permalink / raw)
To: Alexandre Bailon
Cc: ohad, bjorn.andersson, robh+dt, matthias.bgg, linux-remoteproc,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
stephane.leprovost, gpain, khilman
Hi Alex,
On Thu, Aug 19, 2021 at 05:13:38PM +0200, Alexandre Bailon wrote:
> This adds a driver to control the APU present in the MT8183.
> This loads the firmware and start the DSP.
>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
> drivers/remoteproc/Kconfig | 10 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/mtk_apu.c | 440 +++++++++++++++++++++++++++++++++++
> 3 files changed, 451 insertions(+)
> create mode 100644 drivers/remoteproc/mtk_apu.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 9a6eedc3994a5..6c106a6c3ad5d 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -53,6 +53,16 @@ config MTK_SCP
>
> It's safe to say N here.
>
> +config MTK_APU
> + tristate "Mediatek APU remoteproc support"
> + depends on ARCH_MEDIATEK
> + depends on MTK_IOMMU
> + help
> + Say y to support the Mediatek's Accelerated Processing Unit (APU) via
In the bindings A in APU is for AI or Vision whereas above it is for
accelerated. Please pick one and stick with it.
> + the remote processor framework.
> +
> + It's safe to say N here.
> +
> config OMAP_REMOTEPROC
> tristate "OMAP remoteproc support"
> depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index bb26c9e4ef9cf..e395af86c17b0 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o
> obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
> obj-$(CONFIG_INGENIC_VPU_RPROC) += ingenic_rproc.o
> obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
> +obj-$(CONFIG_MTK_APU) += mtk_apu.o
> obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
> obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
> diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
> new file mode 100644
> index 0000000000000..0e3f63987bd85
> --- /dev/null
> +++ b/drivers/remoteproc/mtk_apu.c
> @@ -0,0 +1,440 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 BayLibre SAS
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/iommu.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/remoteproc.h>
> +
> +#include "remoteproc_internal.h"
> +
> +#define SW_RST (0x0000000C)
> +#define SW_RST_OCD_HALT_ON_RST BIT(12)
> +#define SW_RST_IPU_D_RST BIT(8)
> +#define SW_RST_IPU_B_RST BIT(4)
> +#define CORE_CTRL (0x00000110)
> +#define CORE_CTRL_PDEBUG_ENABLE BIT(31)
> +#define CORE_CTRL_SRAM_64K_iMEM (0x00 << 27)
> +#define CORE_CTRL_SRAM_96K_iMEM (0x01 << 27)
> +#define CORE_CTRL_SRAM_128K_iMEM (0x02 << 27)
> +#define CORE_CTRL_SRAM_192K_iMEM (0x03 << 27)
> +#define CORE_CTRL_SRAM_256K_iMEM (0x04 << 27)
> +#define CORE_CTRL_PBCLK_ENABLE BIT(26)
> +#define CORE_CTRL_RUN_STALL BIT(23)
> +#define CORE_CTRL_STATE_VECTOR_SELECT BIT(19)
> +#define CORE_CTRL_PIF_GATED BIT(17)
> +#define CORE_CTRL_NMI BIT(0)
> +#define CORE_XTENSA_INT (0x00000114)
> +#define CORE_CTL_XTENSA_INT (0x00000118)
> +#define CORE_DEFAULT0 (0x0000013C)
> +#define CORE_DEFAULT0_QOS_SWAP_0 (0x00 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_1 (0x01 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_2 (0x02 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_3 (0x03 << 28)
> +#define CORE_DEFAULT0_ARUSER_USE_IOMMU (0x10 << 23)
> +#define CORE_DEFAULT0_AWUSER_USE_IOMMU (0x10 << 18)
> +#define CORE_DEFAULT1 (0x00000140)
> +#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU (0x10 << 0)
> +#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU (0x10 << 5)
> +#define CORE_XTENSA_ALTRESETVEC (0x000001F8)
> +
> +#define RSC_VENDOR_CARVEOUT (RSC_VENDOR_START + 1)
> +
> +#define APU_RESET_DELAY (27)
> +
> +struct mtk_apu_rproc {
> + struct device *dev;
> + void __iomem *base;
> + int irq;
> + struct clk_bulk_data clks[3];
> + struct list_head mappings;
> +};
> +
> +static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + struct device *dev = rproc->dev.parent;
> + struct rproc_mem_entry *mapping;
> + struct iommu_domain *domain;
> + int ret;
> + u64 pa;
> +
> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> + if (!mapping)
> + return -ENOMEM;
> +
> + if (!entry->va)
> + pa = entry->dma;
> + else
> + pa = rproc_va_to_pa(entry->va);
> +
> + if ((strcmp(entry->name, "vdev0vring0") == 0 ||
> + strcmp(entry->name, "vdev0vring1") == 0)) {
If my guesswork is correct (see below) the above is required to deal with
multiple remote processors. Why not just look for "vring0" and "vring1"?. That
way we wouldn't have to keep the "vdev0" part for carveouts that really are
"vdev1".
> + entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
> + if (IS_ERR_OR_NULL(entry->va)) {
> + dev_err(dev, "Unable to map memory region: %pa+%lx\n",
> + &entry->dma, entry->len);
> + ret = PTR_ERR(mapping->va);
> + goto free_mapping;
> + }
> + mapping->va = entry->va;
> + }
> +
> + domain = iommu_get_domain_for_dev(dev);
Here @domain can be NULL
> + ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
... and iommu_map() isn't dealing with that condition properly. Please
check for a NULL condition before proceeding.
> + if (ret) {
> + dev_err(dev, "iommu_map failed: %d\n", ret);
> + goto err_memunmap;
> + }
> +
> + mapping->da = entry->da;
> + mapping->len = entry->len;
> + list_add_tail(&mapping->node, &apu_rproc->mappings);
> +
> + return 0;
> +
> +err_memunmap:
> + memunmap(mapping->va);
> +free_mapping:
> + kfree(mapping);
> +
> + return ret;
> +}
> +
> +static void mtk_apu_iommu_unmap_all(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + struct device *dev = rproc->dev.parent;
> + struct rproc_mem_entry *entry, *tmp;
> + struct iommu_domain *domain;
> +
> + /* clean up iommu mapping entries */
> + list_for_each_entry_safe(entry, tmp, &apu_rproc->mappings, node) {
> + size_t unmapped;
> +
> + domain = iommu_get_domain_for_dev(dev);
> + unmapped = iommu_unmap(domain, entry->da, entry->len);
Same as above
> + if (unmapped != entry->len) {
> + /* nothing much to do besides complaining */
> + dev_err(dev, "failed to unmap %zx/%zu\n", entry->len,
> + unmapped);
> + }
> + memunmap(entry->va);
> +
> + list_del(&entry->node);
> + kfree(entry);
> + }
> +}
> +
> +static int mtk_apu_rproc_prepare(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + int ret;
> +
> + ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
> + apu_rproc->clks);
> + if (ret)
> + dev_err(apu_rproc->dev, "Failed to enable clocks\n");
> +
> + return ret;
> +}
> +
> +static int mtk_apu_rproc_unprepare(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +
> + clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
> + apu_rproc->clks);
> +
> + return 0;
> +}
> +
> +static int mtk_apu_rproc_start(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + u32 core_ctrl;
> +
> + /* Set reset vector of APU firmware boot address */
> + writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
> +
> + /* Turn on the clocks and stall the APU */
> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
> + core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
> + CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
> + CORE_CTRL_PIF_GATED;
> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> + /* Reset the APU: this requires 27 ns to be effective on any platform */
> + writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
> + apu_rproc->base + SW_RST);
> + ndelay(APU_RESET_DELAY);
> + writel(0, apu_rproc->base + SW_RST);
> +
> + core_ctrl &= ~CORE_CTRL_PIF_GATED;
> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> + /* Configure memory accesses to go through the IOMMU */
> + writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
> + CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
> + writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
> + CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
> + apu_rproc->base + CORE_DEFAULT1);
> +
> + /* Release the APU */
> + core_ctrl &= ~CORE_CTRL_RUN_STALL;
> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> + return 0;
> +}
> +
> +static int mtk_apu_rproc_stop(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + u32 core_ctrl;
> +
> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
> + writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
> +
> + mtk_apu_iommu_unmap_all(rproc);
> +
> + return 0;
> +}
> +
> +static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +
> + writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
> +}
> +
> +static int mtk_apu_load(struct rproc *rproc, const struct firmware *fw)
> +
> +{
> + struct rproc_mem_entry *entry, *tmp;
> + int ret;
> +
> + ret = rproc_elf_load_segments(rproc, fw);
> + if (ret)
> + return ret;
> +
> + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
> + ret = mtk_apu_iommu_map(rproc, entry);
> + if (ret)
> + goto err_unmap_all;
> + }
> +
> + return 0;
> +
> +err_unmap_all:
> + mtk_apu_iommu_unmap_all(rproc);
> +
> + return ret;
> +}
> +
> +static struct reserved_mem *of_reserved_mem_by_name(struct rproc *rproc,
> + const char *name)
> +{
> + struct device *dev = rproc->dev.parent;
> + struct device_node *np = dev->of_node;
> + struct device_node *target;
> + struct reserved_mem *rmem;
> + int idx;
> +
> + idx = of_property_match_string(np, "memory-region-names", name);
> + if (idx < 0) {
> + dev_err(dev, "failed to find %s memory\n", name);
> + return NULL;
> + }
> +
> + target = of_parse_phandle(np, "memory-region", idx);
> + if (!target)
> + return NULL;
There is no comment to explain the above gymnastic and as such I have to guess
the firmware resources always have "vdev0" prepended to buffer, vring0 and
vring1. If I am correct please add a comment to describe the situation, ditto
if I am not correct.
> +
> + rmem = of_reserved_mem_lookup(target);
> + if (!rmem)
> + dev_err(dev, "unable to acquire memory-region\n");
> + of_node_put(target);
> +
> + return rmem;
> +}
> +
> +static int mtk_apu_handle_rsc(struct rproc *rproc, u32 rsc_type, void *rsc,
> + int offset, int avail)
> +{
> + struct device *dev = rproc->dev.parent;
> +
> + if (rsc_type == RSC_VENDOR_CARVEOUT) {
> + struct fw_rsc_carveout *rsc_carveout = rsc;
> + struct rproc_mem_entry *mem;
> + struct reserved_mem *rmem;
> +
> + rmem = of_reserved_mem_by_name(rproc, rsc_carveout->name);
s/of_reserved_mem_by_name/mtk_of_reserved_mem_by_name
> + if (!rmem)
> + return -ENOMEM;
> +
> + if (rmem->size < rsc_carveout->len) {
I think '!=' would be more appropriate than '<'.
> + dev_err(dev, "The reserved memory is too small\n");
> + return -ENOMEM;
> + }
> +
> + mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base,
> + rsc_carveout->len, rsc_carveout->da,
> + NULL, NULL, rsc_carveout->name);
Why not naming the carveout the way it is in the DT rather than keeping with
what's in the firmware image?
> + if (!mem)
> + return -ENOMEM;
> +
> + mem->flags = rsc_carveout->flags;
> + rsc_carveout->pa = rmem->base;
> + rproc_add_carveout(rproc, mem);
> + }
> +
> + return RSC_HANDLED;
> +}
> +
> +static const struct rproc_ops mtk_apu_rproc_ops = {
> + .prepare = mtk_apu_rproc_prepare,
> + .unprepare = mtk_apu_rproc_unprepare,
> + .start = mtk_apu_rproc_start,
> + .stop = mtk_apu_rproc_stop,
> + .kick = mtk_apu_rproc_kick,
> + .load = mtk_apu_load,
> + .parse_fw = rproc_elf_load_rsc_table,
> + .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> + .sanity_check = rproc_elf_sanity_check,
> + .get_boot_addr = rproc_elf_get_boot_addr,
> + .handle_rsc = mtk_apu_handle_rsc,
> +};
> +
> +static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
> +{
> + struct rproc *rproc = data;
> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> +
> + writel(1, apu_rproc->base + CORE_XTENSA_INT);
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t handle_event(int irq, void *data)
> +{
> + struct rproc *rproc = data;
> +
> + rproc_vq_interrupt(rproc, 0);
> + rproc_vq_interrupt(rproc, 1);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int mtk_apu_rproc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mtk_apu_rproc *apu_rproc;
> + struct rproc *rproc;
> + struct resource *res;
> + int ret;
> +
> + rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
> + sizeof(*apu_rproc));
> + if (!rproc)
> + return -ENOMEM;
> +
> + rproc->recovery_disabled = true;
> + rproc->has_iommu = false;
> + rproc->auto_boot = false;
> +
> + apu_rproc = rproc->priv;
> + apu_rproc->dev = dev;
> + INIT_LIST_HEAD(&apu_rproc->mappings);
> +
> + platform_set_drvdata(pdev, rproc);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + apu_rproc->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(apu_rproc->base)) {
> + dev_err(dev, "Failed to map mmio\n");
> + ret = PTR_ERR(apu_rproc->base);
> + goto free_rproc;
> + }
> +
> + apu_rproc->irq = platform_get_irq(pdev, 0);
> + if (apu_rproc->irq < 0) {
> + ret = apu_rproc->irq;
> + goto free_rproc;
> + }
> +
> + ret = devm_request_threaded_irq(dev, apu_rproc->irq,
> + mtk_apu_rproc_callback, handle_event,
> + IRQF_SHARED | IRQF_ONESHOT,
> + NULL, rproc);
I'm pretty sure I commented on that during my last review - is there a need to
do a threaded irq here? As far as I can tell there is no advantage in doing so
in this context. Please add a comment to justify using a threaded irq.
Thanks,
Mathieu
> + if (ret) {
> + dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
> + goto free_rproc;
> + }
> +
> + apu_rproc->clks[0].id = "ipu";
> + apu_rproc->clks[1].id = "axi";
> + apu_rproc->clks[2].id = "jtag";
> +
> + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
> + apu_rproc->clks);
> + if (ret) {
> + dev_err(dev, "Failed to get clocks\n");
> + goto free_rproc;
> + }
> +
> + ret = rproc_add(rproc);
> + if (ret) {
> + dev_err(dev, "rproc_add failed: %d\n", ret);
> + goto free_rproc;
> + }
> +
> + return 0;
> +
> +free_rproc:
> + rproc_free(rproc);
> +
> + return ret;
> +}
> +
> +static int mtk_apu_rproc_remove(struct platform_device *pdev)
> +{
> + struct rproc *rproc = platform_get_drvdata(pdev);
> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> + struct device *dev = &pdev->dev;
> +
> + disable_irq(apu_rproc->irq);
> +
> + rproc_del(rproc);
> + of_reserved_mem_device_release(dev);
> + rproc_free(rproc);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mtk_apu_rproc_of_match[] = {
> + { .compatible = "mediatek,mt8183-apu", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
> +
> +static struct platform_driver mtk_apu_rproc_driver = {
> + .probe = mtk_apu_rproc_probe,
> + .remove = mtk_apu_rproc_remove,
> + .driver = {
> + .name = "mtk_apu-rproc",
> + .of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
> + },
> +};
> +module_platform_driver(mtk_apu_rproc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Alexandre Bailon");
> +MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
> --
> 2.31.1
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
@ 2021-08-30 18:19 ` Mathieu Poirier
0 siblings, 0 replies; 27+ messages in thread
From: Mathieu Poirier @ 2021-08-30 18:19 UTC (permalink / raw)
To: Alexandre Bailon
Cc: ohad, bjorn.andersson, robh+dt, matthias.bgg, linux-remoteproc,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
stephane.leprovost, gpain, khilman
Hi Alex,
On Thu, Aug 19, 2021 at 05:13:38PM +0200, Alexandre Bailon wrote:
> This adds a driver to control the APU present in the MT8183.
> This loads the firmware and start the DSP.
>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
> drivers/remoteproc/Kconfig | 10 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/mtk_apu.c | 440 +++++++++++++++++++++++++++++++++++
> 3 files changed, 451 insertions(+)
> create mode 100644 drivers/remoteproc/mtk_apu.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 9a6eedc3994a5..6c106a6c3ad5d 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -53,6 +53,16 @@ config MTK_SCP
>
> It's safe to say N here.
>
> +config MTK_APU
> + tristate "Mediatek APU remoteproc support"
> + depends on ARCH_MEDIATEK
> + depends on MTK_IOMMU
> + help
> + Say y to support the Mediatek's Accelerated Processing Unit (APU) via
In the bindings A in APU is for AI or Vision whereas above it is for
accelerated. Please pick one and stick with it.
> + the remote processor framework.
> +
> + It's safe to say N here.
> +
> config OMAP_REMOTEPROC
> tristate "OMAP remoteproc support"
> depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index bb26c9e4ef9cf..e395af86c17b0 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o
> obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
> obj-$(CONFIG_INGENIC_VPU_RPROC) += ingenic_rproc.o
> obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
> +obj-$(CONFIG_MTK_APU) += mtk_apu.o
> obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
> obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
> diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
> new file mode 100644
> index 0000000000000..0e3f63987bd85
> --- /dev/null
> +++ b/drivers/remoteproc/mtk_apu.c
> @@ -0,0 +1,440 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 BayLibre SAS
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/iommu.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/remoteproc.h>
> +
> +#include "remoteproc_internal.h"
> +
> +#define SW_RST (0x0000000C)
> +#define SW_RST_OCD_HALT_ON_RST BIT(12)
> +#define SW_RST_IPU_D_RST BIT(8)
> +#define SW_RST_IPU_B_RST BIT(4)
> +#define CORE_CTRL (0x00000110)
> +#define CORE_CTRL_PDEBUG_ENABLE BIT(31)
> +#define CORE_CTRL_SRAM_64K_iMEM (0x00 << 27)
> +#define CORE_CTRL_SRAM_96K_iMEM (0x01 << 27)
> +#define CORE_CTRL_SRAM_128K_iMEM (0x02 << 27)
> +#define CORE_CTRL_SRAM_192K_iMEM (0x03 << 27)
> +#define CORE_CTRL_SRAM_256K_iMEM (0x04 << 27)
> +#define CORE_CTRL_PBCLK_ENABLE BIT(26)
> +#define CORE_CTRL_RUN_STALL BIT(23)
> +#define CORE_CTRL_STATE_VECTOR_SELECT BIT(19)
> +#define CORE_CTRL_PIF_GATED BIT(17)
> +#define CORE_CTRL_NMI BIT(0)
> +#define CORE_XTENSA_INT (0x00000114)
> +#define CORE_CTL_XTENSA_INT (0x00000118)
> +#define CORE_DEFAULT0 (0x0000013C)
> +#define CORE_DEFAULT0_QOS_SWAP_0 (0x00 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_1 (0x01 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_2 (0x02 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_3 (0x03 << 28)
> +#define CORE_DEFAULT0_ARUSER_USE_IOMMU (0x10 << 23)
> +#define CORE_DEFAULT0_AWUSER_USE_IOMMU (0x10 << 18)
> +#define CORE_DEFAULT1 (0x00000140)
> +#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU (0x10 << 0)
> +#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU (0x10 << 5)
> +#define CORE_XTENSA_ALTRESETVEC (0x000001F8)
> +
> +#define RSC_VENDOR_CARVEOUT (RSC_VENDOR_START + 1)
> +
> +#define APU_RESET_DELAY (27)
> +
> +struct mtk_apu_rproc {
> + struct device *dev;
> + void __iomem *base;
> + int irq;
> + struct clk_bulk_data clks[3];
> + struct list_head mappings;
> +};
> +
> +static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + struct device *dev = rproc->dev.parent;
> + struct rproc_mem_entry *mapping;
> + struct iommu_domain *domain;
> + int ret;
> + u64 pa;
> +
> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> + if (!mapping)
> + return -ENOMEM;
> +
> + if (!entry->va)
> + pa = entry->dma;
> + else
> + pa = rproc_va_to_pa(entry->va);
> +
> + if ((strcmp(entry->name, "vdev0vring0") == 0 ||
> + strcmp(entry->name, "vdev0vring1") == 0)) {
If my guesswork is correct (see below) the above is required to deal with
multiple remote processors. Why not just look for "vring0" and "vring1"?. That
way we wouldn't have to keep the "vdev0" part for carveouts that really are
"vdev1".
> + entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
> + if (IS_ERR_OR_NULL(entry->va)) {
> + dev_err(dev, "Unable to map memory region: %pa+%lx\n",
> + &entry->dma, entry->len);
> + ret = PTR_ERR(mapping->va);
> + goto free_mapping;
> + }
> + mapping->va = entry->va;
> + }
> +
> + domain = iommu_get_domain_for_dev(dev);
Here @domain can be NULL
> + ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
... and iommu_map() isn't dealing with that condition properly. Please
check for a NULL condition before proceeding.
> + if (ret) {
> + dev_err(dev, "iommu_map failed: %d\n", ret);
> + goto err_memunmap;
> + }
> +
> + mapping->da = entry->da;
> + mapping->len = entry->len;
> + list_add_tail(&mapping->node, &apu_rproc->mappings);
> +
> + return 0;
> +
> +err_memunmap:
> + memunmap(mapping->va);
> +free_mapping:
> + kfree(mapping);
> +
> + return ret;
> +}
> +
> +static void mtk_apu_iommu_unmap_all(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + struct device *dev = rproc->dev.parent;
> + struct rproc_mem_entry *entry, *tmp;
> + struct iommu_domain *domain;
> +
> + /* clean up iommu mapping entries */
> + list_for_each_entry_safe(entry, tmp, &apu_rproc->mappings, node) {
> + size_t unmapped;
> +
> + domain = iommu_get_domain_for_dev(dev);
> + unmapped = iommu_unmap(domain, entry->da, entry->len);
Same as above
> + if (unmapped != entry->len) {
> + /* nothing much to do besides complaining */
> + dev_err(dev, "failed to unmap %zx/%zu\n", entry->len,
> + unmapped);
> + }
> + memunmap(entry->va);
> +
> + list_del(&entry->node);
> + kfree(entry);
> + }
> +}
> +
> +static int mtk_apu_rproc_prepare(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + int ret;
> +
> + ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
> + apu_rproc->clks);
> + if (ret)
> + dev_err(apu_rproc->dev, "Failed to enable clocks\n");
> +
> + return ret;
> +}
> +
> +static int mtk_apu_rproc_unprepare(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +
> + clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
> + apu_rproc->clks);
> +
> + return 0;
> +}
> +
> +static int mtk_apu_rproc_start(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + u32 core_ctrl;
> +
> + /* Set reset vector of APU firmware boot address */
> + writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
> +
> + /* Turn on the clocks and stall the APU */
> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
> + core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
> + CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
> + CORE_CTRL_PIF_GATED;
> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> + /* Reset the APU: this requires 27 ns to be effective on any platform */
> + writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
> + apu_rproc->base + SW_RST);
> + ndelay(APU_RESET_DELAY);
> + writel(0, apu_rproc->base + SW_RST);
> +
> + core_ctrl &= ~CORE_CTRL_PIF_GATED;
> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> + /* Configure memory accesses to go through the IOMMU */
> + writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
> + CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
> + writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
> + CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
> + apu_rproc->base + CORE_DEFAULT1);
> +
> + /* Release the APU */
> + core_ctrl &= ~CORE_CTRL_RUN_STALL;
> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> + return 0;
> +}
> +
> +static int mtk_apu_rproc_stop(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + u32 core_ctrl;
> +
> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
> + writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
> +
> + mtk_apu_iommu_unmap_all(rproc);
> +
> + return 0;
> +}
> +
> +static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +
> + writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
> +}
> +
> +static int mtk_apu_load(struct rproc *rproc, const struct firmware *fw)
> +
> +{
> + struct rproc_mem_entry *entry, *tmp;
> + int ret;
> +
> + ret = rproc_elf_load_segments(rproc, fw);
> + if (ret)
> + return ret;
> +
> + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
> + ret = mtk_apu_iommu_map(rproc, entry);
> + if (ret)
> + goto err_unmap_all;
> + }
> +
> + return 0;
> +
> +err_unmap_all:
> + mtk_apu_iommu_unmap_all(rproc);
> +
> + return ret;
> +}
> +
> +static struct reserved_mem *of_reserved_mem_by_name(struct rproc *rproc,
> + const char *name)
> +{
> + struct device *dev = rproc->dev.parent;
> + struct device_node *np = dev->of_node;
> + struct device_node *target;
> + struct reserved_mem *rmem;
> + int idx;
> +
> + idx = of_property_match_string(np, "memory-region-names", name);
> + if (idx < 0) {
> + dev_err(dev, "failed to find %s memory\n", name);
> + return NULL;
> + }
> +
> + target = of_parse_phandle(np, "memory-region", idx);
> + if (!target)
> + return NULL;
There is no comment to explain the above gymnastic and as such I have to guess
the firmware resources always have "vdev0" prepended to buffer, vring0 and
vring1. If I am correct please add a comment to describe the situation, ditto
if I am not correct.
> +
> + rmem = of_reserved_mem_lookup(target);
> + if (!rmem)
> + dev_err(dev, "unable to acquire memory-region\n");
> + of_node_put(target);
> +
> + return rmem;
> +}
> +
> +static int mtk_apu_handle_rsc(struct rproc *rproc, u32 rsc_type, void *rsc,
> + int offset, int avail)
> +{
> + struct device *dev = rproc->dev.parent;
> +
> + if (rsc_type == RSC_VENDOR_CARVEOUT) {
> + struct fw_rsc_carveout *rsc_carveout = rsc;
> + struct rproc_mem_entry *mem;
> + struct reserved_mem *rmem;
> +
> + rmem = of_reserved_mem_by_name(rproc, rsc_carveout->name);
s/of_reserved_mem_by_name/mtk_of_reserved_mem_by_name
> + if (!rmem)
> + return -ENOMEM;
> +
> + if (rmem->size < rsc_carveout->len) {
I think '!=' would be more appropriate than '<'.
> + dev_err(dev, "The reserved memory is too small\n");
> + return -ENOMEM;
> + }
> +
> + mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base,
> + rsc_carveout->len, rsc_carveout->da,
> + NULL, NULL, rsc_carveout->name);
Why not naming the carveout the way it is in the DT rather than keeping with
what's in the firmware image?
> + if (!mem)
> + return -ENOMEM;
> +
> + mem->flags = rsc_carveout->flags;
> + rsc_carveout->pa = rmem->base;
> + rproc_add_carveout(rproc, mem);
> + }
> +
> + return RSC_HANDLED;
> +}
> +
> +static const struct rproc_ops mtk_apu_rproc_ops = {
> + .prepare = mtk_apu_rproc_prepare,
> + .unprepare = mtk_apu_rproc_unprepare,
> + .start = mtk_apu_rproc_start,
> + .stop = mtk_apu_rproc_stop,
> + .kick = mtk_apu_rproc_kick,
> + .load = mtk_apu_load,
> + .parse_fw = rproc_elf_load_rsc_table,
> + .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> + .sanity_check = rproc_elf_sanity_check,
> + .get_boot_addr = rproc_elf_get_boot_addr,
> + .handle_rsc = mtk_apu_handle_rsc,
> +};
> +
> +static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
> +{
> + struct rproc *rproc = data;
> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> +
> + writel(1, apu_rproc->base + CORE_XTENSA_INT);
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t handle_event(int irq, void *data)
> +{
> + struct rproc *rproc = data;
> +
> + rproc_vq_interrupt(rproc, 0);
> + rproc_vq_interrupt(rproc, 1);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int mtk_apu_rproc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mtk_apu_rproc *apu_rproc;
> + struct rproc *rproc;
> + struct resource *res;
> + int ret;
> +
> + rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
> + sizeof(*apu_rproc));
> + if (!rproc)
> + return -ENOMEM;
> +
> + rproc->recovery_disabled = true;
> + rproc->has_iommu = false;
> + rproc->auto_boot = false;
> +
> + apu_rproc = rproc->priv;
> + apu_rproc->dev = dev;
> + INIT_LIST_HEAD(&apu_rproc->mappings);
> +
> + platform_set_drvdata(pdev, rproc);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + apu_rproc->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(apu_rproc->base)) {
> + dev_err(dev, "Failed to map mmio\n");
> + ret = PTR_ERR(apu_rproc->base);
> + goto free_rproc;
> + }
> +
> + apu_rproc->irq = platform_get_irq(pdev, 0);
> + if (apu_rproc->irq < 0) {
> + ret = apu_rproc->irq;
> + goto free_rproc;
> + }
> +
> + ret = devm_request_threaded_irq(dev, apu_rproc->irq,
> + mtk_apu_rproc_callback, handle_event,
> + IRQF_SHARED | IRQF_ONESHOT,
> + NULL, rproc);
I'm pretty sure I commented on that during my last review - is there a need to
do a threaded irq here? As far as I can tell there is no advantage in doing so
in this context. Please add a comment to justify using a threaded irq.
Thanks,
Mathieu
> + if (ret) {
> + dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
> + goto free_rproc;
> + }
> +
> + apu_rproc->clks[0].id = "ipu";
> + apu_rproc->clks[1].id = "axi";
> + apu_rproc->clks[2].id = "jtag";
> +
> + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
> + apu_rproc->clks);
> + if (ret) {
> + dev_err(dev, "Failed to get clocks\n");
> + goto free_rproc;
> + }
> +
> + ret = rproc_add(rproc);
> + if (ret) {
> + dev_err(dev, "rproc_add failed: %d\n", ret);
> + goto free_rproc;
> + }
> +
> + return 0;
> +
> +free_rproc:
> + rproc_free(rproc);
> +
> + return ret;
> +}
> +
> +static int mtk_apu_rproc_remove(struct platform_device *pdev)
> +{
> + struct rproc *rproc = platform_get_drvdata(pdev);
> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> + struct device *dev = &pdev->dev;
> +
> + disable_irq(apu_rproc->irq);
> +
> + rproc_del(rproc);
> + of_reserved_mem_device_release(dev);
> + rproc_free(rproc);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mtk_apu_rproc_of_match[] = {
> + { .compatible = "mediatek,mt8183-apu", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
> +
> +static struct platform_driver mtk_apu_rproc_driver = {
> + .probe = mtk_apu_rproc_probe,
> + .remove = mtk_apu_rproc_remove,
> + .driver = {
> + .name = "mtk_apu-rproc",
> + .of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
> + },
> +};
> +module_platform_driver(mtk_apu_rproc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Alexandre Bailon");
> +MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
> --
> 2.31.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
@ 2021-08-30 18:19 ` Mathieu Poirier
0 siblings, 0 replies; 27+ messages in thread
From: Mathieu Poirier @ 2021-08-30 18:19 UTC (permalink / raw)
To: Alexandre Bailon
Cc: ohad, bjorn.andersson, robh+dt, matthias.bgg, linux-remoteproc,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
stephane.leprovost, gpain, khilman
Hi Alex,
On Thu, Aug 19, 2021 at 05:13:38PM +0200, Alexandre Bailon wrote:
> This adds a driver to control the APU present in the MT8183.
> This loads the firmware and start the DSP.
>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
> drivers/remoteproc/Kconfig | 10 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/mtk_apu.c | 440 +++++++++++++++++++++++++++++++++++
> 3 files changed, 451 insertions(+)
> create mode 100644 drivers/remoteproc/mtk_apu.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 9a6eedc3994a5..6c106a6c3ad5d 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -53,6 +53,16 @@ config MTK_SCP
>
> It's safe to say N here.
>
> +config MTK_APU
> + tristate "Mediatek APU remoteproc support"
> + depends on ARCH_MEDIATEK
> + depends on MTK_IOMMU
> + help
> + Say y to support the Mediatek's Accelerated Processing Unit (APU) via
In the bindings A in APU is for AI or Vision whereas above it is for
accelerated. Please pick one and stick with it.
> + the remote processor framework.
> +
> + It's safe to say N here.
> +
> config OMAP_REMOTEPROC
> tristate "OMAP remoteproc support"
> depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index bb26c9e4ef9cf..e395af86c17b0 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o
> obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
> obj-$(CONFIG_INGENIC_VPU_RPROC) += ingenic_rproc.o
> obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
> +obj-$(CONFIG_MTK_APU) += mtk_apu.o
> obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
> obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
> diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
> new file mode 100644
> index 0000000000000..0e3f63987bd85
> --- /dev/null
> +++ b/drivers/remoteproc/mtk_apu.c
> @@ -0,0 +1,440 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 BayLibre SAS
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/iommu.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/remoteproc.h>
> +
> +#include "remoteproc_internal.h"
> +
> +#define SW_RST (0x0000000C)
> +#define SW_RST_OCD_HALT_ON_RST BIT(12)
> +#define SW_RST_IPU_D_RST BIT(8)
> +#define SW_RST_IPU_B_RST BIT(4)
> +#define CORE_CTRL (0x00000110)
> +#define CORE_CTRL_PDEBUG_ENABLE BIT(31)
> +#define CORE_CTRL_SRAM_64K_iMEM (0x00 << 27)
> +#define CORE_CTRL_SRAM_96K_iMEM (0x01 << 27)
> +#define CORE_CTRL_SRAM_128K_iMEM (0x02 << 27)
> +#define CORE_CTRL_SRAM_192K_iMEM (0x03 << 27)
> +#define CORE_CTRL_SRAM_256K_iMEM (0x04 << 27)
> +#define CORE_CTRL_PBCLK_ENABLE BIT(26)
> +#define CORE_CTRL_RUN_STALL BIT(23)
> +#define CORE_CTRL_STATE_VECTOR_SELECT BIT(19)
> +#define CORE_CTRL_PIF_GATED BIT(17)
> +#define CORE_CTRL_NMI BIT(0)
> +#define CORE_XTENSA_INT (0x00000114)
> +#define CORE_CTL_XTENSA_INT (0x00000118)
> +#define CORE_DEFAULT0 (0x0000013C)
> +#define CORE_DEFAULT0_QOS_SWAP_0 (0x00 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_1 (0x01 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_2 (0x02 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_3 (0x03 << 28)
> +#define CORE_DEFAULT0_ARUSER_USE_IOMMU (0x10 << 23)
> +#define CORE_DEFAULT0_AWUSER_USE_IOMMU (0x10 << 18)
> +#define CORE_DEFAULT1 (0x00000140)
> +#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU (0x10 << 0)
> +#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU (0x10 << 5)
> +#define CORE_XTENSA_ALTRESETVEC (0x000001F8)
> +
> +#define RSC_VENDOR_CARVEOUT (RSC_VENDOR_START + 1)
> +
> +#define APU_RESET_DELAY (27)
> +
> +struct mtk_apu_rproc {
> + struct device *dev;
> + void __iomem *base;
> + int irq;
> + struct clk_bulk_data clks[3];
> + struct list_head mappings;
> +};
> +
> +static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + struct device *dev = rproc->dev.parent;
> + struct rproc_mem_entry *mapping;
> + struct iommu_domain *domain;
> + int ret;
> + u64 pa;
> +
> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> + if (!mapping)
> + return -ENOMEM;
> +
> + if (!entry->va)
> + pa = entry->dma;
> + else
> + pa = rproc_va_to_pa(entry->va);
> +
> + if ((strcmp(entry->name, "vdev0vring0") == 0 ||
> + strcmp(entry->name, "vdev0vring1") == 0)) {
If my guesswork is correct (see below) the above is required to deal with
multiple remote processors. Why not just look for "vring0" and "vring1"?. That
way we wouldn't have to keep the "vdev0" part for carveouts that really are
"vdev1".
> + entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
> + if (IS_ERR_OR_NULL(entry->va)) {
> + dev_err(dev, "Unable to map memory region: %pa+%lx\n",
> + &entry->dma, entry->len);
> + ret = PTR_ERR(mapping->va);
> + goto free_mapping;
> + }
> + mapping->va = entry->va;
> + }
> +
> + domain = iommu_get_domain_for_dev(dev);
Here @domain can be NULL
> + ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
... and iommu_map() isn't dealing with that condition properly. Please
check for a NULL condition before proceeding.
> + if (ret) {
> + dev_err(dev, "iommu_map failed: %d\n", ret);
> + goto err_memunmap;
> + }
> +
> + mapping->da = entry->da;
> + mapping->len = entry->len;
> + list_add_tail(&mapping->node, &apu_rproc->mappings);
> +
> + return 0;
> +
> +err_memunmap:
> + memunmap(mapping->va);
> +free_mapping:
> + kfree(mapping);
> +
> + return ret;
> +}
> +
> +static void mtk_apu_iommu_unmap_all(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + struct device *dev = rproc->dev.parent;
> + struct rproc_mem_entry *entry, *tmp;
> + struct iommu_domain *domain;
> +
> + /* clean up iommu mapping entries */
> + list_for_each_entry_safe(entry, tmp, &apu_rproc->mappings, node) {
> + size_t unmapped;
> +
> + domain = iommu_get_domain_for_dev(dev);
> + unmapped = iommu_unmap(domain, entry->da, entry->len);
Same as above
> + if (unmapped != entry->len) {
> + /* nothing much to do besides complaining */
> + dev_err(dev, "failed to unmap %zx/%zu\n", entry->len,
> + unmapped);
> + }
> + memunmap(entry->va);
> +
> + list_del(&entry->node);
> + kfree(entry);
> + }
> +}
> +
> +static int mtk_apu_rproc_prepare(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + int ret;
> +
> + ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
> + apu_rproc->clks);
> + if (ret)
> + dev_err(apu_rproc->dev, "Failed to enable clocks\n");
> +
> + return ret;
> +}
> +
> +static int mtk_apu_rproc_unprepare(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +
> + clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
> + apu_rproc->clks);
> +
> + return 0;
> +}
> +
> +static int mtk_apu_rproc_start(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + u32 core_ctrl;
> +
> + /* Set reset vector of APU firmware boot address */
> + writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
> +
> + /* Turn on the clocks and stall the APU */
> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
> + core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
> + CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
> + CORE_CTRL_PIF_GATED;
> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> + /* Reset the APU: this requires 27 ns to be effective on any platform */
> + writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
> + apu_rproc->base + SW_RST);
> + ndelay(APU_RESET_DELAY);
> + writel(0, apu_rproc->base + SW_RST);
> +
> + core_ctrl &= ~CORE_CTRL_PIF_GATED;
> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> + /* Configure memory accesses to go through the IOMMU */
> + writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
> + CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
> + writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
> + CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
> + apu_rproc->base + CORE_DEFAULT1);
> +
> + /* Release the APU */
> + core_ctrl &= ~CORE_CTRL_RUN_STALL;
> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> + return 0;
> +}
> +
> +static int mtk_apu_rproc_stop(struct rproc *rproc)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> + u32 core_ctrl;
> +
> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
> + writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
> +
> + mtk_apu_iommu_unmap_all(rproc);
> +
> + return 0;
> +}
> +
> +static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
> +{
> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +
> + writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
> +}
> +
> +static int mtk_apu_load(struct rproc *rproc, const struct firmware *fw)
> +
> +{
> + struct rproc_mem_entry *entry, *tmp;
> + int ret;
> +
> + ret = rproc_elf_load_segments(rproc, fw);
> + if (ret)
> + return ret;
> +
> + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
> + ret = mtk_apu_iommu_map(rproc, entry);
> + if (ret)
> + goto err_unmap_all;
> + }
> +
> + return 0;
> +
> +err_unmap_all:
> + mtk_apu_iommu_unmap_all(rproc);
> +
> + return ret;
> +}
> +
> +static struct reserved_mem *of_reserved_mem_by_name(struct rproc *rproc,
> + const char *name)
> +{
> + struct device *dev = rproc->dev.parent;
> + struct device_node *np = dev->of_node;
> + struct device_node *target;
> + struct reserved_mem *rmem;
> + int idx;
> +
> + idx = of_property_match_string(np, "memory-region-names", name);
> + if (idx < 0) {
> + dev_err(dev, "failed to find %s memory\n", name);
> + return NULL;
> + }
> +
> + target = of_parse_phandle(np, "memory-region", idx);
> + if (!target)
> + return NULL;
There is no comment to explain the above gymnastic and as such I have to guess
the firmware resources always have "vdev0" prepended to buffer, vring0 and
vring1. If I am correct please add a comment to describe the situation, ditto
if I am not correct.
> +
> + rmem = of_reserved_mem_lookup(target);
> + if (!rmem)
> + dev_err(dev, "unable to acquire memory-region\n");
> + of_node_put(target);
> +
> + return rmem;
> +}
> +
> +static int mtk_apu_handle_rsc(struct rproc *rproc, u32 rsc_type, void *rsc,
> + int offset, int avail)
> +{
> + struct device *dev = rproc->dev.parent;
> +
> + if (rsc_type == RSC_VENDOR_CARVEOUT) {
> + struct fw_rsc_carveout *rsc_carveout = rsc;
> + struct rproc_mem_entry *mem;
> + struct reserved_mem *rmem;
> +
> + rmem = of_reserved_mem_by_name(rproc, rsc_carveout->name);
s/of_reserved_mem_by_name/mtk_of_reserved_mem_by_name
> + if (!rmem)
> + return -ENOMEM;
> +
> + if (rmem->size < rsc_carveout->len) {
I think '!=' would be more appropriate than '<'.
> + dev_err(dev, "The reserved memory is too small\n");
> + return -ENOMEM;
> + }
> +
> + mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base,
> + rsc_carveout->len, rsc_carveout->da,
> + NULL, NULL, rsc_carveout->name);
Why not naming the carveout the way it is in the DT rather than keeping with
what's in the firmware image?
> + if (!mem)
> + return -ENOMEM;
> +
> + mem->flags = rsc_carveout->flags;
> + rsc_carveout->pa = rmem->base;
> + rproc_add_carveout(rproc, mem);
> + }
> +
> + return RSC_HANDLED;
> +}
> +
> +static const struct rproc_ops mtk_apu_rproc_ops = {
> + .prepare = mtk_apu_rproc_prepare,
> + .unprepare = mtk_apu_rproc_unprepare,
> + .start = mtk_apu_rproc_start,
> + .stop = mtk_apu_rproc_stop,
> + .kick = mtk_apu_rproc_kick,
> + .load = mtk_apu_load,
> + .parse_fw = rproc_elf_load_rsc_table,
> + .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> + .sanity_check = rproc_elf_sanity_check,
> + .get_boot_addr = rproc_elf_get_boot_addr,
> + .handle_rsc = mtk_apu_handle_rsc,
> +};
> +
> +static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
> +{
> + struct rproc *rproc = data;
> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> +
> + writel(1, apu_rproc->base + CORE_XTENSA_INT);
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t handle_event(int irq, void *data)
> +{
> + struct rproc *rproc = data;
> +
> + rproc_vq_interrupt(rproc, 0);
> + rproc_vq_interrupt(rproc, 1);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int mtk_apu_rproc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mtk_apu_rproc *apu_rproc;
> + struct rproc *rproc;
> + struct resource *res;
> + int ret;
> +
> + rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
> + sizeof(*apu_rproc));
> + if (!rproc)
> + return -ENOMEM;
> +
> + rproc->recovery_disabled = true;
> + rproc->has_iommu = false;
> + rproc->auto_boot = false;
> +
> + apu_rproc = rproc->priv;
> + apu_rproc->dev = dev;
> + INIT_LIST_HEAD(&apu_rproc->mappings);
> +
> + platform_set_drvdata(pdev, rproc);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + apu_rproc->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(apu_rproc->base)) {
> + dev_err(dev, "Failed to map mmio\n");
> + ret = PTR_ERR(apu_rproc->base);
> + goto free_rproc;
> + }
> +
> + apu_rproc->irq = platform_get_irq(pdev, 0);
> + if (apu_rproc->irq < 0) {
> + ret = apu_rproc->irq;
> + goto free_rproc;
> + }
> +
> + ret = devm_request_threaded_irq(dev, apu_rproc->irq,
> + mtk_apu_rproc_callback, handle_event,
> + IRQF_SHARED | IRQF_ONESHOT,
> + NULL, rproc);
I'm pretty sure I commented on that during my last review - is there a need to
do a threaded irq here? As far as I can tell there is no advantage in doing so
in this context. Please add a comment to justify using a threaded irq.
Thanks,
Mathieu
> + if (ret) {
> + dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
> + goto free_rproc;
> + }
> +
> + apu_rproc->clks[0].id = "ipu";
> + apu_rproc->clks[1].id = "axi";
> + apu_rproc->clks[2].id = "jtag";
> +
> + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
> + apu_rproc->clks);
> + if (ret) {
> + dev_err(dev, "Failed to get clocks\n");
> + goto free_rproc;
> + }
> +
> + ret = rproc_add(rproc);
> + if (ret) {
> + dev_err(dev, "rproc_add failed: %d\n", ret);
> + goto free_rproc;
> + }
> +
> + return 0;
> +
> +free_rproc:
> + rproc_free(rproc);
> +
> + return ret;
> +}
> +
> +static int mtk_apu_rproc_remove(struct platform_device *pdev)
> +{
> + struct rproc *rproc = platform_get_drvdata(pdev);
> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> + struct device *dev = &pdev->dev;
> +
> + disable_irq(apu_rproc->irq);
> +
> + rproc_del(rproc);
> + of_reserved_mem_device_release(dev);
> + rproc_free(rproc);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mtk_apu_rproc_of_match[] = {
> + { .compatible = "mediatek,mt8183-apu", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
> +
> +static struct platform_driver mtk_apu_rproc_driver = {
> + .probe = mtk_apu_rproc_probe,
> + .remove = mtk_apu_rproc_remove,
> + .driver = {
> + .name = "mtk_apu-rproc",
> + .of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
> + },
> +};
> +module_platform_driver(mtk_apu_rproc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Alexandre Bailon");
> +MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
2021-08-30 18:19 ` Mathieu Poirier
(?)
@ 2021-09-08 8:21 ` Alexandre Bailon
-1 siblings, 0 replies; 27+ messages in thread
From: Alexandre Bailon @ 2021-09-08 8:21 UTC (permalink / raw)
To: Mathieu Poirier
Cc: ohad, bjorn.andersson, robh+dt, matthias.bgg, linux-remoteproc,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
stephane.leprovost, gpain, khilman
Hi Mathieu,
Le 30/08/2021 à 20:19, Mathieu Poirier a écrit :
> Hi Alex,
>
> On Thu, Aug 19, 2021 at 05:13:38PM +0200, Alexandre Bailon wrote:
>> This adds a driver to control the APU present in the MT8183.
>> This loads the firmware and start the DSP.
>>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>> drivers/remoteproc/Kconfig | 10 +
>> drivers/remoteproc/Makefile | 1 +
>> drivers/remoteproc/mtk_apu.c | 440 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 451 insertions(+)
>> create mode 100644 drivers/remoteproc/mtk_apu.c
>>
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index 9a6eedc3994a5..6c106a6c3ad5d 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -53,6 +53,16 @@ config MTK_SCP
>>
>> It's safe to say N here.
>>
>> +config MTK_APU
>> + tristate "Mediatek APU remoteproc support"
>> + depends on ARCH_MEDIATEK
>> + depends on MTK_IOMMU
>> + help
>> + Say y to support the Mediatek's Accelerated Processing Unit (APU) via
> In the bindings A in APU is for AI or Vision whereas above it is for
> accelerated. Please pick one and stick with it.
Indeed, the A stands for AI so I missed that one.
>> + the remote processor framework.
>> +
>> + It's safe to say N here.
>> +
>> config OMAP_REMOTEPROC
>> tristate "OMAP remoteproc support"
>> depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
>> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
>> index bb26c9e4ef9cf..e395af86c17b0 100644
>> --- a/drivers/remoteproc/Makefile
>> +++ b/drivers/remoteproc/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o
>> obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
>> obj-$(CONFIG_INGENIC_VPU_RPROC) += ingenic_rproc.o
>> obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
>> +obj-$(CONFIG_MTK_APU) += mtk_apu.o
>> obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
>> obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
>> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
>> diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
>> new file mode 100644
>> index 0000000000000..0e3f63987bd85
>> --- /dev/null
>> +++ b/drivers/remoteproc/mtk_apu.c
>> @@ -0,0 +1,440 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020 BayLibre SAS
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/iommu.h>
>> +#include <linux/irq.h>
>> +#include <linux/module.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/remoteproc.h>
>> +
>> +#include "remoteproc_internal.h"
>> +
>> +#define SW_RST (0x0000000C)
>> +#define SW_RST_OCD_HALT_ON_RST BIT(12)
>> +#define SW_RST_IPU_D_RST BIT(8)
>> +#define SW_RST_IPU_B_RST BIT(4)
>> +#define CORE_CTRL (0x00000110)
>> +#define CORE_CTRL_PDEBUG_ENABLE BIT(31)
>> +#define CORE_CTRL_SRAM_64K_iMEM (0x00 << 27)
>> +#define CORE_CTRL_SRAM_96K_iMEM (0x01 << 27)
>> +#define CORE_CTRL_SRAM_128K_iMEM (0x02 << 27)
>> +#define CORE_CTRL_SRAM_192K_iMEM (0x03 << 27)
>> +#define CORE_CTRL_SRAM_256K_iMEM (0x04 << 27)
>> +#define CORE_CTRL_PBCLK_ENABLE BIT(26)
>> +#define CORE_CTRL_RUN_STALL BIT(23)
>> +#define CORE_CTRL_STATE_VECTOR_SELECT BIT(19)
>> +#define CORE_CTRL_PIF_GATED BIT(17)
>> +#define CORE_CTRL_NMI BIT(0)
>> +#define CORE_XTENSA_INT (0x00000114)
>> +#define CORE_CTL_XTENSA_INT (0x00000118)
>> +#define CORE_DEFAULT0 (0x0000013C)
>> +#define CORE_DEFAULT0_QOS_SWAP_0 (0x00 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_1 (0x01 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_2 (0x02 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_3 (0x03 << 28)
>> +#define CORE_DEFAULT0_ARUSER_USE_IOMMU (0x10 << 23)
>> +#define CORE_DEFAULT0_AWUSER_USE_IOMMU (0x10 << 18)
>> +#define CORE_DEFAULT1 (0x00000140)
>> +#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU (0x10 << 0)
>> +#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU (0x10 << 5)
>> +#define CORE_XTENSA_ALTRESETVEC (0x000001F8)
>> +
>> +#define RSC_VENDOR_CARVEOUT (RSC_VENDOR_START + 1)
>> +
>> +#define APU_RESET_DELAY (27)
>> +
>> +struct mtk_apu_rproc {
>> + struct device *dev;
>> + void __iomem *base;
>> + int irq;
>> + struct clk_bulk_data clks[3];
>> + struct list_head mappings;
>> +};
>> +
>> +static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + struct device *dev = rproc->dev.parent;
>> + struct rproc_mem_entry *mapping;
>> + struct iommu_domain *domain;
>> + int ret;
>> + u64 pa;
>> +
>> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
>> + if (!mapping)
>> + return -ENOMEM;
>> +
>> + if (!entry->va)
>> + pa = entry->dma;
>> + else
>> + pa = rproc_va_to_pa(entry->va);
>> +
>> + if ((strcmp(entry->name, "vdev0vring0") == 0 ||
>> + strcmp(entry->name, "vdev0vring1") == 0)) {
> If my guesswork is correct (see below) the above is required to deal with
> multiple remote processors. Why not just look for "vring0" and "vring1"?. That
> way we wouldn't have to keep the "vdev0" part for carveouts that really are
> "vdev1".
This makes sense. I will do that change.
>
>> + entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
>> + if (IS_ERR_OR_NULL(entry->va)) {
>> + dev_err(dev, "Unable to map memory region: %pa+%lx\n",
>> + &entry->dma, entry->len);
>> + ret = PTR_ERR(mapping->va);
>> + goto free_mapping;
>> + }
>> + mapping->va = entry->va;
>> + }
>> +
>> + domain = iommu_get_domain_for_dev(dev);
> Here @domain can be NULL
>
>> + ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
> ... and iommu_map() isn't dealing with that condition properly. Please
> check for a NULL condition before proceeding.
>
>> + if (ret) {
>> + dev_err(dev, "iommu_map failed: %d\n", ret);
>> + goto err_memunmap;
>> + }
>> +
>> + mapping->da = entry->da;
>> + mapping->len = entry->len;
>> + list_add_tail(&mapping->node, &apu_rproc->mappings);
>> +
>> + return 0;
>> +
>> +err_memunmap:
>> + memunmap(mapping->va);
>> +free_mapping:
>> + kfree(mapping);
>> +
>> + return ret;
>> +}
>> +
>> +static void mtk_apu_iommu_unmap_all(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + struct device *dev = rproc->dev.parent;
>> + struct rproc_mem_entry *entry, *tmp;
>> + struct iommu_domain *domain;
>> +
>> + /* clean up iommu mapping entries */
>> + list_for_each_entry_safe(entry, tmp, &apu_rproc->mappings, node) {
>> + size_t unmapped;
>> +
>> + domain = iommu_get_domain_for_dev(dev);
>> + unmapped = iommu_unmap(domain, entry->da, entry->len);
> Same as above
>
>> + if (unmapped != entry->len) {
>> + /* nothing much to do besides complaining */
>> + dev_err(dev, "failed to unmap %zx/%zu\n", entry->len,
>> + unmapped);
>> + }
>> + memunmap(entry->va);
>> +
>> + list_del(&entry->node);
>> + kfree(entry);
>> + }
>> +}
>> +
>> +static int mtk_apu_rproc_prepare(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + int ret;
>> +
>> + ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
>> + apu_rproc->clks);
>> + if (ret)
>> + dev_err(apu_rproc->dev, "Failed to enable clocks\n");
>> +
>> + return ret;
>> +}
>> +
>> +static int mtk_apu_rproc_unprepare(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +
>> + clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
>> + apu_rproc->clks);
>> +
>> + return 0;
>> +}
>> +
>> +static int mtk_apu_rproc_start(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + u32 core_ctrl;
>> +
>> + /* Set reset vector of APU firmware boot address */
>> + writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
>> +
>> + /* Turn on the clocks and stall the APU */
>> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
>> + core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
>> + CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
>> + CORE_CTRL_PIF_GATED;
>> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> + /* Reset the APU: this requires 27 ns to be effective on any platform */
>> + writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
>> + apu_rproc->base + SW_RST);
>> + ndelay(APU_RESET_DELAY);
>> + writel(0, apu_rproc->base + SW_RST);
>> +
>> + core_ctrl &= ~CORE_CTRL_PIF_GATED;
>> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> + /* Configure memory accesses to go through the IOMMU */
>> + writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
>> + CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
>> + writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
>> + CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
>> + apu_rproc->base + CORE_DEFAULT1);
>> +
>> + /* Release the APU */
>> + core_ctrl &= ~CORE_CTRL_RUN_STALL;
>> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> + return 0;
>> +}
>> +
>> +static int mtk_apu_rproc_stop(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + u32 core_ctrl;
>> +
>> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
>> + writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
>> +
>> + mtk_apu_iommu_unmap_all(rproc);
>> +
>> + return 0;
>> +}
>> +
>> +static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +
>> + writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
>> +}
>> +
>> +static int mtk_apu_load(struct rproc *rproc, const struct firmware *fw)
>> +
>> +{
>> + struct rproc_mem_entry *entry, *tmp;
>> + int ret;
>> +
>> + ret = rproc_elf_load_segments(rproc, fw);
>> + if (ret)
>> + return ret;
>> +
>> + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
>> + ret = mtk_apu_iommu_map(rproc, entry);
>> + if (ret)
>> + goto err_unmap_all;
>> + }
>> +
>> + return 0;
>> +
>> +err_unmap_all:
>> + mtk_apu_iommu_unmap_all(rproc);
>> +
>> + return ret;
>> +}
>> +
>> +static struct reserved_mem *of_reserved_mem_by_name(struct rproc *rproc,
>> + const char *name)
>> +{
>> + struct device *dev = rproc->dev.parent;
>> + struct device_node *np = dev->of_node;
>> + struct device_node *target;
>> + struct reserved_mem *rmem;
>> + int idx;
>> +
>> + idx = of_property_match_string(np, "memory-region-names", name);
>> + if (idx < 0) {
>> + dev_err(dev, "failed to find %s memory\n", name);
>> + return NULL;
>> + }
>> +
>> + target = of_parse_phandle(np, "memory-region", idx);
>> + if (!target)
>> + return NULL;
> There is no comment to explain the above gymnastic and as such I have to guess
> the firmware resources always have "vdev0" prepended to buffer, vring0 and
> vring1. If I am correct please add a comment to describe the situation, ditto
> if I am not correct.
OK, I will add much more documentation to explain.
>
>> +
>> + rmem = of_reserved_mem_lookup(target);
>> + if (!rmem)
>> + dev_err(dev, "unable to acquire memory-region\n");
>> + of_node_put(target);
>> +
>> + return rmem;
>> +}
>> +
>> +static int mtk_apu_handle_rsc(struct rproc *rproc, u32 rsc_type, void *rsc,
>> + int offset, int avail)
>> +{
>> + struct device *dev = rproc->dev.parent;
>> +
>> + if (rsc_type == RSC_VENDOR_CARVEOUT) {
>> + struct fw_rsc_carveout *rsc_carveout = rsc;
>> + struct rproc_mem_entry *mem;
>> + struct reserved_mem *rmem;
>> +
>> + rmem = of_reserved_mem_by_name(rproc, rsc_carveout->name);
> s/of_reserved_mem_by_name/mtk_of_reserved_mem_by_name
>
>> + if (!rmem)
>> + return -ENOMEM;
>> +
>> + if (rmem->size < rsc_carveout->len) {
> I think '!=' would be more appropriate than '<'.
>
>> + dev_err(dev, "The reserved memory is too small\n");
>> + return -ENOMEM;
>> + }
>> +
>> + mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base,
>> + rsc_carveout->len, rsc_carveout->da,
>> + NULL, NULL, rsc_carveout->name);
> Why not naming the carveout the way it is in the DT rather than keeping with
> what's in the firmware image?
The main reason is that we may load different firmware at runtime, with
very different memory layouts.
Some firmware only need few MB of memory whereas some other may requires
about 64 MB of memory.
Using the resource table here gives much more flexibility than DT that
is expected to be "static".
>
>> + if (!mem)
>> + return -ENOMEM;
>> +
>> + mem->flags = rsc_carveout->flags;
>> + rsc_carveout->pa = rmem->base;
>> + rproc_add_carveout(rproc, mem);
>> + }
>> +
>> + return RSC_HANDLED;
>> +}
>> +
>> +static const struct rproc_ops mtk_apu_rproc_ops = {
>> + .prepare = mtk_apu_rproc_prepare,
>> + .unprepare = mtk_apu_rproc_unprepare,
>> + .start = mtk_apu_rproc_start,
>> + .stop = mtk_apu_rproc_stop,
>> + .kick = mtk_apu_rproc_kick,
>> + .load = mtk_apu_load,
>> + .parse_fw = rproc_elf_load_rsc_table,
>> + .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
>> + .sanity_check = rproc_elf_sanity_check,
>> + .get_boot_addr = rproc_elf_get_boot_addr,
>> + .handle_rsc = mtk_apu_handle_rsc,
>> +};
>> +
>> +static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
>> +{
>> + struct rproc *rproc = data;
>> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
>> +
>> + writel(1, apu_rproc->base + CORE_XTENSA_INT);
>> +
>> + return IRQ_WAKE_THREAD;
>> +}
>> +
>> +static irqreturn_t handle_event(int irq, void *data)
>> +{
>> + struct rproc *rproc = data;
>> +
>> + rproc_vq_interrupt(rproc, 0);
>> + rproc_vq_interrupt(rproc, 1);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int mtk_apu_rproc_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct mtk_apu_rproc *apu_rproc;
>> + struct rproc *rproc;
>> + struct resource *res;
>> + int ret;
>> +
>> + rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
>> + sizeof(*apu_rproc));
>> + if (!rproc)
>> + return -ENOMEM;
>> +
>> + rproc->recovery_disabled = true;
>> + rproc->has_iommu = false;
>> + rproc->auto_boot = false;
>> +
>> + apu_rproc = rproc->priv;
>> + apu_rproc->dev = dev;
>> + INIT_LIST_HEAD(&apu_rproc->mappings);
>> +
>> + platform_set_drvdata(pdev, rproc);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + apu_rproc->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(apu_rproc->base)) {
>> + dev_err(dev, "Failed to map mmio\n");
>> + ret = PTR_ERR(apu_rproc->base);
>> + goto free_rproc;
>> + }
>> +
>> + apu_rproc->irq = platform_get_irq(pdev, 0);
>> + if (apu_rproc->irq < 0) {
>> + ret = apu_rproc->irq;
>> + goto free_rproc;
>> + }
>> +
>> + ret = devm_request_threaded_irq(dev, apu_rproc->irq,
>> + mtk_apu_rproc_callback, handle_event,
>> + IRQF_SHARED | IRQF_ONESHOT,
>> + NULL, rproc);
> I'm pretty sure I commented on that during my last review - is there a need to
> do a threaded irq here? As far as I can tell there is no advantage in doing so
> in this context. Please add a comment to justify using a threaded irq.
My apologize if I forgot to address a comment you made.
It used to be required before I moved to virtio / rpmsg to manage the
communication before
the CPU and the APU (the driver was doing a lot of thing in the callback).
Now, with the latest architecture, I am not sure if that is still
required since the callback is just
going through a list of request, copy rpmsg data and then notify DRM
that a request has been received.
Thanks,
Alexandre
>
> Thanks,
> Mathieu
>
>> + if (ret) {
>> + dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
>> + goto free_rproc;
>> + }
>> +
>> + apu_rproc->clks[0].id = "ipu";
>> + apu_rproc->clks[1].id = "axi";
>> + apu_rproc->clks[2].id = "jtag";
>> +
>> + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
>> + apu_rproc->clks);
>> + if (ret) {
>> + dev_err(dev, "Failed to get clocks\n");
>> + goto free_rproc;
>> + }
>> +
>> + ret = rproc_add(rproc);
>> + if (ret) {
>> + dev_err(dev, "rproc_add failed: %d\n", ret);
>> + goto free_rproc;
>> + }
>> +
>> + return 0;
>> +
>> +free_rproc:
>> + rproc_free(rproc);
>> +
>> + return ret;
>> +}
>> +
>> +static int mtk_apu_rproc_remove(struct platform_device *pdev)
>> +{
>> + struct rproc *rproc = platform_get_drvdata(pdev);
>> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
>> + struct device *dev = &pdev->dev;
>> +
>> + disable_irq(apu_rproc->irq);
>> +
>> + rproc_del(rproc);
>> + of_reserved_mem_device_release(dev);
>> + rproc_free(rproc);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id mtk_apu_rproc_of_match[] = {
>> + { .compatible = "mediatek,mt8183-apu", },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
>> +
>> +static struct platform_driver mtk_apu_rproc_driver = {
>> + .probe = mtk_apu_rproc_probe,
>> + .remove = mtk_apu_rproc_remove,
>> + .driver = {
>> + .name = "mtk_apu-rproc",
>> + .of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
>> + },
>> +};
>> +module_platform_driver(mtk_apu_rproc_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Alexandre Bailon");
>> +MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
>> --
>> 2.31.1
>>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
@ 2021-09-08 8:21 ` Alexandre Bailon
0 siblings, 0 replies; 27+ messages in thread
From: Alexandre Bailon @ 2021-09-08 8:21 UTC (permalink / raw)
To: Mathieu Poirier
Cc: ohad, bjorn.andersson, robh+dt, matthias.bgg, linux-remoteproc,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
stephane.leprovost, gpain, khilman
Hi Mathieu,
Le 30/08/2021 à 20:19, Mathieu Poirier a écrit :
> Hi Alex,
>
> On Thu, Aug 19, 2021 at 05:13:38PM +0200, Alexandre Bailon wrote:
>> This adds a driver to control the APU present in the MT8183.
>> This loads the firmware and start the DSP.
>>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>> drivers/remoteproc/Kconfig | 10 +
>> drivers/remoteproc/Makefile | 1 +
>> drivers/remoteproc/mtk_apu.c | 440 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 451 insertions(+)
>> create mode 100644 drivers/remoteproc/mtk_apu.c
>>
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index 9a6eedc3994a5..6c106a6c3ad5d 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -53,6 +53,16 @@ config MTK_SCP
>>
>> It's safe to say N here.
>>
>> +config MTK_APU
>> + tristate "Mediatek APU remoteproc support"
>> + depends on ARCH_MEDIATEK
>> + depends on MTK_IOMMU
>> + help
>> + Say y to support the Mediatek's Accelerated Processing Unit (APU) via
> In the bindings A in APU is for AI or Vision whereas above it is for
> accelerated. Please pick one and stick with it.
Indeed, the A stands for AI so I missed that one.
>> + the remote processor framework.
>> +
>> + It's safe to say N here.
>> +
>> config OMAP_REMOTEPROC
>> tristate "OMAP remoteproc support"
>> depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
>> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
>> index bb26c9e4ef9cf..e395af86c17b0 100644
>> --- a/drivers/remoteproc/Makefile
>> +++ b/drivers/remoteproc/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o
>> obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
>> obj-$(CONFIG_INGENIC_VPU_RPROC) += ingenic_rproc.o
>> obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
>> +obj-$(CONFIG_MTK_APU) += mtk_apu.o
>> obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
>> obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
>> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
>> diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
>> new file mode 100644
>> index 0000000000000..0e3f63987bd85
>> --- /dev/null
>> +++ b/drivers/remoteproc/mtk_apu.c
>> @@ -0,0 +1,440 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020 BayLibre SAS
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/iommu.h>
>> +#include <linux/irq.h>
>> +#include <linux/module.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/remoteproc.h>
>> +
>> +#include "remoteproc_internal.h"
>> +
>> +#define SW_RST (0x0000000C)
>> +#define SW_RST_OCD_HALT_ON_RST BIT(12)
>> +#define SW_RST_IPU_D_RST BIT(8)
>> +#define SW_RST_IPU_B_RST BIT(4)
>> +#define CORE_CTRL (0x00000110)
>> +#define CORE_CTRL_PDEBUG_ENABLE BIT(31)
>> +#define CORE_CTRL_SRAM_64K_iMEM (0x00 << 27)
>> +#define CORE_CTRL_SRAM_96K_iMEM (0x01 << 27)
>> +#define CORE_CTRL_SRAM_128K_iMEM (0x02 << 27)
>> +#define CORE_CTRL_SRAM_192K_iMEM (0x03 << 27)
>> +#define CORE_CTRL_SRAM_256K_iMEM (0x04 << 27)
>> +#define CORE_CTRL_PBCLK_ENABLE BIT(26)
>> +#define CORE_CTRL_RUN_STALL BIT(23)
>> +#define CORE_CTRL_STATE_VECTOR_SELECT BIT(19)
>> +#define CORE_CTRL_PIF_GATED BIT(17)
>> +#define CORE_CTRL_NMI BIT(0)
>> +#define CORE_XTENSA_INT (0x00000114)
>> +#define CORE_CTL_XTENSA_INT (0x00000118)
>> +#define CORE_DEFAULT0 (0x0000013C)
>> +#define CORE_DEFAULT0_QOS_SWAP_0 (0x00 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_1 (0x01 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_2 (0x02 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_3 (0x03 << 28)
>> +#define CORE_DEFAULT0_ARUSER_USE_IOMMU (0x10 << 23)
>> +#define CORE_DEFAULT0_AWUSER_USE_IOMMU (0x10 << 18)
>> +#define CORE_DEFAULT1 (0x00000140)
>> +#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU (0x10 << 0)
>> +#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU (0x10 << 5)
>> +#define CORE_XTENSA_ALTRESETVEC (0x000001F8)
>> +
>> +#define RSC_VENDOR_CARVEOUT (RSC_VENDOR_START + 1)
>> +
>> +#define APU_RESET_DELAY (27)
>> +
>> +struct mtk_apu_rproc {
>> + struct device *dev;
>> + void __iomem *base;
>> + int irq;
>> + struct clk_bulk_data clks[3];
>> + struct list_head mappings;
>> +};
>> +
>> +static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + struct device *dev = rproc->dev.parent;
>> + struct rproc_mem_entry *mapping;
>> + struct iommu_domain *domain;
>> + int ret;
>> + u64 pa;
>> +
>> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
>> + if (!mapping)
>> + return -ENOMEM;
>> +
>> + if (!entry->va)
>> + pa = entry->dma;
>> + else
>> + pa = rproc_va_to_pa(entry->va);
>> +
>> + if ((strcmp(entry->name, "vdev0vring0") == 0 ||
>> + strcmp(entry->name, "vdev0vring1") == 0)) {
> If my guesswork is correct (see below) the above is required to deal with
> multiple remote processors. Why not just look for "vring0" and "vring1"?. That
> way we wouldn't have to keep the "vdev0" part for carveouts that really are
> "vdev1".
This makes sense. I will do that change.
>
>> + entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
>> + if (IS_ERR_OR_NULL(entry->va)) {
>> + dev_err(dev, "Unable to map memory region: %pa+%lx\n",
>> + &entry->dma, entry->len);
>> + ret = PTR_ERR(mapping->va);
>> + goto free_mapping;
>> + }
>> + mapping->va = entry->va;
>> + }
>> +
>> + domain = iommu_get_domain_for_dev(dev);
> Here @domain can be NULL
>
>> + ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
> ... and iommu_map() isn't dealing with that condition properly. Please
> check for a NULL condition before proceeding.
>
>> + if (ret) {
>> + dev_err(dev, "iommu_map failed: %d\n", ret);
>> + goto err_memunmap;
>> + }
>> +
>> + mapping->da = entry->da;
>> + mapping->len = entry->len;
>> + list_add_tail(&mapping->node, &apu_rproc->mappings);
>> +
>> + return 0;
>> +
>> +err_memunmap:
>> + memunmap(mapping->va);
>> +free_mapping:
>> + kfree(mapping);
>> +
>> + return ret;
>> +}
>> +
>> +static void mtk_apu_iommu_unmap_all(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + struct device *dev = rproc->dev.parent;
>> + struct rproc_mem_entry *entry, *tmp;
>> + struct iommu_domain *domain;
>> +
>> + /* clean up iommu mapping entries */
>> + list_for_each_entry_safe(entry, tmp, &apu_rproc->mappings, node) {
>> + size_t unmapped;
>> +
>> + domain = iommu_get_domain_for_dev(dev);
>> + unmapped = iommu_unmap(domain, entry->da, entry->len);
> Same as above
>
>> + if (unmapped != entry->len) {
>> + /* nothing much to do besides complaining */
>> + dev_err(dev, "failed to unmap %zx/%zu\n", entry->len,
>> + unmapped);
>> + }
>> + memunmap(entry->va);
>> +
>> + list_del(&entry->node);
>> + kfree(entry);
>> + }
>> +}
>> +
>> +static int mtk_apu_rproc_prepare(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + int ret;
>> +
>> + ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
>> + apu_rproc->clks);
>> + if (ret)
>> + dev_err(apu_rproc->dev, "Failed to enable clocks\n");
>> +
>> + return ret;
>> +}
>> +
>> +static int mtk_apu_rproc_unprepare(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +
>> + clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
>> + apu_rproc->clks);
>> +
>> + return 0;
>> +}
>> +
>> +static int mtk_apu_rproc_start(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + u32 core_ctrl;
>> +
>> + /* Set reset vector of APU firmware boot address */
>> + writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
>> +
>> + /* Turn on the clocks and stall the APU */
>> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
>> + core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
>> + CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
>> + CORE_CTRL_PIF_GATED;
>> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> + /* Reset the APU: this requires 27 ns to be effective on any platform */
>> + writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
>> + apu_rproc->base + SW_RST);
>> + ndelay(APU_RESET_DELAY);
>> + writel(0, apu_rproc->base + SW_RST);
>> +
>> + core_ctrl &= ~CORE_CTRL_PIF_GATED;
>> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> + /* Configure memory accesses to go through the IOMMU */
>> + writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
>> + CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
>> + writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
>> + CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
>> + apu_rproc->base + CORE_DEFAULT1);
>> +
>> + /* Release the APU */
>> + core_ctrl &= ~CORE_CTRL_RUN_STALL;
>> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> + return 0;
>> +}
>> +
>> +static int mtk_apu_rproc_stop(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + u32 core_ctrl;
>> +
>> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
>> + writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
>> +
>> + mtk_apu_iommu_unmap_all(rproc);
>> +
>> + return 0;
>> +}
>> +
>> +static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +
>> + writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
>> +}
>> +
>> +static int mtk_apu_load(struct rproc *rproc, const struct firmware *fw)
>> +
>> +{
>> + struct rproc_mem_entry *entry, *tmp;
>> + int ret;
>> +
>> + ret = rproc_elf_load_segments(rproc, fw);
>> + if (ret)
>> + return ret;
>> +
>> + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
>> + ret = mtk_apu_iommu_map(rproc, entry);
>> + if (ret)
>> + goto err_unmap_all;
>> + }
>> +
>> + return 0;
>> +
>> +err_unmap_all:
>> + mtk_apu_iommu_unmap_all(rproc);
>> +
>> + return ret;
>> +}
>> +
>> +static struct reserved_mem *of_reserved_mem_by_name(struct rproc *rproc,
>> + const char *name)
>> +{
>> + struct device *dev = rproc->dev.parent;
>> + struct device_node *np = dev->of_node;
>> + struct device_node *target;
>> + struct reserved_mem *rmem;
>> + int idx;
>> +
>> + idx = of_property_match_string(np, "memory-region-names", name);
>> + if (idx < 0) {
>> + dev_err(dev, "failed to find %s memory\n", name);
>> + return NULL;
>> + }
>> +
>> + target = of_parse_phandle(np, "memory-region", idx);
>> + if (!target)
>> + return NULL;
> There is no comment to explain the above gymnastic and as such I have to guess
> the firmware resources always have "vdev0" prepended to buffer, vring0 and
> vring1. If I am correct please add a comment to describe the situation, ditto
> if I am not correct.
OK, I will add much more documentation to explain.
>
>> +
>> + rmem = of_reserved_mem_lookup(target);
>> + if (!rmem)
>> + dev_err(dev, "unable to acquire memory-region\n");
>> + of_node_put(target);
>> +
>> + return rmem;
>> +}
>> +
>> +static int mtk_apu_handle_rsc(struct rproc *rproc, u32 rsc_type, void *rsc,
>> + int offset, int avail)
>> +{
>> + struct device *dev = rproc->dev.parent;
>> +
>> + if (rsc_type == RSC_VENDOR_CARVEOUT) {
>> + struct fw_rsc_carveout *rsc_carveout = rsc;
>> + struct rproc_mem_entry *mem;
>> + struct reserved_mem *rmem;
>> +
>> + rmem = of_reserved_mem_by_name(rproc, rsc_carveout->name);
> s/of_reserved_mem_by_name/mtk_of_reserved_mem_by_name
>
>> + if (!rmem)
>> + return -ENOMEM;
>> +
>> + if (rmem->size < rsc_carveout->len) {
> I think '!=' would be more appropriate than '<'.
>
>> + dev_err(dev, "The reserved memory is too small\n");
>> + return -ENOMEM;
>> + }
>> +
>> + mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base,
>> + rsc_carveout->len, rsc_carveout->da,
>> + NULL, NULL, rsc_carveout->name);
> Why not naming the carveout the way it is in the DT rather than keeping with
> what's in the firmware image?
The main reason is that we may load different firmware at runtime, with
very different memory layouts.
Some firmware only need few MB of memory whereas some other may requires
about 64 MB of memory.
Using the resource table here gives much more flexibility than DT that
is expected to be "static".
>
>> + if (!mem)
>> + return -ENOMEM;
>> +
>> + mem->flags = rsc_carveout->flags;
>> + rsc_carveout->pa = rmem->base;
>> + rproc_add_carveout(rproc, mem);
>> + }
>> +
>> + return RSC_HANDLED;
>> +}
>> +
>> +static const struct rproc_ops mtk_apu_rproc_ops = {
>> + .prepare = mtk_apu_rproc_prepare,
>> + .unprepare = mtk_apu_rproc_unprepare,
>> + .start = mtk_apu_rproc_start,
>> + .stop = mtk_apu_rproc_stop,
>> + .kick = mtk_apu_rproc_kick,
>> + .load = mtk_apu_load,
>> + .parse_fw = rproc_elf_load_rsc_table,
>> + .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
>> + .sanity_check = rproc_elf_sanity_check,
>> + .get_boot_addr = rproc_elf_get_boot_addr,
>> + .handle_rsc = mtk_apu_handle_rsc,
>> +};
>> +
>> +static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
>> +{
>> + struct rproc *rproc = data;
>> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
>> +
>> + writel(1, apu_rproc->base + CORE_XTENSA_INT);
>> +
>> + return IRQ_WAKE_THREAD;
>> +}
>> +
>> +static irqreturn_t handle_event(int irq, void *data)
>> +{
>> + struct rproc *rproc = data;
>> +
>> + rproc_vq_interrupt(rproc, 0);
>> + rproc_vq_interrupt(rproc, 1);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int mtk_apu_rproc_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct mtk_apu_rproc *apu_rproc;
>> + struct rproc *rproc;
>> + struct resource *res;
>> + int ret;
>> +
>> + rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
>> + sizeof(*apu_rproc));
>> + if (!rproc)
>> + return -ENOMEM;
>> +
>> + rproc->recovery_disabled = true;
>> + rproc->has_iommu = false;
>> + rproc->auto_boot = false;
>> +
>> + apu_rproc = rproc->priv;
>> + apu_rproc->dev = dev;
>> + INIT_LIST_HEAD(&apu_rproc->mappings);
>> +
>> + platform_set_drvdata(pdev, rproc);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + apu_rproc->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(apu_rproc->base)) {
>> + dev_err(dev, "Failed to map mmio\n");
>> + ret = PTR_ERR(apu_rproc->base);
>> + goto free_rproc;
>> + }
>> +
>> + apu_rproc->irq = platform_get_irq(pdev, 0);
>> + if (apu_rproc->irq < 0) {
>> + ret = apu_rproc->irq;
>> + goto free_rproc;
>> + }
>> +
>> + ret = devm_request_threaded_irq(dev, apu_rproc->irq,
>> + mtk_apu_rproc_callback, handle_event,
>> + IRQF_SHARED | IRQF_ONESHOT,
>> + NULL, rproc);
> I'm pretty sure I commented on that during my last review - is there a need to
> do a threaded irq here? As far as I can tell there is no advantage in doing so
> in this context. Please add a comment to justify using a threaded irq.
My apologize if I forgot to address a comment you made.
It used to be required before I moved to virtio / rpmsg to manage the
communication before
the CPU and the APU (the driver was doing a lot of thing in the callback).
Now, with the latest architecture, I am not sure if that is still
required since the callback is just
going through a list of request, copy rpmsg data and then notify DRM
that a request has been received.
Thanks,
Alexandre
>
> Thanks,
> Mathieu
>
>> + if (ret) {
>> + dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
>> + goto free_rproc;
>> + }
>> +
>> + apu_rproc->clks[0].id = "ipu";
>> + apu_rproc->clks[1].id = "axi";
>> + apu_rproc->clks[2].id = "jtag";
>> +
>> + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
>> + apu_rproc->clks);
>> + if (ret) {
>> + dev_err(dev, "Failed to get clocks\n");
>> + goto free_rproc;
>> + }
>> +
>> + ret = rproc_add(rproc);
>> + if (ret) {
>> + dev_err(dev, "rproc_add failed: %d\n", ret);
>> + goto free_rproc;
>> + }
>> +
>> + return 0;
>> +
>> +free_rproc:
>> + rproc_free(rproc);
>> +
>> + return ret;
>> +}
>> +
>> +static int mtk_apu_rproc_remove(struct platform_device *pdev)
>> +{
>> + struct rproc *rproc = platform_get_drvdata(pdev);
>> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
>> + struct device *dev = &pdev->dev;
>> +
>> + disable_irq(apu_rproc->irq);
>> +
>> + rproc_del(rproc);
>> + of_reserved_mem_device_release(dev);
>> + rproc_free(rproc);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id mtk_apu_rproc_of_match[] = {
>> + { .compatible = "mediatek,mt8183-apu", },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
>> +
>> +static struct platform_driver mtk_apu_rproc_driver = {
>> + .probe = mtk_apu_rproc_probe,
>> + .remove = mtk_apu_rproc_remove,
>> + .driver = {
>> + .name = "mtk_apu-rproc",
>> + .of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
>> + },
>> +};
>> +module_platform_driver(mtk_apu_rproc_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Alexandre Bailon");
>> +MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
>> --
>> 2.31.1
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU
@ 2021-09-08 8:21 ` Alexandre Bailon
0 siblings, 0 replies; 27+ messages in thread
From: Alexandre Bailon @ 2021-09-08 8:21 UTC (permalink / raw)
To: Mathieu Poirier
Cc: ohad, bjorn.andersson, robh+dt, matthias.bgg, linux-remoteproc,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
stephane.leprovost, gpain, khilman
Hi Mathieu,
Le 30/08/2021 à 20:19, Mathieu Poirier a écrit :
> Hi Alex,
>
> On Thu, Aug 19, 2021 at 05:13:38PM +0200, Alexandre Bailon wrote:
>> This adds a driver to control the APU present in the MT8183.
>> This loads the firmware and start the DSP.
>>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>> drivers/remoteproc/Kconfig | 10 +
>> drivers/remoteproc/Makefile | 1 +
>> drivers/remoteproc/mtk_apu.c | 440 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 451 insertions(+)
>> create mode 100644 drivers/remoteproc/mtk_apu.c
>>
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index 9a6eedc3994a5..6c106a6c3ad5d 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -53,6 +53,16 @@ config MTK_SCP
>>
>> It's safe to say N here.
>>
>> +config MTK_APU
>> + tristate "Mediatek APU remoteproc support"
>> + depends on ARCH_MEDIATEK
>> + depends on MTK_IOMMU
>> + help
>> + Say y to support the Mediatek's Accelerated Processing Unit (APU) via
> In the bindings A in APU is for AI or Vision whereas above it is for
> accelerated. Please pick one and stick with it.
Indeed, the A stands for AI so I missed that one.
>> + the remote processor framework.
>> +
>> + It's safe to say N here.
>> +
>> config OMAP_REMOTEPROC
>> tristate "OMAP remoteproc support"
>> depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
>> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
>> index bb26c9e4ef9cf..e395af86c17b0 100644
>> --- a/drivers/remoteproc/Makefile
>> +++ b/drivers/remoteproc/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV) += remoteproc_cdev.o
>> obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
>> obj-$(CONFIG_INGENIC_VPU_RPROC) += ingenic_rproc.o
>> obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
>> +obj-$(CONFIG_MTK_APU) += mtk_apu.o
>> obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
>> obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
>> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
>> diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
>> new file mode 100644
>> index 0000000000000..0e3f63987bd85
>> --- /dev/null
>> +++ b/drivers/remoteproc/mtk_apu.c
>> @@ -0,0 +1,440 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020 BayLibre SAS
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/iommu.h>
>> +#include <linux/irq.h>
>> +#include <linux/module.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/remoteproc.h>
>> +
>> +#include "remoteproc_internal.h"
>> +
>> +#define SW_RST (0x0000000C)
>> +#define SW_RST_OCD_HALT_ON_RST BIT(12)
>> +#define SW_RST_IPU_D_RST BIT(8)
>> +#define SW_RST_IPU_B_RST BIT(4)
>> +#define CORE_CTRL (0x00000110)
>> +#define CORE_CTRL_PDEBUG_ENABLE BIT(31)
>> +#define CORE_CTRL_SRAM_64K_iMEM (0x00 << 27)
>> +#define CORE_CTRL_SRAM_96K_iMEM (0x01 << 27)
>> +#define CORE_CTRL_SRAM_128K_iMEM (0x02 << 27)
>> +#define CORE_CTRL_SRAM_192K_iMEM (0x03 << 27)
>> +#define CORE_CTRL_SRAM_256K_iMEM (0x04 << 27)
>> +#define CORE_CTRL_PBCLK_ENABLE BIT(26)
>> +#define CORE_CTRL_RUN_STALL BIT(23)
>> +#define CORE_CTRL_STATE_VECTOR_SELECT BIT(19)
>> +#define CORE_CTRL_PIF_GATED BIT(17)
>> +#define CORE_CTRL_NMI BIT(0)
>> +#define CORE_XTENSA_INT (0x00000114)
>> +#define CORE_CTL_XTENSA_INT (0x00000118)
>> +#define CORE_DEFAULT0 (0x0000013C)
>> +#define CORE_DEFAULT0_QOS_SWAP_0 (0x00 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_1 (0x01 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_2 (0x02 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_3 (0x03 << 28)
>> +#define CORE_DEFAULT0_ARUSER_USE_IOMMU (0x10 << 23)
>> +#define CORE_DEFAULT0_AWUSER_USE_IOMMU (0x10 << 18)
>> +#define CORE_DEFAULT1 (0x00000140)
>> +#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU (0x10 << 0)
>> +#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU (0x10 << 5)
>> +#define CORE_XTENSA_ALTRESETVEC (0x000001F8)
>> +
>> +#define RSC_VENDOR_CARVEOUT (RSC_VENDOR_START + 1)
>> +
>> +#define APU_RESET_DELAY (27)
>> +
>> +struct mtk_apu_rproc {
>> + struct device *dev;
>> + void __iomem *base;
>> + int irq;
>> + struct clk_bulk_data clks[3];
>> + struct list_head mappings;
>> +};
>> +
>> +static int mtk_apu_iommu_map(struct rproc *rproc, struct rproc_mem_entry *entry)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + struct device *dev = rproc->dev.parent;
>> + struct rproc_mem_entry *mapping;
>> + struct iommu_domain *domain;
>> + int ret;
>> + u64 pa;
>> +
>> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
>> + if (!mapping)
>> + return -ENOMEM;
>> +
>> + if (!entry->va)
>> + pa = entry->dma;
>> + else
>> + pa = rproc_va_to_pa(entry->va);
>> +
>> + if ((strcmp(entry->name, "vdev0vring0") == 0 ||
>> + strcmp(entry->name, "vdev0vring1") == 0)) {
> If my guesswork is correct (see below) the above is required to deal with
> multiple remote processors. Why not just look for "vring0" and "vring1"?. That
> way we wouldn't have to keep the "vdev0" part for carveouts that really are
> "vdev1".
This makes sense. I will do that change.
>
>> + entry->va = memremap(entry->dma, entry->len, MEMREMAP_WC);
>> + if (IS_ERR_OR_NULL(entry->va)) {
>> + dev_err(dev, "Unable to map memory region: %pa+%lx\n",
>> + &entry->dma, entry->len);
>> + ret = PTR_ERR(mapping->va);
>> + goto free_mapping;
>> + }
>> + mapping->va = entry->va;
>> + }
>> +
>> + domain = iommu_get_domain_for_dev(dev);
> Here @domain can be NULL
>
>> + ret = iommu_map(domain, entry->da, pa, entry->len, entry->flags);
> ... and iommu_map() isn't dealing with that condition properly. Please
> check for a NULL condition before proceeding.
>
>> + if (ret) {
>> + dev_err(dev, "iommu_map failed: %d\n", ret);
>> + goto err_memunmap;
>> + }
>> +
>> + mapping->da = entry->da;
>> + mapping->len = entry->len;
>> + list_add_tail(&mapping->node, &apu_rproc->mappings);
>> +
>> + return 0;
>> +
>> +err_memunmap:
>> + memunmap(mapping->va);
>> +free_mapping:
>> + kfree(mapping);
>> +
>> + return ret;
>> +}
>> +
>> +static void mtk_apu_iommu_unmap_all(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + struct device *dev = rproc->dev.parent;
>> + struct rproc_mem_entry *entry, *tmp;
>> + struct iommu_domain *domain;
>> +
>> + /* clean up iommu mapping entries */
>> + list_for_each_entry_safe(entry, tmp, &apu_rproc->mappings, node) {
>> + size_t unmapped;
>> +
>> + domain = iommu_get_domain_for_dev(dev);
>> + unmapped = iommu_unmap(domain, entry->da, entry->len);
> Same as above
>
>> + if (unmapped != entry->len) {
>> + /* nothing much to do besides complaining */
>> + dev_err(dev, "failed to unmap %zx/%zu\n", entry->len,
>> + unmapped);
>> + }
>> + memunmap(entry->va);
>> +
>> + list_del(&entry->node);
>> + kfree(entry);
>> + }
>> +}
>> +
>> +static int mtk_apu_rproc_prepare(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + int ret;
>> +
>> + ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
>> + apu_rproc->clks);
>> + if (ret)
>> + dev_err(apu_rproc->dev, "Failed to enable clocks\n");
>> +
>> + return ret;
>> +}
>> +
>> +static int mtk_apu_rproc_unprepare(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +
>> + clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
>> + apu_rproc->clks);
>> +
>> + return 0;
>> +}
>> +
>> +static int mtk_apu_rproc_start(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + u32 core_ctrl;
>> +
>> + /* Set reset vector of APU firmware boot address */
>> + writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
>> +
>> + /* Turn on the clocks and stall the APU */
>> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
>> + core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
>> + CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
>> + CORE_CTRL_PIF_GATED;
>> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> + /* Reset the APU: this requires 27 ns to be effective on any platform */
>> + writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
>> + apu_rproc->base + SW_RST);
>> + ndelay(APU_RESET_DELAY);
>> + writel(0, apu_rproc->base + SW_RST);
>> +
>> + core_ctrl &= ~CORE_CTRL_PIF_GATED;
>> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> + /* Configure memory accesses to go through the IOMMU */
>> + writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
>> + CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
>> + writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
>> + CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
>> + apu_rproc->base + CORE_DEFAULT1);
>> +
>> + /* Release the APU */
>> + core_ctrl &= ~CORE_CTRL_RUN_STALL;
>> + writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> + return 0;
>> +}
>> +
>> +static int mtk_apu_rproc_stop(struct rproc *rproc)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> + u32 core_ctrl;
>> +
>> + core_ctrl = readl(apu_rproc->base + CORE_CTRL);
>> + writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
>> +
>> + mtk_apu_iommu_unmap_all(rproc);
>> +
>> + return 0;
>> +}
>> +
>> +static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
>> +{
>> + struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +
>> + writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
>> +}
>> +
>> +static int mtk_apu_load(struct rproc *rproc, const struct firmware *fw)
>> +
>> +{
>> + struct rproc_mem_entry *entry, *tmp;
>> + int ret;
>> +
>> + ret = rproc_elf_load_segments(rproc, fw);
>> + if (ret)
>> + return ret;
>> +
>> + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
>> + ret = mtk_apu_iommu_map(rproc, entry);
>> + if (ret)
>> + goto err_unmap_all;
>> + }
>> +
>> + return 0;
>> +
>> +err_unmap_all:
>> + mtk_apu_iommu_unmap_all(rproc);
>> +
>> + return ret;
>> +}
>> +
>> +static struct reserved_mem *of_reserved_mem_by_name(struct rproc *rproc,
>> + const char *name)
>> +{
>> + struct device *dev = rproc->dev.parent;
>> + struct device_node *np = dev->of_node;
>> + struct device_node *target;
>> + struct reserved_mem *rmem;
>> + int idx;
>> +
>> + idx = of_property_match_string(np, "memory-region-names", name);
>> + if (idx < 0) {
>> + dev_err(dev, "failed to find %s memory\n", name);
>> + return NULL;
>> + }
>> +
>> + target = of_parse_phandle(np, "memory-region", idx);
>> + if (!target)
>> + return NULL;
> There is no comment to explain the above gymnastic and as such I have to guess
> the firmware resources always have "vdev0" prepended to buffer, vring0 and
> vring1. If I am correct please add a comment to describe the situation, ditto
> if I am not correct.
OK, I will add much more documentation to explain.
>
>> +
>> + rmem = of_reserved_mem_lookup(target);
>> + if (!rmem)
>> + dev_err(dev, "unable to acquire memory-region\n");
>> + of_node_put(target);
>> +
>> + return rmem;
>> +}
>> +
>> +static int mtk_apu_handle_rsc(struct rproc *rproc, u32 rsc_type, void *rsc,
>> + int offset, int avail)
>> +{
>> + struct device *dev = rproc->dev.parent;
>> +
>> + if (rsc_type == RSC_VENDOR_CARVEOUT) {
>> + struct fw_rsc_carveout *rsc_carveout = rsc;
>> + struct rproc_mem_entry *mem;
>> + struct reserved_mem *rmem;
>> +
>> + rmem = of_reserved_mem_by_name(rproc, rsc_carveout->name);
> s/of_reserved_mem_by_name/mtk_of_reserved_mem_by_name
>
>> + if (!rmem)
>> + return -ENOMEM;
>> +
>> + if (rmem->size < rsc_carveout->len) {
> I think '!=' would be more appropriate than '<'.
>
>> + dev_err(dev, "The reserved memory is too small\n");
>> + return -ENOMEM;
>> + }
>> +
>> + mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base,
>> + rsc_carveout->len, rsc_carveout->da,
>> + NULL, NULL, rsc_carveout->name);
> Why not naming the carveout the way it is in the DT rather than keeping with
> what's in the firmware image?
The main reason is that we may load different firmware at runtime, with
very different memory layouts.
Some firmware only need few MB of memory whereas some other may requires
about 64 MB of memory.
Using the resource table here gives much more flexibility than DT that
is expected to be "static".
>
>> + if (!mem)
>> + return -ENOMEM;
>> +
>> + mem->flags = rsc_carveout->flags;
>> + rsc_carveout->pa = rmem->base;
>> + rproc_add_carveout(rproc, mem);
>> + }
>> +
>> + return RSC_HANDLED;
>> +}
>> +
>> +static const struct rproc_ops mtk_apu_rproc_ops = {
>> + .prepare = mtk_apu_rproc_prepare,
>> + .unprepare = mtk_apu_rproc_unprepare,
>> + .start = mtk_apu_rproc_start,
>> + .stop = mtk_apu_rproc_stop,
>> + .kick = mtk_apu_rproc_kick,
>> + .load = mtk_apu_load,
>> + .parse_fw = rproc_elf_load_rsc_table,
>> + .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
>> + .sanity_check = rproc_elf_sanity_check,
>> + .get_boot_addr = rproc_elf_get_boot_addr,
>> + .handle_rsc = mtk_apu_handle_rsc,
>> +};
>> +
>> +static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
>> +{
>> + struct rproc *rproc = data;
>> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
>> +
>> + writel(1, apu_rproc->base + CORE_XTENSA_INT);
>> +
>> + return IRQ_WAKE_THREAD;
>> +}
>> +
>> +static irqreturn_t handle_event(int irq, void *data)
>> +{
>> + struct rproc *rproc = data;
>> +
>> + rproc_vq_interrupt(rproc, 0);
>> + rproc_vq_interrupt(rproc, 1);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int mtk_apu_rproc_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct mtk_apu_rproc *apu_rproc;
>> + struct rproc *rproc;
>> + struct resource *res;
>> + int ret;
>> +
>> + rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
>> + sizeof(*apu_rproc));
>> + if (!rproc)
>> + return -ENOMEM;
>> +
>> + rproc->recovery_disabled = true;
>> + rproc->has_iommu = false;
>> + rproc->auto_boot = false;
>> +
>> + apu_rproc = rproc->priv;
>> + apu_rproc->dev = dev;
>> + INIT_LIST_HEAD(&apu_rproc->mappings);
>> +
>> + platform_set_drvdata(pdev, rproc);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + apu_rproc->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(apu_rproc->base)) {
>> + dev_err(dev, "Failed to map mmio\n");
>> + ret = PTR_ERR(apu_rproc->base);
>> + goto free_rproc;
>> + }
>> +
>> + apu_rproc->irq = platform_get_irq(pdev, 0);
>> + if (apu_rproc->irq < 0) {
>> + ret = apu_rproc->irq;
>> + goto free_rproc;
>> + }
>> +
>> + ret = devm_request_threaded_irq(dev, apu_rproc->irq,
>> + mtk_apu_rproc_callback, handle_event,
>> + IRQF_SHARED | IRQF_ONESHOT,
>> + NULL, rproc);
> I'm pretty sure I commented on that during my last review - is there a need to
> do a threaded irq here? As far as I can tell there is no advantage in doing so
> in this context. Please add a comment to justify using a threaded irq.
My apologize if I forgot to address a comment you made.
It used to be required before I moved to virtio / rpmsg to manage the
communication before
the CPU and the APU (the driver was doing a lot of thing in the callback).
Now, with the latest architecture, I am not sure if that is still
required since the callback is just
going through a list of request, copy rpmsg data and then notify DRM
that a request has been received.
Thanks,
Alexandre
>
> Thanks,
> Mathieu
>
>> + if (ret) {
>> + dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
>> + goto free_rproc;
>> + }
>> +
>> + apu_rproc->clks[0].id = "ipu";
>> + apu_rproc->clks[1].id = "axi";
>> + apu_rproc->clks[2].id = "jtag";
>> +
>> + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
>> + apu_rproc->clks);
>> + if (ret) {
>> + dev_err(dev, "Failed to get clocks\n");
>> + goto free_rproc;
>> + }
>> +
>> + ret = rproc_add(rproc);
>> + if (ret) {
>> + dev_err(dev, "rproc_add failed: %d\n", ret);
>> + goto free_rproc;
>> + }
>> +
>> + return 0;
>> +
>> +free_rproc:
>> + rproc_free(rproc);
>> +
>> + return ret;
>> +}
>> +
>> +static int mtk_apu_rproc_remove(struct platform_device *pdev)
>> +{
>> + struct rproc *rproc = platform_get_drvdata(pdev);
>> + struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
>> + struct device *dev = &pdev->dev;
>> +
>> + disable_irq(apu_rproc->irq);
>> +
>> + rproc_del(rproc);
>> + of_reserved_mem_device_release(dev);
>> + rproc_free(rproc);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id mtk_apu_rproc_of_match[] = {
>> + { .compatible = "mediatek,mt8183-apu", },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
>> +
>> +static struct platform_driver mtk_apu_rproc_driver = {
>> + .probe = mtk_apu_rproc_probe,
>> + .remove = mtk_apu_rproc_remove,
>> + .driver = {
>> + .name = "mtk_apu-rproc",
>> + .of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
>> + },
>> +};
>> +module_platform_driver(mtk_apu_rproc_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Alexandre Bailon");
>> +MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
>> --
>> 2.31.1
>>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 27+ messages in thread