| Message ID | 20200703044127.27438-1-chao.hao@mediatek.com |
|---|---|
| Headers | show |
| Series | MT6779 IOMMU SUPPORT | expand |
On Fri, 2020-07-03 at 12:41 +0800, Chao Hao wrote: > Given the fact that we are adding more and more plat_data bool values, > it would make sense to use a u32 flags register and add the appropriate > macro definitions to set and check for a flag present. > No functional change. > > Cc: Yong Wu <yong.wu@mediatek.com> > Suggested-by: Matthias Brugger <matthias.bgg@gmail.com> > Signed-off-by: Chao Hao <chao.hao@mediatek.com> > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/iommu/mtk_iommu.c | 28 +++++++++++++++++----------- > drivers/iommu/mtk_iommu.h | 7 +------ > 2 files changed, 18 insertions(+), 17 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 88d3df5b91c2..40ca564d97af 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -100,6 +100,15 @@ > #define MTK_M4U_TO_LARB(id) (((id) >> 5) & 0xf) > #define MTK_M4U_TO_PORT(id) ((id) & 0x1f) > > +#define HAS_4GB_MODE BIT(0) > +/* HW will use the EMI clock if there isn't the "bclk". */ > +#define HAS_BCLK BIT(1) > +#define HAS_VLD_PA_RNG BIT(2) > +#define RESET_AXI BIT(3) > + > +#define MTK_IOMMU_HAS_FLAG(pdata, _x) \ > + ((((pdata)->flags) & (_x)) == (_x)) > + > struct mtk_iommu_domain { > struct io_pgtable_cfg cfg; > struct io_pgtable_ops *iop; > @@ -563,7 +572,8 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > upper_32_bits(data->protect_base); > writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR); > > - if (data->enable_4GB && data->plat_data->has_vld_pa_rng) { > + if (data->enable_4GB && > + MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_VLD_PA_RNG)) { > /* > * If 4GB mode is enabled, the validate PA range is from > * 0x1_0000_0000 to 0x1_ffff_ffff. here record bit[32:30]. > @@ -573,7 +583,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > } > writel_relaxed(0, data->base + REG_MMU_DCM_DIS); > > - if (data->plat_data->reset_axi) { > + if (MTK_IOMMU_HAS_FLAG(data->plat_data, RESET_AXI)) { > /* The register is called STANDARD_AXI_MODE in this case */ > writel_relaxed(0, data->base + REG_MMU_MISC_CTRL); > } > @@ -618,7 +628,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > > /* Whether the current dram is over 4GB */ > data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT)); > - if (!data->plat_data->has_4gb_mode) > + if (!MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) > data->enable_4GB = false; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -631,7 +641,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > if (data->irq < 0) > return data->irq; > > - if (data->plat_data->has_bclk) { > + if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_BCLK)) { > data->bclk = devm_clk_get(dev, "bclk"); > if (IS_ERR(data->bclk)) > return PTR_ERR(data->bclk); > @@ -763,23 +773,19 @@ static const struct dev_pm_ops mtk_iommu_pm_ops = { > > static const struct mtk_iommu_plat_data mt2712_data = { > .m4u_plat = M4U_MT2712, > - .has_4gb_mode = true, > - .has_bclk = true, > - .has_vld_pa_rng = true, > + .flags = HAS_4GB_MODE | HAS_BCLK | HAS_VLD_PA_RNG, > .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, > }; > > static const struct mtk_iommu_plat_data mt8173_data = { > .m4u_plat = M4U_MT8173, > - .has_4gb_mode = true, > - .has_bclk = true, > - .reset_axi = true, > + .flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI, > .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, > + .flags = RESET_AXI, > .larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1}, > }; > > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > index 7212e6fcf982..5225a9170aaa 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -39,12 +39,7 @@ enum mtk_iommu_plat { > > struct mtk_iommu_plat_data { > enum mtk_iommu_plat m4u_plat; > - bool has_4gb_mode; > - > - /* HW will use the EMI clock if there isn't the "bclk". */ > - bool has_bclk; > - bool has_vld_pa_rng; > - bool reset_axi; > + u32 flags; How about using bit field instead? eg u32 has_bclk:1; In this way, we don't need to change code. Joe.C
On 04/07/2020 03:16, Yingjoe Chen wrote: > On Fri, 2020-07-03 at 12:41 +0800, Chao Hao wrote: >> Given the fact that we are adding more and more plat_data bool values, >> it would make sense to use a u32 flags register and add the appropriate >> macro definitions to set and check for a flag present. >> No functional change. >> >> Cc: Yong Wu <yong.wu@mediatek.com> >> Suggested-by: Matthias Brugger <matthias.bgg@gmail.com> >> Signed-off-by: Chao Hao <chao.hao@mediatek.com> >> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> >> --- >> drivers/iommu/mtk_iommu.c | 28 +++++++++++++++++----------- >> drivers/iommu/mtk_iommu.h | 7 +------ >> 2 files changed, 18 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c >> index 88d3df5b91c2..40ca564d97af 100644 >> --- a/drivers/iommu/mtk_iommu.c >> +++ b/drivers/iommu/mtk_iommu.c >> @@ -100,6 +100,15 @@ >> #define MTK_M4U_TO_LARB(id) (((id) >> 5) & 0xf) >> #define MTK_M4U_TO_PORT(id) ((id) & 0x1f) >> >> +#define HAS_4GB_MODE BIT(0) >> +/* HW will use the EMI clock if there isn't the "bclk". */ >> +#define HAS_BCLK BIT(1) >> +#define HAS_VLD_PA_RNG BIT(2) >> +#define RESET_AXI BIT(3) >> + >> +#define MTK_IOMMU_HAS_FLAG(pdata, _x) \ >> + ((((pdata)->flags) & (_x)) == (_x)) >> + >> struct mtk_iommu_domain { >> struct io_pgtable_cfg cfg; >> struct io_pgtable_ops *iop; >> @@ -563,7 +572,8 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) >> upper_32_bits(data->protect_base); >> writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR); >> >> - if (data->enable_4GB && data->plat_data->has_vld_pa_rng) { >> + if (data->enable_4GB && >> + MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_VLD_PA_RNG)) { >> /* >> * If 4GB mode is enabled, the validate PA range is from >> * 0x1_0000_0000 to 0x1_ffff_ffff. here record bit[32:30]. >> @@ -573,7 +583,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) >> } >> writel_relaxed(0, data->base + REG_MMU_DCM_DIS); >> >> - if (data->plat_data->reset_axi) { >> + if (MTK_IOMMU_HAS_FLAG(data->plat_data, RESET_AXI)) { >> /* The register is called STANDARD_AXI_MODE in this case */ >> writel_relaxed(0, data->base + REG_MMU_MISC_CTRL); >> } >> @@ -618,7 +628,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) >> >> /* Whether the current dram is over 4GB */ >> data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT)); >> - if (!data->plat_data->has_4gb_mode) >> + if (!MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) >> data->enable_4GB = false; >> >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> @@ -631,7 +641,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) >> if (data->irq < 0) >> return data->irq; >> >> - if (data->plat_data->has_bclk) { >> + if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_BCLK)) { >> data->bclk = devm_clk_get(dev, "bclk"); >> if (IS_ERR(data->bclk)) >> return PTR_ERR(data->bclk); >> @@ -763,23 +773,19 @@ static const struct dev_pm_ops mtk_iommu_pm_ops = { >> >> static const struct mtk_iommu_plat_data mt2712_data = { >> .m4u_plat = M4U_MT2712, >> - .has_4gb_mode = true, >> - .has_bclk = true, >> - .has_vld_pa_rng = true, >> + .flags = HAS_4GB_MODE | HAS_BCLK | HAS_VLD_PA_RNG, >> .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, >> }; >> >> static const struct mtk_iommu_plat_data mt8173_data = { >> .m4u_plat = M4U_MT8173, >> - .has_4gb_mode = true, >> - .has_bclk = true, >> - .reset_axi = true, >> + .flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI, >> .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, >> + .flags = RESET_AXI, >> .larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1}, >> }; >> >> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h >> index 7212e6fcf982..5225a9170aaa 100644 >> --- a/drivers/iommu/mtk_iommu.h >> +++ b/drivers/iommu/mtk_iommu.h >> @@ -39,12 +39,7 @@ enum mtk_iommu_plat { >> >> struct mtk_iommu_plat_data { >> enum mtk_iommu_plat m4u_plat; >> - bool has_4gb_mode; >> - >> - /* HW will use the EMI clock if there isn't the "bclk". */ >> - bool has_bclk; >> - bool has_vld_pa_rng; >> - bool reset_axi; >> + u32 flags; > > > How about using bit field instead? eg > > u32 has_bclk:1; > > In this way, we don't need to change code. > Actually I proposed to use the flag approach because I didn't want to bloat the mtk_iommu_plat_data structure with new variables for every new feature, being it a bit field or a bool. Regards, Matthias
On 03/07/2020 06:41, Chao Hao wrote: > Add F_MMU_IN_ORDER_WR_EN_MASK and F_MMU_STANDARD_AXI_MODE_EN_MASK > definitions in MISC_CTRL register. > F_MMU_STANDARD_AXI_MODE_EN_MASK: > If we set F_MMU_STANDARD_AXI_MODE_EN_MASK (bit[3][19] = 0, not follow > standard AXI protocol), the iommu will priorize sending of urgent read > command over a normal read command. This improves the performance. > F_MMU_IN_ORDER_WR_EN_MASK: > If we set F_MMU_IN_ORDER_WR_EN_MASK (bit[1][17] = 0, out-of-order write), > the iommu will re-order write commands and send the write commands with > higher priority. Otherwise the sending of write commands will be done in > order. The feature is controlled by OUT_ORDER_WR_EN platform data flag. > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > Suggested-by: Yong Wu <yong.wu@mediatek.com> > Signed-off-by: Chao Hao <chao.hao@mediatek.com> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/iommu/mtk_iommu.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 40ca564d97af..219d7aa6f059 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -42,6 +42,9 @@ > #define F_INVLD_EN1 BIT(1) > > #define REG_MMU_MISC_CTRL 0x048 > +#define F_MMU_IN_ORDER_WR_EN_MASK (BIT(1) | BIT(17)) > +#define F_MMU_STANDARD_AXI_MODE_MASK (BIT(3) | BIT(19)) > + > #define REG_MMU_DCM_DIS 0x050 > > #define REG_MMU_CTRL_REG 0x110 > @@ -105,6 +108,7 @@ > #define HAS_BCLK BIT(1) > #define HAS_VLD_PA_RNG BIT(2) > #define RESET_AXI BIT(3) > +#define OUT_ORDER_WR_EN BIT(4) > > #define MTK_IOMMU_HAS_FLAG(pdata, _x) \ > ((((pdata)->flags) & (_x)) == (_x)) > @@ -585,8 +589,14 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > > if (MTK_IOMMU_HAS_FLAG(data->plat_data, RESET_AXI)) { > /* The register is called STANDARD_AXI_MODE in this case */ > - writel_relaxed(0, data->base + REG_MMU_MISC_CTRL); > + 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, OUT_ORDER_WR_EN)) > + regval &= ~F_MMU_IN_ORDER_WR_EN_MASK; > } > + writel_relaxed(regval, data->base + REG_MMU_MISC_CTRL); > > if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0, > dev_name(data->dev), (void *)data)) { >
On 03/07/2020 06:41, 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 variables, 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 extend larb_remap[] to larb_remap[8][4] for this. > larb_remap[x][y]: x means common-id above, y means subcommon_id above. > > We can also distinguish if the M4U HW has sub_common by HAS_SUB_COMM > macro. > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > Signed-off-by: Chao Hao <chao.hao@mediatek.com> > Reviewed-by: Yong Wu <yong.wu@mediatek.com> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/iommu/mtk_iommu.c | 21 ++++++++++++++------- > drivers/iommu/mtk_iommu.h | 5 ++++- > 2 files changed, 18 insertions(+), 8 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 533b8f76f592..0d96dcd8612b 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) > > @@ -109,6 +111,7 @@ > #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 MTK_IOMMU_HAS_FLAG(pdata, _x) \ > ((((pdata)->flags) & (_x)) == (_x)) > @@ -239,7 +242,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 */ > @@ -255,10 +258,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 (MTK_IOMMU_HAS_FLAG(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)) { > @@ -785,21 +792,21 @@ static const struct mtk_iommu_plat_data mt2712_data = { > .m4u_plat = M4U_MT2712, > .flags = HAS_4GB_MODE | HAS_BCLK | HAS_VLD_PA_RNG, > .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 = { > .m4u_plat = M4U_MT8173, > .flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI, > .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, > .flags = RESET_AXI, > .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 cf53f5e80d22..46d0d47b22e1 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -17,6 +17,9 @@ > #include <linux/spinlock.h> > #include <soc/mediatek/smi.h> > > +#define MTK_LARB_COM_MAX 8 > +#define MTK_LARB_SUBCOM_MAX 4 > + > struct mtk_iommu_suspend_reg { > union { > u32 standard_axi_mode;/* v1 */ > @@ -41,7 +44,7 @@ struct mtk_iommu_plat_data { > enum mtk_iommu_plat m4u_plat; > u32 flags; > u32 inv_sel_reg; > - unsigned char larbid_remap[MTK_LARB_NR_MAX]; > + unsigned char larbid_remap[MTK_LARB_COM_MAX][MTK_LARB_SUBCOM_MAX]; > }; > > struct mtk_iommu_domain; >
On 03/07/2020 06:41, Chao Hao wrote: > The MMU_CTRL register of MT8173 is different from other SoCs. > The in_order_wr_en is bit[9] which is zero by default. > Other SoCs have the vitcim_tlb_en feature mapped to bit[12]. > This bit is set to one by default. We need to preserve the bit > when setting F_MMU_TF_PROT_TO_PROGRAM_ADDR as otherwise the > bit will be cleared and IOMMU performance will drop. > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > Cc: Yong Wu <yong.wu@mediatek.com> > Signed-off-by: Chao Hao <chao.hao@mediatek.com> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > 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 e71003037ffa..a816030d00f1 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -555,11 +555,13 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > return ret; > } > > - if (data->plat_data->m4u_plat == M4U_MT8173) > + 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 = F_MMU_TF_PROT_TO_PROGRAM_ADDR; > + } else { > + regval = readl_relaxed(data->base + REG_MMU_CTRL_REG); > + regval |= F_MMU_TF_PROT_TO_PROGRAM_ADDR; > + } > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG); > > regval = F_L2_MULIT_HIT_EN | >
Hi Matthias and Yingjoe, Thanks for your comments! On Mon, 2020-07-06 at 17:17 +0200, Matthias Brugger wrote: > > On 04/07/2020 03:16, Yingjoe Chen wrote: > > On Fri, 2020-07-03 at 12:41 +0800, Chao Hao wrote: > >> Given the fact that we are adding more and more plat_data bool values, > >> it would make sense to use a u32 flags register and add the appropriate > >> macro definitions to set and check for a flag present. > >> No functional change. > >> > >> Cc: Yong Wu <yong.wu@mediatek.com> > >> Suggested-by: Matthias Brugger <matthias.bgg@gmail.com> > >> Signed-off-by: Chao Hao <chao.hao@mediatek.com> > >> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > >> --- > >> drivers/iommu/mtk_iommu.c | 28 +++++++++++++++++----------- > >> drivers/iommu/mtk_iommu.h | 7 +------ > >> 2 files changed, 18 insertions(+), 17 deletions(-) > >> > >> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > >> index 88d3df5b91c2..40ca564d97af 100644 > >> --- a/drivers/iommu/mtk_iommu.c > >> +++ b/drivers/iommu/mtk_iommu.c > >> @@ -100,6 +100,15 @@ > >> #define MTK_M4U_TO_LARB(id) (((id) >> 5) & 0xf) > >> #define MTK_M4U_TO_PORT(id) ((id) & 0x1f) > >> > >> +#define HAS_4GB_MODE BIT(0) > >> +/* HW will use the EMI clock if there isn't the "bclk". */ > >> +#define HAS_BCLK BIT(1) > >> +#define HAS_VLD_PA_RNG BIT(2) > >> +#define RESET_AXI BIT(3) > >> + > >> +#define MTK_IOMMU_HAS_FLAG(pdata, _x) \ > >> + ((((pdata)->flags) & (_x)) == (_x)) > >> + > >> struct mtk_iommu_domain { > >> struct io_pgtable_cfg cfg; > >> struct io_pgtable_ops *iop; > >> @@ -563,7 +572,8 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > >> upper_32_bits(data->protect_base); > >> writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR); > >> > >> - if (data->enable_4GB && data->plat_data->has_vld_pa_rng) { > >> + if (data->enable_4GB && > >> + MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_VLD_PA_RNG)) { > >> /* > >> * If 4GB mode is enabled, the validate PA range is from > >> * 0x1_0000_0000 to 0x1_ffff_ffff. here record bit[32:30]. > >> @@ -573,7 +583,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > >> } > >> writel_relaxed(0, data->base + REG_MMU_DCM_DIS); > >> > >> - if (data->plat_data->reset_axi) { > >> + if (MTK_IOMMU_HAS_FLAG(data->plat_data, RESET_AXI)) { > >> /* The register is called STANDARD_AXI_MODE in this case */ > >> writel_relaxed(0, data->base + REG_MMU_MISC_CTRL); > >> } > >> @@ -618,7 +628,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > >> > >> /* Whether the current dram is over 4GB */ > >> data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT)); > >> - if (!data->plat_data->has_4gb_mode) > >> + if (!MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) > >> data->enable_4GB = false; > >> > >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> @@ -631,7 +641,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > >> if (data->irq < 0) > >> return data->irq; > >> > >> - if (data->plat_data->has_bclk) { > >> + if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_BCLK)) { > >> data->bclk = devm_clk_get(dev, "bclk"); > >> if (IS_ERR(data->bclk)) > >> return PTR_ERR(data->bclk); > >> @@ -763,23 +773,19 @@ static const struct dev_pm_ops mtk_iommu_pm_ops = { > >> > >> static const struct mtk_iommu_plat_data mt2712_data = { > >> .m4u_plat = M4U_MT2712, > >> - .has_4gb_mode = true, > >> - .has_bclk = true, > >> - .has_vld_pa_rng = true, > >> + .flags = HAS_4GB_MODE | HAS_BCLK | HAS_VLD_PA_RNG, > >> .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, > >> }; > >> > >> static const struct mtk_iommu_plat_data mt8173_data = { > >> .m4u_plat = M4U_MT8173, > >> - .has_4gb_mode = true, > >> - .has_bclk = true, > >> - .reset_axi = true, > >> + .flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI, > >> .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, > >> + .flags = RESET_AXI, > >> .larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1}, > >> }; > >> > >> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > >> index 7212e6fcf982..5225a9170aaa 100644 > >> --- a/drivers/iommu/mtk_iommu.h > >> +++ b/drivers/iommu/mtk_iommu.h > >> @@ -39,12 +39,7 @@ enum mtk_iommu_plat { > >> > >> struct mtk_iommu_plat_data { > >> enum mtk_iommu_plat m4u_plat; > >> - bool has_4gb_mode; > >> - > >> - /* HW will use the EMI clock if there isn't the "bclk". */ > >> - bool has_bclk; > >> - bool has_vld_pa_rng; > >> - bool reset_axi; > >> + u32 flags; > > > > > > How about using bit field instead? eg > > > > u32 has_bclk:1; > > > > In this way, we don't need to change code. > > > > Actually I proposed to use the flag approach because I didn't want to bloat the > mtk_iommu_plat_data structure with new variables for every new feature, being it > a bit field or a bool. > Regards, > Matthias @Yingjoe, If you don't have other concerns, we will use Matthias's proposal, thanks
On 03/07/2020 06:41, Chao Hao wrote: > Some platforms(ex: mt6779) need to improve performance by setting > REG_MMU_WR_LEN_CTRL register. And we can use WR_THROT_EN macro 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. > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > Signed-off-by: Chao Hao <chao.hao@mediatek.com> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/iommu/mtk_iommu.c | 11 +++++++++++ > drivers/iommu/mtk_iommu.h | 1 + > 2 files changed, 12 insertions(+) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 0d96dcd8612b..5c8e141668fc 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -46,6 +46,8 @@ > #define F_MMU_STANDARD_AXI_MODE_MASK (BIT(3) | BIT(19)) > > #define REG_MMU_DCM_DIS 0x050 > +#define REG_MMU_WR_LEN_CTRL 0x054 > +#define F_MMU_WR_THROT_DIS_MASK (BIT(5) | BIT(21)) > > #define REG_MMU_CTRL_REG 0x110 > #define F_MMU_TF_PROT_TO_PROGRAM_ADDR (2 << 4) > @@ -112,6 +114,7 @@ > #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 MTK_IOMMU_HAS_FLAG(pdata, _x) \ > ((((pdata)->flags) & (_x)) == (_x)) > @@ -593,6 +596,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 (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 &= ~F_MMU_WR_THROT_DIS_MASK; > + writel_relaxed(regval, data->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 */ > @@ -747,6 +756,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_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); > @@ -771,6 +781,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_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); > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > index 46d0d47b22e1..31edd05e2eb1 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -31,6 +31,7 @@ struct mtk_iommu_suspend_reg { > u32 int_main_control; > u32 ivrp_paddr; > u32 vld_pa_rng; > + u32 wr_len_ctrl; > }; > > enum mtk_iommu_plat { >
On Fri, Jul 03, 2020 at 12:41:17PM +0800, Chao Hao wrote: > Chao Hao (10): > dt-bindings: mediatek: Add bindings for MT6779 > iommu/mediatek: Rename the register STANDARD_AXI_MODE(0x48) to MISC_CTRL > iommu/mediatek: Use a u32 flags to describe different HW features > iommu/mediatek: Setting 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_CTRL register definition > iommu/mediatek: Extend protect pa alignment value > iommu/mediatek: Modify MMU_CTRL register setting > iommu/mediatek: Add mt6779 basic support Applied, thanks.
On Fri, 2020-07-10 at 16:13 +0200, Joerg Roedel wrote: > On Fri, Jul 03, 2020 at 12:41:17PM +0800, Chao Hao wrote: > > Chao Hao (10): > > dt-bindings: mediatek: Add bindings for MT6779 > > iommu/mediatek: Rename the register STANDARD_AXI_MODE(0x48) to MISC_CTRL > > iommu/mediatek: Use a u32 flags to describe different HW features > > iommu/mediatek: Setting 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_CTRL register definition > > iommu/mediatek: Extend protect pa alignment value > > iommu/mediatek: Modify MMU_CTRL register setting > > iommu/mediatek: Add mt6779 basic support > > Applied, thanks. Hi Joerg, Thanks for the apply. The SMI part always go with the IOMMU, Could you also help apply the mt6779 SMI basical part [1][2]. Both has already got reviewed-by from Rob and Matthias. and the [3] in that patchset is for performance improvement, it's not so necessary, it can be send in another patchset. [1] https://lore.kernel.org/patchwork/patch/1176833/ [2] https://lore.kernel.org/patchwork/patch/1176831/ [3] https://lore.kernel.org/patchwork/patch/1176832/
On Sat, Jul 11, 2020 at 03:11:33PM +0800, Yong Wu wrote: > The SMI part always go with the IOMMU, Could you also help apply the > mt6779 SMI basical part [1][2]. Both has already got reviewed-by from > Rob and Matthias. and the [3] in that patchset is for performance > improvement, it's not so necessary, it can be send in another patchset. > > > [1] https://lore.kernel.org/patchwork/patch/1176833/ > [2] https://lore.kernel.org/patchwork/patch/1176831/ Okay, applied these two to the arm/mediatek branch. Joerg
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: v6: 1. Fix build error for "PATCH v5 02/10". 2. Use more precise definitions and commit messages. v5: 1. Split "iommu/mediatek: Add mt6779 IOMMU basic support(patch v4)" to three patches(from PATCH v5 08/10 to PATCH v5 10/10). 2. Use macro definitions to replace bool values in mtk_iommu_plat_data structure http://lists.infradead.org/pipermail/linux-mediatek/2020-June/013586.html 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. https://lkml.org/lkml/2020/6/16/1741 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 (10): dt-bindings: mediatek: Add bindings for MT6779 iommu/mediatek: Rename the register STANDARD_AXI_MODE(0x48) to MISC_CTRL iommu/mediatek: Use a u32 flags to describe different HW features iommu/mediatek: Setting 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_CTRL register definition iommu/mediatek: Extend protect pa alignment value iommu/mediatek: Modify MMU_CTRL register setting iommu/mediatek: Add mt6779 basic support .../bindings/iommu/mediatek,iommu.txt | 2 + drivers/iommu/mtk_iommu.c | 110 +++++++--- drivers/iommu/mtk_iommu.h | 20 +- include/dt-bindings/memory/mt6779-larb-port.h | 206 ++++++++++++++++++ 4 files changed, 299 insertions(+), 39 deletions(-) -- 2.18.0