2021-09-23 12:02:00

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 00/33] MT8195 IOMMU SUPPORT

This patchset adds MT8195 iommu support.

MT8195 have 3 IOMMU HWs. 2 IOMMU HW is for multimedia, and 1 IOMMU HW is
for infra-master, like PCIe/USB.

About the 2 MM IOMMU HW, something like this:

IOMMU(VDO) IOMMU(VPP)
| |
SMI_COMMON(VDO) SMI_COMMON(VPP)
--------------- ----------------
| | ... | | ...
larb0 larb2 ... larb1 larb3 ...

these two MM IOMMU HW share a pgtable.

About the INFRA IOMMU, it don't have larbs, the master connects the iommu
directly. It use a independent pgtable.

Also, mt8195 IOMMU bank supports. Normally the IOMMU register size only
is 0x1000. In this IOMMU HW, the register size is 5 * 0x1000. each 0x1000
is a bank. the banks' register look like this:
----------------------------------------
|bank0 | bank1 | bank2 | bank3 | bank4|
----------------------------------------
|global |
|control| null
|regs |
-----------------------------------------
|bank |bank |bank |bank |bank |
|regs |regs |regs |regs |regs |
| | | | | |
-----------------------------------------
All the banks share some global control registers, and each bank have its
special bank registers, like pgtable base register, tlb operation registers,
the fault status registers.

In mt8195, we enable this bank feature for infra iommu, We put PCIe in bank0
and USB in bank4. they have independent pgtable.

patch[1..23]: support mt8195 iommu.
patch[24..33]: support bank feature.

TODO: there is another APU_IOMMU in mt8195, this should depend on APU patches.
thus, we need add that feature after that.

Change note:
v3: 1) base on v5.15-rc1
2) Adjust devlink with smi-common, not use the property(sub-sommon).
3) Adjust tlb_flush_all flow,
a) Fix tlb_flush_all only is supported in bank0.
b) add tlb-flush-all in the resume callback.
c) remove the pm status checking in tlb-flush-all.
The reason are showed in the commit message.
4) Allow IOMMU_DOMAIN_UNMANAGED since PCIe VFIO use that.
5) Fix a clk warning and a null abort when unbind the iommu driver.

v2: https://lore.kernel.org/linux-mediatek/[email protected]/
1) Base on v5.14-rc1.
2) Fix build fail for arm32.
3) Fix dt-binding issue from Rob.
4) Fix the bank issue when tlb flush. v1 always use bank->base.
5) adjust devlink with smi-common since the node may be smi-sub-common.
6) other changes: like reword some commit message(removing many
"This patch..."); seperate serveral patches.

v1: https://lore.kernel.org/linux-mediatek/[email protected]/
Base on v5.13-rc1

Yong Wu (33):
dt-bindings: mediatek: mt8195: Add binding for MM IOMMU
dt-bindings: mediatek: mt8195: Add binding for infra IOMMU
iommu/mediatek: Fix 2 HW sharing pgtable issue
iommu/mediatek: Remove clk_disable in mtk_iommu_remove
iommu/mediatek: Adapt sharing and non-sharing pgtable case
iommu/mediatek: Add 12G~16G support for multi domains
iommu/mediatek: Add a flag DCM_DISABLE
iommu/mediatek: Add a flag NON_STD_AXI
iommu/mediatek: Remove for_each_m4u in tlb_sync_all
iommu/mediatek: Add tlb_lock in tlb_flush_all
iommu/mediatek: Remove the granule in the tlb flush
iommu/mediatek: Always tlb_flush_all when each PM resume
iommu/mediatek: Remove the power status checking in tlb flush all
iommu/mediatek: Always enable output PA over 32bits in isr
iommu/mediatek: Add SUB_COMMON_3BITS flag
iommu/mediatek: Add IOMMU_TYPE flag
iommu/mediatek: Contain MM IOMMU flow with the MM TYPE
iommu/mediatek: Adjust device link when it is sub-common
iommu/mediatek: Add list_del in mtk_iommu_remove
iommu/mediatek: Allow IOMMU_DOMAIN_UNMANAGED for PCIe VFIO
iommu/mediatek: Add infra iommu support
iommu/mediatek: Add PCIe support
iommu/mediatek: Add mt8195 support
iommu/mediatek: Only adjust code about register base
iommu/mediatek: Just move code position in hw_init
iommu/mediatek: Add mtk_iommu_bank_data structure
iommu/mediatek: Initialise bank HW for each a bank
iommu/mediatek: Add bank_nr and bank_enable
iommu/mediatek: Change the domid to iova_region_id
iommu/mediatek: Get the proper bankid for multi banks
iommu/mediatek: Initialise/Remove for multi bank dev
iommu/mediatek: Backup/restore regsiters for multi banks
iommu/mediatek: mt8195: Enable multi banks for infra iommu

.../bindings/iommu/mediatek,iommu.yaml | 20 +-
drivers/iommu/mtk_iommu.c | 799 ++++++++++++------
drivers/iommu/mtk_iommu.h | 56 +-
.../dt-bindings/memory/mt8195-memory-port.h | 408 +++++++++
include/dt-bindings/memory/mtk-memory-port.h | 2 +
5 files changed, 1014 insertions(+), 271 deletions(-)
create mode 100644 include/dt-bindings/memory/mt8195-memory-port.h

--
2.18.0



2021-09-23 12:02:23

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 06/33] iommu/mediatek: Add 12G~16G support for multi domains

In mt8192, we preassign 0-4G; 4G-8G; 8G-12G for different multimedia
engines. This depends on the "dma-ranges=" in the iommu consumer's dtsi
node.

Adds 12G-16G region here. and reword the previous comment. we don't limit
which master locate in which region.

CCU still is 8G-12G. Don't change it here.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index ee10c56e8f96..00d9ae3dc88a 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -178,10 +178,12 @@ static const struct mtk_iommu_iova_region single_domain[] = {
};

static const struct mtk_iommu_iova_region mt8192_multi_dom[] = {
- { .iova_base = 0x0, .size = SZ_4G}, /* disp: 0 ~ 4G */
+ { .iova_base = 0x0, .size = SZ_4G}, /* 0 ~ 4G */
#if IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT)
- { .iova_base = SZ_4G, .size = SZ_4G}, /* vdec: 4G ~ 8G */
- { .iova_base = SZ_4G * 2, .size = SZ_4G}, /* CAM/MDP: 8G ~ 12G */
+ { .iova_base = SZ_4G, .size = SZ_4G}, /* 4G ~ 8G */
+ { .iova_base = SZ_4G * 2, .size = SZ_4G}, /* 8G ~ 12G */
+ { .iova_base = SZ_4G * 3, .size = SZ_4G}, /* 12G ~ 16G */
+
{ .iova_base = 0x240000000ULL, .size = 0x4000000}, /* CCU0 */
{ .iova_base = 0x244000000ULL, .size = 0x4000000}, /* CCU1 */
#endif
--
2.18.0

2021-09-23 12:02:55

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 03/33] iommu/mediatek: Fix 2 HW sharing pgtable issue

In the commit 4f956c97d26b ("iommu/mediatek: Move domain_finalise into
attach_device"), I overlooked the sharing pgtable case.
After that commit, the "data" in the mtk_iommu_domain_finalise always is
the data of the current IOMMU HW. Fix this for the sharing pgtable case.

Only affect mt2712 which is the only SoC that share pgtable currently.

Fixes: 4f956c97d26b ("iommu/mediatek: Move domain_finalise into attach_device")
Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index d837adfd1da5..3e8f0164d26c 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -451,7 +451,7 @@ static void mtk_iommu_domain_free(struct iommu_domain *domain)
static int mtk_iommu_attach_device(struct iommu_domain *domain,
struct device *dev)
{
- struct mtk_iommu_data *data = dev_iommu_priv_get(dev);
+ struct mtk_iommu_data *data = dev_iommu_priv_get(dev), *frstdata;
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
struct device *m4udev = data->dev;
int ret, domid;
@@ -461,7 +461,10 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
return domid;

if (!dom->data) {
- if (mtk_iommu_domain_finalise(dom, data, domid))
+ /* Data is in the frstdata in sharing pgtable case. */
+ frstdata = mtk_iommu_get_m4u_data();
+
+ if (mtk_iommu_domain_finalise(dom, frstdata, domid))
return -ENODEV;
dom->data = data;
}
--
2.18.0

2021-09-23 12:03:15

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 05/33] iommu/mediatek: Adapt sharing and non-sharing pgtable case

In previous mt2712, Both IOMMUs are MM IOMMU, and they will share pgtable.
However in the latest SoC, another is infra IOMMU, there is no reason to
share pgtable between MM with INFRA IOMMU. This patch manage to
implement the two case(sharing and non-sharing pgtable).

Currently we use for_each_m4u to loop the 2 HWs. Add the list_head into
this macro.
In the sharing pgtable case, the list_head is the global "m4ulist".
In the non-sharing pgtable case, the list_head is hw_list_head which is a
variable in the "data". then for_each_m4u will only loop itself.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 47 ++++++++++++++++++++++-----------------
drivers/iommu/mtk_iommu.h | 7 ++++++
2 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 5c24e8e10a73..ee10c56e8f96 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -118,6 +118,7 @@
#define WR_THROT_EN BIT(6)
#define HAS_LEGACY_IVRP_PADDR BIT(7)
#define IOVA_34_EN BIT(8)
+#define SHARE_PGTABLE BIT(9) /* 2 HW share pgtable */

#define MTK_IOMMU_HAS_FLAG(pdata, _x) \
((((pdata)->flags) & (_x)) == (_x))
@@ -165,7 +166,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data);

static LIST_HEAD(m4ulist); /* List all the M4U HWs */

-#define for_each_m4u(data) list_for_each_entry(data, &m4ulist, list)
+#define for_each_m4u(data, head) list_for_each_entry(data, head, list)

struct mtk_iommu_iova_region {
dma_addr_t iova_base;
@@ -186,21 +187,10 @@ static const struct mtk_iommu_iova_region mt8192_multi_dom[] = {
#endif
};

-/*
- * There may be 1 or 2 M4U HWs, But we always expect they are in the same domain
- * for the performance.
- *
- * Here always return the mtk_iommu_data of the first probed M4U where the
- * iommu domain information is recorded.
- */
-static struct mtk_iommu_data *mtk_iommu_get_m4u_data(void)
+/* If 2 M4U share a domain(use the same hwlist), Put the corresponding info in first data.*/
+static struct mtk_iommu_data *mtk_iommu_get_frst_data(struct list_head *hwlist)
{
- struct mtk_iommu_data *data;
-
- for_each_m4u(data)
- return data;
-
- return NULL;
+ return list_first_entry(hwlist, struct mtk_iommu_data, list);
}

static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
@@ -210,7 +200,9 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)

static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
{
- for_each_m4u(data) {
+ struct list_head *head = data->hw_list;
+
+ for_each_m4u(data, head) {
if (pm_runtime_get_if_in_use(data->dev) <= 0)
continue;

@@ -227,12 +219,13 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
size_t granule,
struct mtk_iommu_data *data)
{
+ struct list_head *head = data->hw_list;
bool has_pm = !!data->dev->pm_domain;
unsigned long flags;
int ret;
u32 tmp;

- for_each_m4u(data) {
+ for_each_m4u(data, head) {
if (has_pm) {
if (pm_runtime_get_if_in_use(data->dev) <= 0)
continue;
@@ -453,6 +446,7 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
{
struct mtk_iommu_data *data = dev_iommu_priv_get(dev), *frstdata;
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+ struct list_head *hw_list = data->hw_list;
struct device *m4udev = data->dev;
int ret, domid;

@@ -462,7 +456,7 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,

if (!dom->data) {
/* Data is in the frstdata in sharing pgtable case. */
- frstdata = mtk_iommu_get_m4u_data();
+ frstdata = mtk_iommu_get_frst_data(hw_list);

if (mtk_iommu_domain_finalise(dom, frstdata, domid))
return -ENODEV;
@@ -584,10 +578,12 @@ static void mtk_iommu_release_device(struct device *dev)

static struct iommu_group *mtk_iommu_device_group(struct device *dev)
{
- struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
+ struct mtk_iommu_data *c_data = dev_iommu_priv_get(dev), *data;
+ struct list_head *hw_list = c_data->hw_list;
struct iommu_group *group;
int domid;

+ data = mtk_iommu_get_frst_data(hw_list);
if (!data)
return ERR_PTR(-ENODEV);

@@ -888,7 +884,15 @@ static int mtk_iommu_probe(struct platform_device *pdev)
goto out_sysfs_remove;

spin_lock_init(&data->tlb_lock);
- list_add_tail(&data->list, &m4ulist);
+
+ if (MTK_IOMMU_HAS_FLAG(data->plat_data, SHARE_PGTABLE)) {
+ list_add_tail(&data->list, data->plat_data->hw_list);
+ data->hw_list = data->plat_data->hw_list;
+ } else {
+ INIT_LIST_HEAD(&data->hw_list_head);
+ list_add_tail(&data->list, &data->hw_list_head);
+ data->hw_list = &data->hw_list_head;
+ }

if (!iommu_present(&platform_bus_type)) {
ret = bus_set_iommu(&platform_bus_type, &mtk_iommu_ops);
@@ -991,7 +995,8 @@ static const struct dev_pm_ops mtk_iommu_pm_ops = {

static const struct mtk_iommu_plat_data mt2712_data = {
.m4u_plat = M4U_MT2712,
- .flags = HAS_4GB_MODE | HAS_BCLK | HAS_VLD_PA_RNG,
+ .flags = HAS_4GB_MODE | HAS_BCLK | HAS_VLD_PA_RNG | SHARE_PGTABLE,
+ .hw_list = &m4ulist,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index f81fa8862ed0..027a42396557 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -55,6 +55,7 @@ struct mtk_iommu_plat_data {
u32 flags;
u32 inv_sel_reg;

+ struct list_head *hw_list;
unsigned int iova_region_nr;
const struct mtk_iommu_iova_region *iova_region;
unsigned char larbid_remap[MTK_LARB_COM_MAX][MTK_LARB_SUBCOM_MAX];
@@ -80,6 +81,12 @@ struct mtk_iommu_data {

struct dma_iommu_mapping *mapping; /* For mtk_iommu_v1.c */

+ /*
+ * In the sharing pgtable case, list data->list to the global list like m4ulist.
+ * In the non-sharing pgtable case, list data->list to the itself hw_list_head.
+ */
+ struct list_head *hw_list;
+ struct list_head hw_list_head;
struct list_head list;
struct mtk_smi_larb_iommu larb_imu[MTK_LARB_NR_MAX];
};
--
2.18.0

2021-09-23 12:03:44

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 10/33] iommu/mediatek: Add tlb_lock in tlb_flush_all

The tlb_flush_all also touches the registers about tlb operations.
Add spinlock in it to protect the tlb registers. since the tlb_range
already hold the spinlock, move it a bit outside the spinlock to
print log.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 0b4c30baa864..ab484d20b441 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -204,15 +204,24 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
return container_of(dom, struct mtk_iommu_domain, domain);
}

-static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
+static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data *data)
{
- if (pm_runtime_get_if_in_use(data->dev) <= 0)
- return;
+ unsigned long flags;

+ spin_lock_irqsave(&data->tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
data->base + data->plat_data->inv_sel_reg);
writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
wmb(); /* Make sure the tlb flush all done */
+ spin_unlock_irqrestore(&data->tlb_lock, flags);
+}
+
+static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
+{
+ if (pm_runtime_get_if_in_use(data->dev) <= 0)
+ return;
+
+ mtk_iommu_tlb_do_flush_all(data);

pm_runtime_put(data->dev);
}
@@ -247,14 +256,16 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
/* tlb sync */
ret = readl_poll_timeout_atomic(data->base + REG_MMU_CPE_DONE,
tmp, tmp != 0, 10, 1000);
+
+ /* Clear the CPE status */
+ writel_relaxed(0, data->base + REG_MMU_CPE_DONE);
+ spin_unlock_irqrestore(&data->tlb_lock, flags);
+
if (ret) {
dev_warn(data->dev,
"Partial TLB flush timed out, falling back to full flush\n");
- mtk_iommu_tlb_flush_all(data);
+ mtk_iommu_tlb_do_flush_all(data);
}
- /* Clear the CPE status */
- writel_relaxed(0, data->base + REG_MMU_CPE_DONE);
- spin_unlock_irqrestore(&data->tlb_lock, flags);

if (has_pm)
pm_runtime_put(data->dev);
--
2.18.0

2021-09-23 12:03:54

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 11/33] iommu/mediatek: Remove the granule in the tlb flush

The MediaTek IOMMU don't care about granule when tlb flushing.
Remove this variable.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index ab484d20b441..44cf5547d084 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -227,7 +227,6 @@ static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
}

static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
- size_t granule,
struct mtk_iommu_data *data)
{
struct list_head *head = data->hw_list;
@@ -541,8 +540,7 @@ static void mtk_iommu_iotlb_sync(struct iommu_domain *domain,
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
size_t length = gather->end - gather->start + 1;

- mtk_iommu_tlb_flush_range_sync(gather->start, length, gather->pgsize,
- dom->data);
+ mtk_iommu_tlb_flush_range_sync(gather->start, length, dom->data);
}

static void mtk_iommu_sync_map(struct iommu_domain *domain, unsigned long iova,
@@ -550,7 +548,7 @@ static void mtk_iommu_sync_map(struct iommu_domain *domain, unsigned long iova,
{
struct mtk_iommu_domain *dom = to_mtk_domain(domain);

- mtk_iommu_tlb_flush_range_sync(iova, size, size, dom->data);
+ mtk_iommu_tlb_flush_range_sync(iova, size, dom->data);
}

static phys_addr_t mtk_iommu_iova_to_phys(struct iommu_domain *domain,
--
2.18.0

2021-09-23 12:04:11

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 07/33] iommu/mediatek: Add a flag DCM_DISABLE

In the infra iommu, we should disable DCM. add a new flag for this.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 00d9ae3dc88a..4e1b01fd58b0 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -51,6 +51,8 @@
#define F_MMU_STANDARD_AXI_MODE_MASK (BIT(3) | BIT(19))

#define REG_MMU_DCM_DIS 0x050
+#define F_MMU_DCM BIT(8)
+
#define REG_MMU_WR_LEN_CTRL 0x054
#define F_MMU_WR_THROT_DIS_MASK (BIT(5) | BIT(21))

@@ -119,6 +121,7 @@
#define HAS_LEGACY_IVRP_PADDR BIT(7)
#define IOVA_34_EN BIT(8)
#define SHARE_PGTABLE BIT(9) /* 2 HW share pgtable */
+#define DCM_DISABLE BIT(10)

#define MTK_IOMMU_HAS_FLAG(pdata, _x) \
((((pdata)->flags) & (_x)) == (_x))
@@ -722,7 +725,11 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
regval = F_MMU_VLD_PA_RNG(7, 4);
writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
}
- writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
+ if (MTK_IOMMU_HAS_FLAG(data->plat_data, DCM_DISABLE))
+ writel_relaxed(F_MMU_DCM, data->base + REG_MMU_DCM_DIS);
+ else
+ writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
+
if (MTK_IOMMU_HAS_FLAG(data->plat_data, WR_THROT_EN)) {
/* write command throttling mode */
regval = readl_relaxed(data->base + REG_MMU_WR_LEN_CTRL);
--
2.18.0

2021-09-23 12:04:11

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 14/33] iommu/mediatek: Always enable output PA over 32bits in isr

Currently the output PA[32:33] is contained by the flag IOVA_34.
This is not right. the iova_34 has no relation with pa[32:33], the 32bits
iova still could map to pa[32:33]. Move it out from the flag.

No need fix tag since currently only mt8192 use the calulation and it
always has this IOVA_34 flag.

Prepare for the IOMMU that still use IOVA 32bits but its dram size may be
over 4GB.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 4a33b6c6b1db..ef98f19ce86e 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -289,11 +289,11 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
if (MTK_IOMMU_HAS_FLAG(data->plat_data, IOVA_34_EN)) {
va34_32 = FIELD_GET(F_MMU_INVAL_VA_34_32_MASK, fault_iova);
- pa34_32 = FIELD_GET(F_MMU_INVAL_PA_34_32_MASK, fault_iova);
fault_iova = fault_iova & F_MMU_INVAL_VA_31_12_MASK;
fault_iova |= (u64)va34_32 << 32;
- fault_pa |= (u64)pa34_32 << 32;
}
+ pa34_32 = FIELD_GET(F_MMU_INVAL_PA_34_32_MASK, fault_iova);
+ fault_pa |= (u64)pa34_32 << 32;

fault_port = F_MMU_INT_ID_PORT_ID(regval);
if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM)) {
--
2.18.0

2021-09-23 12:04:46

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 18/33] iommu/mediatek: Adjust device link when it is sub-common

For MM IOMMU, We always add device link between smi-common and IOMMU HW.
In mt8195, we add smi-sub-common. Thus, if the node is sub-common, we still
need find again to get smi-common, then do device link.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2ebbb3412cb9..faea4a0d922f 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -786,9 +786,9 @@ static int mtk_iommu_mm_dts_parse(struct device *dev,
struct component_match **match,
struct mtk_iommu_data *data)
{
+ struct device_node *larbnode, *smicomm_node, *smi_subcomm_node;
struct platform_device *plarbdev;
struct device_link *link;
- struct device_node *larbnode, *smicomm_node;
int i, larb_nr, ret;

larb_nr = of_count_phandle_with_args(dev->of_node, "mediatek,larbs", NULL);
@@ -822,11 +822,21 @@ static int mtk_iommu_mm_dts_parse(struct device *dev,
compare_of, larbnode);
}

- /* Get smi-common dev from the last larb. */
- smicomm_node = of_parse_phandle(larbnode, "mediatek,smi", 0);
- if (!smicomm_node)
+ /* Get smi-(sub)-common dev from the last larb. */
+ smi_subcomm_node = of_parse_phandle(larbnode, "mediatek,smi", 0);
+ if (!smi_subcomm_node)
return -EINVAL;

+ /*
+ * It may have two level smi-common. the node is smi-sub-common if it
+ * has a new mediatek,smi property. otherwise it is smi-commmon.
+ */
+ smicomm_node = of_parse_phandle(smi_subcomm_node, "mediatek,smi", 0);
+ if (smicomm_node)
+ of_node_put(smi_subcomm_node);
+ else
+ smicomm_node = smi_subcomm_node;
+
plarbdev = of_find_device_by_node(smicomm_node);
of_node_put(smicomm_node);
data->smicomm_dev = &plarbdev->dev;
--
2.18.0

2021-09-23 12:04:46

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 09/33] iommu/mediatek: Remove for_each_m4u in tlb_sync_all

The tlb_sync_all is called from these three functions:
a) flush_iotlb_all: it will be called for each a iommu HW.
b) tlb_flush_range_sync: it already has for_each_m4u.
c) in irq: When IOMMU HW translation fault, Only need flush itself.

Thus, No need for_each_m4u in this tlb_sync_all. Remove it.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6f4f6624e3ac..0b4c30baa864 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -206,19 +206,15 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)

static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
{
- struct list_head *head = data->hw_list;
-
- for_each_m4u(data, head) {
- if (pm_runtime_get_if_in_use(data->dev) <= 0)
- continue;
+ if (pm_runtime_get_if_in_use(data->dev) <= 0)
+ return;

- writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
- data->base + data->plat_data->inv_sel_reg);
- writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
- wmb(); /* Make sure the tlb flush all done */
+ writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
+ data->base + data->plat_data->inv_sel_reg);
+ writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
+ wmb(); /* Make sure the tlb flush all done */

- pm_runtime_put(data->dev);
- }
+ pm_runtime_put(data->dev);
}

static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
--
2.18.0

2021-09-23 12:05:03

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 21/33] iommu/mediatek: Add infra iommu support

The infra iommu enable bits in mt8195 is in the pericfg register segment,
use regmap to update it.

If infra iommu master translation fault, It don't have the larbid/portid,
thus print out the whole register value.

Since regmap_update_bits may fail, add return value for mtk_iommu_config.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 36 +++++++++++++++++++++++++++++-------
drivers/iommu/mtk_iommu.h | 2 ++
2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index d103e4f33078..37d6dfb4feab 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -112,6 +112,8 @@

#define MTK_PROTECT_PA_ALIGN 256

+#define PERICFG_IOMMU_1 0x714
+
#define HAS_4GB_MODE BIT(0)
/* HW will use the EMI clock if there isn't the "bclk". */
#define HAS_BCLK BIT(1)
@@ -324,8 +326,8 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) {
dev_err_ratelimited(
data->dev,
- "fault type=0x%x iova=0x%llx pa=0x%llx larb=%d port=%d layer=%d %s\n",
- int_state, fault_iova, fault_pa, fault_larb, fault_port,
+ "fault type=0x%x iova=0x%llx pa=0x%llx master=0x%x(larb=%d port=%d) layer=%d %s\n",
+ int_state, fault_iova, fault_pa, regval, fault_larb, fault_port,
layer, write ? "write" : "read");
}

@@ -369,14 +371,15 @@ static int mtk_iommu_get_domain_id(struct device *dev,
return -EINVAL;
}

