This patchset adds mt6779 iommu support.
mt6779 has two iommus, they are MM_IOMMU(M4U) and APU_IOMMU which used ARM Short-Descriptor translation format.
The mt6779's MM_IOMMU-SMI and APU_IOMMU HW diagram is as below, it is only a brief diagram:
EMI
|
--------------------------------------
| |
MM_IOMMU APU_IOMMU
| |
SMI_COMMOM----------- APU_BUS
| | |
SMI_LARB(0~11) | |
| | |
| | --------------
| | | | |
Multimedia engine CCU VPU MDLA EMDA
All the connections are hardware fixed, software can not adjust it.
Compared with mt8183, SMI_BUS_ID width has changed from 10 to 12. SMI Larb number is described in bit[11:7],
Port number is described in bit[6:2]. In addition, there are some registers has changed in mt6779, so we need
to redefine and reuse them.
The patchset only used MM_IOMMU, so we only add MM_IOMMU basic function, such as smi_larb port definition, registers
definition and hardware initialization.
change notes:
v4:
1. Rebase on v5.8-rc1.
2. Fix coding style.
3. Add F_MMU_IN_DRDER_WR_EN definition in MISC_CTRL to improve performance.
v3:
1. Rebase on v5.7-rc1.
2. Remove unused port definition,ex:APU and CCU port in mt6779-larb-port.h.
3. Remove "change single domain to multiple domain" part(from PATCH v2 09/19 to PATCH v2 19/19).
4. Redesign mt6779 basic part
(1)Add some register definition and reuse them.
(2)Redesign smi larb bus ID to analyze IOMMU translation fault.
(3)Only init MM_IOMMU and not use APU_IOMMU.
http://lists.infradead.org/pipermail/linux-mediatek/2020-May/029811.html
v2:
1. Rebase on v5.5-rc1.
2. Delete M4U_PORT_UNKNOWN define because of not use it.
3. Correct coding format.
4. Rename offset=0x48 register.
5. Split "iommu/mediatek: Add mt6779 IOMMU basic support(patch v1)" to several patches(patch v2).
http://lists.infradead.org/pipermail/linux-mediatek/2020-January/026131.html
v1:
http://lists.infradead.org/pipermail/linux-mediatek/2019-November/024567.html
Chao Hao (7):
dt-bindings: mediatek: Add bindings for MT6779
iommu/mediatek: Rename the register STANDARD_AXI_MODE(0x48) to
MISC_CTRL
iommu/mediatek: Set MISC_CTRL register
iommu/mediatek: Move inv_sel_reg into the plat_data
iommu/mediatek: Add sub_comm id in translation fault
iommu/mediatek: Add REG_MMU_WR_LEN definition preparing for mt6779
iommu/mediatek: Add mt6779 basic support
.../bindings/iommu/mediatek,iommu.txt | 2 +
drivers/iommu/mtk_iommu.c | 92 ++++++--
drivers/iommu/mtk_iommu.h | 10 +-
include/dt-bindings/memory/mt6779-larb-port.h | 206 ++++++++++++++++++
4 files changed, 285 insertions(+), 25 deletions(-)
--
2.18.0
Some platforms(ex: mt6779) have a new register called by REG_MMU_WR_LEN
to improve performance.
This patch add this register definition.
Signed-off-by: Chao Hao <[email protected]>
---
drivers/iommu/mtk_iommu.c | 10 ++++++++++
drivers/iommu/mtk_iommu.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index a687e8db0e51..c706bca6487e 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -46,6 +46,8 @@
#define F_MMU_STANDARD_AXI_MODE_BIT (BIT(3) | BIT(19))
#define REG_MMU_DCM_DIS 0x050
+#define REG_MMU_WR_LEN 0x054
+#define F_MMU_WR_THROT_DIS_BIT (BIT(5) | BIT(21))
#define REG_MMU_CTRL_REG 0x110
#define F_MMU_TF_PROT_TO_PROGRAM_ADDR (2 << 4)
@@ -581,6 +583,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
}
writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
+ if (data->plat_data->has_wr_len) {
+ /* write command throttling mode */
+ regval = readl_relaxed(data->base + REG_MMU_WR_LEN);
+ regval &= ~F_MMU_WR_THROT_DIS_BIT;
+ writel_relaxed(regval, data->base + REG_MMU_WR_LEN);
+ }
if (data->plat_data->reset_axi) {
/* The register is called STANDARD_AXI_MODE in this case */
@@ -737,6 +745,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device *dev)
struct mtk_iommu_suspend_reg *reg = &data->reg;
void __iomem *base = data->base;
+ reg->wr_len = readl_relaxed(base + REG_MMU_WR_LEN);
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);
@@ -761,6 +770,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
dev_err(data->dev, "Failed to enable clk(%d) in resume\n", ret);
return ret;
}
+ writel_relaxed(reg->wr_len, base + REG_MMU_WR_LEN);
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);
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index d51ff99c2c71..9971cedd72ea 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -25,6 +25,7 @@ struct mtk_iommu_suspend_reg {
u32 int_main_control;
u32 ivrp_paddr;
u32 vld_pa_rng;
+ u32 wr_len;
};
enum mtk_iommu_plat {
@@ -43,6 +44,7 @@ struct mtk_iommu_plat_data {
bool has_misc_ctrl;
bool has_sub_comm;
bool has_vld_pa_rng;
+ bool has_wr_len;
bool reset_axi;
u32 inv_sel_reg;
unsigned char larbid_remap[8][4];
--
2.18.0
The max larb number that a iommu HW support is 8(larb0~larb7 in the below
diagram).
If the larb's number is over 8, we use a sub_common for merging
several larbs into one larb. At this case, we will extend larb_id:
bit[11:9] means common-id;
bit[8:7] means subcommon-id;
From these two variable, we could get the real larb number when
translation fault happen.
The diagram is as below:
EMI
|
IOMMU
|
-----------------
| |
common1 common0
| |
-----------------
|
smi common
|
------------------------------------
| | | | | |
3'd0 3'd1 3'd2 3'd3 ... 3'd7 <-common_id(max is 8)
| | | | | |
Larb0 Larb1 | Larb3 ... Larb7
|
smi sub common
|
--------------------------
| | | |
2'd0 2'd1 2'd2 2'd3 <-sub_common_id(max is 4)
| | | |
Larb8 Larb9 Larb10 Larb11
In this patch we extern larb_remap[] to larb_remap[8][4] for this.
larb_remap[x][y]: x mean common-id above, y means subcommon_id above.
We can also distinguish if the M4U HW has sub_common by has_sub_comm
property.
Signed-off-by: Chao Hao <[email protected]>
Reviewed-by: Yong Wu <[email protected]>
---
drivers/iommu/mtk_iommu.c | 20 +++++++++++++-------
drivers/iommu/mtk_iommu.h | 3 ++-
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index f23919feba4e..a687e8db0e51 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -91,6 +91,8 @@
#define REG_MMU1_INVLD_PA 0x148
#define REG_MMU0_INT_ID 0x150
#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_LARB_ID(a) (((a) >> 7) & 0x7)
#define F_MMU_INT_ID_PORT_ID(a) (((a) >> 2) & 0x1f)
@@ -229,7 +231,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;
u32 int_state, regval, fault_iova, fault_pa;
- unsigned int fault_larb, fault_port;
+ unsigned int fault_larb, fault_port, sub_comm = 0;
bool layer, write;
/* Read error info from registers */
@@ -245,10 +247,14 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
}
layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
- fault_larb = F_MMU_INT_ID_LARB_ID(regval);
fault_port = F_MMU_INT_ID_PORT_ID(regval);
-
- fault_larb = data->plat_data->larbid_remap[fault_larb];
+ if (data->plat_data->has_sub_comm) {
+ fault_larb = F_MMU_INT_ID_COMM_ID(regval);
+ sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
+ } else {
+ fault_larb = F_MMU_INT_ID_LARB_ID(regval);
+ }
+ 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)) {
@@ -778,7 +784,7 @@ static const struct mtk_iommu_plat_data mt2712_data = {
.has_bclk = true,
.has_vld_pa_rng = true,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
- .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
+ .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
};
static const struct mtk_iommu_plat_data mt8173_data = {
@@ -787,14 +793,14 @@ static const struct mtk_iommu_plat_data mt8173_data = {
.has_bclk = true,
.reset_axi = true,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
- .larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */
+ .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
};
static const struct mtk_iommu_plat_data mt8183_data = {
.m4u_plat = M4U_MT8183,
.reset_axi = true,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
- .larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1},
+ .larbid_remap = {{0}, {4}, {5}, {6}, {7}, {2}, {3}, {1}},
};
static const struct of_device_id mtk_iommu_of_ids[] = {
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index afd7a2de5c1e..d51ff99c2c71 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -41,10 +41,11 @@ struct mtk_iommu_plat_data {
/* HW will use the EMI clock if there isn't the "bclk". */
bool has_bclk;
bool has_misc_ctrl;
+ bool has_sub_comm;
bool has_vld_pa_rng;
bool reset_axi;
u32 inv_sel_reg;
- unsigned char larbid_remap[MTK_LARB_NR_MAX];
+ unsigned char larbid_remap[8][4];
};
struct mtk_iommu_domain;
--
2.18.0
On 17/06/2020 05:00, Chao Hao wrote:
> The max larb number that a iommu HW support is 8(larb0~larb7 in the below
> diagram).
> If the larb's number is over 8, we use a sub_common for merging
> several larbs into one larb. At this case, we will extend larb_id:
> bit[11:9] means common-id;
> bit[8:7] means subcommon-id;
> From these two variable, we could get the real larb number when
> translation fault happen.
> The diagram is as below:
> EMI
> |
> IOMMU
> |
> -----------------
> | |
> common1 common0
> | |
> -----------------
> |
> smi common
> |
> ------------------------------------
> | | | | | |
> 3'd0 3'd1 3'd2 3'd3 ... 3'd7 <-common_id(max is 8)
> | | | | | |
> Larb0 Larb1 | Larb3 ... Larb7
> |
> smi sub common
> |
> --------------------------
> | | | |
> 2'd0 2'd1 2'd2 2'd3 <-sub_common_id(max is 4)
> | | | |
> Larb8 Larb9 Larb10 Larb11
>
> In this patch we extern larb_remap[] to larb_remap[8][4] for this.
extern -> extend
> larb_remap[x][y]: x mean common-id above, y means subcommon_id above.
mean -> means
>
> We can also distinguish if the M4U HW has sub_common by has_sub_comm
> property.
>
> Signed-off-by: Chao Hao <[email protected]>
> Reviewed-by: Yong Wu <[email protected]>
> ---
> drivers/iommu/mtk_iommu.c | 20 +++++++++++++-------
> drivers/iommu/mtk_iommu.h | 3 ++-
> 2 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index f23919feba4e..a687e8db0e51 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -91,6 +91,8 @@
> #define REG_MMU1_INVLD_PA 0x148
> #define REG_MMU0_INT_ID 0x150
> #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_LARB_ID(a) (((a) >> 7) & 0x7)
> #define F_MMU_INT_ID_PORT_ID(a) (((a) >> 2) & 0x1f)
>
> @@ -229,7 +231,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;
> u32 int_state, regval, fault_iova, fault_pa;
> - unsigned int fault_larb, fault_port;
> + unsigned int fault_larb, fault_port, sub_comm = 0;
> bool layer, write;
>
> /* Read error info from registers */
> @@ -245,10 +247,14 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
> }
> layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
> write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
> - fault_larb = F_MMU_INT_ID_LARB_ID(regval);
> fault_port = F_MMU_INT_ID_PORT_ID(regval);
> -
> - fault_larb = data->plat_data->larbid_remap[fault_larb];
> + if (data->plat_data->has_sub_comm) {
> + fault_larb = F_MMU_INT_ID_COMM_ID(regval);
> + sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
> + } else {
> + fault_larb = F_MMU_INT_ID_LARB_ID(regval);
> + }
> + 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)) {
> @@ -778,7 +784,7 @@ static const struct mtk_iommu_plat_data mt2712_data = {
> .has_bclk = true,
> .has_vld_pa_rng = true,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> - .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
> + .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
> };
>
> static const struct mtk_iommu_plat_data mt8173_data = {
> @@ -787,14 +793,14 @@ static const struct mtk_iommu_plat_data mt8173_data = {
> .has_bclk = true,
> .reset_axi = true,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> - .larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */
> + .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
> };
>
> static const struct mtk_iommu_plat_data mt8183_data = {
> .m4u_plat = M4U_MT8183,
> .reset_axi = true,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> - .larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1},
> + .larbid_remap = {{0}, {4}, {5}, {6}, {7}, {2}, {3}, {1}},
> };
>
> static const struct of_device_id mtk_iommu_of_ids[] = {
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index afd7a2de5c1e..d51ff99c2c71 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -41,10 +41,11 @@ struct mtk_iommu_plat_data {
> /* HW will use the EMI clock if there isn't the "bclk". */
> bool has_bclk;
> bool has_misc_ctrl;
> + bool has_sub_comm;
> bool has_vld_pa_rng;
> bool reset_axi;
> u32 inv_sel_reg;
> - unsigned char larbid_remap[MTK_LARB_NR_MAX];
> + unsigned char larbid_remap[8][4];
MTK_LARB_NR_MAX is 16, why do you decrease it to 8?
Should we use a define for the subcommon as well?
Regards,
Matthias
> };
>
> struct mtk_iommu_domain;
>
On 17/06/2020 05:00, Chao Hao wrote:
> Some platforms(ex: mt6779) have a new register called by REG_MMU_WR_LEN
> to improve performance.
> This patch add this register definition.
Please be more specific what this register is about.
>
> Signed-off-by: Chao Hao <[email protected]>
> ---
> drivers/iommu/mtk_iommu.c | 10 ++++++++++
> drivers/iommu/mtk_iommu.h | 2 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index a687e8db0e51..c706bca6487e 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -46,6 +46,8 @@
> #define F_MMU_STANDARD_AXI_MODE_BIT (BIT(3) | BIT(19))
>
> #define REG_MMU_DCM_DIS 0x050
> +#define REG_MMU_WR_LEN 0x054
> +#define F_MMU_WR_THROT_DIS_BIT (BIT(5) | BIT(21))
>
> #define REG_MMU_CTRL_REG 0x110
> #define F_MMU_TF_PROT_TO_PROGRAM_ADDR (2 << 4)
> @@ -581,6 +583,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
> }
> writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
> + if (data->plat_data->has_wr_len) {
> + /* write command throttling mode */
> + regval = readl_relaxed(data->base + REG_MMU_WR_LEN);
> + regval &= ~F_MMU_WR_THROT_DIS_BIT;
> + writel_relaxed(regval, data->base + REG_MMU_WR_LEN);
> + }
>
> if (data->plat_data->reset_axi) {
> /* The register is called STANDARD_AXI_MODE in this case */
> @@ -737,6 +745,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device *dev)
> struct mtk_iommu_suspend_reg *reg = &data->reg;
> void __iomem *base = data->base;
>
> + reg->wr_len = readl_relaxed(base + REG_MMU_WR_LEN);
Can we read/write the register without any side effect although hardware has not
implemented it (!has_wr_len)?
> 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);
> @@ -761,6 +770,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
> dev_err(data->dev, "Failed to enable clk(%d) in resume\n", ret);
> return ret;
> }
> + writel_relaxed(reg->wr_len, base + REG_MMU_WR_LEN);
> 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);
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index d51ff99c2c71..9971cedd72ea 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -25,6 +25,7 @@ struct mtk_iommu_suspend_reg {
> u32 int_main_control;
> u32 ivrp_paddr;
> u32 vld_pa_rng;
> + u32 wr_len;
> };
>
> enum mtk_iommu_plat {
> @@ -43,6 +44,7 @@ struct mtk_iommu_plat_data {
> bool has_misc_ctrl;
> bool has_sub_comm;
> bool has_vld_pa_rng;
> + bool has_wr_len;
Given the fact that we are adding more and more plat_data bool values, I think
it would make sense to use a u32 flags register and add the appropriate macro
definitions to set and check for a flag present.
Regards,
Matthias
> bool reset_axi;
> u32 inv_sel_reg;
> unsigned char larbid_remap[8][4];
>
Hi Matthias,
Thanks very much for your review.
On Wed, 2020-06-17 at 11:17 +0200, Matthias Brugger wrote:
>
> On 17/06/2020 05:00, Chao Hao wrote:
> > The max larb number that a iommu HW support is 8(larb0~larb7 in the below
> > diagram).
> > If the larb's number is over 8, we use a sub_common for merging
> > several larbs into one larb. At this case, we will extend larb_id:
> > bit[11:9] means common-id;
> > bit[8:7] means subcommon-id;
> > From these two variable, we could get the real larb number when
> > translation fault happen.
> > The diagram is as below:
> > EMI
> > |
> > IOMMU
> > |
> > -----------------
> > | |
> > common1 common0
> > | |
> > -----------------
> > |
> > smi common
> > |
> > ------------------------------------
> > | | | | | |
> > 3'd0 3'd1 3'd2 3'd3 ... 3'd7 <-common_id(max is 8)
> > | | | | | |
> > Larb0 Larb1 | Larb3 ... Larb7
> > |
> > smi sub common
> > |
> > --------------------------
> > | | | |
> > 2'd0 2'd1 2'd2 2'd3 <-sub_common_id(max is 4)
> > | | | |
> > Larb8 Larb9 Larb10 Larb11
> >
> > In this patch we extern larb_remap[] to larb_remap[8][4] for this.
>
> extern -> extend
>
> > larb_remap[x][y]: x mean common-id above, y means subcommon_id above.
>
> mean -> means
>
> >
> > We can also distinguish if the M4U HW has sub_common by has_sub_comm
> > property.
> >
> > Signed-off-by: Chao Hao <[email protected]>
> > Reviewed-by: Yong Wu <[email protected]>
> > ---
> > drivers/iommu/mtk_iommu.c | 20 +++++++++++++-------
> > drivers/iommu/mtk_iommu.h | 3 ++-
> > 2 files changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index f23919feba4e..a687e8db0e51 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -91,6 +91,8 @@
> > #define REG_MMU1_INVLD_PA 0x148
> > #define REG_MMU0_INT_ID 0x150
> > #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_LARB_ID(a) (((a) >> 7) & 0x7)
> > #define F_MMU_INT_ID_PORT_ID(a) (((a) >> 2) & 0x1f)
> >
> > @@ -229,7 +231,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;
> > u32 int_state, regval, fault_iova, fault_pa;
> > - unsigned int fault_larb, fault_port;
> > + unsigned int fault_larb, fault_port, sub_comm = 0;
> > bool layer, write;
> >
> > /* Read error info from registers */
> > @@ -245,10 +247,14 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
> > }
> > layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
> > write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
> > - fault_larb = F_MMU_INT_ID_LARB_ID(regval);
> > fault_port = F_MMU_INT_ID_PORT_ID(regval);
> > -
> > - fault_larb = data->plat_data->larbid_remap[fault_larb];
> > + if (data->plat_data->has_sub_comm) {
> > + fault_larb = F_MMU_INT_ID_COMM_ID(regval);
> > + sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
> > + } else {
> > + fault_larb = F_MMU_INT_ID_LARB_ID(regval);
> > + }
> > + 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)) {
> > @@ -778,7 +784,7 @@ static const struct mtk_iommu_plat_data mt2712_data = {
> > .has_bclk = true,
> > .has_vld_pa_rng = true,
> > .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> > - .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
> > + .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
> > };
> >
> > static const struct mtk_iommu_plat_data mt8173_data = {
> > @@ -787,14 +793,14 @@ static const struct mtk_iommu_plat_data mt8173_data = {
> > .has_bclk = true,
> > .reset_axi = true,
> > .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> > - .larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */
> > + .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
> > };
> >
> > static const struct mtk_iommu_plat_data mt8183_data = {
> > .m4u_plat = M4U_MT8183,
> > .reset_axi = true,
> > .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> > - .larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1},
> > + .larbid_remap = {{0}, {4}, {5}, {6}, {7}, {2}, {3}, {1}},
> > };
> >
> > static const struct of_device_id mtk_iommu_of_ids[] = {
> > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> > index afd7a2de5c1e..d51ff99c2c71 100644
> > --- a/drivers/iommu/mtk_iommu.h
> > +++ b/drivers/iommu/mtk_iommu.h
> > @@ -41,10 +41,11 @@ struct mtk_iommu_plat_data {
> > /* HW will use the EMI clock if there isn't the "bclk". */
> > bool has_bclk;
> > bool has_misc_ctrl;
> > + bool has_sub_comm;
> > bool has_vld_pa_rng;
> > bool reset_axi;
> > u32 inv_sel_reg;
> > - unsigned char larbid_remap[MTK_LARB_NR_MAX];
> > + unsigned char larbid_remap[8][4];
>
> MTK_LARB_NR_MAX is 16, why do you decrease it to 8?
From the diagram above, the max number of the larbs that could connected
with a IOMMU HW is 8. thus, 8 is right here for each a IOMMU HW.
as I commented when v3. mt2712 have the larbs over 8 since it has 2
IOMMU HWes.
and MTK_LARB_NR_MAX means the max larbs number that this SoC support.
Keep its value as is.
> Should we use a define for the subcommon as well?
>
> Regards,
> Matthias
>
> > };
> >
> > struct mtk_iommu_domain;
> >
On Wed, 2020-06-17 at 19:11 +0800, Yong Wu wrote:
> Hi Matthias,
>
> Thanks very much for your review.
>
> On Wed, 2020-06-17 at 11:17 +0200, Matthias Brugger wrote:
> >
> > On 17/06/2020 05:00, Chao Hao wrote:
> > > The max larb number that a iommu HW support is 8(larb0~larb7 in the below
> > > diagram).
> > > If the larb's number is over 8, we use a sub_common for merging
> > > several larbs into one larb. At this case, we will extend larb_id:
> > > bit[11:9] means common-id;
> > > bit[8:7] means subcommon-id;
> > > From these two variable, we could get the real larb number when
> > > translation fault happen.
> > > The diagram is as below:
> > > EMI
> > > |
> > > IOMMU
> > > |
> > > -----------------
> > > | |
> > > common1 common0
> > > | |
> > > -----------------
> > > |
> > > smi common
> > > |
> > > ------------------------------------
> > > | | | | | |
> > > 3'd0 3'd1 3'd2 3'd3 ... 3'd7 <-common_id(max is 8)
> > > | | | | | |
> > > Larb0 Larb1 | Larb3 ... Larb7
> > > |
> > > smi sub common
> > > |
> > > --------------------------
> > > | | | |
> > > 2'd0 2'd1 2'd2 2'd3 <-sub_common_id(max is 4)
> > > | | | |
> > > Larb8 Larb9 Larb10 Larb11
> > >
> > > In this patch we extern larb_remap[] to larb_remap[8][4] for this.
> >
> > extern -> extend
> >
> > > larb_remap[x][y]: x mean common-id above, y means subcommon_id above.
> >
> > mean -> means
> >
> > >
> > > We can also distinguish if the M4U HW has sub_common by has_sub_comm
> > > property.
> > >
> > > Signed-off-by: Chao Hao <[email protected]>
> > > Reviewed-by: Yong Wu <[email protected]>
> > > ---
> > > drivers/iommu/mtk_iommu.c | 20 +++++++++++++-------
> > > drivers/iommu/mtk_iommu.h | 3 ++-
> > > 2 files changed, 15 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > > index f23919feba4e..a687e8db0e51 100644
> > > --- a/drivers/iommu/mtk_iommu.c
> > > +++ b/drivers/iommu/mtk_iommu.c
> > > @@ -91,6 +91,8 @@
> > > #define REG_MMU1_INVLD_PA 0x148
> > > #define REG_MMU0_INT_ID 0x150
> > > #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_LARB_ID(a) (((a) >> 7) & 0x7)
> > > #define F_MMU_INT_ID_PORT_ID(a) (((a) >> 2) & 0x1f)
> > >
> > > @@ -229,7 +231,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;
> > > u32 int_state, regval, fault_iova, fault_pa;
> > > - unsigned int fault_larb, fault_port;
> > > + unsigned int fault_larb, fault_port, sub_comm = 0;
> > > bool layer, write;
> > >
> > > /* Read error info from registers */
> > > @@ -245,10 +247,14 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
> > > }
> > > layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
> > > write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
> > > - fault_larb = F_MMU_INT_ID_LARB_ID(regval);
> > > fault_port = F_MMU_INT_ID_PORT_ID(regval);
> > > -
> > > - fault_larb = data->plat_data->larbid_remap[fault_larb];
> > > + if (data->plat_data->has_sub_comm) {
> > > + fault_larb = F_MMU_INT_ID_COMM_ID(regval);
> > > + sub_comm = F_MMU_INT_ID_SUB_COMM_ID(regval);
> > > + } else {
> > > + fault_larb = F_MMU_INT_ID_LARB_ID(regval);
> > > + }
> > > + 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)) {
> > > @@ -778,7 +784,7 @@ static const struct mtk_iommu_plat_data mt2712_data = {
> > > .has_bclk = true,
> > > .has_vld_pa_rng = true,
> > > .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> > > - .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
> > > + .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
> > > };
> > >
> > > static const struct mtk_iommu_plat_data mt8173_data = {
> > > @@ -787,14 +793,14 @@ static const struct mtk_iommu_plat_data mt8173_data = {
> > > .has_bclk = true,
> > > .reset_axi = true,
> > > .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> > > - .larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */
> > > + .larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
> > > };
> > >
> > > static const struct mtk_iommu_plat_data mt8183_data = {
> > > .m4u_plat = M4U_MT8183,
> > > .reset_axi = true,
> > > .inv_sel_reg = REG_MMU_INV_SEL_GEN1,
> > > - .larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1},
> > > + .larbid_remap = {{0}, {4}, {5}, {6}, {7}, {2}, {3}, {1}},
> > > };
> > >
> > > static const struct of_device_id mtk_iommu_of_ids[] = {
> > > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> > > index afd7a2de5c1e..d51ff99c2c71 100644
> > > --- a/drivers/iommu/mtk_iommu.h
> > > +++ b/drivers/iommu/mtk_iommu.h
> > > @@ -41,10 +41,11 @@ struct mtk_iommu_plat_data {
> > > /* HW will use the EMI clock if there isn't the "bclk". */
> > > bool has_bclk;
> > > bool has_misc_ctrl;
> > > + bool has_sub_comm;
> > > bool has_vld_pa_rng;
> > > bool reset_axi;
> > > u32 inv_sel_reg;
> > > - unsigned char larbid_remap[MTK_LARB_NR_MAX];
> > > + unsigned char larbid_remap[8][4];
> >
> > MTK_LARB_NR_MAX is 16, why do you decrease it to 8?
>
> From the diagram above, the max number of the larbs that could connected
> with a IOMMU HW is 8. thus, 8 is right here for each a IOMMU HW.
>
> as I commented when v3. mt2712 have the larbs over 8 since it has 2
> IOMMU HWes.
>
> and MTK_LARB_NR_MAX means the max larbs number that this SoC support.
> Keep its value as is.
>
>
> > Should we use a define for the subcommon as well?
> >
> > Regards,
> > Matthias
> >
Hi Matthias and yong,
Thanks very much for your review.
HW diagram is as belove and whether we need to use macro definitions to
show it, maybe more clearer? like this:
#define LARB_COMMON_MAX 8
#define LARB_SUB_COMMON_MAX 4
unsigned char larbid_remap[LARB_COMMON_MAX][LARB_SUB_COMMON_MAX];
>
> > > };
> > >
> > > struct mtk_iommu_domain;
> > >
>
>
On Wed, 2020-06-17 at 11:22 +0200, Matthias Brugger wrote:
>
> On 17/06/2020 05:00, Chao Hao wrote:
> > Some platforms(ex: mt6779) have a new register called by REG_MMU_WR_LEN
> > to improve performance.
> > This patch add this register definition.
>
> Please be more specific what this register is about.
>
OK. thanks.
We can use "has_wr_len" flag to control whether we need to set the
register. If the register uses default value, iommu will send command to
EMI without restriction, when the number of commands become more and
more, it will drop the EMI performance. So when more than
ten_commands(default value) don't be handled for EMI, IOMMU will stop
send command to EMI for keeping EMI's performace by enabling write
throttling mechanism(bit[5][21]=0) in MMU_WR_LEN_CTRL register.
I will write description above to commit message in next version
> >
> > Signed-off-by: Chao Hao <[email protected]>
> > ---
> > drivers/iommu/mtk_iommu.c | 10 ++++++++++
> > drivers/iommu/mtk_iommu.h | 2 ++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index a687e8db0e51..c706bca6487e 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -46,6 +46,8 @@
> > #define F_MMU_STANDARD_AXI_MODE_BIT (BIT(3) | BIT(19))
> >
> > #define REG_MMU_DCM_DIS 0x050
> > +#define REG_MMU_WR_LEN 0x054
> > +#define F_MMU_WR_THROT_DIS_BIT (BIT(5) | BIT(21))
> >
> > #define REG_MMU_CTRL_REG 0x110
> > #define F_MMU_TF_PROT_TO_PROGRAM_ADDR (2 << 4)
> > @@ -581,6 +583,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> > writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
> > }
> > writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
> > + if (data->plat_data->has_wr_len) {
> > + /* write command throttling mode */
> > + regval = readl_relaxed(data->base + REG_MMU_WR_LEN);
> > + regval &= ~F_MMU_WR_THROT_DIS_BIT;
> > + writel_relaxed(regval, data->base + REG_MMU_WR_LEN);
> > + }
> >
> > if (data->plat_data->reset_axi) {
> > /* The register is called STANDARD_AXI_MODE in this case */
> > @@ -737,6 +745,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device *dev)
> > struct mtk_iommu_suspend_reg *reg = &data->reg;
> > void __iomem *base = data->base;
> >
> > + reg->wr_len = readl_relaxed(base + REG_MMU_WR_LEN);
>
> Can we read/write the register without any side effect although hardware has not
> implemented it (!has_wr_len)?
It doesn't have side effect. Becasue all the MTK platform have the
register for iommu HW. If we need to have requirement for performance,
we can set it by has_wr_len.
But I'm Sorry, the name of flag(has_wr_len) is not exact, I will rename
it in next version, ex: "wr_throt_en"
>
>
> > 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);
> > @@ -761,6 +770,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
> > dev_err(data->dev, "Failed to enable clk(%d) in resume\n", ret);
> > return ret;
> > }
> > + writel_relaxed(reg->wr_len, base + REG_MMU_WR_LEN);
> > 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);
> > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> > index d51ff99c2c71..9971cedd72ea 100644
> > --- a/drivers/iommu/mtk_iommu.h
> > +++ b/drivers/iommu/mtk_iommu.h
> > @@ -25,6 +25,7 @@ struct mtk_iommu_suspend_reg {
> > u32 int_main_control;
> > u32 ivrp_paddr;
> > u32 vld_pa_rng;
> > + u32 wr_len;
> > };
> >
> > enum mtk_iommu_plat {
> > @@ -43,6 +44,7 @@ struct mtk_iommu_plat_data {
> > bool has_misc_ctrl;
> > bool has_sub_comm;
> > bool has_vld_pa_rng;
> > + bool has_wr_len;
>
> Given the fact that we are adding more and more plat_data bool values, I think
> it would make sense to use a u32 flags register and add the appropriate macro
> definitions to set and check for a flag present.
Thanks for your advice.
do you mean like this:
struct plat_flag {
#define HAS_4GB_MODE BIT(0)
#define HAS_BCLK BIT(1)
#define REST_AXI BIT(2)
... ...
u32 flag;
};
struct mtk_iommu_plat_data {
......
struct plat_flag flag;
......
};
> Regards,
> Matthias
>
> > bool reset_axi;
> > u32 inv_sel_reg;
> > unsigned char larbid_remap[8][4];
> >
On 19/06/2020 12:56, chao hao wrote:
> On Wed, 2020-06-17 at 11:22 +0200, Matthias Brugger wrote:
>>
>> On 17/06/2020 05:00, Chao Hao wrote:
>>> Some platforms(ex: mt6779) have a new register called by REG_MMU_WR_LEN
>>> to improve performance.
>>> This patch add this register definition.
>>
>> Please be more specific what this register is about.
>>
> OK. thanks.
> We can use "has_wr_len" flag to control whether we need to set the
> register. If the register uses default value, iommu will send command to
> EMI without restriction, when the number of commands become more and
> more, it will drop the EMI performance. So when more than
> ten_commands(default value) don't be handled for EMI, IOMMU will stop
> send command to EMI for keeping EMI's performace by enabling write
> throttling mechanism(bit[5][21]=0) in MMU_WR_LEN_CTRL register.
>
> I will write description above to commit message in next version
>
>>>
>>> Signed-off-by: Chao Hao <[email protected]>
>>> ---
>>> drivers/iommu/mtk_iommu.c | 10 ++++++++++
>>> drivers/iommu/mtk_iommu.h | 2 ++
>>> 2 files changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>>> index a687e8db0e51..c706bca6487e 100644
>>> --- a/drivers/iommu/mtk_iommu.c
>>> +++ b/drivers/iommu/mtk_iommu.c
>>> @@ -46,6 +46,8 @@
>>> #define F_MMU_STANDARD_AXI_MODE_BIT (BIT(3) | BIT(19))
>>>
>>> #define REG_MMU_DCM_DIS 0x050
>>> +#define REG_MMU_WR_LEN 0x054
>>> +#define F_MMU_WR_THROT_DIS_BIT (BIT(5) | BIT(21))
>>>
>>> #define REG_MMU_CTRL_REG 0x110
>>> #define F_MMU_TF_PROT_TO_PROGRAM_ADDR (2 << 4)
>>> @@ -581,6 +583,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
>>> writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
>>> }
>>> writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
>>> + if (data->plat_data->has_wr_len) {
>>> + /* write command throttling mode */
>>> + regval = readl_relaxed(data->base + REG_MMU_WR_LEN);
>>> + regval &= ~F_MMU_WR_THROT_DIS_BIT;
>>> + writel_relaxed(regval, data->base + REG_MMU_WR_LEN);
>>> + }
>>>
>>> if (data->plat_data->reset_axi) {
>>> /* The register is called STANDARD_AXI_MODE in this case */
>>> @@ -737,6 +745,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device *dev)
>>> struct mtk_iommu_suspend_reg *reg = &data->reg;
>>> void __iomem *base = data->base;
>>>
>>> + reg->wr_len = readl_relaxed(base + REG_MMU_WR_LEN);
>>
>> Can we read/write the register without any side effect although hardware has not
>> implemented it (!has_wr_len)?
>
> It doesn't have side effect. Becasue all the MTK platform have the
> register for iommu HW. If we need to have requirement for performance,
> we can set it by has_wr_len.
> But I'm Sorry, the name of flag(has_wr_len) is not exact, I will rename
> it in next version, ex: "wr_throt_en"
>
>>
>>
>>> 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);
>>> @@ -761,6 +770,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
>>> dev_err(data->dev, "Failed to enable clk(%d) in resume\n", ret);
>>> return ret;
>>> }
>>> + writel_relaxed(reg->wr_len, base + REG_MMU_WR_LEN);
>>> 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);
>>> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
>>> index d51ff99c2c71..9971cedd72ea 100644
>>> --- a/drivers/iommu/mtk_iommu.h
>>> +++ b/drivers/iommu/mtk_iommu.h
>>> @@ -25,6 +25,7 @@ struct mtk_iommu_suspend_reg {
>>> u32 int_main_control;
>>> u32 ivrp_paddr;
>>> u32 vld_pa_rng;
>>> + u32 wr_len;
>>> };
>>>
>>> enum mtk_iommu_plat {
>>> @@ -43,6 +44,7 @@ struct mtk_iommu_plat_data {
>>> bool has_misc_ctrl;
>>> bool has_sub_comm;
>>> bool has_vld_pa_rng;
>>> + bool has_wr_len;
>>
>> Given the fact that we are adding more and more plat_data bool values, I think
>> it would make sense to use a u32 flags register and add the appropriate macro
>> definitions to set and check for a flag present.
>
> Thanks for your advice.
> do you mean like this:
> struct plat_flag {
>
> #define HAS_4GB_MODE BIT(0)
> #define HAS_BCLK BIT(1)
> #define REST_AXI BIT(2)
> ... ...
>
> u32 flag;
> };
>
> struct mtk_iommu_plat_data {
> ......
> struct plat_flag flag;
> ......
> };
>
Nearly, I mean something like this:
#define HAS_4GB_MODE BIT(0)
#define HAS_BCLK BIT(1)
#define REST_AXI BIT(2)
#define MTK_IOMMU_HAS_FLAG(pdata, _x) \
((((pdata)->flags) & (_x)) == (_x))
struct mtk_iommu_plat_data {
...
u32 flags;
...
}
if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_BCLK)
...
Regards,
Matthias
>
>> Regards,
>> Matthias
>>
>>> bool reset_axi;
>>> u32 inv_sel_reg;
>>> unsigned char larbid_remap[8][4];
>>>
>
On Sun, 2020-06-21 at 13:01 +0200, Matthias Brugger wrote:
>
> On 19/06/2020 12:56, chao hao wrote:
> > On Wed, 2020-06-17 at 11:22 +0200, Matthias Brugger wrote:
> >>
> >> On 17/06/2020 05:00, Chao Hao wrote:
> >>> Some platforms(ex: mt6779) have a new register called by REG_MMU_WR_LEN
> >>> to improve performance.
> >>> This patch add this register definition.
> >>
> >> Please be more specific what this register is about.
> >>
> > OK. thanks.
> > We can use "has_wr_len" flag to control whether we need to set the
> > register. If the register uses default value, iommu will send command to
> > EMI without restriction, when the number of commands become more and
> > more, it will drop the EMI performance. So when more than
> > ten_commands(default value) don't be handled for EMI, IOMMU will stop
> > send command to EMI for keeping EMI's performace by enabling write
> > throttling mechanism(bit[5][21]=0) in MMU_WR_LEN_CTRL register.
> >
> > I will write description above to commit message in next version
> >
> >>>
> >>> Signed-off-by: Chao Hao <[email protected]>
> >>> ---
> >>> drivers/iommu/mtk_iommu.c | 10 ++++++++++
> >>> drivers/iommu/mtk_iommu.h | 2 ++
> >>> 2 files changed, 12 insertions(+)
> >>>
> >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> >>> index a687e8db0e51..c706bca6487e 100644
> >>> --- a/drivers/iommu/mtk_iommu.c
> >>> +++ b/drivers/iommu/mtk_iommu.c
> >>> @@ -46,6 +46,8 @@
> >>> #define F_MMU_STANDARD_AXI_MODE_BIT (BIT(3) | BIT(19))
> >>>
> >>> #define REG_MMU_DCM_DIS 0x050
> >>> +#define REG_MMU_WR_LEN 0x054
> >>> +#define F_MMU_WR_THROT_DIS_BIT (BIT(5) | BIT(21))
> >>>
> >>> #define REG_MMU_CTRL_REG 0x110
> >>> #define F_MMU_TF_PROT_TO_PROGRAM_ADDR (2 << 4)
> >>> @@ -581,6 +583,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> >>> writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
> >>> }
> >>> writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
> >>> + if (data->plat_data->has_wr_len) {
> >>> + /* write command throttling mode */
> >>> + regval = readl_relaxed(data->base + REG_MMU_WR_LEN);
> >>> + regval &= ~F_MMU_WR_THROT_DIS_BIT;
> >>> + writel_relaxed(regval, data->base + REG_MMU_WR_LEN);
> >>> + }
> >>>
> >>> if (data->plat_data->reset_axi) {
> >>> /* The register is called STANDARD_AXI_MODE in this case */
> >>> @@ -737,6 +745,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device *dev)
> >>> struct mtk_iommu_suspend_reg *reg = &data->reg;
> >>> void __iomem *base = data->base;
> >>>
> >>> + reg->wr_len = readl_relaxed(base + REG_MMU_WR_LEN);
> >>
> >> Can we read/write the register without any side effect although hardware has not
> >> implemented it (!has_wr_len)?
> >
> > It doesn't have side effect. Becasue all the MTK platform have the
> > register for iommu HW. If we need to have requirement for performance,
> > we can set it by has_wr_len.
> > But I'm Sorry, the name of flag(has_wr_len) is not exact, I will rename
> > it in next version, ex: "wr_throt_en"
> >
> >>
> >>
> >>> 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);
> >>> @@ -761,6 +770,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
> >>> dev_err(data->dev, "Failed to enable clk(%d) in resume\n", ret);
> >>> return ret;
> >>> }
> >>> + writel_relaxed(reg->wr_len, base + REG_MMU_WR_LEN);
> >>> 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);
> >>> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> >>> index d51ff99c2c71..9971cedd72ea 100644
> >>> --- a/drivers/iommu/mtk_iommu.h
> >>> +++ b/drivers/iommu/mtk_iommu.h
> >>> @@ -25,6 +25,7 @@ struct mtk_iommu_suspend_reg {
> >>> u32 int_main_control;
> >>> u32 ivrp_paddr;
> >>> u32 vld_pa_rng;
> >>> + u32 wr_len;
> >>> };
> >>>
> >>> enum mtk_iommu_plat {
> >>> @@ -43,6 +44,7 @@ struct mtk_iommu_plat_data {
> >>> bool has_misc_ctrl;
> >>> bool has_sub_comm;
> >>> bool has_vld_pa_rng;
> >>> + bool has_wr_len;
> >>
> >> Given the fact that we are adding more and more plat_data bool values, I think
> >> it would make sense to use a u32 flags register and add the appropriate macro
> >> definitions to set and check for a flag present.
> >
> > Thanks for your advice.
> > do you mean like this:
> > struct plat_flag {
> >
> > #define HAS_4GB_MODE BIT(0)
> > #define HAS_BCLK BIT(1)
> > #define REST_AXI BIT(2)
> > ... ...
> >
> > u32 flag;
> > };
> >
> > struct mtk_iommu_plat_data {
> > ......
> > struct plat_flag flag;
> > ......
> > };
> >
>
> Nearly, I mean something like this:
>
> #define HAS_4GB_MODE BIT(0)
> #define HAS_BCLK BIT(1)
> #define REST_AXI BIT(2)
>
> #define MTK_IOMMU_HAS_FLAG(pdata, _x) \
> ((((pdata)->flags) & (_x)) == (_x))
>
> struct mtk_iommu_plat_data {
> ...
> u32 flags;
> ...
> }
>
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_BCLK)
> ...
>
Ok, got it, thanks
> Regards,
> Matthias
>
> >
> >> Regards,
> >> Matthias
> >>
> >>> bool reset_axi;
> >>> u32 inv_sel_reg;
> >>> unsigned char larbid_remap[8][4];
> >>>
> >