-static void mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,
- bool enable, unsigned int domid)
+static int mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,
+ bool enable, unsigned int domid)
{
struct mtk_smi_larb_iommu *larb_mmu;
unsigned int larbid, portid;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
const struct mtk_iommu_iova_region *region;
- int i;
+ u32 peri_mmuen, peri_mmuen_msk;
+ int i, ret = 0;

for (i = 0; i < fwspec->num_ids; ++i) {
larbid = MTK_M4U_TO_LARB(fwspec->ids[i]);
@@ -396,8 +399,19 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,
larb_mmu->mmu |= MTK_SMI_MMU_EN(portid);
else
larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid);
+ } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA)) {
+ peri_mmuen_msk = BIT(portid);
+ peri_mmuen = enable ? peri_mmuen_msk : 0;
+
+ ret = regmap_update_bits(data->pericfg, PERICFG_IOMMU_1,
+ peri_mmuen_msk, peri_mmuen);
+ if (ret)
+ dev_err(dev, "%s iommu(%s) inframaster 0x%x fail(%d).\n",
+ enable ? "enable" : "disable",
+ dev_name(data->dev), peri_mmuen_msk, ret);
}
}
+ return ret;
}

static int mtk_iommu_domain_finalise(struct mtk_iommu_domain *dom,
@@ -504,8 +518,7 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
pm_runtime_put(m4udev);
}

- mtk_iommu_config(data, dev, true, domid);
- return 0;
+ return mtk_iommu_config(data, dev, true, domid);
}

static void mtk_iommu_detach_device(struct iommu_domain *domain,
@@ -921,6 +934,15 @@ static int mtk_iommu_probe(struct platform_device *pdev)
ret = mtk_iommu_mm_dts_parse(dev, &match, data);
if (ret)
goto out_runtime_disable;
+ } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
+ data->plat_data->pericfg_comp_str) {
+ infracfg = syscon_regmap_lookup_by_compatible(data->plat_data->pericfg_comp_str);
+ if (IS_ERR(infracfg)) {
+ ret = PTR_ERR(infracfg);
+ goto out_runtime_disable;
+ }
+
+ data->pericfg = infracfg;
}

platform_set_drvdata(pdev, data);
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 5b32277fee99..d83c79bf800a 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -55,6 +55,7 @@ struct mtk_iommu_plat_data {
u32 flags;
u32 inv_sel_reg;

+ char *pericfg_comp_str;
struct list_head *hw_list;
unsigned int iova_region_nr;
const struct mtk_iommu_iova_region *iova_region;
@@ -80,6 +81,7 @@ struct mtk_iommu_data {
struct device *smicomm_dev;

struct dma_iommu_mapping *mapping; /* For mtk_iommu_v1.c */
+ struct regmap *pericfg;

/*
* In the sharing pgtable case, list data->list to the global list like m4ulist.
--
2.18.0

2021-09-23 12:05:18

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 23/33] iommu/mediatek: Add mt8195 support

mt8195 has 3 IOMMU, containing 2 MM IOMMUs, one is for vdo, the other
is for vpp. and 1 INFRA IOMMU.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 42 +++++++++++++++++++++++++++++++++++++++
drivers/iommu/mtk_iommu.h | 1 +
2 files changed, 43 insertions(+)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 3f1fd8036345..df823ee3541a 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1163,6 +1163,45 @@ static const struct mtk_iommu_plat_data mt8192_data = {
{0, 14, 16}, {0, 13, 18, 17}},
};

+static const struct mtk_iommu_plat_data mt8195_data_infra = {
+ .m4u_plat = M4U_MT8195,
+ .flags = WR_THROT_EN | DCM_DISABLE |
+ MTK_IOMMU_TYPE_INFRA | IFA_IOMMU_PCIe_SUPPORT,
+ .pericfg_comp_str = "mediatek,mt8195-pericfg_ao",
+ .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
+ .iova_region = single_domain,
+ .iova_region_nr = ARRAY_SIZE(single_domain),
+};
+
+static const struct mtk_iommu_plat_data mt8195_data_vdo = {
+ .m4u_plat = M4U_MT8195,
+ .flags = HAS_BCLK | HAS_SUB_COMM_2BITS | OUT_ORDER_WR_EN |
+ WR_THROT_EN | NOT_STD_AXI_MODE | IOVA_34_EN |
+ SHARE_PGTABLE | MTK_IOMMU_TYPE_MM,
+ .hw_list = &m4ulist,
+ .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
+ .iova_region = mt8192_multi_dom,
+ .iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
+ .larbid_remap = {{2, 0}, {21}, {24}, {7}, {19}, {9, 10, 11},
+ {13, 17, 15/* 17b */, 25}, {5}},
+};
+
+static const struct mtk_iommu_plat_data mt8195_data_vpp = {
+ .m4u_plat = M4U_MT8195,
+ .flags = HAS_BCLK | HAS_SUB_COMM_3BITS | OUT_ORDER_WR_EN |
+ WR_THROT_EN | NOT_STD_AXI_MODE | IOVA_34_EN |
+ SHARE_PGTABLE | MTK_IOMMU_TYPE_MM,
+ .hw_list = &m4ulist,
+ .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
+ .iova_region = mt8192_multi_dom,
+ .iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
+ .larbid_remap = {{1}, {3}, {22, 0, 0, 0, 23}, {8},
+ {20}, {12},
+ /* 16: 16a; 29: 16b; 30: CCUtop0; 31: CCUtop1 */
+ {14, 16, 29, 26, 30, 31, 18},
+ {4, 0, 0, 0, 6}},
+};
+
static const struct of_device_id mtk_iommu_of_ids[] = {
{ .compatible = "mediatek,mt2712-m4u", .data = &mt2712_data},
{ .compatible = "mediatek,mt6779-m4u", .data = &mt6779_data},
@@ -1170,6 +1209,9 @@ static const struct of_device_id mtk_iommu_of_ids[] = {
{ .compatible = "mediatek,mt8173-m4u", .data = &mt8173_data},
{ .compatible = "mediatek,mt8183-m4u", .data = &mt8183_data},
{ .compatible = "mediatek,mt8192-m4u", .data = &mt8192_data},
+ { .compatible = "mediatek,mt8195-iommu-infra", .data = &mt8195_data_infra},
+ { .compatible = "mediatek,mt8195-iommu-vdo", .data = &mt8195_data_vdo},
+ { .compatible = "mediatek,mt8195-iommu-vpp", .data = &mt8195_data_vpp},
{}
};

diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index d83c79bf800a..a9dc79c5a724 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -46,6 +46,7 @@ enum mtk_iommu_plat {
M4U_MT8173,
M4U_MT8183,
M4U_MT8192,
+ M4U_MT8195,
};

struct mtk_iommu_iova_region;
--
2.18.0

2021-09-23 12:05:26

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 08/33] iommu/mediatek: Add a flag NON_STD_AXI

Add a new flag NON_STD_AXI, All the previous SoC support this flag.
Prepare for adding infra and apu iommu which don't support this.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 4e1b01fd58b0..6f4f6624e3ac 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -122,6 +122,7 @@
#define IOVA_34_EN BIT(8)
#define SHARE_PGTABLE BIT(9) /* 2 HW share pgtable */
#define DCM_DISABLE BIT(10)
+#define NOT_STD_AXI_MODE BIT(11)

#define MTK_IOMMU_HAS_FLAG(pdata, _x) \
((((pdata)->flags) & (_x)) == (_x))
@@ -742,7 +743,8 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
regval = 0;
} else {
regval = readl_relaxed(data->base + REG_MMU_MISC_CTRL);
- regval &= ~F_MMU_STANDARD_AXI_MODE_MASK;
+ if (MTK_IOMMU_HAS_FLAG(data->plat_data, NOT_STD_AXI_MODE))
+ regval &= ~F_MMU_STANDARD_AXI_MODE_MASK;
if (MTK_IOMMU_HAS_FLAG(data->plat_data, OUT_ORDER_WR_EN))
regval &= ~F_MMU_IN_ORDER_WR_EN_MASK;
}
@@ -1004,7 +1006,8 @@ static const struct dev_pm_ops mtk_iommu_pm_ops = {

static const struct mtk_iommu_plat_data mt2712_data = {
.m4u_plat = M4U_MT2712,
- .flags = HAS_4GB_MODE | HAS_BCLK | HAS_VLD_PA_RNG | SHARE_PGTABLE,
+ .flags = HAS_4GB_MODE | HAS_BCLK | HAS_VLD_PA_RNG | SHARE_PGTABLE |
+ NOT_STD_AXI_MODE,
.hw_list = &m4ulist,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.iova_region = single_domain,
@@ -1014,7 +1017,8 @@ static const struct mtk_iommu_plat_data mt2712_data = {

static const struct mtk_iommu_plat_data mt6779_data = {
.m4u_plat = M4U_MT6779,
- .flags = HAS_SUB_COMM | OUT_ORDER_WR_EN | WR_THROT_EN,
+ .flags = HAS_SUB_COMM | OUT_ORDER_WR_EN | WR_THROT_EN |
+ NOT_STD_AXI_MODE,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
@@ -1023,7 +1027,7 @@ static const struct mtk_iommu_plat_data mt6779_data = {

static const struct mtk_iommu_plat_data mt8167_data = {
.m4u_plat = M4U_MT8167,
- .flags = RESET_AXI | HAS_LEGACY_IVRP_PADDR,
+ .flags = RESET_AXI | HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
@@ -1033,7 +1037,7 @@ static const struct mtk_iommu_plat_data mt8167_data = {
static const struct mtk_iommu_plat_data mt8173_data = {
.m4u_plat = M4U_MT8173,
.flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI |
- HAS_LEGACY_IVRP_PADDR,
+ HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
@@ -1052,7 +1056,7 @@ static const struct mtk_iommu_plat_data mt8183_data = {
static const struct mtk_iommu_plat_data mt8192_data = {
.m4u_plat = M4U_MT8192,
.flags = HAS_BCLK | HAS_SUB_COMM | OUT_ORDER_WR_EN |
- WR_THROT_EN | IOVA_34_EN,
+ WR_THROT_EN | IOVA_34_EN | NOT_STD_AXI_MODE,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
.iova_region = mt8192_multi_dom,
.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
--
2.18.0

2021-09-23 12:05:37

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 25/33] iommu/mediatek: Just move code position in hw_init

No functional change too, prepare for mt8195 IOMMU support bank functions.
Some global control settings are in bank0 while the other banks have
their bank independent setting. Here only move the global control
settings and the independent registers together.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 48 +++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 725e000eee0f..ac31fe8b7aaf 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -731,30 +731,6 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
}
writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);

- regval = F_L2_MULIT_HIT_EN |
- F_TABLE_WALK_FAULT_INT_EN |
- F_PREETCH_FIFO_OVERFLOW_INT_EN |
- F_MISS_FIFO_OVERFLOW_INT_EN |
- F_PREFETCH_FIFO_ERR_INT_EN |
- F_MISS_FIFO_ERR_INT_EN;
- writel_relaxed(regval, data->base + REG_MMU_INT_CONTROL0);
-
- regval = F_INT_TRANSLATION_FAULT |
- F_INT_MAIN_MULTI_HIT_FAULT |
- F_INT_INVALID_PA_FAULT |
- F_INT_ENTRY_REPLACEMENT_FAULT |
- F_INT_TLB_MISS_FAULT |
- F_INT_MISS_TRANSACTION_FIFO_FAULT |
- F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
- writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
-
- if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_LEGACY_IVRP_PADDR))
- regval = (data->protect_base >> 1) | (data->enable_4GB << 31);
- else
- regval = lower_32_bits(data->protect_base) |
- upper_32_bits(data->protect_base);
- writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
-
if (data->enable_4GB &&
MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_VLD_PA_RNG)) {
/*
@@ -788,6 +764,30 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
}
writel_relaxed(regval, data->base + REG_MMU_MISC_CTRL);

+ regval = F_L2_MULIT_HIT_EN |
+ F_TABLE_WALK_FAULT_INT_EN |
+ F_PREETCH_FIFO_OVERFLOW_INT_EN |
+ F_MISS_FIFO_OVERFLOW_INT_EN |
+ F_PREFETCH_FIFO_ERR_INT_EN |
+ F_MISS_FIFO_ERR_INT_EN;
+ writel_relaxed(regval, data->base + REG_MMU_INT_CONTROL0);
+
+ regval = F_INT_TRANSLATION_FAULT |
+ F_INT_MAIN_MULTI_HIT_FAULT |
+ F_INT_INVALID_PA_FAULT |
+ F_INT_ENTRY_REPLACEMENT_FAULT |
+ F_INT_TLB_MISS_FAULT |
+ F_INT_MISS_TRANSACTION_FIFO_FAULT |
+ F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
+ writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
+
+ if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_LEGACY_IVRP_PADDR))
+ regval = (data->protect_base >> 1) | (data->enable_4GB << 31);
+ else
+ regval = lower_32_bits(data->protect_base) |
+ upper_32_bits(data->protect_base);
+ writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
+
if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
dev_name(data->dev), (void *)data)) {
writel_relaxed(0, data->base + REG_MMU_PT_BASE_ADDR);
--
2.18.0

2021-09-23 12:05:49

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 26/33] iommu/mediatek: Add mtk_iommu_bank_data structure

Prepare for supporting multi-banks for the IOMMU HW, No functional change.

Add a new structure(mtk_iommu_bank_data) for each a bank. Each a bank have
the independent HW base/IRQ/tlb-range ops, and each a bank has its special
iommu-domain(independent pgtable), thus, also move the domain information
into it.

In previous SoC, we have only one bank which could be treated as bank0(
bankid always is 0 for the previous SoC).

After adding this structure, the tlb operations and irq could use
bank_data as parameter.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 137 ++++++++++++++++++++++----------------
drivers/iommu/mtk_iommu.h | 24 ++++++-
2 files changed, 99 insertions(+), 62 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index ac31fe8b7aaf..6ef3eb3cad92 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -146,7 +146,7 @@ struct mtk_iommu_domain {
struct io_pgtable_cfg cfg;
struct io_pgtable_ops *iop;

- struct mtk_iommu_data *data;
+ struct mtk_iommu_bank_data *bank;
struct iommu_domain domain;
};

@@ -221,25 +221,29 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)

static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
{
- void __iomem *base = data->base;
+ /* Tlb flush all always is in bank0. */
+ struct mtk_iommu_bank_data *bank = &data->bank[0];
+ void __iomem *base = bank->base;
unsigned long flags;

/*
* No need get power status since the HW PM status nearly is active
* when entering here.
*/
- spin_lock_irqsave(&data->tlb_lock, flags);
+ spin_lock_irqsave(&bank->tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + data->plat_data->inv_sel_reg);
writel_relaxed(F_ALL_INVLD, base + REG_MMU_INVALIDATE);
wmb(); /* Make sure the tlb flush all done */
- spin_unlock_irqrestore(&data->tlb_lock, flags);
+ spin_unlock_irqrestore(&bank->tlb_lock, flags);
}

static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
- struct mtk_iommu_data *data)
+ struct mtk_iommu_bank_data *bank)
{
- struct list_head *head = data->hw_list;
- bool has_pm = !!data->dev->pm_domain;
+ struct list_head *head = bank->pdata->hw_list;
+ bool has_pm = !!bank->pdev->pm_domain;
+ struct mtk_iommu_bank_data *curbank;
+ struct mtk_iommu_data *data;
unsigned long flags;
void __iomem *base;
int ret;
@@ -251,9 +255,10 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
continue;
}

- base = data->base;
+ curbank = &data->bank[bank->id];
+ base = curbank->base;

- spin_lock_irqsave(&data->tlb_lock, flags);
+ spin_lock_irqsave(&curbank->tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
base + data->plat_data->inv_sel_reg);

@@ -268,7 +273,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,

/* Clear the CPE status */
writel_relaxed(0, base + REG_MMU_CPE_DONE);
- spin_unlock_irqrestore(&data->tlb_lock, flags);
+ spin_unlock_irqrestore(&curbank->tlb_lock, flags);

if (ret) {
dev_warn(data->dev,
@@ -283,12 +288,13 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,

static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
{
- struct mtk_iommu_data *data = dev_id;
- struct mtk_iommu_domain *dom = data->m4u_dom;
+ struct mtk_iommu_bank_data *bank = dev_id;
+ struct mtk_iommu_data *data = bank->pdata;
+ struct mtk_iommu_domain *dom = bank->m4u_dom;
unsigned int fault_larb = 0, fault_port = 0, sub_comm = 0;
u32 int_state, regval, va34_32, pa34_32;
const struct mtk_iommu_plat_data *plat_data = data->plat_data;
- void __iomem *base = data->base;
+ void __iomem *base = bank->base;
u64 fault_iova, fault_pa;
bool layer, write;

@@ -327,10 +333,10 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
fault_larb = data->plat_data->larbid_remap[fault_larb][sub_comm];
}

- if (report_iommu_fault(&dom->domain, data->dev, fault_iova,
+ if (report_iommu_fault(&dom->domain, bank->pdev, fault_iova,
write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) {
dev_err_ratelimited(
- data->dev,
+ bank->pdev,
"fault type=0x%x iova=0x%llx pa=0x%llx master=0x%x(larb=%d port=%d) layer=%d %s\n",
int_state, fault_iova, fault_pa, regval, fault_larb, fault_port,
layer, write ? "write" : "read");
@@ -427,12 +433,14 @@ static int mtk_iommu_domain_finalise(struct mtk_iommu_domain *dom,
unsigned int domid)
{
const struct mtk_iommu_iova_region *region;
-
- /* Use the exist domain as there is only one pgtable here. */
- if (data->m4u_dom) {
- dom->iop = data->m4u_dom->iop;
- dom->cfg = data->m4u_dom->cfg;
- dom->domain.pgsize_bitmap = data->m4u_dom->cfg.pgsize_bitmap;
+ struct mtk_iommu_domain *m4u_dom;
+
+ /* Always use bank0 in sharing pgtable case */
+ m4u_dom = data->bank[0].m4u_dom;
+ if (m4u_dom) {
+ dom->iop = m4u_dom->iop;
+ dom->cfg = m4u_dom->cfg;
+ dom->domain.pgsize_bitmap = m4u_dom->cfg.pgsize_bitmap;
goto update_iova_region;
}

@@ -493,23 +501,26 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
struct mtk_iommu_data *data = dev_iommu_priv_get(dev), *frstdata;
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
struct list_head *hw_list = data->hw_list;
+ struct mtk_iommu_bank_data *bank;
struct device *m4udev = data->dev;
+ unsigned int bankid = 0;
int ret, domid;

domid = mtk_iommu_get_domain_id(dev, data->plat_data);
if (domid < 0)
return domid;

- if (!dom->data) {
+ bank = &data->bank[bankid];
+ if (!dom->bank) {
/* Data is in the frstdata in sharing pgtable case. */
frstdata = mtk_iommu_get_frst_data(hw_list);

if (mtk_iommu_domain_finalise(dom, frstdata, domid))
return -ENODEV;
- dom->data = data;
+ dom->bank = bank;
}

- if (!data->m4u_dom) { /* Initialize the M4U HW */
+ if (!bank->m4u_dom) { /* Initialize the M4U HW */
ret = pm_runtime_resume_and_get(m4udev);
if (ret < 0)
return ret;
@@ -519,9 +530,9 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
pm_runtime_put(m4udev);
return ret;
}
- data->m4u_dom = dom;
+ bank->m4u_dom = dom;
writel(dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
- data->base + REG_MMU_PT_BASE_ADDR);
+ bank->base + REG_MMU_PT_BASE_ADDR);

pm_runtime_put(m4udev);
}
@@ -543,7 +554,7 @@ static int mtk_iommu_map(struct iommu_domain *domain, unsigned long iova,
struct mtk_iommu_domain *dom = to_mtk_domain(domain);

/* The "4GB mode" M4U physically can not use the lower remap of Dram. */
- if (dom->data->enable_4GB)
+ if (dom->bank->pdata->enable_4GB)
paddr |= BIT_ULL(32);

/* Synchronize with the tlb_lock */
@@ -564,7 +575,7 @@ static void mtk_iommu_flush_iotlb_all(struct iommu_domain *domain)
{
struct mtk_iommu_domain *dom = to_mtk_domain(domain);

- mtk_iommu_tlb_flush_all(dom->data);
+ mtk_iommu_tlb_flush_all(dom->bank->pdata);
}

static void mtk_iommu_iotlb_sync(struct iommu_domain *domain,
@@ -573,7 +584,7 @@ static void mtk_iommu_iotlb_sync(struct iommu_domain *domain,
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
size_t length = gather->end - gather->start + 1;

- mtk_iommu_tlb_flush_range_sync(gather->start, length, dom->data);
+ mtk_iommu_tlb_flush_range_sync(gather->start, length, dom->bank);
}

static void mtk_iommu_sync_map(struct iommu_domain *domain, unsigned long iova,
@@ -581,7 +592,7 @@ static void mtk_iommu_sync_map(struct iommu_domain *domain, unsigned long iova,
{
struct mtk_iommu_domain *dom = to_mtk_domain(domain);

- mtk_iommu_tlb_flush_range_sync(iova, size, dom->data);
+ mtk_iommu_tlb_flush_range_sync(iova, size, dom->bank);
}

static phys_addr_t mtk_iommu_iova_to_phys(struct iommu_domain *domain,
@@ -591,7 +602,7 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct iommu_domain *domain,
phys_addr_t pa;

pa = dom->iop->iova_to_phys(dom->iop, iova);
- if (dom->data->enable_4GB && pa >= MTK_IOMMU_4GB_MODE_REMAP_BASE)
+ if (dom->bank->pdata->enable_4GB && pa >= MTK_IOMMU_4GB_MODE_REMAP_BASE)
pa &= ~BIT_ULL(32);

return pa;
@@ -720,16 +731,17 @@ static const struct iommu_ops mtk_iommu_ops = {

static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
{
+ const struct mtk_iommu_bank_data *bank0 = &data->bank[0];
u32 regval;

if (data->plat_data->m4u_plat == M4U_MT8173) {
regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173;
} else {
- regval = readl_relaxed(data->base + REG_MMU_CTRL_REG);
+ regval = readl_relaxed(bank0->base + REG_MMU_CTRL_REG);
regval |= F_MMU_TF_PROT_TO_PROGRAM_ADDR;
}
- writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
+ writel_relaxed(regval, bank0->base + REG_MMU_CTRL_REG);

if (data->enable_4GB &&
MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_VLD_PA_RNG)) {
@@ -738,31 +750,31 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
* 0x1_0000_0000 to 0x1_ffff_ffff. here record bit[32:30].
*/
regval = F_MMU_VLD_PA_RNG(7, 4);
- writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
+ writel_relaxed(regval, bank0->base + REG_MMU_VLD_PA_RNG);
}
if (MTK_IOMMU_HAS_FLAG(data->plat_data, DCM_DISABLE))
- writel_relaxed(F_MMU_DCM, data->base + REG_MMU_DCM_DIS);
+ writel_relaxed(F_MMU_DCM, bank0->base + REG_MMU_DCM_DIS);
else
- writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
+ writel_relaxed(0, bank0->base + REG_MMU_DCM_DIS);

if (MTK_IOMMU_HAS_FLAG(data->plat_data, WR_THROT_EN)) {
/* write command throttling mode */
- regval = readl_relaxed(data->base + REG_MMU_WR_LEN_CTRL);
+ regval = readl_relaxed(bank0->base + REG_MMU_WR_LEN_CTRL);
regval &= ~F_MMU_WR_THROT_DIS_MASK;
- writel_relaxed(regval, data->base + REG_MMU_WR_LEN_CTRL);
+ writel_relaxed(regval, bank0->base + REG_MMU_WR_LEN_CTRL);
}

if (MTK_IOMMU_HAS_FLAG(data->plat_data, RESET_AXI)) {
/* The register is called STANDARD_AXI_MODE in this case */
regval = 0;
} else {
- regval = readl_relaxed(data->base + REG_MMU_MISC_CTRL);
+ regval = readl_relaxed(bank0->base + REG_MMU_MISC_CTRL);
if (MTK_IOMMU_HAS_FLAG(data->plat_data, NOT_STD_AXI_MODE))
regval &= ~F_MMU_STANDARD_AXI_MODE_MASK;
if (MTK_IOMMU_HAS_FLAG(data->plat_data, OUT_ORDER_WR_EN))
regval &= ~F_MMU_IN_ORDER_WR_EN_MASK;
}
- writel_relaxed(regval, data->base + REG_MMU_MISC_CTRL);
+ writel_relaxed(regval, bank0->base + REG_MMU_MISC_CTRL);

regval = F_L2_MULIT_HIT_EN |
F_TABLE_WALK_FAULT_INT_EN |
@@ -770,7 +782,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
F_MISS_FIFO_OVERFLOW_INT_EN |
F_PREFETCH_FIFO_ERR_INT_EN |
F_MISS_FIFO_ERR_INT_EN;
- writel_relaxed(regval, data->base + REG_MMU_INT_CONTROL0);
+ writel_relaxed(regval, bank0->base + REG_MMU_INT_CONTROL0);

regval = F_INT_TRANSLATION_FAULT |
F_INT_MAIN_MULTI_HIT_FAULT |
@@ -779,19 +791,19 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
F_INT_TLB_MISS_FAULT |
F_INT_MISS_TRANSACTION_FIFO_FAULT |
F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
- writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
+ writel_relaxed(regval, bank0->base + REG_MMU_INT_MAIN_CONTROL);

if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_LEGACY_IVRP_PADDR))
regval = (data->protect_base >> 1) | (data->enable_4GB << 31);
else
regval = lower_32_bits(data->protect_base) |
upper_32_bits(data->protect_base);
- writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
+ writel_relaxed(regval, bank0->base + REG_MMU_IVRP_PADDR);

- if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
- dev_name(data->dev), (void *)data)) {
- writel_relaxed(0, data->base + REG_MMU_PT_BASE_ADDR);
- dev_err(data->dev, "Failed @ IRQ-%d Request\n", data->irq);
+ if (devm_request_irq(bank0->pdev, bank0->irq, mtk_iommu_isr, 0,
+ dev_name(bank0->pdev), (void *)bank0)) {
+ writel_relaxed(0, bank0->base + REG_MMU_PT_BASE_ADDR);
+ dev_err(bank0->pdev, "Failed @ IRQ-%d Request\n", bank0->irq);
return -ENODEV;
}

@@ -884,6 +896,8 @@ static int mtk_iommu_probe(struct platform_device *pdev)
int ret;
u32 val;
char *p;
+ struct mtk_iommu_bank_data *bank;
+ void __iomem *base;

data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -921,14 +935,20 @@ static int mtk_iommu_probe(struct platform_device *pdev)
}

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- data->base = devm_ioremap_resource(dev, res);
- if (IS_ERR(data->base))
- return PTR_ERR(data->base);
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
ioaddr = res->start;

- data->irq = platform_get_irq(pdev, 0);
- if (data->irq < 0)
- return data->irq;
+ bank = &data->bank[0];
+ bank->id = 0;
+ bank->base = base;
+ bank->irq = platform_get_irq(pdev, 0);
+ if (bank->irq < 0)
+ return bank->irq;
+ bank->pdev = dev;
+ bank->pdata = data;
+ spin_lock_init(&bank->tlb_lock);

if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_BCLK)) {
data->bclk = devm_clk_get(dev, "bclk");
@@ -964,8 +984,6 @@ static int mtk_iommu_probe(struct platform_device *pdev)
if (ret)
goto out_sysfs_remove;

- spin_lock_init(&data->tlb_lock);
-
if (MTK_IOMMU_HAS_FLAG(data->plat_data, SHARE_PGTABLE)) {
list_add_tail(&data->list, data->plat_data->hw_list);
data->hw_list = data->plat_data->hw_list;
@@ -1015,6 +1033,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
static int mtk_iommu_remove(struct platform_device *pdev)
{
struct mtk_iommu_data *data = platform_get_drvdata(pdev);
+ struct mtk_iommu_bank_data *bank = &data->bank[0];

iommu_device_sysfs_remove(&data->iommu);
iommu_device_unregister(&data->iommu);
@@ -1031,7 +1050,7 @@ static int mtk_iommu_remove(struct platform_device *pdev)
#endif
}
pm_runtime_disable(&pdev->dev);
- devm_free_irq(&pdev->dev, data->irq, data);
+ devm_free_irq(&pdev->dev, bank->irq, bank);
return 0;
}

@@ -1039,7 +1058,7 @@ static int __maybe_unused mtk_iommu_runtime_suspend(struct device *dev)
{
struct mtk_iommu_data *data = dev_get_drvdata(dev);
struct mtk_iommu_suspend_reg *reg = &data->reg;
- void __iomem *base = data->base;
+ void __iomem *base = data->bank[0].base;

reg->wr_len_ctrl = readl_relaxed(base + REG_MMU_WR_LEN_CTRL);
reg->misc_ctrl = readl_relaxed(base + REG_MMU_MISC_CTRL);
@@ -1057,8 +1076,8 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
{
struct mtk_iommu_data *data = dev_get_drvdata(dev);
struct mtk_iommu_suspend_reg *reg = &data->reg;
- struct mtk_iommu_domain *m4u_dom = data->m4u_dom;
- void __iomem *base = data->base;
+ struct mtk_iommu_domain *m4u_dom = data->bank[0].m4u_dom;
+ void __iomem *base = data->bank[0].base;
int ret;

ret = clk_prepare_enable(data->bclk);
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index a9dc79c5a724..881fade8d39a 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -24,6 +24,8 @@

#define MTK_IOMMU_GROUP_MAX 8

+#define MTK_IOMMU_BANK_MAX 5
+
struct mtk_iommu_suspend_reg {
union {
u32 standard_axi_mode;/* v1 */
@@ -65,17 +67,33 @@ struct mtk_iommu_plat_data {

struct mtk_iommu_domain;

-struct mtk_iommu_data {
+struct mtk_iommu_bank_data {
void __iomem *base;
int irq;
+ unsigned int id;
+ struct device *pdev;
+ struct mtk_iommu_data *pdata; /* parent data */
+ spinlock_t tlb_lock; /* lock for tlb range flush */
+ struct mtk_iommu_domain *m4u_dom; /* Each bank has a domain */
+};
+
+struct mtk_iommu_data {
+ union {
+ struct { /* only for gen1 */
+ void __iomem *base;
+ int irq;
+ struct mtk_iommu_domain *m4u_dom;
+ };
+
+ /* only for gen2 that support multi-banks */
+ struct mtk_iommu_bank_data bank[MTK_IOMMU_BANK_MAX];
+ };
struct device *dev;
struct clk *bclk;
phys_addr_t protect_base; /* protect memory base */
struct mtk_iommu_suspend_reg reg;
- struct mtk_iommu_domain *m4u_dom;
struct iommu_group *m4u_group[MTK_IOMMU_GROUP_MAX];
bool enable_4GB;
- spinlock_t tlb_lock; /* lock for tlb range flush */

struct iommu_device iommu;
const struct mtk_iommu_plat_data *plat_data;
--
2.18.0

2021-09-23 12:05:55

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 28/33] iommu/mediatek: Add bank_nr and bank_enable

Prepare for supporting multi banks, Adds two variables in the plat_data:
bank_nr: the bank number that this SoC support;
bank_enable: list if the banks is enabled.

Add them for all the current SoC, bank_nr always is 1 and only
bank_enable[0] is enabled.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 18 ++++++++++++++++++
drivers/iommu/mtk_iommu.h | 3 +++
2 files changed, 21 insertions(+)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index c139675d99e6..4ad85469f68f 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1134,6 +1134,8 @@ static const struct mtk_iommu_plat_data mt2712_data = {
NOT_STD_AXI_MODE | MTK_IOMMU_TYPE_MM,
.hw_list = &m4ulist,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
@@ -1144,6 +1146,8 @@ static const struct mtk_iommu_plat_data mt6779_data = {
.flags = HAS_SUB_COMM_2BITS | OUT_ORDER_WR_EN | WR_THROT_EN |
NOT_STD_AXI_MODE | MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}},
@@ -1154,6 +1158,8 @@ static const struct mtk_iommu_plat_data mt8167_data = {
.flags = RESET_AXI | HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE |
MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {1}, {2}}, /* Linear mapping. */
@@ -1165,6 +1171,8 @@ static const struct mtk_iommu_plat_data mt8173_data = {
HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE |
MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
@@ -1174,6 +1182,8 @@ static const struct mtk_iommu_plat_data mt8183_data = {
.m4u_plat = M4U_MT8183,
.flags = RESET_AXI | MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {4}, {5}, {6}, {7}, {2}, {3}, {1}},
@@ -1185,6 +1195,8 @@ static const struct mtk_iommu_plat_data mt8192_data = {
WR_THROT_EN | IOVA_34_EN | NOT_STD_AXI_MODE |
MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = mt8192_multi_dom,
.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
.larbid_remap = {{0}, {1}, {4, 5}, {7}, {2}, {9, 11, 19, 20},
@@ -1196,6 +1208,8 @@ static const struct mtk_iommu_plat_data mt8195_data_infra = {
.flags = WR_THROT_EN | DCM_DISABLE |
MTK_IOMMU_TYPE_INFRA | IFA_IOMMU_PCIe_SUPPORT,
.pericfg_comp_str = "mediatek,mt8195-pericfg_ao",
+ .bank_nr = 1,
+ .bank_enable = {true},
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
@@ -1208,6 +1222,8 @@ static const struct mtk_iommu_plat_data mt8195_data_vdo = {
SHARE_PGTABLE | MTK_IOMMU_TYPE_MM,
.hw_list = &m4ulist,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = mt8192_multi_dom,
.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
.larbid_remap = {{2, 0}, {21}, {24}, {7}, {19}, {9, 10, 11},
@@ -1221,6 +1237,8 @@ static const struct mtk_iommu_plat_data mt8195_data_vpp = {
SHARE_PGTABLE | MTK_IOMMU_TYPE_MM,
.hw_list = &m4ulist,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = mt8192_multi_dom,
.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
.larbid_remap = {{1}, {3}, {22, 0, 0, 0, 23}, {8},
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 881fade8d39a..dc0190edebf0 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -62,6 +62,9 @@ struct mtk_iommu_plat_data {
struct list_head *hw_list;
unsigned int iova_region_nr;
const struct mtk_iommu_iova_region *iova_region;
+
+ unsigned int bank_nr;
+ bool bank_enable[MTK_IOMMU_BANK_MAX];
unsigned char larbid_remap[MTK_LARB_COM_MAX][MTK_LARB_SUBCOM_MAX];
};

--
2.18.0

2021-09-23 12:06:02

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 13/33] iommu/mediatek: Remove the power status checking in tlb flush all

To simplify the code, Remove the power status checking in the
tlb_flush_all, remove this:
if (pm_runtime_get_if_in_use(data->dev) <= 0)
continue;

After this patch, the mtk_iommu_tlb_flush_all will be called from
a) isr
b) pm runtime resume callback
c) tlb flush range fail case
d) iommu_create_device_direct_mappings
-> iommu_flush_iotlb_all
In first three cases, the power and clock always are enabled; d) is direct
mapping, the tlb flush is unnecessay since we already have tlb_flush_all
in the pm_runtime_resume callback. When the iommu's power status is
changed to active, the tlb always is clean.

In addition, there still are 2 reasons that don't add PM status checking
in the tlb flush all:
a) Write tlb flush all register also is ok even though the HW has no
power and clocks. Write ignore.
b) pm_runtime_get_if_in_use(m4udev) is 0 when the tlb_flush_all
is called frm pm_runtime_resume cb. From this point, we can not add
this code above in this tlb_flush_all.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index e9e94944ed91..4a33b6c6b1db 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -204,10 +204,14 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
return container_of(dom, struct mtk_iommu_domain, domain);
}

-static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data *data)
+static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
{
unsigned long flags;

+ /*
+ * No need get power status since the HW PM status nearly is active
+ * when entering here.
+ */
spin_lock_irqsave(&data->tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
data->base + data->plat_data->inv_sel_reg);
@@ -216,16 +220,6 @@ static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data *data)
spin_unlock_irqrestore(&data->tlb_lock, flags);
}

-static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
-{
- if (pm_runtime_get_if_in_use(data->dev) <= 0)
- return;
-
- mtk_iommu_tlb_do_flush_all(data);
-
- pm_runtime_put(data->dev);
-}
-
static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
struct mtk_iommu_data *data)
{
@@ -263,7 +257,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
if (ret) {
dev_warn(data->dev,
"Partial TLB flush timed out, falling back to full flush\n");
- mtk_iommu_tlb_do_flush_all(data);
+ mtk_iommu_tlb_flush_all(data);
}

if (has_pm)
@@ -993,7 +987,7 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
*
* Thus, Make sure the tlb always is clean after each PM resume.
*/
- mtk_iommu_tlb_do_flush_all(data);
+ mtk_iommu_tlb_flush_all(data);

/*
* Uppon first resume, only enable the clk and return, since the values of the
--
2.18.0

2021-09-23 12:06:02

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 12/33] iommu/mediatek: Always tlb_flush_all when each PM resume

Prepare for 2 HWs that sharing pgtable in different power-domains.

When there are 2 M4U HWs, it may has problem in the flush_range in which
we get the pm_status via the m4u dev, BUT that function don't reflect the
real power-domain status of the HW since there may be other HW also use
that power-domain.

The function dma_alloc_attrs help allocate the iommu buffer which
need the corresponding power domain since tlb flush is needed when
preparing iova. BUT this function only is for allocating buffer,
we have no good reason to request the user always call pm_runtime_get
before calling dma_alloc_xxx. Therefore, we add a tlb_flush_all
in the pm_runtime_resume to make sure the tlb always is clean.

Another solution is always call pm_runtime_get in the tlb_flush_range.
This will trigger pm runtime resume/backup so often when the iommu
power is not active at some time(means user don't call pm_runtime_get
before calling dma_alloc_xxx), This may cause the performance drop.
thus we don't use this.

In other case, the iommu's power should always be active via device
link with smi.

The previous SoC don't have PM except mt8192. the mt8192 IOMMU is display's
power-domain which nearly always is enabled. thus no need fix tags here.
Prepare for mt8195.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 44cf5547d084..e9e94944ed91 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -984,6 +984,17 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
return ret;
}

+ /*
+ * Users may allocate dma buffer before they call pm_runtime_get, then
+ * it will lack the necessary tlb flush.
+ *
+ * We have no good reason to request the users always call dma_alloc_xx
+ * after pm_runtime_get_sync.
+ *
+ * Thus, Make sure the tlb always is clean after each PM resume.
+ */
+ mtk_iommu_tlb_do_flush_all(data);
+
/*
* Uppon first resume, only enable the clk and return, since the values of the
* registers are not yet set.
--
2.18.0

2021-09-23 12:06:12

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 27/33] iommu/mediatek: Initialise bank HW for each a bank

The mt8195 IOMMU HW max support 5 banks, and regarding the banks'
registers, it looks like:

----------------------------------------
|bank0 | bank1 | bank2 | bank3 | bank4|
----------------------------------------
|global |
|control| null
|regs |
-----------------------------------------
|bank |bank |bank |bank |bank |
|regs |regs |regs |regs |regs |
| | | | | |
-----------------------------------------

Each bank has some special bank registers and it share bank0's global
control registers. this patch initialise the bank hw with the bankid.

In the hw_init, we always initialise bank0's control register since
we don't know if the bank0 is initialised.

Additionally, About each bank's register base, always delta 0x1000.
like bank[x + 1] = bank[x] + 0x1000.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6ef3eb3cad92..c139675d99e6 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -152,7 +152,7 @@ struct mtk_iommu_domain {

static const struct iommu_ops mtk_iommu_ops;

-static int mtk_iommu_hw_init(const struct mtk_iommu_data *data);
+static int mtk_iommu_hw_init(const struct mtk_iommu_data *data, unsigned int bankid);

#define MTK_IOMMU_TLB_ADDR(iova) ({ \
dma_addr_t _addr = iova; \
@@ -520,12 +520,12 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
dom->bank = bank;
}

- if (!bank->m4u_dom) { /* Initialize the M4U HW */
+ if (!bank->m4u_dom) { /* Initialize the M4U HW for each a BANK */
ret = pm_runtime_resume_and_get(m4udev);
if (ret < 0)
return ret;

- ret = mtk_iommu_hw_init(data);
+ ret = mtk_iommu_hw_init(data, bankid);
if (ret) {
pm_runtime_put(m4udev);
return ret;
@@ -729,11 +729,16 @@ static const struct iommu_ops mtk_iommu_ops = {
.owner = THIS_MODULE,
};

-static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
+static int mtk_iommu_hw_init(const struct mtk_iommu_data *data, unsigned int bankid)
{
+ const struct mtk_iommu_bank_data *bankx = &data->bank[bankid];
const struct mtk_iommu_bank_data *bank0 = &data->bank[0];
u32 regval;

+ /*
+ * Global control settings are in bank0. May re-init these global registers
+ * since no sure if there is bank0 consumers.
+ */
if (data->plat_data->m4u_plat == M4U_MT8173) {
regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173;
@@ -776,13 +781,14 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
}
writel_relaxed(regval, bank0->base + REG_MMU_MISC_CTRL);

+ /* Independent settings for each bank */
regval = F_L2_MULIT_HIT_EN |
F_TABLE_WALK_FAULT_INT_EN |
F_PREETCH_FIFO_OVERFLOW_INT_EN |
F_MISS_FIFO_OVERFLOW_INT_EN |
F_PREFETCH_FIFO_ERR_INT_EN |
F_MISS_FIFO_ERR_INT_EN;
- writel_relaxed(regval, bank0->base + REG_MMU_INT_CONTROL0);
+ writel_relaxed(regval, bankx->base + REG_MMU_INT_CONTROL0);

regval = F_INT_TRANSLATION_FAULT |
F_INT_MAIN_MULTI_HIT_FAULT |
@@ -791,19 +797,19 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
F_INT_TLB_MISS_FAULT |
F_INT_MISS_TRANSACTION_FIFO_FAULT |
F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
- writel_relaxed(regval, bank0->base + REG_MMU_INT_MAIN_CONTROL);
+ writel_relaxed(regval, bankx->base + REG_MMU_INT_MAIN_CONTROL);

if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_LEGACY_IVRP_PADDR))
regval = (data->protect_base >> 1) | (data->enable_4GB << 31);
else
regval = lower_32_bits(data->protect_base) |
upper_32_bits(data->protect_base);
- writel_relaxed(regval, bank0->base + REG_MMU_IVRP_PADDR);
+ writel_relaxed(regval, bankx->base + REG_MMU_IVRP_PADDR);

- if (devm_request_irq(bank0->pdev, bank0->irq, mtk_iommu_isr, 0,
- dev_name(bank0->pdev), (void *)bank0)) {
- writel_relaxed(0, bank0->base + REG_MMU_PT_BASE_ADDR);
- dev_err(bank0->pdev, "Failed @ IRQ-%d Request\n", bank0->irq);
+ if (devm_request_irq(bankx->pdev, bankx->irq, mtk_iommu_isr, 0,
+ dev_name(bankx->pdev), (void *)bankx)) {
+ writel_relaxed(0, bankx->base + REG_MMU_PT_BASE_ADDR);
+ dev_err(bankx->pdev, "Failed @ IRQ-%d Request\n", bankx->irq);
return -ENODEV;
}

--
2.18.0

2021-09-23 12:06:23

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 29/33] iommu/mediatek: Change the domid to iova_region_id

Prepare for adding bankid, also no functional change.

In the previous SoC, each a iova_region is a domain; In the multi-banks
case, each a bank is a domain, then the original function name
"mtk_iommu_get_domain_id" is not proper. Use "iova_region_id" instead of
"domain_id".

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 46 +++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 4ad85469f68f..6e3f7b97f3f5 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -352,8 +352,8 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}

-static int mtk_iommu_get_domain_id(struct device *dev,
- const struct mtk_iommu_plat_data *plat_data)
+static int mtk_iommu_get_iova_region_id(struct device *dev,
+ const struct mtk_iommu_plat_data *plat_data)
{
const struct mtk_iommu_iova_region *rgn = plat_data->iova_region;
const struct bus_dma_region *dma_rgn = dev->dma_range_map;
@@ -383,7 +383,7 @@ static int mtk_iommu_get_domain_id(struct device *dev,
}

static int mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,
- bool enable, unsigned int domid)
+ bool enable, unsigned int regionid)
{
struct mtk_smi_larb_iommu *larb_mmu;
unsigned int larbid, portid;
@@ -399,12 +399,12 @@ static int mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,
if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
larb_mmu = &data->larb_imu[larbid];

- region = data->plat_data->iova_region + domid;
+ region = data->plat_data->iova_region + regionid;
larb_mmu->bank[portid] = upper_32_bits(region->iova_base);

- dev_dbg(dev, "%s iommu for larb(%s) port %d dom %d bank %d.\n",
+ dev_dbg(dev, "%s iommu for larb(%s) port %d region %d rgn-bank %d.\n",
enable ? "enable" : "disable", dev_name(larb_mmu->dev),
- portid, domid, larb_mmu->bank[portid]);
+ portid, regionid, larb_mmu->bank[portid]);

if (enable)
larb_mmu->mmu |= MTK_SMI_MMU_EN(portid);
@@ -430,7 +430,7 @@ static int mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,

static int mtk_iommu_domain_finalise(struct mtk_iommu_domain *dom,
struct mtk_iommu_data *data,
- unsigned int domid)
+ unsigned int region_id)
{
const struct mtk_iommu_iova_region *region;
struct mtk_iommu_domain *m4u_dom;
@@ -469,7 +469,7 @@ static int mtk_iommu_domain_finalise(struct mtk_iommu_domain *dom,

update_iova_region:
/* Update the iova region for this domain */
- region = data->plat_data->iova_region + domid;
+ region = data->plat_data->iova_region + region_id;
dom->domain.geometry.aperture_start = region->iova_base;
dom->domain.geometry.aperture_end = region->iova_base + region->size - 1;
dom->domain.geometry.force_aperture = true;
@@ -504,18 +504,18 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
struct mtk_iommu_bank_data *bank;
struct device *m4udev = data->dev;
unsigned int bankid = 0;
- int ret, domid;
+ int ret, region_id;

- domid = mtk_iommu_get_domain_id(dev, data->plat_data);
- if (domid < 0)
- return domid;
+ region_id = mtk_iommu_get_iova_region_id(dev, data->plat_data);
+ if (region_id < 0)
+ return region_id;

bank = &data->bank[bankid];
if (!dom->bank) {
/* Data is in the frstdata in sharing pgtable case. */
frstdata = mtk_iommu_get_frst_data(hw_list);

- if (mtk_iommu_domain_finalise(dom, frstdata, domid))
+ if (mtk_iommu_domain_finalise(dom, frstdata, region_id))
return -ENODEV;
dom->bank = bank;
}
@@ -537,7 +537,7 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
pm_runtime_put(m4udev);
}

- return mtk_iommu_config(data, dev, true, domid);
+ return mtk_iommu_config(data, dev, true, region_id);
}

static void mtk_iommu_detach_device(struct iommu_domain *domain,
@@ -636,21 +636,21 @@ static struct iommu_group *mtk_iommu_device_group(struct device *dev)
struct mtk_iommu_data *c_data = dev_iommu_priv_get(dev), *data;
struct list_head *hw_list = c_data->hw_list;
struct iommu_group *group;
- int domid;
+ int regionid;

data = mtk_iommu_get_frst_data(hw_list);
if (!data)
return ERR_PTR(-ENODEV);

- domid = mtk_iommu_get_domain_id(dev, data->plat_data);
- if (domid < 0)
- return ERR_PTR(domid);
+ regionid = mtk_iommu_get_iova_region_id(dev, data->plat_data);
+ if (regionid < 0)
+ return ERR_PTR(regionid);

- group = data->m4u_group[domid];
+ group = data->m4u_group[regionid];
if (!group) {
group = iommu_group_alloc();
if (!IS_ERR(group))
- data->m4u_group[domid] = group;
+ data->m4u_group[regionid] = group;
} else {
iommu_group_ref_get(group);
}
@@ -683,14 +683,14 @@ static void mtk_iommu_get_resv_regions(struct device *dev,
struct list_head *head)
{
struct mtk_iommu_data *data = dev_iommu_priv_get(dev);
- unsigned int domid = mtk_iommu_get_domain_id(dev, data->plat_data), i;
+ unsigned int regionid = mtk_iommu_get_iova_region_id(dev, data->plat_data), i;
const struct mtk_iommu_iova_region *resv, *curdom;
struct iommu_resv_region *region;
int prot = IOMMU_WRITE | IOMMU_READ;

- if ((int)domid < 0)
+ if ((int)regionid < 0)
return;
- curdom = data->plat_data->iova_region + domid;
+ curdom = data->plat_data->iova_region + regionid;
for (i = 0; i < data->plat_data->iova_region_nr; i++) {
resv = data->plat_data->iova_region + i;

--
2.18.0

2021-09-23 12:06:29

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 15/33] iommu/mediatek: Add SUB_COMMON_3BITS flag

In prevous SoC, the sub common id occupy 2 bits. the mt8195's sub common
id has 3bits. Add a new flag for this. and rename the prevous flag to
_2BITS. For readable, I put these two flags together, then move the
other flags. no functional change.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 26 ++++++++++++++++----------
drivers/iommu/mtk_iommu.h | 2 +-
2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index ef98f19ce86e..d557c3437d67 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -105,6 +105,8 @@
#define REG_MMU1_INT_ID 0x154
#define F_MMU_INT_ID_COMM_ID(a) (((a) >> 9) & 0x7)
#define F_MMU_INT_ID_SUB_COMM_ID(a) (((a) >> 7) & 0x3)
+#define F_MMU_INT_ID_COMM_ID_EXT(a) (((a) >> 10) & 0x7)
+#define F_MMU_INT_ID_SUB_COMM_ID_EXT(a) (((a) >> 7) & 0x7)
#define F_MMU_INT_ID_LARB_ID(a) (((a) >> 7) & 0x7)
#define F_MMU_INT_ID_PORT_ID(a) (((a) >> 2) & 0x1f)

@@ -116,13 +118,14 @@
#define HAS_VLD_PA_RNG BIT(2)
#define RESET_AXI BIT(3)
#define OUT_ORDER_WR_EN BIT(4)
-#define HAS_SUB_COMM BIT(5)
-#define WR_THROT_EN BIT(6)
-#define HAS_LEGACY_IVRP_PADDR BIT(7)
-#define IOVA_34_EN BIT(8)
-#define SHARE_PGTABLE BIT(9) /* 2 HW share pgtable */
-#define DCM_DISABLE BIT(10)
-#define NOT_STD_AXI_MODE BIT(11)
+#define HAS_SUB_COMM_2BITS BIT(5)
+#define HAS_SUB_COMM_3BITS BIT(6)
+#define WR_THROT_EN BIT(7)
+#define HAS_LEGACY_IVRP_PADDR BIT(8)
+#define IOVA_34_EN BIT(9)
+#define SHARE_PGTABLE BIT(10) /* 2 HW share pgtable */
+#define DCM_DISABLE BIT(11)
+#define NOT_STD_AXI_MODE BIT(12)

#define MTK_IOMMU_HAS_FLAG(pdata, _x) \
((((pdata)->flags) & (_x)) == (_x))
@@ -296,9 +299,12 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
fault_pa |= (u64)pa34_32 << 32;

fault_port = F_MMU_INT_ID_PORT_ID(regval);
- if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM)) {
+ if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM_2BITS)) {
fault_larb = F_MMU_INT_ID_COMM_ID(regval);
sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
+ } else if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM_3BITS)) {
+ fault_larb = F_MMU_INT_ID_COMM_ID_EXT(regval);
+ sub_comm = F_MMU_INT_ID_SUB_COMM_ID_EXT(regval);
} else {
fault_larb = F_MMU_INT_ID_LARB_ID(regval);
}
@@ -1027,7 +1033,7 @@ static const struct mtk_iommu_plat_data mt2712_data = {

static const struct mtk_iommu_plat_data mt6779_data = {
.m4u_plat = M4U_MT6779,
- .flags = HAS_SUB_COMM | OUT_ORDER_WR_EN | WR_THROT_EN |
+ .flags = HAS_SUB_COMM_2BITS | OUT_ORDER_WR_EN | WR_THROT_EN |
NOT_STD_AXI_MODE,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
.iova_region = single_domain,
@@ -1065,7 +1071,7 @@ static const struct mtk_iommu_plat_data mt8183_data = {

static const struct mtk_iommu_plat_data mt8192_data = {
.m4u_plat = M4U_MT8192,
- .flags = HAS_BCLK | HAS_SUB_COMM | OUT_ORDER_WR_EN |
+ .flags = HAS_BCLK | HAS_SUB_COMM_2BITS | OUT_ORDER_WR_EN |
WR_THROT_EN | IOVA_34_EN | NOT_STD_AXI_MODE,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
.iova_region = mt8192_multi_dom,
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 027a42396557..5b32277fee99 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -20,7 +20,7 @@
#include <dt-bindings/memory/mtk-memory-port.h>

#define MTK_LARB_COM_MAX 8
-#define MTK_LARB_SUBCOM_MAX 4
+#define MTK_LARB_SUBCOM_MAX 8

#define MTK_IOMMU_GROUP_MAX 8

--
2.18.0

2021-09-23 12:06:31

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 33/33] iommu/mediatek: mt8195: Enable multi banks for infra iommu

Enable the multi-bank functions for infra-iommu. We put PCIE in bank0
and USB in the last bank(bank4). and we don't use the other banks
currently, disable them.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 3cb18ed28132..90be8ebbc98a 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1273,8 +1273,11 @@ static const struct mtk_iommu_plat_data mt8195_data_infra = {
.flags = WR_THROT_EN | DCM_DISABLE |
MTK_IOMMU_TYPE_INFRA | IFA_IOMMU_PCIe_SUPPORT,
.pericfg_comp_str = "mediatek,mt8195-pericfg_ao",
- .bank_nr = 1,
- .bank_enable = {true},
+ .bank_nr = 5,
+ .bank_enable = {true, false, false, false, true},
+ .bank_portmsk = {[0] = GENMASK(19, 16), /* PCIe */
+ [4] = GENMASK(31, 20), /* USB */
+ },
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
--
2.18.0

2021-09-23 12:06:43

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 16/33] iommu/mediatek: Add IOMMU_TYPE flag

Add IOMMU_TYPE definition. In the mt8195, we have another IOMMU_TYPE:
infra iommu, also there will be another APU_IOMMU, thus, use 2bits for the
IOMMU_TYPE.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index d557c3437d67..2665b0707f13 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -126,9 +126,17 @@
#define SHARE_PGTABLE BIT(10) /* 2 HW share pgtable */
#define DCM_DISABLE BIT(11)
#define NOT_STD_AXI_MODE BIT(12)
+/* 2 bits: iommu type */
+#define MTK_IOMMU_TYPE_MM (0x0 << 13)
+#define MTK_IOMMU_TYPE_INFRA (0x1 << 13)
+#define MTK_IOMMU_TYPE_MASK (0x3 << 13)

-#define MTK_IOMMU_HAS_FLAG(pdata, _x) \
- ((((pdata)->flags) & (_x)) == (_x))
+#define MTK_IOMMU_HAS_FLAG(pdata, _x) (!!(((pdata)->flags) & (_x)))
+
+#define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask) \
+ ((((pdata)->flags) & (mask)) == (_x))
+#define MTK_IOMMU_IS_TYPE(pdata, _x) MTK_IOMMU_HAS_FLAG_MASK(pdata, _x,\
+ MTK_IOMMU_TYPE_MASK)

struct mtk_iommu_domain {
struct io_pgtable_cfg cfg;
--
2.18.0

2021-09-23 12:06:52

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 22/33] iommu/mediatek: Add PCIe support

Currently the code for of_iommu_configure_dev_id is like this:

static int of_iommu_configure_dev_id(struct device_node *master_np,
struct device *dev,
const u32 *id)
{
struct of_phandle_args iommu_spec = { .args_count = 1 };

err = of_map_id(master_np, *id, "iommu-map",
"iommu-map-mask", &iommu_spec.np,
iommu_spec.args);
...
}

It supports only one id output. BUT our PCIe HW has two ID(one is for
writing, the other is for reading). I'm not sure if we should change
of_map_id to support output MAX_PHANDLE_ARGS.

Here add the solution in ourselve drivers. If it's pcie case, enable one
more bit.

Not all infra iommu support PCIe, thus add a PCIe support flag here.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 37d6dfb4feab..3f1fd8036345 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -20,6 +20,7 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/regmap.h>
@@ -132,6 +133,7 @@
#define MTK_IOMMU_TYPE_MM (0x0 << 13)
#define MTK_IOMMU_TYPE_INFRA (0x1 << 13)
#define MTK_IOMMU_TYPE_MASK (0x3 << 13)
+#define IFA_IOMMU_PCIe_SUPPORT BIT(15)

#define MTK_IOMMU_HAS_FLAG(pdata, _x) (!!(((pdata)->flags) & (_x)))

@@ -401,8 +403,11 @@ static int mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,
larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid);
} else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA)) {
peri_mmuen_msk = BIT(portid);
- peri_mmuen = enable ? peri_mmuen_msk : 0;
+ /* PCIdev has only one output id, enable the next writing bit for PCIe */
+ if (dev_is_pci(dev))
+ peri_mmuen_msk |= BIT(portid + 1);

+ peri_mmuen = enable ? peri_mmuen_msk : 0;
ret = regmap_update_bits(data->pericfg, PERICFG_IOMMU_1,
peri_mmuen_msk, peri_mmuen);
if (ret)
@@ -977,6 +982,15 @@ static int mtk_iommu_probe(struct platform_device *pdev)
ret = component_master_add_with_match(dev, &mtk_iommu_com_ops, match);
if (ret)
goto out_bus_set_null;
+ } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
+ MTK_IOMMU_HAS_FLAG(data->plat_data, IFA_IOMMU_PCIe_SUPPORT)) {
+ #ifdef CONFIG_PCI
+ if (!iommu_present(&pci_bus_type)) {
+ ret = bus_set_iommu(&pci_bus_type, &mtk_iommu_ops);
+ if (ret) /* PCIe fail don't affect platform_bus. */
+ goto out_list_del;
+ }
+ #endif
}
return ret;

@@ -1007,6 +1021,11 @@ static int mtk_iommu_remove(struct platform_device *pdev)
if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
device_link_remove(data->smicomm_dev, &pdev->dev);
component_master_del(&pdev->dev, &mtk_iommu_com_ops);
+ } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
+ MTK_IOMMU_HAS_FLAG(data->plat_data, IFA_IOMMU_PCIe_SUPPORT)) {
+ #ifdef CONFIG_PCI
+ bus_set_iommu(&pci_bus_type, NULL);
+ #endif
}
pm_runtime_disable(&pdev->dev);
devm_free_irq(&pdev->dev, data->irq, data);
--
2.18.0

2021-09-23 12:07:09

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 17/33] iommu/mediatek: Contain MM IOMMU flow with the MM TYPE

Prepare for supporting INFRA_IOMMU, and APU_IOMMU later.

For Infra IOMMU/APU IOMMU, it doesn't have the "larb""port". thus, Use
the MM flag contain the MM_IOMMU special flow, Also, it moves a big
chunk code about parsing the mediatek,larbs into a function, this is
only needed for MM IOMMU. and all the current SoC are MM_IOMMU.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 193 +++++++++++++++++++++-----------------
1 file changed, 109 insertions(+), 84 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2665b0707f13..2ebbb3412cb9 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -280,7 +280,7 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
{
struct mtk_iommu_data *data = dev_id;
struct mtk_iommu_domain *dom = data->m4u_dom;
- unsigned int fault_larb, fault_port, sub_comm = 0;
+ unsigned int fault_larb = 0, fault_port = 0, sub_comm = 0;
u32 int_state, regval, va34_32, pa34_32;
u64 fault_iova, fault_pa;
bool layer, write;
@@ -306,17 +306,19 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
pa34_32 = FIELD_GET(F_MMU_INVAL_PA_34_32_MASK, fault_iova);
fault_pa |= (u64)pa34_32 << 32;

- fault_port = F_MMU_INT_ID_PORT_ID(regval);
- if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM_2BITS)) {
- fault_larb = F_MMU_INT_ID_COMM_ID(regval);
- sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
- } else if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM_3BITS)) {
- fault_larb = F_MMU_INT_ID_COMM_ID_EXT(regval);
- sub_comm = F_MMU_INT_ID_SUB_COMM_ID_EXT(regval);
- } else {
- fault_larb = F_MMU_INT_ID_LARB_ID(regval);
+ if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
+ fault_port = F_MMU_INT_ID_PORT_ID(regval);
+ if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM_2BITS)) {
+ fault_larb = F_MMU_INT_ID_COMM_ID(regval);
+ sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
+ } else if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM_3BITS)) {
+ fault_larb = F_MMU_INT_ID_COMM_ID_EXT(regval);
+ sub_comm = F_MMU_INT_ID_SUB_COMM_ID_EXT(regval);
+ } else {
+ fault_larb = F_MMU_INT_ID_LARB_ID(regval);
+ }
+ fault_larb = data->plat_data->larbid_remap[fault_larb][sub_comm];
}
- fault_larb = data->plat_data->larbid_remap[fault_larb][sub_comm];

if (report_iommu_fault(&dom->domain, data->dev, fault_iova,
write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) {
@@ -380,19 +382,21 @@ static void mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,
larbid = MTK_M4U_TO_LARB(fwspec->ids[i]);
portid = MTK_M4U_TO_PORT(fwspec->ids[i]);

- larb_mmu = &data->larb_imu[larbid];
+ if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
+ larb_mmu = &data->larb_imu[larbid];

- region = data->plat_data->iova_region + domid;
- larb_mmu->bank[portid] = upper_32_bits(region->iova_base);
+ region = data->plat_data->iova_region + domid;
+ larb_mmu->bank[portid] = upper_32_bits(region->iova_base);

- dev_dbg(dev, "%s iommu for larb(%s) port %d dom %d bank %d.\n",
- enable ? "enable" : "disable", dev_name(larb_mmu->dev),
- portid, domid, larb_mmu->bank[portid]);
+ dev_dbg(dev, "%s iommu for larb(%s) port %d dom %d bank %d.\n",
+ enable ? "enable" : "disable", dev_name(larb_mmu->dev),
+ portid, domid, larb_mmu->bank[portid]);

- if (enable)
- larb_mmu->mmu |= MTK_SMI_MMU_EN(portid);
- else
- larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid);
+ if (enable)
+ larb_mmu->mmu |= MTK_SMI_MMU_EN(portid);
+ else
+ larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid);
+ }
}
}

@@ -778,19 +782,75 @@ static const struct component_master_ops mtk_iommu_com_ops = {
.unbind = mtk_iommu_unbind,
};

+static int mtk_iommu_mm_dts_parse(struct device *dev,
+ struct component_match **match,
+ struct mtk_iommu_data *data)
+{
+ struct platform_device *plarbdev;
+ struct device_link *link;
+ struct device_node *larbnode, *smicomm_node;
+ int i, larb_nr, ret;
+
+ larb_nr = of_count_phandle_with_args(dev->of_node, "mediatek,larbs", NULL);
+ if (larb_nr < 0)
+ return larb_nr;
+
+ for (i = 0; i < larb_nr; i++) {
+ u32 id;
+
+ larbnode = of_parse_phandle(dev->of_node, "mediatek,larbs", i);
+ if (!larbnode)
+ return -EINVAL;
+
+ if (!of_device_is_available(larbnode)) {
+ of_node_put(larbnode);
+ continue;
+ }
+
+ ret = of_property_read_u32(larbnode, "mediatek,larb-id", &id);
+ if (ret)/* The id is consecutive if there is no this property */
+ id = i;
+
+ plarbdev = of_find_device_by_node(larbnode);
+ if (!plarbdev) {
+ of_node_put(larbnode);
+ return -EPROBE_DEFER;
+ }
+ data->larb_imu[id].dev = &plarbdev->dev;
+
+ component_match_add_release(dev, match, release_of,
+ compare_of, larbnode);
+ }
+
+ /* Get smi-common dev from the last larb. */
+ smicomm_node = of_parse_phandle(larbnode, "mediatek,smi", 0);
+ if (!smicomm_node)
+ return -EINVAL;
+
+ plarbdev = of_find_device_by_node(smicomm_node);
+ of_node_put(smicomm_node);
+ data->smicomm_dev = &plarbdev->dev;
+
+ link = device_link_add(data->smicomm_dev, dev,
+ DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
+
+ if (!link) {
+ dev_err(dev, "Unable link %s.\n", dev_name(data->smicomm_dev));
+ return PTR_ERR(link);
+ }
+ return 0;
+}
+
static int mtk_iommu_probe(struct platform_device *pdev)
{
struct mtk_iommu_data *data;
struct device *dev = &pdev->dev;
- struct device_node *larbnode, *smicomm_node;
- struct platform_device *plarbdev;
- struct device_link *link;
struct resource *res;
resource_size_t ioaddr;
struct component_match *match = NULL;
struct regmap *infracfg;
void *protect;
- int i, larb_nr, ret;
+ int ret;
u32 val;
char *p;

@@ -845,55 +905,12 @@ static int mtk_iommu_probe(struct platform_device *pdev)
return PTR_ERR(data->bclk);
}

- larb_nr = of_count_phandle_with_args(dev->of_node,
- "mediatek,larbs", NULL);
- if (larb_nr < 0)
- return larb_nr;
-
- for (i = 0; i < larb_nr; i++) {
- u32 id;
-
- larbnode = of_parse_phandle(dev->of_node, "mediatek,larbs", i);
- if (!larbnode)
- return -EINVAL;
-
- if (!of_device_is_available(larbnode)) {
- of_node_put(larbnode);
- continue;
- }
-
- ret = of_property_read_u32(larbnode, "mediatek,larb-id", &id);
- if (ret)/* The id is consecutive if there is no this property */
- id = i;
-
- plarbdev = of_find_device_by_node(larbnode);
- if (!plarbdev) {
- of_node_put(larbnode);
- return -EPROBE_DEFER;
- }
- data->larb_imu[id].dev = &plarbdev->dev;
-
- component_match_add_release(dev, &match, release_of,
- compare_of, larbnode);
- }
-
- /* Get smi-common dev from the last larb. */
- smicomm_node = of_parse_phandle(larbnode, "mediatek,smi", 0);
- if (!smicomm_node)
- return -EINVAL;
-
- plarbdev = of_find_device_by_node(smicomm_node);
- of_node_put(smicomm_node);
- data->smicomm_dev = &plarbdev->dev;
-
pm_runtime_enable(dev);

- link = device_link_add(data->smicomm_dev, dev,
- DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
- if (!link) {
- dev_err(dev, "Unable to link %s.\n", dev_name(data->smicomm_dev));
- ret = -EINVAL;
- goto out_runtime_disable;
+ if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
+ ret = mtk_iommu_mm_dts_parse(dev, &match, data);
+ if (ret)
+ goto out_runtime_disable;
}

platform_set_drvdata(pdev, data);
@@ -924,9 +941,11 @@ static int mtk_iommu_probe(struct platform_device *pdev)
goto out_list_del;
}

- ret = component_master_add_with_match(dev, &mtk_iommu_com_ops, match);
- if (ret)
- goto out_bus_set_null;
+ if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
+ ret = component_master_add_with_match(dev, &mtk_iommu_com_ops, match);
+ if (ret)
+ goto out_bus_set_null;
+ }
return ret;

out_bus_set_null:
@@ -937,7 +956,8 @@ static int mtk_iommu_probe(struct platform_device *pdev)
out_sysfs_remove:
iommu_device_sysfs_remove(&data->iommu);
out_link_remove:
- device_link_remove(data->smicomm_dev, dev);
+ if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM))
+ device_link_remove(data->smicomm_dev, dev);
out_runtime_disable:
pm_runtime_disable(dev);
return ret;
@@ -953,10 +973,12 @@ static int mtk_iommu_remove(struct platform_device *pdev)
if (iommu_present(&platform_bus_type))
bus_set_iommu(&platform_bus_type, NULL);

- device_link_remove(data->smicomm_dev, &pdev->dev);
+ if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
+ device_link_remove(data->smicomm_dev, &pdev->dev);
+ component_master_del(&pdev->dev, &mtk_iommu_com_ops);
+ }
pm_runtime_disable(&pdev->dev);
devm_free_irq(&pdev->dev, data->irq, data);
- component_master_del(&pdev->dev, &mtk_iommu_com_ops);
return 0;
}

@@ -1031,7 +1053,7 @@ static const struct dev_pm_ops mtk_iommu_pm_ops = {
static const struct mtk_iommu_plat_data mt2712_data = {
.m4u_plat = M4U_MT2712,
.flags = HAS_4GB_MODE | HAS_BCLK | HAS_VLD_PA_RNG | SHARE_PGTABLE |
- NOT_STD_AXI_MODE,
+ NOT_STD_AXI_MODE | MTK_IOMMU_TYPE_MM,
.hw_list = &m4ulist,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.iova_region = single_domain,
@@ -1042,7 +1064,7 @@ static const struct mtk_iommu_plat_data mt2712_data = {
static const struct mtk_iommu_plat_data mt6779_data = {
.m4u_plat = M4U_MT6779,
.flags = HAS_SUB_COMM_2BITS | OUT_ORDER_WR_EN | WR_THROT_EN |
- NOT_STD_AXI_MODE,
+ NOT_STD_AXI_MODE | MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
@@ -1051,7 +1073,8 @@ static const struct mtk_iommu_plat_data mt6779_data = {

static const struct mtk_iommu_plat_data mt8167_data = {
.m4u_plat = M4U_MT8167,
- .flags = RESET_AXI | HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE,
+ .flags = RESET_AXI | HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE |
+ MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
@@ -1061,7 +1084,8 @@ static const struct mtk_iommu_plat_data mt8167_data = {
static const struct mtk_iommu_plat_data mt8173_data = {
.m4u_plat = M4U_MT8173,
.flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI |
- HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE,
+ HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE |
+ MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
@@ -1070,7 +1094,7 @@ static const struct mtk_iommu_plat_data mt8173_data = {

static const struct mtk_iommu_plat_data mt8183_data = {
.m4u_plat = M4U_MT8183,
- .flags = RESET_AXI,
+ .flags = RESET_AXI | MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
@@ -1080,7 +1104,8 @@ static const struct mtk_iommu_plat_data mt8183_data = {
static const struct mtk_iommu_plat_data mt8192_data = {
.m4u_plat = M4U_MT8192,
.flags = HAS_BCLK | HAS_SUB_COMM_2BITS | OUT_ORDER_WR_EN |
- WR_THROT_EN | IOVA_34_EN | NOT_STD_AXI_MODE,
+ WR_THROT_EN | IOVA_34_EN | NOT_STD_AXI_MODE |
+ MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
.iova_region = mt8192_multi_dom,
.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
--
2.18.0

2021-09-23 12:07:10

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 19/33] iommu/mediatek: Add list_del in mtk_iommu_remove

Lack the list_del in the mtk_iommu_remove, and remove
bus_set_iommu(*, NULL) since there may be several iommu HWs.
we can not bus_set_iommu null when one iommu driver unbind.
This issue is not so important, No need fix tags.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index faea4a0d922f..2818b984c905 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -980,8 +980,7 @@ static int mtk_iommu_remove(struct platform_device *pdev)
iommu_device_sysfs_remove(&data->iommu);
iommu_device_unregister(&data->iommu);

- if (iommu_present(&platform_bus_type))
- bus_set_iommu(&platform_bus_type, NULL);
+ list_del(&data->list);

if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
device_link_remove(data->smicomm_dev, &pdev->dev);
--
2.18.0

2021-09-23 12:07:12

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 20/33] iommu/mediatek: Allow IOMMU_DOMAIN_UNMANAGED for PCIe VFIO

Allow the type IOMMU_DOMAIN_UNMANAGED since vfio_iommu_type1.c always call
iommu_domain_alloc. The PCIe EP works ok when going through vfio.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2818b984c905..d103e4f33078 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -450,7 +450,7 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
{
struct mtk_iommu_domain *dom;

- if (type != IOMMU_DOMAIN_DMA)
+ if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
return NULL;

dom = kzalloc(sizeof(*dom), GFP_KERNEL);
--
2.18.0

2021-09-23 12:07:35

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 24/33] iommu/mediatek: Only adjust code about register base

No functional change. Use "base" instead of the data->base. This is
avoid to touch too many lines in the next patches.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 51 +++++++++++++++++++++------------------
1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index df823ee3541a..725e000eee0f 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -221,6 +221,7 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)

static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
{
+ void __iomem *base = data->base;
unsigned long flags;

/*
@@ -228,9 +229,8 @@ static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
* when entering here.
*/
spin_lock_irqsave(&data->tlb_lock, flags);
- writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
- data->base + data->plat_data->inv_sel_reg);
- writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
+ writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + data->plat_data->inv_sel_reg);
+ writel_relaxed(F_ALL_INVLD, base + REG_MMU_INVALIDATE);
wmb(); /* Make sure the tlb flush all done */
spin_unlock_irqrestore(&data->tlb_lock, flags);
}
@@ -241,6 +241,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
struct list_head *head = data->hw_list;
bool has_pm = !!data->dev->pm_domain;
unsigned long flags;
+ void __iomem *base;
int ret;
u32 tmp;

@@ -250,23 +251,23 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
continue;
}

+ base = data->base;
+
spin_lock_irqsave(&data->tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
- data->base + data->plat_data->inv_sel_reg);
+ base + data->plat_data->inv_sel_reg);

- writel_relaxed(MTK_IOMMU_TLB_ADDR(iova),
- data->base + REG_MMU_INVLD_START_A);
+ writel_relaxed(MTK_IOMMU_TLB_ADDR(iova), base + REG_MMU_INVLD_START_A);
writel_relaxed(MTK_IOMMU_TLB_ADDR(iova + size - 1),
- data->base + REG_MMU_INVLD_END_A);
- writel_relaxed(F_MMU_INV_RANGE,
- data->base + REG_MMU_INVALIDATE);
+ base + REG_MMU_INVLD_END_A);
+ writel_relaxed(F_MMU_INV_RANGE, base + REG_MMU_INVALIDATE);

/* tlb sync */
- ret = readl_poll_timeout_atomic(data->base + REG_MMU_CPE_DONE,
+ ret = readl_poll_timeout_atomic(base + REG_MMU_CPE_DONE,
tmp, tmp != 0, 10, 1000);

/* Clear the CPE status */
- writel_relaxed(0, data->base + REG_MMU_CPE_DONE);
+ writel_relaxed(0, base + REG_MMU_CPE_DONE);
spin_unlock_irqrestore(&data->tlb_lock, flags);

if (ret) {
@@ -286,23 +287,25 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
struct mtk_iommu_domain *dom = data->m4u_dom;
unsigned int fault_larb = 0, fault_port = 0, sub_comm = 0;
u32 int_state, regval, va34_32, pa34_32;
+ const struct mtk_iommu_plat_data *plat_data = data->plat_data;
+ void __iomem *base = data->base;
u64 fault_iova, fault_pa;
bool layer, write;

/* Read error info from registers */
- int_state = readl_relaxed(data->base + REG_MMU_FAULT_ST1);
+ int_state = readl_relaxed(base + REG_MMU_FAULT_ST1);
if (int_state & F_REG_MMU0_FAULT_MASK) {
- regval = readl_relaxed(data->base + REG_MMU0_INT_ID);
- fault_iova = readl_relaxed(data->base + REG_MMU0_FAULT_VA);
- fault_pa = readl_relaxed(data->base + REG_MMU0_INVLD_PA);
+ regval = readl_relaxed(base + REG_MMU0_INT_ID);
+ fault_iova = readl_relaxed(base + REG_MMU0_FAULT_VA);
+ fault_pa = readl_relaxed(base + REG_MMU0_INVLD_PA);
} else {
- regval = readl_relaxed(data->base + REG_MMU1_INT_ID);
- fault_iova = readl_relaxed(data->base + REG_MMU1_FAULT_VA);
- fault_pa = readl_relaxed(data->base + REG_MMU1_INVLD_PA);
+ regval = readl_relaxed(base + REG_MMU1_INT_ID);
+ fault_iova = readl_relaxed(base + REG_MMU1_FAULT_VA);
+ fault_pa = readl_relaxed(base + REG_MMU1_INVLD_PA);
}
layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
- if (MTK_IOMMU_HAS_FLAG(data->plat_data, IOVA_34_EN)) {
+ if (MTK_IOMMU_HAS_FLAG(plat_data, IOVA_34_EN)) {
va34_32 = FIELD_GET(F_MMU_INVAL_VA_34_32_MASK, fault_iova);
fault_iova = fault_iova & F_MMU_INVAL_VA_31_12_MASK;
fault_iova |= (u64)va34_32 << 32;
@@ -310,12 +313,12 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
pa34_32 = FIELD_GET(F_MMU_INVAL_PA_34_32_MASK, fault_iova);
fault_pa |= (u64)pa34_32 << 32;

- if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
+ if (MTK_IOMMU_IS_TYPE(plat_data, MTK_IOMMU_TYPE_MM)) {
fault_port = F_MMU_INT_ID_PORT_ID(regval);
- if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM_2BITS)) {
+ if (MTK_IOMMU_HAS_FLAG(plat_data, HAS_SUB_COMM_2BITS)) {
fault_larb = F_MMU_INT_ID_COMM_ID(regval);
sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
- } else if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_SUB_COMM_3BITS)) {
+ } else if (MTK_IOMMU_HAS_FLAG(plat_data, HAS_SUB_COMM_3BITS)) {
fault_larb = F_MMU_INT_ID_COMM_ID_EXT(regval);
sub_comm = F_MMU_INT_ID_SUB_COMM_ID_EXT(regval);
} else {
@@ -334,9 +337,9 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
}

/* Interrupt clear */
- regval = readl_relaxed(data->base + REG_MMU_INT_CONTROL0);
+ regval = readl_relaxed(base + REG_MMU_INT_CONTROL0);
regval |= F_INT_CLR_BIT;
- writel_relaxed(regval, data->base + REG_MMU_INT_CONTROL0);
+ writel_relaxed(regval, base + REG_MMU_INT_CONTROL0);

mtk_iommu_tlb_flush_all(data);

--
2.18.0

2021-09-23 12:08:29

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 30/33] iommu/mediatek: Get the proper bankid for multi banks

We preassign some ports in a special bank via the new defined
bank_portmsk. Put it in the plat_data means it is not expected to be
adjusted dynamically.

If the iommu id in the iommu consumer's dtsi node is inside this
bank_portmsk, then we switch it to this special iommu bank, and initialise
the IOMMU bank HW.

Each a bank has the independent pgtable(4GB iova range). Each a bank
is a independent iommu domain/group. Currently we don't separate different
iova ranges inside a bank.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 38 +++++++++++++++++++++++++++++++++++---
drivers/iommu/mtk_iommu.h | 1 +
2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6e3f7b97f3f5..5d648f50cbe1 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -352,6 +352,30 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}

+static unsigned int mtk_iommu_get_bank_id(struct device *dev,
+ const struct mtk_iommu_plat_data *plat_data)
+{
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ unsigned int i, portmsk = 0, bankid = 0; /* bank default is 0 */
+
+ if (plat_data->bank_nr == 1)
+ return bankid;
+
+ for (i = 0; i < fwspec->num_ids; i++)
+ portmsk |= BIT(MTK_M4U_TO_PORT(fwspec->ids[i]));
+
+ for (i = 0; i < plat_data->bank_nr; i++) {
+ if (!plat_data->bank_enable[i])
+ continue;
+
+ if (portmsk & plat_data->bank_portmsk[i]) {
+ bankid = i;
+ break;
+ }
+ }
+ return bankid;
+}
+
static int mtk_iommu_get_iova_region_id(struct device *dev,
const struct mtk_iommu_plat_data *plat_data)
{
@@ -503,13 +527,14 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
struct list_head *hw_list = data->hw_list;
struct mtk_iommu_bank_data *bank;
struct device *m4udev = data->dev;
- unsigned int bankid = 0;
+ unsigned int bankid;
int ret, region_id;

region_id = mtk_iommu_get_iova_region_id(dev, data->plat_data);
if (region_id < 0)
return region_id;

+ bankid = mtk_iommu_get_bank_id(dev, data->plat_data);
bank = &data->bank[bankid];
if (!dom->bank) {
/* Data is in the frstdata in sharing pgtable case. */
@@ -636,6 +661,7 @@ static struct iommu_group *mtk_iommu_device_group(struct device *dev)
struct mtk_iommu_data *c_data = dev_iommu_priv_get(dev), *data;
struct list_head *hw_list = c_data->hw_list;
struct iommu_group *group;
+ unsigned int bankid, groupid;
int regionid;

data = mtk_iommu_get_frst_data(hw_list);
@@ -645,12 +671,18 @@ static struct iommu_group *mtk_iommu_device_group(struct device *dev)
regionid = mtk_iommu_get_iova_region_id(dev, data->plat_data);
if (regionid < 0)
return ERR_PTR(regionid);
+ bankid = mtk_iommu_get_bank_id(dev, data->plat_data);

- group = data->m4u_group[regionid];
+ /*
+ * If the bank function is enabled, each a bank is a iommu group/domain.
+ * otherwise, each a iova region is a iommu group/domain.
+ */
+ groupid = bankid ? bankid : regionid;
+ group = data->m4u_group[groupid];
if (!group) {
group = iommu_group_alloc();
if (!IS_ERR(group))
- data->m4u_group[regionid] = group;
+ data->m4u_group[groupid] = group;
} else {
iommu_group_ref_get(group);
}
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index dc0190edebf0..cf4b3d10cf2c 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -65,6 +65,7 @@ struct mtk_iommu_plat_data {

unsigned int bank_nr;
bool bank_enable[MTK_IOMMU_BANK_MAX];
+ unsigned int bank_portmsk[MTK_IOMMU_BANK_MAX];
unsigned char larbid_remap[MTK_LARB_COM_MAX][MTK_LARB_SUBCOM_MAX];
};

--
2.18.0

2021-09-23 12:08:34

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 31/33] iommu/mediatek: Initialise/Remove for multi bank dev

The registers for each bank of the IOMMU base are in order, delta is
0x1000. Initialise the base for each bank.

For all the previous SoC, we only have bank0. thus use "do {} while()"
to allow bank0 always go.

When removing the device, Not always all the banks are initialised, it
depend on if there is masters for that bank.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 44 +++++++++++++++++++++++++++------------
1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 5d648f50cbe1..3925d1d4f2cf 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -112,6 +112,7 @@
#define F_MMU_INT_ID_PORT_ID(a) (((a) >> 2) & 0x1f)

#define MTK_PROTECT_PA_ALIGN 256
+#define MTK_IOMMU_BANK_SZ 0x1000

#define PERICFG_IOMMU_1 0x714

@@ -927,11 +928,11 @@ static int mtk_iommu_probe(struct platform_device *pdev)
struct mtk_iommu_data *data;
struct device *dev = &pdev->dev;
struct resource *res;
- resource_size_t ioaddr;
+ resource_size_t ioaddr, size;
struct component_match *match = NULL;
struct regmap *infracfg;
void *protect;
- int ret;
+ int ret, i = 0;
u32 val;
char *p;
struct mtk_iommu_bank_data *bank;
@@ -973,20 +974,31 @@ static int mtk_iommu_probe(struct platform_device *pdev)
}

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ size = resource_size(res);
+ if (size < data->plat_data->bank_nr * MTK_IOMMU_BANK_SZ) {
+ dev_err(dev, "banknr %d. res %pR is not enough.\n",
+ data->plat_data->bank_nr, res);
+ return -EINVAL;
+ }
base = devm_ioremap_resource(dev, res);
if (IS_ERR(base))
return PTR_ERR(base);
ioaddr = res->start;

- bank = &data->bank[0];
- bank->id = 0;
- bank->base = base;
- bank->irq = platform_get_irq(pdev, 0);
- if (bank->irq < 0)
- return bank->irq;
- bank->pdev = dev;
- bank->pdata = data;
- spin_lock_init(&bank->tlb_lock);
+ do {
+ if (!data->plat_data->bank_enable[i])
+ continue;
+ bank = &data->bank[i];
+ bank->id = i;
+ bank->base = base + i * MTK_IOMMU_BANK_SZ;
+
+ bank->irq = platform_get_irq(pdev, i);
+ if (bank->irq < 0)
+ return bank->irq;
+ bank->pdev = dev;
+ bank->pdata = data;
+ spin_lock_init(&bank->tlb_lock);
+ } while (++i < data->plat_data->bank_nr);

if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_BCLK)) {
data->bclk = devm_clk_get(dev, "bclk");
@@ -1071,7 +1083,8 @@ static int mtk_iommu_probe(struct platform_device *pdev)
static int mtk_iommu_remove(struct platform_device *pdev)
{
struct mtk_iommu_data *data = platform_get_drvdata(pdev);
- struct mtk_iommu_bank_data *bank = &data->bank[0];
+ struct mtk_iommu_bank_data *bank;
+ int i;

iommu_device_sysfs_remove(&data->iommu);
iommu_device_unregister(&data->iommu);
@@ -1088,7 +1101,12 @@ static int mtk_iommu_remove(struct platform_device *pdev)
#endif
}
pm_runtime_disable(&pdev->dev);
- devm_free_irq(&pdev->dev, bank->irq, bank);
+ for (i = 0; i < data->plat_data->bank_nr; i++) {
+ bank = &data->bank[i];
+ if (!bank->m4u_dom)
+ continue;
+ devm_free_irq(&pdev->dev, bank->irq, bank);
+ }
return 0;
}

--
2.18.0

2021-09-23 12:08:36

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH v3 32/33] iommu/mediatek: Backup/restore regsiters for multi banks

Each bank has some independent registers. thus backup/restore them for
each a bank when suspend and resume.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 39 +++++++++++++++++++++++++++------------
drivers/iommu/mtk_iommu.h | 14 +++++++++++---
2 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 3925d1d4f2cf..3cb18ed28132 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1114,16 +1114,23 @@ static int __maybe_unused mtk_iommu_runtime_suspend(struct device *dev)
{
struct mtk_iommu_data *data = dev_get_drvdata(dev);
struct mtk_iommu_suspend_reg *reg = &data->reg;
- void __iomem *base = data->bank[0].base;
+ void __iomem *base;
+ int i = 0;

+ base = data->bank[i].base;
reg->wr_len_ctrl = readl_relaxed(base + REG_MMU_WR_LEN_CTRL);
reg->misc_ctrl = readl_relaxed(base + REG_MMU_MISC_CTRL);
reg->dcm_dis = readl_relaxed(base + REG_MMU_DCM_DIS);
reg->ctrl_reg = readl_relaxed(base + REG_MMU_CTRL_REG);
- reg->int_control0 = readl_relaxed(base + REG_MMU_INT_CONTROL0);
- reg->int_main_control = readl_relaxed(base + REG_MMU_INT_MAIN_CONTROL);
- reg->ivrp_paddr = readl_relaxed(base + REG_MMU_IVRP_PADDR);
reg->vld_pa_rng = readl_relaxed(base + REG_MMU_VLD_PA_RNG);
+ do {
+ if (!data->plat_data->bank_enable[i])
+ continue;
+ base = data->bank[i].base;
+ reg->int_control[i] = readl_relaxed(base + REG_MMU_INT_CONTROL0);
+ reg->int_main_control[i] = readl_relaxed(base + REG_MMU_INT_MAIN_CONTROL);
+ reg->ivrp_paddr[i] = readl_relaxed(base + REG_MMU_IVRP_PADDR);
+ } while (++i < data->plat_data->bank_nr);
clk_disable_unprepare(data->bclk);
return 0;
}
@@ -1132,9 +1139,9 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
{
struct mtk_iommu_data *data = dev_get_drvdata(dev);
struct mtk_iommu_suspend_reg *reg = &data->reg;
- struct mtk_iommu_domain *m4u_dom = data->bank[0].m4u_dom;
- void __iomem *base = data->bank[0].base;
- int ret;
+ struct mtk_iommu_domain *m4u_dom;
+ void __iomem *base;
+ int ret, i = 0;

ret = clk_prepare_enable(data->bclk);
if (ret) {
@@ -1157,18 +1164,26 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
* Uppon first resume, only enable the clk and return, since the values of the
* registers are not yet set.
*/
- if (!m4u_dom)
+ if (!reg->wr_len_ctrl)
return 0;

+ base = data->bank[i].base;
writel_relaxed(reg->wr_len_ctrl, base + REG_MMU_WR_LEN_CTRL);
writel_relaxed(reg->misc_ctrl, base + REG_MMU_MISC_CTRL);
writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
writel_relaxed(reg->ctrl_reg, base + REG_MMU_CTRL_REG);
- writel_relaxed(reg->int_control0, base + REG_MMU_INT_CONTROL0);
- writel_relaxed(reg->int_main_control, base + REG_MMU_INT_MAIN_CONTROL);
- writel_relaxed(reg->ivrp_paddr, base + REG_MMU_IVRP_PADDR);
writel_relaxed(reg->vld_pa_rng, base + REG_MMU_VLD_PA_RNG);
- writel(m4u_dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK, base + REG_MMU_PT_BASE_ADDR);
+ do {
+ m4u_dom = data->bank[i].m4u_dom;
+ if (!data->plat_data->bank_enable[i] || !m4u_dom)
+ continue;
+ base = data->bank[i].base;
+ writel_relaxed(reg->int_control[i], base + REG_MMU_INT_CONTROL0);
+ writel_relaxed(reg->int_main_control[i], base + REG_MMU_INT_MAIN_CONTROL);
+ writel_relaxed(reg->ivrp_paddr[i], base + REG_MMU_IVRP_PADDR);
+ writel(m4u_dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
+ base + REG_MMU_PT_BASE_ADDR);
+ } while (++i < data->plat_data->bank_nr);
return 0;
}

diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index cf4b3d10cf2c..e781ad583131 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -33,11 +33,19 @@ struct mtk_iommu_suspend_reg {
};
u32 dcm_dis;
u32 ctrl_reg;
- u32 int_control0;
- u32 int_main_control;
- u32 ivrp_paddr;
u32 vld_pa_rng;
u32 wr_len_ctrl;
+ union {
+ struct { /* only for gen1 */
+ u32 int_control0;
+ };
+
+ struct { /* only for gen2 that support multi-banks */
+ u32 int_control[MTK_IOMMU_BANK_MAX];
+ u32 int_main_control[MTK_IOMMU_BANK_MAX];
+ u32 ivrp_paddr[MTK_IOMMU_BANK_MAX];
+ };
+ };
};

enum mtk_iommu_plat {
--
2.18.0

2021-10-22 14:05:56

by Dafna Hirschfeld

[permalink] [raw]
Subject: Re: [PATCH v3 13/33] iommu/mediatek: Remove the power status checking in tlb flush all

Hi


On 23.09.21 13:58, Yong Wu wrote:
> To simplify the code, Remove the power status checking in the
> tlb_flush_all, remove this:
> if (pm_runtime_get_if_in_use(data->dev) <= 0)
> continue;
>
> After this patch, the mtk_iommu_tlb_flush_all will be called from
> a) isr
> b) pm runtime resume callback
> c) tlb flush range fail case
> d) iommu_create_device_direct_mappings
> -> iommu_flush_iotlb_all
> In first three cases, the power and clock always are enabled; d) is direct

Regarding case "c) tlb flush range fail case", I found out that this often happens
when the iommu is used while it is runtime suspended. For example the mtk-vcodec
encoder driver calls "pm_runtime_resume_and_get" only when it starts streaming but
buffers allocation is done in 'v4l2_reqbufs' before "pm_runtime_resume_and_get" is called
and then I see the warning "Partial TLB flush timed out, falling back to full flush"
I am not sure how to fix that issue, but it seems that case 'c)' might indicate that
power and clock are actually not enabled.

> mapping, the tlb flush is unnecessay since we already have tlb_flush_all
> in the pm_runtime_resume callback. When the iommu's power status is
> changed to active, the tlb always is clean.
>
> In addition, there still are 2 reasons that don't add PM status checking
> in the tlb flush all:
> a) Write tlb flush all register also is ok even though the HW has no
> power and clocks. Write ignore.
> b) pm_runtime_get_if_in_use(m4udev) is 0 when the tlb_flush_all
> is called frm pm_runtime_resume cb. From this point, we can not add
> this code above in this tlb_flush_all.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> drivers/iommu/mtk_iommu.c | 20 +++++++-------------
> 1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index e9e94944ed91..4a33b6c6b1db 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -204,10 +204,14 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
> return container_of(dom, struct mtk_iommu_domain, domain);
> }
>
> -static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data *data)
> +static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> {
> unsigned long flags;
>
> + /*
> + * No need get power status since the HW PM status nearly is active
> + * when entering here.
> + */
> spin_lock_irqsave(&data->tlb_lock, flags);
> writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> data->base + data->plat_data->inv_sel_reg);
> @@ -216,16 +220,6 @@ static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data *data)
> spin_unlock_irqrestore(&data->tlb_lock, flags);
> }
>
> -static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> -{
> - if (pm_runtime_get_if_in_use(data->dev) <= 0)
> - return;
> -
> - mtk_iommu_tlb_do_flush_all(data);
> -
> - pm_runtime_put(data->dev);
> -}
> -
> static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> struct mtk_iommu_data *data)
> {
> @@ -263,7 +257,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> if (ret) {
> dev_warn(data->dev,
> "Partial TLB flush timed out, falling back to full flush\n");
> - mtk_iommu_tlb_do_flush_all(data);
> + mtk_iommu_tlb_flush_all(data);
> }
>
> if (has_pm)
> @@ -993,7 +987,7 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
> *
> * Thus, Make sure the tlb always is clean after each PM resume.
> */
> - mtk_iommu_tlb_do_flush_all(data);
> + mtk_iommu_tlb_flush_all(data);
>
> /*
> * Uppon first resume, only enable the clk and return, since the values of the
>

2021-10-25 04:12:15

by Yong Wu (吴勇)

[permalink] [raw]
Subject: Re: [PATCH v3 13/33] iommu/mediatek: Remove the power status checking in tlb flush all

On Fri, 2021-10-22 at 16:03 +0200, Dafna Hirschfeld wrote:
> Hi
>
>
> On 23.09.21 13:58, Yong Wu wrote:
> > To simplify the code, Remove the power status checking in the
> > tlb_flush_all, remove this:
> > if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > continue;
> >
> > After this patch, the mtk_iommu_tlb_flush_all will be called from
> > a) isr
> > b) pm runtime resume callback
> > c) tlb flush range fail case
> > d) iommu_create_device_direct_mappings
> > -> iommu_flush_iotlb_all
> > In first three cases, the power and clock always are enabled; d) is
> > direct
>
> Regarding case "c) tlb flush range fail case", I found out that this
> often happens when the iommu is used while it is runtime suspended.

Which SoC and branch are you testing on?

> For example the mtk-vcodec encoder driver calls
> "pm_runtime_resume_and_get" only when it starts
> streaming but
> buffers allocation is done in 'v4l2_reqbufs' before
> "pm_runtime_resume_and_get" is called

This is the case I tried to fix in [14/33].
pm_runtime_get_if_in_use should return when the iommu device's pm is
not active when vcodec allocate buffer before pm_runtime_resume_and
get.

Do you have the devicelink patchset in your branch? if not, the vcodec
should call mtk_smi_larb_get to enable the power/clock for larbs, then
the iommu's device is active via devicelink between smi and iommu
device.

> and then I see the warning "Partial TLB flush timed out, falling back
> to full flush"
> I am not sure how to fix that issue, but it seems that case 'c)'
> might indicate that
> power and clock are actually not enabled.
>
> > mapping, the tlb flush is unnecessay since we already have
> > tlb_flush_all
> > in the pm_runtime_resume callback. When the iommu's power status is
> > changed to active, the tlb always is clean.
> >
> > In addition, there still are 2 reasons that don't add PM status
> > checking
> > in the tlb flush all:
> > a) Write tlb flush all register also is ok even though the HW has
> > no
> > power and clocks. Write ignore.
> > b) pm_runtime_get_if_in_use(m4udev) is 0 when the tlb_flush_all
> > is called frm pm_runtime_resume cb. From this point, we can not add
> > this code above in this tlb_flush_all.
> >
> > Signed-off-by: Yong Wu <[email protected]>
> > ---
> > drivers/iommu/mtk_iommu.c | 20 +++++++-------------
> > 1 file changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index e9e94944ed91..4a33b6c6b1db 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -204,10 +204,14 @@ static struct mtk_iommu_domain
> > *to_mtk_domain(struct iommu_domain *dom)
> > return container_of(dom, struct mtk_iommu_domain, domain);
> > }
> >
> > -static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data
> > *data)
> > +static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> > {
> > unsigned long flags;
> >
> > + /*
> > + * No need get power status since the HW PM status nearly is
> > active
> > + * when entering here.
> > + */
> > spin_lock_irqsave(&data->tlb_lock, flags);
> > writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> > data->base + data->plat_data->inv_sel_reg);
> > @@ -216,16 +220,6 @@ static void mtk_iommu_tlb_do_flush_all(struct
> > mtk_iommu_data *data)
> > spin_unlock_irqrestore(&data->tlb_lock, flags);
> > }
> >
> > -static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> > -{
> > - if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > - return;
> > -
> > - mtk_iommu_tlb_do_flush_all(data);
> > -
> > - pm_runtime_put(data->dev);
> > -}
> > -
> > static void mtk_iommu_tlb_flush_range_sync(unsigned long iova,
> > size_t size,
> > struct mtk_iommu_data *data)
> > {
> > @@ -263,7 +257,7 @@ static void
> > mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> > if (ret) {
> > dev_warn(data->dev,
> > "Partial TLB flush timed out, falling
> > back to full flush\n");
> > - mtk_iommu_tlb_do_flush_all(data);
> > + mtk_iommu_tlb_flush_all(data);
> > }
> >
> > if (has_pm)
> > @@ -993,7 +987,7 @@ static int __maybe_unused
> > mtk_iommu_runtime_resume(struct device *dev)
> > *
> > * Thus, Make sure the tlb always is clean after each PM
> > resume.
> > */
> > - mtk_iommu_tlb_do_flush_all(data);
> > + mtk_iommu_tlb_flush_all(data);
> >
> > /*
> > * Uppon first resume, only enable the clk and return, since
> > the values of the
> >

2021-11-04 03:29:55

by Yong Wu (吴勇)

[permalink] [raw]
Subject: Re: [PATCH v3 13/33] iommu/mediatek: Remove the power status checking in tlb flush all

On Mon, 2021-10-25 at 12:03 +0800, Yong Wu wrote:
> On Fri, 2021-10-22 at 16:03 +0200, Dafna Hirschfeld wrote:
> > Hi
> >
> >
> > On 23.09.21 13:58, Yong Wu wrote:
> > > To simplify the code, Remove the power status checking in the
> > > tlb_flush_all, remove this:
> > > if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > > continue;
> > >
> > > After this patch, the mtk_iommu_tlb_flush_all will be called from
> > > a) isr
> > > b) pm runtime resume callback
> > > c) tlb flush range fail case
> > > d) iommu_create_device_direct_mappings
> > > -> iommu_flush_iotlb_all
> > > In first three cases, the power and clock always are enabled; d)
> > > is
> > > direct
> >
> > Regarding case "c) tlb flush range fail case", I found out that
> > this
> > often happens when the iommu is used while it is runtime
> > suspended.
>
> Which SoC and branch are you testing on?
>
> > For example the mtk-vcodec encoder driver calls
> > "pm_runtime_resume_and_get" only when it starts
> > streaming but
> > buffers allocation is done in 'v4l2_reqbufs' before
> > "pm_runtime_resume_and_get" is called
>
> This is the case I tried to fix in [14/33].
> pm_runtime_get_if_in_use should return when the iommu device's pm is
> not active when vcodec allocate buffer before pm_runtime_resume_and
> get.
>
> Do you have the devicelink patchset in your branch? if not, the
> vcodec
> should call mtk_smi_larb_get to enable the power/clock for larbs,
> then
> the iommu's device is active via devicelink between smi and iommu
> device.
>
> > and then I see the warning "Partial TLB flush timed out, falling
> > back
> > to full flush"
> > I am not sure how to fix that issue, but it seems that case 'c)'

Have your issue been fixed? or more information about it.

Thanks.

> > might indicate that
> > power and clock are actually not enabled.
> >
> > > mapping, the tlb flush is unnecessay since we already have
> > > tlb_flush_all
> > > in the pm_runtime_resume callback. When the iommu's power status
> > > is
> > > changed to active, the tlb always is clean.
> > >
> > > In addition, there still are 2 reasons that don't add PM status
> > > checking
> > > in the tlb flush all:
> > > a) Write tlb flush all register also is ok even though the HW has
> > > no
> > > power and clocks. Write ignore.
> > > b) pm_runtime_get_if_in_use(m4udev) is 0 when the tlb_flush_all
> > > is called frm pm_runtime_resume cb. From this point, we can not
> > > add
> > > this code above in this tlb_flush_all.
> > >
> > > Signed-off-by: Yong Wu <[email protected]>
> > > ---
> > > drivers/iommu/mtk_iommu.c | 20 +++++++-------------
> > > 1 file changed, 7 insertions(+), 13 deletions(-)

Subject: Re: [PATCH v3 26/33] iommu/mediatek: Add mtk_iommu_bank_data structure

Il 23/09/21 13:58, Yong Wu ha scritto:
> Prepare for supporting multi-banks for the IOMMU HW, No functional change.
>
> Add a new structure(mtk_iommu_bank_data) for each a bank. Each a bank have
> the independent HW base/IRQ/tlb-range ops, and each a bank has its special
> iommu-domain(independent pgtable), thus, also move the domain information
> into it.
>
> In previous SoC, we have only one bank which could be treated as bank0(
> bankid always is 0 for the previous SoC).
>
> After adding this structure, the tlb operations and irq could use
> bank_data as parameter.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> drivers/iommu/mtk_iommu.c | 137 ++++++++++++++++++++++----------------
> drivers/iommu/mtk_iommu.h | 24 ++++++-
> 2 files changed, 99 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index ac31fe8b7aaf..6ef3eb3cad92 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -146,7 +146,7 @@ struct mtk_iommu_domain {
> struct io_pgtable_cfg cfg;
> struct io_pgtable_ops *iop;
>
> - struct mtk_iommu_data *data;
> + struct mtk_iommu_bank_data *bank;
> struct iommu_domain domain;
> };
>
> @@ -221,25 +221,29 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
>
> static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> {
> - void __iomem *base = data->base;
> + /* Tlb flush all always is in bank0. */
> + struct mtk_iommu_bank_data *bank = &data->bank[0];
> + void __iomem *base = bank->base;
> unsigned long flags;
>
> /*
> * No need get power status since the HW PM status nearly is active
> * when entering here.
> */
> - spin_lock_irqsave(&data->tlb_lock, flags);
> + spin_lock_irqsave(&bank->tlb_lock, flags);
> writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + data->plat_data->inv_sel_reg);
> writel_relaxed(F_ALL_INVLD, base + REG_MMU_INVALIDATE);
> wmb(); /* Make sure the tlb flush all done */
> - spin_unlock_irqrestore(&data->tlb_lock, flags);
> + spin_unlock_irqrestore(&bank->tlb_lock, flags);
> }
>
> static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> - struct mtk_iommu_data *data)
> + struct mtk_iommu_bank_data *bank)
> {
> - struct list_head *head = data->hw_list;
> - bool has_pm = !!data->dev->pm_domain;
> + struct list_head *head = bank->pdata->hw_list;
> + bool has_pm = !!bank->pdev->pm_domain;
> + struct mtk_iommu_bank_data *curbank;
> + struct mtk_iommu_data *data;
> unsigned long flags;
> void __iomem *base;
> int ret;
> @@ -251,9 +255,10 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> continue;
> }
>
> - base = data->base;
> + curbank = &data->bank[bank->id];
> + base = curbank->base;
>
> - spin_lock_irqsave(&data->tlb_lock, flags);
> + spin_lock_irqsave(&curbank->tlb_lock, flags);
> writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> base + data->plat_data->inv_sel_reg);
>
> @@ -268,7 +273,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
>
> /* Clear the CPE status */
> writel_relaxed(0, base + REG_MMU_CPE_DONE);
> - spin_unlock_irqrestore(&data->tlb_lock, flags);
> + spin_unlock_irqrestore(&curbank->tlb_lock, flags);
>
> if (ret) {
> dev_warn(data->dev,
> @@ -283,12 +288,13 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
>
> static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
> {
> - struct mtk_iommu_data *data = dev_id;
> - struct mtk_iommu_domain *dom = data->m4u_dom;
> + struct mtk_iommu_bank_data *bank = dev_id;
> + struct mtk_iommu_data *data = bank->pdata;
> + struct mtk_iommu_domain *dom = bank->m4u_dom;
> unsigned int fault_larb = 0, fault_port = 0, sub_comm = 0;
> u32 int_state, regval, va34_32, pa34_32;
> const struct mtk_iommu_plat_data *plat_data = data->plat_data;
> - void __iomem *base = data->base;
> + void __iomem *base = bank->base;
> u64 fault_iova, fault_pa;
> bool layer, write;
>
> @@ -327,10 +333,10 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
> fault_larb = data->plat_data->larbid_remap[fault_larb][sub_comm];
> }
>
> - if (report_iommu_fault(&dom->domain, data->dev, fault_iova,
> + if (report_iommu_fault(&dom->domain, bank->pdev, fault_iova,
> write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) {
> dev_err_ratelimited(
> - data->dev,
> + bank->pdev,
> "fault type=0x%x iova=0x%llx pa=0x%llx master=0x%x(larb=%d port=%d) layer=%d %s\n",
> int_state, fault_iova, fault_pa, regval, fault_larb, fault_port,
> layer, write ? "write" : "read");
> @@ -427,12 +433,14 @@ static int mtk_iommu_domain_finalise(struct mtk_iommu_domain *dom,
> unsigned int domid)
> {
> const struct mtk_iommu_iova_region *region;
> -
> - /* Use the exist domain as there is only one pgtable here. */
> - if (data->m4u_dom) {
> - dom->iop = data->m4u_dom->iop;
> - dom->cfg = data->m4u_dom->cfg;
> - dom->domain.pgsize_bitmap = data->m4u_dom->cfg.pgsize_bitmap;
> + struct mtk_iommu_domain *m4u_dom;
> +
> + /* Always use bank0 in sharing pgtable case */
> + m4u_dom = data->bank[0].m4u_dom;
> + if (m4u_dom) {
> + dom->iop = m4u_dom->iop;
> + dom->cfg = m4u_dom->cfg;
> + dom->domain.pgsize_bitmap = m4u_dom->cfg.pgsize_bitmap;
> goto update_iova_region;
> }
>
> @@ -493,23 +501,26 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
> struct mtk_iommu_data *data = dev_iommu_priv_get(dev), *frstdata;
> struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> struct list_head *hw_list = data->hw_list;
> + struct mtk_iommu_bank_data *bank;
> struct device *m4udev = data->dev;
> + unsigned int bankid = 0;
> int ret, domid;
>
> domid = mtk_iommu_get_domain_id(dev, data->plat_data);
> if (domid < 0)
> return domid;
>
> - if (!dom->data) {
> + bank = &data->bank[bankid];
> + if (!dom->bank) {
> /* Data is in the frstdata in sharing pgtable case. */
> frstdata = mtk_iommu_get_frst_data(hw_list);
>
> if (mtk_iommu_domain_finalise(dom, frstdata, domid))
> return -ENODEV;
> - dom->data = data;
> + dom->bank = bank;
> }
>
> - if (!data->m4u_dom) { /* Initialize the M4U HW */
> + if (!bank->m4u_dom) { /* Initialize the M4U HW */
> ret = pm_runtime_resume_and_get(m4udev);
> if (ret < 0)
> return ret;
> @@ -519,9 +530,9 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
> pm_runtime_put(m4udev);
> return ret;
> }
> - data->m4u_dom = dom;
> + bank->m4u_dom = dom;
> writel(dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
> - data->base + REG_MMU_PT_BASE_ADDR);
> + bank->base + REG_MMU_PT_BASE_ADDR);
>
> pm_runtime_put(m4udev);
> }
> @@ -543,7 +554,7 @@ static int mtk_iommu_map(struct iommu_domain *domain, unsigned long iova,
> struct mtk_iommu_domain *dom = to_mtk_domain(domain);
>
> /* The "4GB mode" M4U physically can not use the lower remap of Dram. */
> - if (dom->data->enable_4GB)
> + if (dom->bank->pdata->enable_4GB)
> paddr |= BIT_ULL(32);
>
> /* Synchronize with the tlb_lock */
> @@ -564,7 +575,7 @@ static void mtk_iommu_flush_iotlb_all(struct iommu_domain *domain)
> {
> struct mtk_iommu_domain *dom = to_mtk_domain(domain);
>
> - mtk_iommu_tlb_flush_all(dom->data);
> + mtk_iommu_tlb_flush_all(dom->bank->pdata);
> }
>
> static void mtk_iommu_iotlb_sync(struct iommu_domain *domain,
> @@ -573,7 +584,7 @@ static void mtk_iommu_iotlb_sync(struct iommu_domain *domain,
> struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> size_t length = gather->end - gather->start + 1;
>
> - mtk_iommu_tlb_flush_range_sync(gather->start, length, dom->data);
> + mtk_iommu_tlb_flush_range_sync(gather->start, length, dom->bank);
> }
>
> static void mtk_iommu_sync_map(struct iommu_domain *domain, unsigned long iova,
> @@ -581,7 +592,7 @@ static void mtk_iommu_sync_map(struct iommu_domain *domain, unsigned long iova,
> {
> struct mtk_iommu_domain *dom = to_mtk_domain(domain);
>
> - mtk_iommu_tlb_flush_range_sync(iova, size, dom->data);
> + mtk_iommu_tlb_flush_range_sync(iova, size, dom->bank);
> }
>
> static phys_addr_t mtk_iommu_iova_to_phys(struct iommu_domain *domain,
> @@ -591,7 +602,7 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct iommu_domain *domain,
> phys_addr_t pa;
>
> pa = dom->iop->iova_to_phys(dom->iop, iova);
> - if (dom->data->enable_4GB && pa >= MTK_IOMMU_4GB_MODE_REMAP_BASE)
> + if (dom->bank->pdata->enable_4GB && pa >= MTK_IOMMU_4GB_MODE_REMAP_BASE)
> pa &= ~BIT_ULL(32);
>
> return pa;
> @@ -720,16 +731,17 @@ static const struct iommu_ops mtk_iommu_ops = {
>
> static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> {
> + const struct mtk_iommu_bank_data *bank0 = &data->bank[0];
> u32 regval;
>
> if (data->plat_data->m4u_plat == M4U_MT8173) {
> regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
> F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173;
> } else {
> - regval = readl_relaxed(data->base + REG_MMU_CTRL_REG);
> + regval = readl_relaxed(bank0->base + REG_MMU_CTRL_REG);
> regval |= F_MMU_TF_PROT_TO_PROGRAM_ADDR;
> }
> - writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
> + writel_relaxed(regval, bank0->base + REG_MMU_CTRL_REG);
>
> if (data->enable_4GB &&
> MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_VLD_PA_RNG)) {
> @@ -738,31 +750,31 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> * 0x1_0000_0000 to 0x1_ffff_ffff. here record bit[32:30].
> */
> regval = F_MMU_VLD_PA_RNG(7, 4);
> - writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
> + writel_relaxed(regval, bank0->base + REG_MMU_VLD_PA_RNG);
> }
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, DCM_DISABLE))
> - writel_relaxed(F_MMU_DCM, data->base + REG_MMU_DCM_DIS);
> + writel_relaxed(F_MMU_DCM, bank0->base + REG_MMU_DCM_DIS);
> else
> - writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
> + writel_relaxed(0, bank0->base + REG_MMU_DCM_DIS);
>
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, WR_THROT_EN)) {
> /* write command throttling mode */
> - regval = readl_relaxed(data->base + REG_MMU_WR_LEN_CTRL);
> + regval = readl_relaxed(bank0->base + REG_MMU_WR_LEN_CTRL);
> regval &= ~F_MMU_WR_THROT_DIS_MASK;
> - writel_relaxed(regval, data->base + REG_MMU_WR_LEN_CTRL);
> + writel_relaxed(regval, bank0->base + REG_MMU_WR_LEN_CTRL);
> }
>
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, RESET_AXI)) {
> /* The register is called STANDARD_AXI_MODE in this case */
> regval = 0;
> } else {
> - regval = readl_relaxed(data->base + REG_MMU_MISC_CTRL);
> + regval = readl_relaxed(bank0->base + REG_MMU_MISC_CTRL);
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, NOT_STD_AXI_MODE))
> regval &= ~F_MMU_STANDARD_AXI_MODE_MASK;
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, OUT_ORDER_WR_EN))
> regval &= ~F_MMU_IN_ORDER_WR_EN_MASK;
> }
> - writel_relaxed(regval, data->base + REG_MMU_MISC_CTRL);
> + writel_relaxed(regval, bank0->base + REG_MMU_MISC_CTRL);
>
> regval = F_L2_MULIT_HIT_EN |
> F_TABLE_WALK_FAULT_INT_EN |
> @@ -770,7 +782,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> F_MISS_FIFO_OVERFLOW_INT_EN |
> F_PREFETCH_FIFO_ERR_INT_EN |
> F_MISS_FIFO_ERR_INT_EN;
> - writel_relaxed(regval, data->base + REG_MMU_INT_CONTROL0);
> + writel_relaxed(regval, bank0->base + REG_MMU_INT_CONTROL0);
>
> regval = F_INT_TRANSLATION_FAULT |
> F_INT_MAIN_MULTI_HIT_FAULT |
> @@ -779,19 +791,19 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> F_INT_TLB_MISS_FAULT |
> F_INT_MISS_TRANSACTION_FIFO_FAULT |
> F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
> - writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
> + writel_relaxed(regval, bank0->base + REG_MMU_INT_MAIN_CONTROL);
>
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_LEGACY_IVRP_PADDR))
> regval = (data->protect_base >> 1) | (data->enable_4GB << 31);
> else
> regval = lower_32_bits(data->protect_base) |
> upper_32_bits(data->protect_base);
> - writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
> + writel_relaxed(regval, bank0->base + REG_MMU_IVRP_PADDR);
>
> - if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
> - dev_name(data->dev), (void *)data)) {
> - writel_relaxed(0, data->base + REG_MMU_PT_BASE_ADDR);
> - dev_err(data->dev, "Failed @ IRQ-%d Request\n", data->irq);
> + if (devm_request_irq(bank0->pdev, bank0->irq, mtk_iommu_isr, 0,
> + dev_name(bank0->pdev), (void *)bank0)) {
> + writel_relaxed(0, bank0->base + REG_MMU_PT_BASE_ADDR);
> + dev_err(bank0->pdev, "Failed @ IRQ-%d Request\n", bank0->irq);
> return -ENODEV;
> }
>
> @@ -884,6 +896,8 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> int ret;
> u32 val;
> char *p;
> + struct mtk_iommu_bank_data *bank;
> + void __iomem *base;
>
> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> @@ -921,14 +935,20 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> }
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->base = devm_ioremap_resource(dev, res);
> - if (IS_ERR(data->base))
> - return PTR_ERR(data->base);
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> ioaddr = res->start;
>
> - data->irq = platform_get_irq(pdev, 0);
> - if (data->irq < 0)
> - return data->irq;
> + bank = &data->bank[0];
> + bank->id = 0;
> + bank->base = base;
> + bank->irq = platform_get_irq(pdev, 0);
> + if (bank->irq < 0)
> + return bank->irq;
> + bank->pdev = dev;
> + bank->pdata = data;
> + spin_lock_init(&bank->tlb_lock);
>
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_BCLK)) {
> data->bclk = devm_clk_get(dev, "bclk");
> @@ -964,8 +984,6 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> if (ret)
> goto out_sysfs_remove;
>
> - spin_lock_init(&data->tlb_lock);
> -
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, SHARE_PGTABLE)) {
> list_add_tail(&data->list, data->plat_data->hw_list);
> data->hw_list = data->plat_data->hw_list;
> @@ -1015,6 +1033,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> static int mtk_iommu_remove(struct platform_device *pdev)
> {
> struct mtk_iommu_data *data = platform_get_drvdata(pdev);
> + struct mtk_iommu_bank_data *bank = &data->bank[0];
>
> iommu_device_sysfs_remove(&data->iommu);
> iommu_device_unregister(&data->iommu);
> @@ -1031,7 +1050,7 @@ static int mtk_iommu_remove(struct platform_device *pdev)
> #endif
> }
> pm_runtime_disable(&pdev->dev);
> - devm_free_irq(&pdev->dev, data->irq, data);
> + devm_free_irq(&pdev->dev, bank->irq, bank);
> return 0;
> }
>
> @@ -1039,7 +1058,7 @@ static int __maybe_unused mtk_iommu_runtime_suspend(struct device *dev)
> {
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
> struct mtk_iommu_suspend_reg *reg = &data->reg;
> - void __iomem *base = data->base;
> + void __iomem *base = data->bank[0].base;
>
> reg->wr_len_ctrl = readl_relaxed(base + REG_MMU_WR_LEN_CTRL);
> reg->misc_ctrl = readl_relaxed(base + REG_MMU_MISC_CTRL);
> @@ -1057,8 +1076,8 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
> {
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
> struct mtk_iommu_suspend_reg *reg = &data->reg;
> - struct mtk_iommu_domain *m4u_dom = data->m4u_dom;
> - void __iomem *base = data->base;
> + struct mtk_iommu_domain *m4u_dom = data->bank[0].m4u_dom;
> + void __iomem *base = data->bank[0].base;
> int ret;
>
> ret = clk_prepare_enable(data->bclk);
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index a9dc79c5a724..881fade8d39a 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -24,6 +24,8 @@
>
> #define MTK_IOMMU_GROUP_MAX 8
>
> +#define MTK_IOMMU_BANK_MAX 5
> +
> struct mtk_iommu_suspend_reg {
> union {
> u32 standard_axi_mode;/* v1 */
> @@ -65,17 +67,33 @@ struct mtk_iommu_plat_data {
>
> struct mtk_iommu_domain;
>
> -struct mtk_iommu_data {
> +struct mtk_iommu_bank_data {
> void __iomem *base;
> int irq;
> + unsigned int id;
> + struct device *pdev;

`pdev` may be a bit misleading, as it's conventionally read as "platform device"
and not as the intended "parent device"... perhaps calling it parent_dev would be
more appropriate.

> + struct mtk_iommu_data *pdata; /* parent data */

Same here, pdata -> parent_data

> + spinlock_t tlb_lock; /* lock for tlb range flush */
> + struct mtk_iommu_domain *m4u_dom; /* Each bank has a domain */
> +};
> +
> +struct mtk_iommu_data {
> + union {
> + struct { /* only for gen1 */
> + void __iomem *base;
> + int irq;
> + struct mtk_iommu_domain *m4u_dom;
> + };
> +
> + /* only for gen2 that support multi-banks */
> + struct mtk_iommu_bank_data bank[MTK_IOMMU_BANK_MAX];
> + };

Sorry, but I really don't like this union... please, update mtk_iommu_v1 to always
use bank[0] or, more appropriately, dynamically allocate the bank array with a
devm_kzalloc call (as to not waste memory on platforms with less available banks).

In that case, you would have...

> struct device *dev;
> struct clk *bclk;
> phys_addr_t protect_base; /* protect memory base */
> struct mtk_iommu_suspend_reg reg;
> - struct mtk_iommu_domain *m4u_dom;
> struct iommu_group *m4u_group[MTK_IOMMU_GROUP_MAX];
> bool enable_4GB;
> - spinlock_t tlb_lock; /* lock for tlb range flush */

struct mtk_iommu_bank_data *banks;
u8 num_banks;

... where `num_banks` gets copied from the same in mtk_iommu_plat_data, defined
for each SoC, and where `banks` is dynamically allocated in mtk_iommu.c and
mtk_iommu_v1.c's probe() callback.

>
> struct iommu_device iommu;
> const struct mtk_iommu_plat_data *plat_data;
>



Subject: Re: [PATCH v3 28/33] iommu/mediatek: Add bank_nr and bank_enable

Il 23/09/21 13:58, Yong Wu ha scritto:
> Prepare for supporting multi banks, Adds two variables in the plat_data:
> bank_nr: the bank number that this SoC support;
> bank_enable: list if the banks is enabled.
>
> Add them for all the current SoC, bank_nr always is 1 and only
> bank_enable[0] is enabled.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> drivers/iommu/mtk_iommu.c | 18 ++++++++++++++++++
> drivers/iommu/mtk_iommu.h | 3 +++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index c139675d99e6..4ad85469f68f 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -1134,6 +1134,8 @@ static const struct mtk_iommu_plat_data mt2712_data = {
> NOT_STD_AXI_MODE | MTK_IOMMU_TYPE_MM,
> .hw_list = &m4ulist,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> + .bank_nr = 1,
> + .bank_enable = {true},
> .iova_region = single_domain,
> .iova_region_nr = ARRAY_SIZE(single_domain),
> .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
> @@ -1144,6 +1146,8 @@ static const struct mtk_iommu_plat_data mt6779_data = {
> .flags = HAS_SUB_COMM_2BITS | OUT_ORDER_WR_EN | WR_THROT_EN |
> NOT_STD_AXI_MODE | MTK_IOMMU_TYPE_MM,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
> + .bank_nr = 1,
> + .bank_enable = {true},
> .iova_region = single_domain,
> .iova_region_nr = ARRAY_SIZE(single_domain),
> .larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}},
> @@ -1154,6 +1158,8 @@ static const struct mtk_iommu_plat_data mt8167_data = {
> .flags = RESET_AXI | HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE |
> MTK_IOMMU_TYPE_MM,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> + .bank_nr = 1,
> + .bank_enable = {true},
> .iova_region = single_domain,
> .iova_region_nr = ARRAY_SIZE(single_domain),
> .larbid_remap = {{0}, {1}, {2}}, /* Linear mapping. */
> @@ -1165,6 +1171,8 @@ static const struct mtk_iommu_plat_data mt8173_data = {
> HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE |
> MTK_IOMMU_TYPE_MM,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> + .bank_nr = 1,
> + .bank_enable = {true},
> .iova_region = single_domain,
> .iova_region_nr = ARRAY_SIZE(single_domain),
> .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
> @@ -1174,6 +1182,8 @@ static const struct mtk_iommu_plat_data mt8183_data = {
> .m4u_plat = M4U_MT8183,
> .flags = RESET_AXI | MTK_IOMMU_TYPE_MM,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> + .bank_nr = 1,
> + .bank_enable = {true},
> .iova_region = single_domain,
> .iova_region_nr = ARRAY_SIZE(single_domain),
> .larbid_remap = {{0}, {4}, {5}, {6}, {7}, {2}, {3}, {1}},
> @@ -1185,6 +1195,8 @@ static const struct mtk_iommu_plat_data mt8192_data = {
> WR_THROT_EN | IOVA_34_EN | NOT_STD_AXI_MODE |
> MTK_IOMMU_TYPE_MM,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
> + .bank_nr = 1,
> + .bank_enable = {true},
> .iova_region = mt8192_multi_dom,
> .iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
> .larbid_remap = {{0}, {1}, {4, 5}, {7}, {2}, {9, 11, 19, 20},
> @@ -1196,6 +1208,8 @@ static const struct mtk_iommu_plat_data mt8195_data_infra = {
> .flags = WR_THROT_EN | DCM_DISABLE |
> MTK_IOMMU_TYPE_INFRA | IFA_IOMMU_PCIe_SUPPORT,
> .pericfg_comp_str = "mediatek,mt8195-pericfg_ao",
> + .bank_nr = 1,
> + .bank_enable = {true},
> .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
> .iova_region = single_domain,
> .iova_region_nr = ARRAY_SIZE(single_domain),
> @@ -1208,6 +1222,8 @@ static const struct mtk_iommu_plat_data mt8195_data_vdo = {
> SHARE_PGTABLE | MTK_IOMMU_TYPE_MM,
> .hw_list = &m4ulist,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
> + .bank_nr = 1,
> + .bank_enable = {true},
> .iova_region = mt8192_multi_dom,
> .iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
> .larbid_remap = {{2, 0}, {21}, {24}, {7}, {19}, {9, 10, 11},
> @@ -1221,6 +1237,8 @@ static const struct mtk_iommu_plat_data mt8195_data_vpp = {
> SHARE_PGTABLE | MTK_IOMMU_TYPE_MM,
> .hw_list = &m4ulist,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
> + .bank_nr = 1,
> + .bank_enable = {true},
> .iova_region = mt8192_multi_dom,
> .iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
> .larbid_remap = {{1}, {3}, {22, 0, 0, 0, 23}, {8},
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index 881fade8d39a..dc0190edebf0 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -62,6 +62,9 @@ struct mtk_iommu_plat_data {
> struct list_head *hw_list;
> unsigned int iova_region_nr;
> const struct mtk_iommu_iova_region *iova_region;
> +
> + unsigned int bank_nr;

As said in the review for patch 26/33, maybe this can be renamed as `num_banks`
instead... and also, this can be a `u8` to save some memory.

> + bool bank_enable[MTK_IOMMU_BANK_MAX];
> unsigned char larbid_remap[MTK_LARB_COM_MAX][MTK_LARB_SUBCOM_MAX];
> };
>
>


Subject: Re: [PATCH v3 32/33] iommu/mediatek: Backup/restore regsiters for multi banks

Il 23/09/21 13:58, Yong Wu ha scritto:
> Each bank has some independent registers. thus backup/restore them for
> each a bank when suspend and resume.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> drivers/iommu/mtk_iommu.c | 39 +++++++++++++++++++++++++++------------
> drivers/iommu/mtk_iommu.h | 14 +++++++++++---
> 2 files changed, 38 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 3925d1d4f2cf..3cb18ed28132 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -1114,16 +1114,23 @@ static int __maybe_unused mtk_iommu_runtime_suspend(struct device *dev)
> {
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
> struct mtk_iommu_suspend_reg *reg = &data->reg;
> - void __iomem *base = data->bank[0].base;
> + void __iomem *base;
> + int i = 0;
>
> + base = data->bank[i].base;
> reg->wr_len_ctrl = readl_relaxed(base + REG_MMU_WR_LEN_CTRL);
> reg->misc_ctrl = readl_relaxed(base + REG_MMU_MISC_CTRL);
> reg->dcm_dis = readl_relaxed(base + REG_MMU_DCM_DIS);
> reg->ctrl_reg = readl_relaxed(base + REG_MMU_CTRL_REG);
> - reg->int_control0 = readl_relaxed(base + REG_MMU_INT_CONTROL0);
> - reg->int_main_control = readl_relaxed(base + REG_MMU_INT_MAIN_CONTROL);
> - reg->ivrp_paddr = readl_relaxed(base + REG_MMU_IVRP_PADDR);
> reg->vld_pa_rng = readl_relaxed(base + REG_MMU_VLD_PA_RNG);
> + do {
> + if (!data->plat_data->bank_enable[i])
> + continue;
> + base = data->bank[i].base;
> + reg->int_control[i] = readl_relaxed(base + REG_MMU_INT_CONTROL0);
> + reg->int_main_control[i] = readl_relaxed(base + REG_MMU_INT_MAIN_CONTROL);
> + reg->ivrp_paddr[i] = readl_relaxed(base + REG_MMU_IVRP_PADDR);
> + } while (++i < data->plat_data->bank_nr);
> clk_disable_unprepare(data->bclk);
> return 0;
> }
> @@ -1132,9 +1139,9 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
> {
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
> struct mtk_iommu_suspend_reg *reg = &data->reg;
> - struct mtk_iommu_domain *m4u_dom = data->bank[0].m4u_dom;
> - void __iomem *base = data->bank[0].base;
> - int ret;
> + struct mtk_iommu_domain *m4u_dom;
> + void __iomem *base;
> + int ret, i = 0;
>
> ret = clk_prepare_enable(data->bclk);
> if (ret) {
> @@ -1157,18 +1164,26 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
> * Uppon first resume, only enable the clk and return, since the values of the
> * registers are not yet set.
> */
> - if (!m4u_dom)
> + if (!reg->wr_len_ctrl)
> return 0;
>
> + base = data->bank[i].base;
> writel_relaxed(reg->wr_len_ctrl, base + REG_MMU_WR_LEN_CTRL);
> writel_relaxed(reg->misc_ctrl, base + REG_MMU_MISC_CTRL);
> writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
> writel_relaxed(reg->ctrl_reg, base + REG_MMU_CTRL_REG);
> - writel_relaxed(reg->int_control0, base + REG_MMU_INT_CONTROL0);
> - writel_relaxed(reg->int_main_control, base + REG_MMU_INT_MAIN_CONTROL);
> - writel_relaxed(reg->ivrp_paddr, base + REG_MMU_IVRP_PADDR);
> writel_relaxed(reg->vld_pa_rng, base + REG_MMU_VLD_PA_RNG);
> - writel(m4u_dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK, base + REG_MMU_PT_BASE_ADDR);
> + do {
> + m4u_dom = data->bank[i].m4u_dom;
> + if (!data->plat_data->bank_enable[i] || !m4u_dom)
> + continue;
> + base = data->bank[i].base;
> + writel_relaxed(reg->int_control[i], base + REG_MMU_INT_CONTROL0);
> + writel_relaxed(reg->int_main_control[i], base + REG_MMU_INT_MAIN_CONTROL);
> + writel_relaxed(reg->ivrp_paddr[i], base + REG_MMU_IVRP_PADDR);
> + writel(m4u_dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
> + base + REG_MMU_PT_BASE_ADDR);
> + } while (++i < data->plat_data->bank_nr);
> return 0;
> }
>
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index cf4b3d10cf2c..e781ad583131 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -33,11 +33,19 @@ struct mtk_iommu_suspend_reg {
> };
> u32 dcm_dis;
> u32 ctrl_reg;
> - u32 int_control0;
> - u32 int_main_control;
> - u32 ivrp_paddr;
> u32 vld_pa_rng;
> u32 wr_len_ctrl;
> + union {
> + struct { /* only for gen1 */
> + u32 int_control0;
> + };
> +
> + struct { /* only for gen2 that support multi-banks */
> + u32 int_control[MTK_IOMMU_BANK_MAX];
> + u32 int_main_control[MTK_IOMMU_BANK_MAX];
> + u32 ivrp_paddr[MTK_IOMMU_BANK_MAX];
> + };
> + };

There's no need for yet another union... just update mtk_iommu_v1.c to use
int_control[0] instead of int_control0.

> };
>
> enum mtk_iommu_plat {
>



Subject: Re: [PATCH v3 29/33] iommu/mediatek: Change the domid to iova_region_id

Il 23/09/21 13:58, Yong Wu ha scritto:
> Prepare for adding bankid, also no functional change.
>
> In the previous SoC, each a iova_region is a domain; In the multi-banks
> case, each a bank is a domain, then the original function name
> "mtk_iommu_get_domain_id" is not proper. Use "iova_region_id" instead of
> "domain_id".
>
> Signed-off-by: Yong Wu <[email protected]>



Reviewed-by: AngeloGioacchino Del Regno <[email protected]>





Subject: Re: [PATCH v3 25/33] iommu/mediatek: Just move code position in hw_init

Il 23/09/21 13:58, Yong Wu ha scritto:
> No functional change too, prepare for mt8195 IOMMU support bank functions.
> Some global control settings are in bank0 while the other banks have
> their bank independent setting. Here only move the global control
> settings and the independent registers together.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>


Subject: Re: [PATCH v3 27/33] iommu/mediatek: Initialise bank HW for each a bank

Il 23/09/21 13:58, Yong Wu ha scritto:
> The mt8195 IOMMU HW max support 5 banks, and regarding the banks'
> registers, it looks like:
>
> ----------------------------------------
> |bank0 | bank1 | bank2 | bank3 | bank4|
> ----------------------------------------
> |global |
> |control| null
> |regs |
> -----------------------------------------
> |bank |bank |bank |bank |bank |
> |regs |regs |regs |regs |regs |
> | | | | | |
> -----------------------------------------
>
> Each bank has some special bank registers and it share bank0's global
> control registers. this patch initialise the bank hw with the bankid.
>
> In the hw_init, we always initialise bank0's control register since
> we don't know if the bank0 is initialised.
>
> Additionally, About each bank's register base, always delta 0x1000.
> like bank[x + 1] = bank[x] + 0x1000.
>
> Signed-off-by: Yong Wu <[email protected]>


Reviewed-by: AngeloGioacchino Del Regno <[email protected]>



Subject: Re: [PATCH v3 24/33] iommu/mediatek: Only adjust code about register base

Il 23/09/21 13:58, Yong Wu ha scritto:
> No functional change. Use "base" instead of the data->base. This is
> avoid to touch too many lines in the next patches.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>



Subject: Re: [PATCH v3 16/33] iommu/mediatek: Add IOMMU_TYPE flag

Il 23/09/21 13:58, Yong Wu ha scritto:
> Add IOMMU_TYPE definition. In the mt8195, we have another IOMMU_TYPE:
> infra iommu, also there will be another APU_IOMMU, thus, use 2bits for the
> IOMMU_TYPE.
>
> Signed-off-by: Yong Wu <[email protected]>



Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

Subject: Re: [PATCH v3 06/33] iommu/mediatek: Add 12G~16G support for multi domains

Il 23/09/21 13:58, Yong Wu ha scritto:
> In mt8192, we preassign 0-4G; 4G-8G; 8G-12G for different multimedia
> engines. This depends on the "dma-ranges=" in the iommu consumer's dtsi
> node.
>
> Adds 12G-16G region here. and reword the previous comment. we don't limit
> which master locate in which region.
>
> CCU still is 8G-12G. Don't change it here.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>


Subject: Re: [PATCH v3 23/33] iommu/mediatek: Add mt8195 support

Il 23/09/21 13:58, Yong Wu ha scritto:
> mt8195 has 3 IOMMU, containing 2 MM IOMMUs, one is for vdo, the other
> is for vpp. and 1 INFRA IOMMU.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>



Subject: Re: [PATCH v3 11/33] iommu/mediatek: Remove the granule in the tlb flush

Il 23/09/21 13:58, Yong Wu ha scritto:
> The MediaTek IOMMU don't care about granule when tlb flushing.
> Remove this variable.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>


Subject: Re: [PATCH v3 08/33] iommu/mediatek: Add a flag NON_STD_AXI

Il 23/09/21 13:58, Yong Wu ha scritto:
> Add a new flag NON_STD_AXI, All the previous SoC support this flag.
> Prepare for adding infra and apu iommu which don't support this.
>
> Signed-off-by: Yong Wu <[email protected]>



Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

Subject: Re: [PATCH v3 21/33] iommu/mediatek: Add infra iommu support

Il 23/09/21 13:58, Yong Wu ha scritto:
> The infra iommu enable bits in mt8195 is in the pericfg register segment,
> use regmap to update it.
>
> If infra iommu master translation fault, It don't have the larbid/portid,
> thus print out the whole register value.
>
> Since regmap_update_bits may fail, add return value for mtk_iommu_config.
>
> Signed-off-by: Yong Wu <[email protected]>



Reviewed-by: AngeloGioacchino Del Regno <[email protected]>



Subject: Re: [PATCH v3 22/33] iommu/mediatek: Add PCIe support

Il 23/09/21 13:58, Yong Wu ha scritto:
> Currently the code for of_iommu_configure_dev_id is like this:
>
> static int of_iommu_configure_dev_id(struct device_node *master_np,
> struct device *dev,
> const u32 *id)
> {
> struct of_phandle_args iommu_spec = { .args_count = 1 };
>
> err = of_map_id(master_np, *id, "iommu-map",
> "iommu-map-mask", &iommu_spec.np,
> iommu_spec.args);
> ...
> }
>
> It supports only one id output. BUT our PCIe HW has two ID(one is for
> writing, the other is for reading). I'm not sure if we should change
> of_map_id to support output MAX_PHANDLE_ARGS.
>
> Here add the solution in ourselve drivers. If it's pcie case, enable one
> more bit.
>
> Not all infra iommu support PCIe, thus add a PCIe support flag here.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> drivers/iommu/mtk_iommu.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 37d6dfb4feab..3f1fd8036345 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -20,6 +20,7 @@
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> +#include <linux/pci.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> @@ -132,6 +133,7 @@
> #define MTK_IOMMU_TYPE_MM (0x0 << 13)
> #define MTK_IOMMU_TYPE_INFRA (0x1 << 13)
> #define MTK_IOMMU_TYPE_MASK (0x3 << 13)
> +#define IFA_IOMMU_PCIe_SUPPORT BIT(15)

This definition looks like "breaking" the naming convention that's used in this
driver... what about MTK_INFRA_IOMMU_PCIE_SUPPORT?

>
> #define MTK_IOMMU_HAS_FLAG(pdata, _x) (!!(((pdata)->flags) & (_x)))
>
> @@ -401,8 +403,11 @@ static int mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,
> larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid);
> } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA)) {
> peri_mmuen_msk = BIT(portid);
> - peri_mmuen = enable ? peri_mmuen_msk : 0;
> + /* PCIdev has only one output id, enable the next writing bit for PCIe */
> + if (dev_is_pci(dev))
> + peri_mmuen_msk |= BIT(portid + 1);
>
> + peri_mmuen = enable ? peri_mmuen_msk : 0;
> ret = regmap_update_bits(data->pericfg, PERICFG_IOMMU_1,
> peri_mmuen_msk, peri_mmuen);
> if (ret)
> @@ -977,6 +982,15 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> ret = component_master_add_with_match(dev, &mtk_iommu_com_ops, match);
> if (ret)
> goto out_bus_set_null;
> + } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
> + MTK_IOMMU_HAS_FLAG(data->plat_data, IFA_IOMMU_PCIe_SUPPORT)) {
> + #ifdef CONFIG_PCI

Please fix the indentation of this ifdef (do not indent).

> + if (!iommu_present(&pci_bus_type)) {
> + ret = bus_set_iommu(&pci_bus_type, &mtk_iommu_ops);
> + if (ret) /* PCIe fail don't affect platform_bus. */
> + goto out_list_del;
> + }
> + #endif
> }
> return ret;
>
> @@ -1007,6 +1021,11 @@ static int mtk_iommu_remove(struct platform_device *pdev)
> if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
> device_link_remove(data->smicomm_dev, &pdev->dev);
> component_master_del(&pdev->dev, &mtk_iommu_com_ops);
> + } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
> + MTK_IOMMU_HAS_FLAG(data->plat_data, IFA_IOMMU_PCIe_SUPPORT)) {
> + #ifdef CONFIG_PCI

ditto.

> + bus_set_iommu(&pci_bus_type, NULL);
> + #endif
> }
> pm_runtime_disable(&pdev->dev);
> devm_free_irq(&pdev->dev, data->irq, data);
>



Subject: Re: [PATCH v3 18/33] iommu/mediatek: Adjust device link when it is sub-common

Il 23/09/21 13:58, Yong Wu ha scritto:
> For MM IOMMU, We always add device link between smi-common and IOMMU HW.
> In mt8195, we add smi-sub-common. Thus, if the node is sub-common, we still
> need find again to get smi-common, then do device link.
>
> Signed-off-by: Yong Wu <[email protected]>



Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

Subject: Re: [PATCH v3 13/33] iommu/mediatek: Remove the power status checking in tlb flush all

Il 23/09/21 13:58, Yong Wu ha scritto:
> To simplify the code, Remove the power status checking in the
> tlb_flush_all, remove this:
> if (pm_runtime_get_if_in_use(data->dev) <= 0)
> continue;
>
> After this patch, the mtk_iommu_tlb_flush_all will be called from
> a) isr
> b) pm runtime resume callback
> c) tlb flush range fail case
> d) iommu_create_device_direct_mappings
> -> iommu_flush_iotlb_all
> In first three cases, the power and clock always are enabled; d) is direct
> mapping, the tlb flush is unnecessay since we already have tlb_flush_all
> in the pm_runtime_resume callback. When the iommu's power status is
> changed to active, the tlb always is clean.
>
> In addition, there still are 2 reasons that don't add PM status checking
> in the tlb flush all:
> a) Write tlb flush all register also is ok even though the HW has no
> power and clocks. Write ignore.

Do you mean that the register write seemingly succeeds but the hardware
discards it?
Please, reword the `a` sentence to be clearer.

> b) pm_runtime_get_if_in_use(m4udev) is 0 when the tlb_flush_all
> is called frm pm_runtime_resume cb. From this point, we can not add
> this code above in this tlb_flush_all.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> drivers/iommu/mtk_iommu.c | 20 +++++++-------------
> 1 file changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index e9e94944ed91..4a33b6c6b1db 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -204,10 +204,14 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
> return container_of(dom, struct mtk_iommu_domain, domain);
> }
>
> -static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data *data)
> +static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> {
> unsigned long flags;
>
> + /*
> + * No need get power status since the HW PM status nearly is active
> + * when entering here.

Please reword this comment to explain the entire situation.

> + */
> spin_lock_irqsave(&data->tlb_lock, flags);
> writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> data->base + data->plat_data->inv_sel_reg);
> @@ -216,16 +220,6 @@ static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data *data)
> spin_unlock_irqrestore(&data->tlb_lock, flags);
> }
>
> -static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> -{
> - if (pm_runtime_get_if_in_use(data->dev) <= 0)
> - return;
> -
> - mtk_iommu_tlb_do_flush_all(data);
> -
> - pm_runtime_put(data->dev);
> -}
> -
> static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> struct mtk_iommu_data *data)
> {
> @@ -263,7 +257,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> if (ret) {
> dev_warn(data->dev,
> "Partial TLB flush timed out, falling back to full flush\n");
> - mtk_iommu_tlb_do_flush_all(data);
> + mtk_iommu_tlb_flush_all(data);
> }
>
> if (has_pm)
> @@ -993,7 +987,7 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
> *
> * Thus, Make sure the tlb always is clean after each PM resume.
> */
> - mtk_iommu_tlb_do_flush_all(data);
> + mtk_iommu_tlb_flush_all(data);
>
> /*
> * Uppon first resume, only enable the clk and return, since the values of the
>


Subject: Re: [PATCH v3 17/33] iommu/mediatek: Contain MM IOMMU flow with the MM TYPE

Il 23/09/21 13:58, Yong Wu ha scritto:
> Prepare for supporting INFRA_IOMMU, and APU_IOMMU later.
>
> For Infra IOMMU/APU IOMMU, it doesn't have the "larb""port". thus, Use
> the MM flag contain the MM_IOMMU special flow, Also, it moves a big
> chunk code about parsing the mediatek,larbs into a function, this is
> only needed for MM IOMMU. and all the current SoC are MM_IOMMU.
>
> Signed-off-by: Yong Wu <[email protected]>



Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

Subject: Re: [PATCH v3 20/33] iommu/mediatek: Allow IOMMU_DOMAIN_UNMANAGED for PCIe VFIO

Il 23/09/21 13:58, Yong Wu ha scritto:
> Allow the type IOMMU_DOMAIN_UNMANAGED since vfio_iommu_type1.c always call
> iommu_domain_alloc. The PCIe EP works ok when going through vfio.
>
> Signed-off-by: Yong Wu <[email protected]>



Acked-by: AngeloGioacchino Del Regno <[email protected]>


Subject: Re: [PATCH v3 07/33] iommu/mediatek: Add a flag DCM_DISABLE

Il 23/09/21 13:58, Yong Wu ha scritto:
> In the infra iommu, we should disable DCM. add a new flag for this.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

Subject: Re: [PATCH v3 09/33] iommu/mediatek: Remove for_each_m4u in tlb_sync_all

Il 23/09/21 13:58, Yong Wu ha scritto:
> The tlb_sync_all is called from these three functions:
> a) flush_iotlb_all: it will be called for each a iommu HW.
> b) tlb_flush_range_sync: it already has for_each_m4u.
> c) in irq: When IOMMU HW translation fault, Only need flush itself.
>
> Thus, No need for_each_m4u in this tlb_sync_all. Remove it.
>
> Signed-off-by: Yong Wu <[email protected]>
> ---
> drivers/iommu/mtk_iommu.c | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 6f4f6624e3ac..0b4c30baa864 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -206,19 +206,15 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
>
> static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> {
> - struct list_head *head = data->hw_list;
> -
> - for_each_m4u(data, head) {
> - if (pm_runtime_get_if_in_use(data->dev) <= 0)
> - continue;
> + if (pm_runtime_get_if_in_use(data->dev) <= 0)
> + return;
>
> - writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> - data->base + data->plat_data->inv_sel_reg);
> - writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
> - wmb(); /* Make sure the tlb flush all done */
> + writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> + data->base + data->plat_data->inv_sel_reg);
> + writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
> + wmb(); /* Make sure the tlb flush all done */

There aren't a lot of writes here - not anymore, since you are no longer doing
this for_each_m4u()...
...so, please change writel_relaxed() to writel() calls, allowing you to also
remove the write barrier at the end (since in the non relaxed version, order
is already ensured).

>
> - pm_runtime_put(data->dev);
> - }
> + pm_runtime_put(data->dev);
> }
>
> static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
>

Subject: Re: [PATCH v3 10/33] iommu/mediatek: Add tlb_lock in tlb_flush_all

Il 23/09/21 13:58, Yong Wu ha scritto:
> The tlb_flush_all also touches the registers about tlb operations.
> Add spinlock in it to protect the tlb registers. since the tlb_range
> already hold the spinlock, move it a bit outside the spinlock to
> print log.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

Subject: Re: [PATCH v3 19/33] iommu/mediatek: Add list_del in mtk_iommu_remove

Il 23/09/21 13:58, Yong Wu ha scritto:
> Lack the list_del in the mtk_iommu_remove, and remove
> bus_set_iommu(*, NULL) since there may be several iommu HWs.
> we can not bus_set_iommu null when one iommu driver unbind.
> This issue is not so important, No need fix tags.
>
> Signed-off-by: Yong Wu <[email protected]>

I would still add a Fixes tag on this commit, as that would schedule it for
backporting - and that's rather important, solidifying older releases.

Please add the required tag, after which...


Reviewed-by: AngeloGioacchino Del Regno <[email protected]>


Subject: Re: [PATCH v3 14/33] iommu/mediatek: Always enable output PA over 32bits in isr

Il 23/09/21 13:58, Yong Wu ha scritto:
> Currently the output PA[32:33] is contained by the flag IOVA_34.
> This is not right. the iova_34 has no relation with pa[32:33], the 32bits
> iova still could map to pa[32:33]. Move it out from the flag.
>
> No need fix tag since currently only mt8192 use the calulation and it
> always has this IOVA_34 flag.
>
> Prepare for the IOMMU that still use IOVA 32bits but its dram size may be
> over 4GB.
>
> Signed-off-by: Yong Wu <[email protected]>



Reviewed-by: AngeloGioacchino Del Regno <[email protected]>


Subject: Re: [PATCH v3 12/33] iommu/mediatek: Always tlb_flush_all when each PM resume

Il 23/09/21 13:58, Yong Wu ha scritto:
> Prepare for 2 HWs that sharing pgtable in different power-domains.
>
> When there are 2 M4U HWs, it may has problem in the flush_range in which
> we get the pm_status via the m4u dev, BUT that function don't reflect the
> real power-domain status of the HW since there may be other HW also use
> that power-domain.
>
> The function dma_alloc_attrs help allocate the iommu buffer which
> need the corresponding power domain since tlb flush is needed when
> preparing iova. BUT this function only is for allocating buffer,
> we have no good reason to request the user always call pm_runtime_get
> before calling dma_alloc_xxx. Therefore, we add a tlb_flush_all
> in the pm_runtime_resume to make sure the tlb always is clean.
>
> Another solution is always call pm_runtime_get in the tlb_flush_range.
> This will trigger pm runtime resume/backup so often when the iommu
> power is not active at some time(means user don't call pm_runtime_get
> before calling dma_alloc_xxx), This may cause the performance drop.
> thus we don't use this.
>
> In other case, the iommu's power should always be active via device
> link with smi.
>
> The previous SoC don't have PM except mt8192. the mt8192 IOMMU is display's
> power-domain which nearly always is enabled. thus no need fix tags here.
> Prepare for mt8195.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>



Subject: Re: [PATCH v3 05/33] iommu/mediatek: Adapt sharing and non-sharing pgtable case

Il 23/09/21 13:58, Yong Wu ha scritto:
> In previous mt2712, Both IOMMUs are MM IOMMU, and they will share pgtable.
> However in the latest SoC, another is infra IOMMU, there is no reason to
> share pgtable between MM with INFRA IOMMU. This patch manage to
> implement the two case(sharing and non-sharing pgtable).
>
> Currently we use for_each_m4u to loop the 2 HWs. Add the list_head into
> this macro.
> In the sharing pgtable case, the list_head is the global "m4ulist".
> In the non-sharing pgtable case, the list_head is hw_list_head which is a
> variable in the "data". then for_each_m4u will only loop itself.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>


Subject: Re: [PATCH v3 15/33] iommu/mediatek: Add SUB_COMMON_3BITS flag

Il 23/09/21 13:58, Yong Wu ha scritto:
> In prevous SoC, the sub common id occupy 2 bits. the mt8195's sub common
> id has 3bits. Add a new flag for this. and rename the prevous flag to
> _2BITS. For readable, I put these two flags together, then move the
> other flags. no functional change.
>
> Signed-off-by: Yong Wu <[email protected]>

Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

2022-01-09 02:46:57

by Yong Wu (吴勇)

[permalink] [raw]
Subject: Re: [PATCH v3 26/33] iommu/mediatek: Add mtk_iommu_bank_data structure

Hi AngeloGioacchino,

Thanks very much for reviewing whole the patchset.

On Tue, 2022-01-04 at 16:53 +0100, AngeloGioacchino Del Regno wrote:
> Il 23/09/21 13:58, Yong Wu ha scritto:
> > Prepare for supporting multi-banks for the IOMMU HW, No functional
> > change.
> >
> > Add a new structure(mtk_iommu_bank_data) for each a bank. Each a
> > bank have
> > the independent HW base/IRQ/tlb-range ops, and each a bank has its
> > special
> > iommu-domain(independent pgtable), thus, also move the domain
> > information
> > into it.
> >
> > In previous SoC, we have only one bank which could be treated as
> > bank0(
> > bankid always is 0 for the previous SoC).
> >
> > After adding this structure, the tlb operations and irq could use
> > bank_data as parameter.
> >
> > Signed-off-by: Yong Wu <[email protected]>
> > ---

[...]

> > -struct mtk_iommu_data {
> > +struct mtk_iommu_bank_data {
> > void __iomem *base;
> > int irq;
> > + unsigned int id;
> > + struct device *pdev;
>
> `pdev` may be a bit misleading, as it's conventionally read as
> "platform device"
> and not as the intended "parent device"... perhaps calling it
> parent_dev would be
> more appropriate.

will rename it. Thanks.

>
> > + struct mtk_iommu_data *pdata; /* parent data */
>
> Same here, pdata -> parent_data

Will fix.

>
> > + spinlock_t tlb_lock; /* lock for tlb range
> > flush */
> > + struct mtk_iommu_domain *m4u_dom; /* Each bank has
> > a domain */
> > +};
> > +
> > +struct mtk_iommu_data {
> > + union {
> > + struct { /* only for gen1 */
> > + void __iomem *base;
> > + int irq;
> > + struct mtk_iommu_domain *m4u_dom;
> > + };
> > +
> > + /* only for gen2 that support multi-banks */
> > + struct mtk_iommu_bank_data bank[MTK_IOMMU_BANK_MAX];
> > + };
>
> Sorry, but I really don't like this union... please, update
> mtk_iommu_v1 to always
> use bank[0] or, more appropriately, dynamically allocate the bank
> array with a
> devm_kzalloc call (as to not waste memory on platforms with less
> available banks).
>
> In that case, you would have...
>
> > struct device *dev;
> > struct clk *bclk;
> > phys_addr_t protect_base; /* protect memory
> > base */
> > struct mtk_iommu_suspend_reg reg;
> > - struct mtk_iommu_domain *m4u_dom;
> > struct iommu_group *m4u_group[MTK_IOMMU_GROUP_MAX];
> > bool enable_4GB;
> > - spinlock_t tlb_lock; /* lock for tlb range
> > flush */
>
> struct mtk_iommu_bank_data *banks;
> u8 num_banks;
>
> ... where `num_banks` gets copied from the same in
> mtk_iommu_plat_data, defined
> for each SoC, and where `banks` is dynamically allocated in
> mtk_iommu.c and
> mtk_iommu_v1.c's probe() callback.

Thanks for this idea. I will try this to see if the code will be too
complicate after changing this. If it is, I will use bank[0] always in
mtk_iommu_v1, this looks simpler.

>
> >
> > struct iommu_device iommu;
> > const struct mtk_iommu_plat_data *plat_data;
> >
>
>


2022-01-09 02:47:34

by Yong Wu (吴勇)

[permalink] [raw]
Subject: Re: [PATCH v3 13/33] iommu/mediatek: Remove the power status checking in tlb flush all

On Tue, 2022-01-04 at 16:55 +0100, AngeloGioacchino Del Regno wrote:
> Il 23/09/21 13:58, Yong Wu ha scritto:
> > To simplify the code, Remove the power status checking in the
> > tlb_flush_all, remove this:
> > if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > continue;
> >
> > After this patch, the mtk_iommu_tlb_flush_all will be called from
> > a) isr
> > b) pm runtime resume callback
> > c) tlb flush range fail case
> > d) iommu_create_device_direct_mappings
> > -> iommu_flush_iotlb_all
> > In first three cases, the power and clock always are enabled; d) is
> > direct
> > mapping, the tlb flush is unnecessay since we already have
> > tlb_flush_all
> > in the pm_runtime_resume callback. When the iommu's power status is
> > changed to active, the tlb always is clean.
> >
> > In addition, there still are 2 reasons that don't add PM status
> > checking
> > in the tlb flush all:
> > a) Write tlb flush all register also is ok even though the HW has
> > no
> > power and clocks. Write ignore.
>
> Do you mean that the register write seemingly succeeds but the
> hardware discards it?
> Please, reword the `a` sentence to be clearer.

Yes. This is the "Write ignore" I tried to explain.

By the way, the tlb operation patches were moved to this patchset[1]
from Dafna. Would you help take a look at that one? I will send the
next mt8195 version based on that patchset.

[1]
https://lore.kernel.org/linux-mediatek/[email protected]/

Thanks.

>
> > b) pm_runtime_get_if_in_use(m4udev) is 0 when the tlb_flush_all
> > is called frm pm_runtime_resume cb. From this point, we can not add
> > this code above in this tlb_flush_all.
> >
> > Signed-off-by: Yong Wu <[email protected]>
> > ---
> > drivers/iommu/mtk_iommu.c | 20 +++++++-------------
> > 1 file changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index e9e94944ed91..4a33b6c6b1db 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -204,10 +204,14 @@ static struct mtk_iommu_domain
> > *to_mtk_domain(struct iommu_domain *dom)
> > return container_of(dom, struct mtk_iommu_domain, domain);
> > }
> >
> > -static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data
> > *data)
> > +static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> > {
> > unsigned long flags;
> >
> > + /*
> > + * No need get power status since the HW PM status nearly is
> > active
> > + * when entering here.
>
> Please reword this comment to explain the entire situation.
>
> > + */
> > spin_lock_irqsave(&data->tlb_lock, flags);
> > writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> > data->base + data->plat_data->inv_sel_reg);
> > @@ -216,16 +220,6 @@ static void mtk_iommu_tlb_do_flush_all(struct
> > mtk_iommu_data *data)
> > spin_unlock_irqrestore(&data->tlb_lock, flags);
> > }
> >
> > -static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> > -{
> > - if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > - return;
> > -
> > - mtk_iommu_tlb_do_flush_all(data);
> > -
> > - pm_runtime_put(data->dev);
> > -}
> > -
> > static void mtk_iommu_tlb_flush_range_sync(unsigned long iova,
> > size_t size,
> > struct mtk_iommu_data *data)
> > {
> > @@ -263,7 +257,7 @@ static void
> > mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> > if (ret) {
> > dev_warn(data->dev,
> > "Partial TLB flush timed out, falling
> > back to full flush\n");
> > - mtk_iommu_tlb_do_flush_all(data);
> > + mtk_iommu_tlb_flush_all(data);
> > }
> >
> > if (has_pm)
> > @@ -993,7 +987,7 @@ static int __maybe_unused
> > mtk_iommu_runtime_resume(struct device *dev)
> > *
> > * Thus, Make sure the tlb always is clean after each PM
> > resume.
> > */
> > - mtk_iommu_tlb_do_flush_all(data);
> > + mtk_iommu_tlb_flush_all(data);
> >
> > /*
> > * Uppon first resume, only enable the clk and return, since
> > the values of the
> >
>
>


2022-01-09 02:47:54

by Yong Wu (吴勇)

[permalink] [raw]
Subject: Re: [PATCH v3 22/33] iommu/mediatek: Add PCIe support

On Tue, 2022-01-04 at 16:54 +0100, AngeloGioacchino Del Regno wrote:
> Il 23/09/21 13:58, Yong Wu ha scritto:
> > Currently the code for of_iommu_configure_dev_id is like this:
> >
> > static int of_iommu_configure_dev_id(struct device_node *master_np,
> > struct device *dev,
> > const u32 *id)
> > {
> > struct of_phandle_args iommu_spec = { .args_count = 1 };
> >
> > err = of_map_id(master_np, *id, "iommu-map",
> > "iommu-map-mask", &iommu_spec.np,
> > iommu_spec.args);
> > ...
> > }
> >
> > It supports only one id output. BUT our PCIe HW has two ID(one is
> > for
> > writing, the other is for reading). I'm not sure if we should
> > change
> > of_map_id to support output MAX_PHANDLE_ARGS.
> >
> > Here add the solution in ourselve drivers. If it's pcie case,
> > enable one
> > more bit.
> >
> > Not all infra iommu support PCIe, thus add a PCIe support flag
> > here.
> >
> > Signed-off-by: Yong Wu <[email protected]>
> > ---
> > drivers/iommu/mtk_iommu.c | 21 ++++++++++++++++++++-
> > 1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 37d6dfb4feab..3f1fd8036345 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -20,6 +20,7 @@
> > #include <linux/of_address.h>
> > #include <linux/of_irq.h>
> > #include <linux/of_platform.h>
> > +#include <linux/pci.h>
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/regmap.h>
> > @@ -132,6 +133,7 @@
> > #define MTK_IOMMU_TYPE_MM (0x0 << 13)
> > #define MTK_IOMMU_TYPE_INFRA (0x1 << 13)
> > #define MTK_IOMMU_TYPE_MASK (0x3 << 13)
> > +#define IFA_IOMMU_PCIe_SUPPORT BIT(15)
>
> This definition looks like "breaking" the naming convention that's
> used in this
> driver... what about MTK_INFRA_IOMMU_PCIE_SUPPORT?

OK for me. I noticed the "PCIE" should called to "PCIe", thus renamed
this.

>
> >
> > #define MTK_IOMMU_HAS_FLAG(pdata, _x) (!!(((pdata)->flags) &
> > (_x)))
> >
> > @@ -401,8 +403,11 @@ static int mtk_iommu_config(struct
> > mtk_iommu_data *data, struct device *dev,
> > larb_mmu->mmu &=
> > ~MTK_SMI_MMU_EN(portid);
> > } else if (MTK_IOMMU_IS_TYPE(data->plat_data,
> > MTK_IOMMU_TYPE_INFRA)) {
> > peri_mmuen_msk = BIT(portid);
> > - peri_mmuen = enable ? peri_mmuen_msk : 0;
> > + /* PCIdev has only one output id, enable the
> > next writing bit for PCIe */
> > + if (dev_is_pci(dev))
> > + peri_mmuen_msk |= BIT(portid + 1);
> >
> > + peri_mmuen = enable ? peri_mmuen_msk : 0;
> > ret = regmap_update_bits(data->pericfg,
> > PERICFG_IOMMU_1,
> > peri_mmuen_msk,
> > peri_mmuen);
> > if (ret)
> > @@ -977,6 +982,15 @@ static int mtk_iommu_probe(struct
> > platform_device *pdev)
> > ret = component_master_add_with_match(dev,
> > &mtk_iommu_com_ops, match);
> > if (ret)
> > goto out_bus_set_null;
> > + } else if (MTK_IOMMU_IS_TYPE(data->plat_data,
> > MTK_IOMMU_TYPE_INFRA) &&
> > + MTK_IOMMU_HAS_FLAG(data->plat_data,
> > IFA_IOMMU_PCIe_SUPPORT)) {
> > + #ifdef CONFIG_PCI
>
> Please fix the indentation of this ifdef (do not indent).

Thanks. Will fix this.

>
> > + if (!iommu_present(&pci_bus_type)) {
> > + ret = bus_set_iommu(&pci_bus_type,
> > &mtk_iommu_ops);
> > + if (ret) /* PCIe fail don't affect
> > platform_bus. */
> > + goto out_list_del;
> > + }
> > + #endif
> > }
> > return ret;
> >
> > @@ -1007,6 +1021,11 @@ static int mtk_iommu_remove(struct
> > platform_device *pdev)
> > if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
> > device_link_remove(data->smicomm_dev, &pdev->dev);
> > component_master_del(&pdev->dev, &mtk_iommu_com_ops);
> > + } else if (MTK_IOMMU_IS_TYPE(data->plat_data,
> > MTK_IOMMU_TYPE_INFRA) &&
> > + MTK_IOMMU_HAS_FLAG(data->plat_data,
> > IFA_IOMMU_PCIe_SUPPORT)) {
> > + #ifdef CONFIG_PCI
>
> ditto.
>
> > + bus_set_iommu(&pci_bus_type, NULL);
> > + #endif
> > }
> > pm_runtime_disable(&pdev->dev);
> > devm_free_irq(&pdev->dev, data->irq, data);
> >
>
>


2022-01-09 02:48:33

by Yong Wu (吴勇)

[permalink] [raw]
Subject: Re: [PATCH v3 09/33] iommu/mediatek: Remove for_each_m4u in tlb_sync_all

On Tue, 2022-01-04 at 16:55 +0100, AngeloGioacchino Del Regno wrote:
> Il 23/09/21 13:58, Yong Wu ha scritto:
> > The tlb_sync_all is called from these three functions:
> > a) flush_iotlb_all: it will be called for each a iommu HW.
> > b) tlb_flush_range_sync: it already has for_each_m4u.
> > c) in irq: When IOMMU HW translation fault, Only need flush itself.
> >
> > Thus, No need for_each_m4u in this tlb_sync_all. Remove it.
> >
> > Signed-off-by: Yong Wu <[email protected]>
> > ---
> > drivers/iommu/mtk_iommu.c | 18 +++++++-----------
> > 1 file changed, 7 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 6f4f6624e3ac..0b4c30baa864 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -206,19 +206,15 @@ static struct mtk_iommu_domain
> > *to_mtk_domain(struct iommu_domain *dom)
> >
> > static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
> > {
> > - struct list_head *head = data->hw_list;
> > -
> > - for_each_m4u(data, head) {
> > - if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > - continue;
> > + if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > + return;
> >
> > - writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> > - data->base + data->plat_data-
> > >inv_sel_reg);
> > - writel_relaxed(F_ALL_INVLD, data->base +
> > REG_MMU_INVALIDATE);
> > - wmb(); /* Make sure the tlb flush all done */
> > + writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> > + data->base + data->plat_data->inv_sel_reg);
> > + writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
> > + wmb(); /* Make sure the tlb flush all done */
>
> There aren't a lot of writes here - not anymore, since you are no
> longer doing
> this for_each_m4u()...
> ...so, please change writel_relaxed() to writel() calls, allowing you
> to also
> remove the write barrier at the end (since in the non relaxed
> version, order is already ensured).

In the "writel", the "__iowmb()" runs before "write_relaxed". Then how
to guarantee the last register was wrote into the HW. Here the flush
all don't have sync(waiting it complete)

>
> >
> > - pm_runtime_put(data->dev);
> > - }
> > + pm_runtime_put(data->dev);
> > }
> >
> > static void mtk_iommu_tlb_flush_range_sync(unsigned long iova,
> > size_t size,
> >


Subject: Re: [PATCH v3 09/33] iommu/mediatek: Remove for_each_m4u in tlb_sync_all

Il 09/01/22 03:48, Yong Wu ha scritto:
> On Tue, 2022-01-04 at 16:55 +0100, AngeloGioacchino Del Regno wrote:
>> Il 23/09/21 13:58, Yong Wu ha scritto:
>>> The tlb_sync_all is called from these three functions:
>>> a) flush_iotlb_all: it will be called for each a iommu HW.
>>> b) tlb_flush_range_sync: it already has for_each_m4u.
>>> c) in irq: When IOMMU HW translation fault, Only need flush itself.
>>>
>>> Thus, No need for_each_m4u in this tlb_sync_all. Remove it.
>>>
>>> Signed-off-by: Yong Wu <[email protected]>
>>> ---
>>> drivers/iommu/mtk_iommu.c | 18 +++++++-----------
>>> 1 file changed, 7 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>>> index 6f4f6624e3ac..0b4c30baa864 100644
>>> --- a/drivers/iommu/mtk_iommu.c
>>> +++ b/drivers/iommu/mtk_iommu.c
>>> @@ -206,19 +206,15 @@ static struct mtk_iommu_domain
>>> *to_mtk_domain(struct iommu_domain *dom)
>>>
>>> static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
>>> {
>>> - struct list_head *head = data->hw_list;
>>> -
>>> - for_each_m4u(data, head) {
>>> - if (pm_runtime_get_if_in_use(data->dev) <= 0)
>>> - continue;
>>> + if (pm_runtime_get_if_in_use(data->dev) <= 0)
>>> + return;
>>>
>>> - writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
>>> - data->base + data->plat_data-
>>>> inv_sel_reg);
>>> - writel_relaxed(F_ALL_INVLD, data->base +
>>> REG_MMU_INVALIDATE);
>>> - wmb(); /* Make sure the tlb flush all done */
>>> + writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
>>> + data->base + data->plat_data->inv_sel_reg);
>>> + writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
>>> + wmb(); /* Make sure the tlb flush all done */
>>
>> There aren't a lot of writes here - not anymore, since you are no
>> longer doing
>> this for_each_m4u()...
>> ...so, please change writel_relaxed() to writel() calls, allowing you
>> to also
>> remove the write barrier at the end (since in the non relaxed
>> version, order is already ensured).
>
> In the "writel", the "__iowmb()" runs before "write_relaxed". Then how
> to guarantee the last register was wrote into the HW. Here the flush
> all don't have sync(waiting it complete)
>

That's right, I'm sorry for the invalid proposal.

Though, there's something else to mention here... if writing
(F_INVLD_EN1 | F_INVLD_EN0) to inv_sel_reg is *required* to happen before
writing F_ALL_INVLD to REG_MMU_INVALIDATE (which I think is exactly the
case here), then, in order to ensure write ordering, you should still use
writel() instead of the relaxed accessor; after which, since (as you mentioned)
there is no sync readback loop, you can keep that wmb() at the end.

>>
>>>
>>> - pm_runtime_put(data->dev);
>>> - }
>>> + pm_runtime_put(data->dev);
>>> }
>>>
>>> static void mtk_iommu_tlb_flush_range_sync(unsigned long iova,
>>> size_t size,
>>>
>



2022-01-10 10:59:52

by Yong Wu (吴勇)

[permalink] [raw]
Subject: Re: [PATCH v3 09/33] iommu/mediatek: Remove for_each_m4u in tlb_sync_all

On Mon, 2022-01-10 at 10:16 +0100, AngeloGioacchino Del Regno wrote:
> Il 09/01/22 03:48, Yong Wu ha scritto:
> > On Tue, 2022-01-04 at 16:55 +0100, AngeloGioacchino Del Regno
> > wrote:
> > > Il 23/09/21 13:58, Yong Wu ha scritto:
> > > > The tlb_sync_all is called from these three functions:
> > > > a) flush_iotlb_all: it will be called for each a iommu HW.
> > > > b) tlb_flush_range_sync: it already has for_each_m4u.
> > > > c) in irq: When IOMMU HW translation fault, Only need flush
> > > > itself.
> > > >
> > > > Thus, No need for_each_m4u in this tlb_sync_all. Remove it.
> > > >
> > > > Signed-off-by: Yong Wu <[email protected]>
> > > > ---
> > > > drivers/iommu/mtk_iommu.c | 18 +++++++-----------
> > > > 1 file changed, 7 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/iommu/mtk_iommu.c
> > > > b/drivers/iommu/mtk_iommu.c
> > > > index 6f4f6624e3ac..0b4c30baa864 100644
> > > > --- a/drivers/iommu/mtk_iommu.c
> > > > +++ b/drivers/iommu/mtk_iommu.c
> > > > @@ -206,19 +206,15 @@ static struct mtk_iommu_domain
> > > > *to_mtk_domain(struct iommu_domain *dom)
> > > >
> > > > static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data
> > > > *data)
> > > > {
> > > > - struct list_head *head = data->hw_list;
> > > > -
> > > > - for_each_m4u(data, head) {
> > > > - if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > > > - continue;
> > > > + if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > > > + return;
> > > >
> > > > - writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> > > > - data->base + data->plat_data-
> > > > > inv_sel_reg);
> > > >
> > > > - writel_relaxed(F_ALL_INVLD, data->base +
> > > > REG_MMU_INVALIDATE);
> > > > - wmb(); /* Make sure the tlb flush all done */
> > > > + writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> > > > + data->base + data->plat_data-
> > > > >inv_sel_reg);
> > > > + writel_relaxed(F_ALL_INVLD, data->base +
> > > > REG_MMU_INVALIDATE);
> > > > + wmb(); /* Make sure the tlb flush all done */
> > >
> > > There aren't a lot of writes here - not anymore, since you are no
> > > longer doing
> > > this for_each_m4u()...
> > > ...so, please change writel_relaxed() to writel() calls, allowing
> > > you
> > > to also
> > > remove the write barrier at the end (since in the non relaxed
> > > version, order is already ensured).
> >
> > In the "writel", the "__iowmb()" runs before "write_relaxed". Then
> > how
> > to guarantee the last register was wrote into the HW. Here the
> > flush
> > all don't have sync(waiting it complete)
> >
>
> That's right, I'm sorry for the invalid proposal.
>
> Though, there's something else to mention here... if writing
> (F_INVLD_EN1 | F_INVLD_EN0) to inv_sel_reg is *required* to happen
> before
> writing F_ALL_INVLD to REG_MMU_INVALIDATE (which I think is exactly
> the
> case here), then, in order to ensure write ordering, you should still
> use
> writel() instead of the relaxed accessor; after which, since (as you
> mentioned)
> there is no sync readback loop, you can keep that wmb() at the end.

The writel_relaxed also makes sure the order. I did try this:


https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/

>
> > >
> > > >
> > > > - pm_runtime_put(data->dev);
> > > > - }
> > > > + pm_runtime_put(data->dev);
> > > > }
> > > >
> > > > static void mtk_iommu_tlb_flush_range_sync(unsigned long
> > > > iova,
> > > > size_t size,
> > > >
>
>


Subject: Re: [PATCH v3 09/33] iommu/mediatek: Remove for_each_m4u in tlb_sync_all

Il 10/01/22 11:59, Yong Wu ha scritto:
> On Mon, 2022-01-10 at 10:16 +0100, AngeloGioacchino Del Regno wrote:
>> Il 09/01/22 03:48, Yong Wu ha scritto:
>>> On Tue, 2022-01-04 at 16:55 +0100, AngeloGioacchino Del Regno
>>> wrote:
>>>> Il 23/09/21 13:58, Yong Wu ha scritto:
>>>>> The tlb_sync_all is called from these three functions:
>>>>> a) flush_iotlb_all: it will be called for each a iommu HW.
>>>>> b) tlb_flush_range_sync: it already has for_each_m4u.
>>>>> c) in irq: When IOMMU HW translation fault, Only need flush
>>>>> itself.
>>>>>
>>>>> Thus, No need for_each_m4u in this tlb_sync_all. Remove it.
>>>>>
>>>>> Signed-off-by: Yong Wu <[email protected]>
>>>>> ---
>>>>> drivers/iommu/mtk_iommu.c | 18 +++++++-----------
>>>>> 1 file changed, 7 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iommu/mtk_iommu.c
>>>>> b/drivers/iommu/mtk_iommu.c
>>>>> index 6f4f6624e3ac..0b4c30baa864 100644
>>>>> --- a/drivers/iommu/mtk_iommu.c
>>>>> +++ b/drivers/iommu/mtk_iommu.c
>>>>> @@ -206,19 +206,15 @@ static struct mtk_iommu_domain
>>>>> *to_mtk_domain(struct iommu_domain *dom)
>>>>>
>>>>> static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data
>>>>> *data)
>>>>> {
>>>>> - struct list_head *head = data->hw_list;
>>>>> -
>>>>> - for_each_m4u(data, head) {
>>>>> - if (pm_runtime_get_if_in_use(data->dev) <= 0)
>>>>> - continue;
>>>>> + if (pm_runtime_get_if_in_use(data->dev) <= 0)
>>>>> + return;
>>>>>
>>>>> - writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
>>>>> - data->base + data->plat_data-
>>>>>> inv_sel_reg);
>>>>>
>>>>> - writel_relaxed(F_ALL_INVLD, data->base +
>>>>> REG_MMU_INVALIDATE);
>>>>> - wmb(); /* Make sure the tlb flush all done */
>>>>> + writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
>>>>> + data->base + data->plat_data-
>>>>>> inv_sel_reg);
>>>>> + writel_relaxed(F_ALL_INVLD, data->base +
>>>>> REG_MMU_INVALIDATE);
>>>>> + wmb(); /* Make sure the tlb flush all done */
>>>>
>>>> There aren't a lot of writes here - not anymore, since you are no
>>>> longer doing
>>>> this for_each_m4u()...
>>>> ...so, please change writel_relaxed() to writel() calls, allowing
>>>> you
>>>> to also
>>>> remove the write barrier at the end (since in the non relaxed
>>>> version, order is already ensured).
>>>
>>> In the "writel", the "__iowmb()" runs before "write_relaxed". Then
>>> how
>>> to guarantee the last register was wrote into the HW. Here the
>>> flush
>>> all don't have sync(waiting it complete)
>>>
>>
>> That's right, I'm sorry for the invalid proposal.
>>
>> Though, there's something else to mention here... if writing
>> (F_INVLD_EN1 | F_INVLD_EN0) to inv_sel_reg is *required* to happen
>> before
>> writing F_ALL_INVLD to REG_MMU_INVALIDATE (which I think is exactly
>> the
>> case here), then, in order to ensure write ordering, you should still
>> use
>> writel() instead of the relaxed accessor; after which, since (as you
>> mentioned)
>> there is no sync readback loop, you can keep that wmb() at the end.
>
> The writel_relaxed also makes sure the order. I did try this:
>
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
>

Ok, that's fair. Means that this patch is fine as it is.
I'll release by R-b on Dafna's patch, as suggested.

Thank you,
- Angelo