Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support
On Fri, 2018-12-21 at 19:31 +0100, Matthias Brugger wrote: > > On 08/12/2018 09:39, Yong Wu wrote: > > The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use > > the ARM Short-descriptor like mt8173, and most of the HW registers > > are the same. > > > > Here list main differences between mt8183 and mt8173/mt2712: > > 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two. > > 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead. > > 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB > > mode". > > 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent > > the bit[33:32] in the physical address of the pgtable base, But the > > standard ttbr0[1] means the S bit which is enabled defaultly, Hence, > > we add a mask. > > 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support. > > 6) the larb-id in smi-common is remapped. M4U should enable > > larbid_remapped support. > > > > Signed-off-by: Yong Wu > > --- [...] > > +static const struct mtk_iommu_plat_data mt8183_data = { > > + .m4u_plat= M4U_MT8183, > > + .larbid_remap_enable = true, > > + .larbid_remapped = {0, 4, 5, 6, 7, 2, 3, 1}, > > Aren't we reinventing the wheel here? > Why can't we use larb-id to get the correct id insteaf of providing another > data > structure for the remapping? Sorry, The remapping id is arbitrary, there is no rule to get it from the larb-id. >From Nicolas's comment, I plan to delete "larbid_remap_enable" and only use "larbid_remap". The other SoCs use the linear mapping here. In addition, I have to apologize that here will may be improved for mt2712. There are 2 smi-common(smi-common0 and smi-common1) in mt2712, actually the remapping relationship for smi-common1 is also different. If it is really needed, I plan to change it from "larbid_remap" to "larbid_remap[2]" which 0 is for smi-common0 and 1 is for smi-common1. Of course, it doesn't affect the iommu functions and only prints the error log when IOMMU translation fault. > > Regards, > Matthias [...] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 03/18] memory: mtk-smi: Use a general config_port interface
On Fri, 2018-12-21 at 18:47 +0100, Matthias Brugger wrote: > > On 08/12/2018 09:39, Yong Wu wrote: > > The config_port of mt2712 and mt8183 are the same. Use a general > > config_port interface instead. > > > > In addition, in mt2712, larb8 and larb9 are the bdpsys larbs which > > are not the normal larb, their register space are different from the > > normal one. thus, we can not call the general config_port. In mt8183, > > IPU0/1 and CCU connect with smi-common directly, they also are not > > the normal larb. Hence, we add a "larb_special_mask" for these special > > larbs. > > > > This is also a preparing patch for adding mt8183 SMI support. > > > > Signed-off-by: Yong Wu > > --- > > drivers/memory/mtk-smi.c | 12 +--- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > > index 8f2d152..3b9ad0e 100644 > > --- a/drivers/memory/mtk-smi.c > > +++ b/drivers/memory/mtk-smi.c > > @@ -53,6 +53,7 @@ struct mtk_smi_larb_gen { > > bool need_larbid; > > int port_in_larb[MTK_LARB_NR_MAX + 1]; > > void (*config_port)(struct device *); > > + unsigned int larb_special_mask; /* The special larbs mask. */ > > I'm not really happy with the name larb_special_mask but I can't think of > anything else. The comment is not needed as it just rewords the name of the > variable. Thanks this comment. then I reword more detail instead this common "special", How about "larb_to_common_directly_mask"? > > Other then that (or even without changing anything): > > Reviewed-by: Matthias Brugger Thanks. > > > }; > > > > struct mtk_smi { > > @@ -176,17 +177,13 @@ void mtk_smi_larb_put(struct device *larbdev) > > return -ENODEV; > > } > > > > -static void mtk_smi_larb_config_port_mt2712(struct device *dev) > > +static void mtk_smi_larb_config_port_gen2_general(struct device *dev) > > { > > struct mtk_smi_larb *larb = dev_get_drvdata(dev); > > u32 reg; > > int i; > > > > - /* > > -* larb 8/9 is the bdpsys larb, the iommu_en is enabled defaultly. > > -* Don't need to set it again. > > -*/ > > - if (larb->larbid == 8 || larb->larbid == 9) > > + if (BIT(larb->larbid) & larb->larb_gen->larb_special_mask) > > return; > > > > for_each_set_bit(i, (unsigned long *)larb->mmu, 32) { > > @@ -261,7 +258,8 @@ static void mtk_smi_larb_config_port_gen1(struct device > > *dev) > > > > static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = { > > .need_larbid = true, > > - .config_port = mtk_smi_larb_config_port_mt2712, > > + .config_port = mtk_smi_larb_config_port_gen2_general, > > + .larb_special_mask = BIT(8) | BIT(9), /* bdpsys */ > > }; > > > > static const struct of_device_id mtk_smi_larb_of_ids[] = { > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support
On Sat, 2018-12-22 at 08:31 +0800, Nicolas Boichat wrote: > On Fri, Dec 21, 2018 at 4:02 PM Yong Wu wrote: > > > > On Fri, 2018-12-21 at 12:43 +0800, Nicolas Boichat wrote: > > > On Sat, Dec 8, 2018 at 4:42 PM Yong Wu wrote: > > > > > > > > The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use > > > > the ARM Short-descriptor like mt8173, and most of the HW registers > > > > are the same. > > > > > > > > Here list main differences between mt8183 and mt8173/mt2712: > > > > 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two. > > > > 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead. > > > > 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB > > > > mode". > > > > 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent > > > > the bit[33:32] in the physical address of the pgtable base, But the > > > > standard ttbr0[1] means the S bit which is enabled defaultly, Hence, > > > > we add a mask. > > > > 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support. > > > > 6) the larb-id in smi-common is remapped. M4U should enable > > > > larbid_remapped support. > > > > > > > > Signed-off-by: Yong Wu > > > > --- > > > > drivers/iommu/mtk_iommu.c | 31 ++- > > > > drivers/iommu/mtk_iommu.h | 1 + > > > > drivers/memory/mtk-smi.c | 19 +++ > > > > 3 files changed, 42 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > > > index 8ab3b69..d91a554 100644 > > > > --- a/drivers/iommu/mtk_iommu.c > > > > +++ b/drivers/iommu/mtk_iommu.c > > > > @@ -36,6 +36,7 @@ > > > > #include "mtk_iommu.h" > > > > > > > > #define REG_MMU_PT_BASE_ADDR 0x000 > > > > +#define MMU_PT_ADDR_MASK GENMASK(31, 7) > > > > > > > > #define REG_MMU_INVALIDATE 0x020 > > > > #define F_ALL_INVLD0x2 > > > > @@ -54,7 +55,7 @@ > > > > #define REG_MMU_CTRL_REG 0x110 > > > > #define F_MMU_PREFETCH_RT_REPLACE_MOD BIT(4) > > > > #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \ > > > > - ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5) > > > > + ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4) > > > > > > Should the shift value be a member of plat_data instead? > > > > It's also ok. > > This TF_PROTECT_SEL MACRO looks a bit complex. I can use a new patch to > > refactor it. > > SGTM. > > > > > > > > /* It's named by F_MMU_TF_PROT_SEL in mt2712. */ > > > > #define F_MMU_TF_PROTECT_SEL(prot, data) \ > > > > (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data)) > > > > @@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct > > > > iommu_domain *domain, > > > > /* Update the pgtable base address register of the M4U HW */ > > > > if (!data->m4u_dom) { > > > > data->m4u_dom = dom; > > > > - writel(dom->cfg.arm_v7s_cfg.ttbr[0], > > > > + writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK, > > > >data->base + REG_MMU_PT_BASE_ADDR); > > > > } > > > > > > > > @@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, > > > > struct of_phandle_args *args) > > > > > > > > static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > > > > { > > > > + enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat; > > > > u32 regval; > > > > int ret; > > > > > > > > @@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct > > > > mtk_iommu_data *data) > > > > } > > > > > > > > regval = F_MMU_TF_PROTECT_SEL(2, data); > > > > - if (data->plat_data->m4u_plat == M4U_MT8173) > > > > + if (m4u_plat == M4U_MT8173) > > > > regval |= F_MMU_PREFETCH_RT_REPLACE_MOD; > > > > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG); > > > > > > > > @@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct > > > > mtk_iommu_data *data) > > > > F_INT_PRETETCH_TRANSATION_FIFO_FAULT; > > > > writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL); > > > > > > > > - if (data->plat_data->m4u_plat == M4U_MT8173) > > > > + if (m4u_plat == M4U_MT8173) > > > > 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 && data->plat_data->m4u_plat != > > > > M4U_MT8173) { > > > > + if (data->enable_4GB && m4u_plat == M4U_MT2712) { > > > > /* > > > > * If 4GB mode is enabled, the validate PA range is from > > > > * 0x1__ to 0x1__. here record
Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support
On Fri, Dec 21, 2018 at 4:02 PM Yong Wu wrote: > > On Fri, 2018-12-21 at 12:43 +0800, Nicolas Boichat wrote: > > On Sat, Dec 8, 2018 at 4:42 PM Yong Wu wrote: > > > > > > The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use > > > the ARM Short-descriptor like mt8173, and most of the HW registers > > > are the same. > > > > > > Here list main differences between mt8183 and mt8173/mt2712: > > > 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two. > > > 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead. > > > 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB > > > mode". > > > 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent > > > the bit[33:32] in the physical address of the pgtable base, But the > > > standard ttbr0[1] means the S bit which is enabled defaultly, Hence, > > > we add a mask. > > > 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support. > > > 6) the larb-id in smi-common is remapped. M4U should enable > > > larbid_remapped support. > > > > > > Signed-off-by: Yong Wu > > > --- > > > drivers/iommu/mtk_iommu.c | 31 ++- > > > drivers/iommu/mtk_iommu.h | 1 + > > > drivers/memory/mtk-smi.c | 19 +++ > > > 3 files changed, 42 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > > index 8ab3b69..d91a554 100644 > > > --- a/drivers/iommu/mtk_iommu.c > > > +++ b/drivers/iommu/mtk_iommu.c > > > @@ -36,6 +36,7 @@ > > > #include "mtk_iommu.h" > > > > > > #define REG_MMU_PT_BASE_ADDR 0x000 > > > +#define MMU_PT_ADDR_MASK GENMASK(31, 7) > > > > > > #define REG_MMU_INVALIDATE 0x020 > > > #define F_ALL_INVLD0x2 > > > @@ -54,7 +55,7 @@ > > > #define REG_MMU_CTRL_REG 0x110 > > > #define F_MMU_PREFETCH_RT_REPLACE_MOD BIT(4) > > > #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \ > > > - ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5) > > > + ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4) > > > > Should the shift value be a member of plat_data instead? > > It's also ok. > This TF_PROTECT_SEL MACRO looks a bit complex. I can use a new patch to > refactor it. SGTM. > > > > > /* It's named by F_MMU_TF_PROT_SEL in mt2712. */ > > > #define F_MMU_TF_PROTECT_SEL(prot, data) \ > > > (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data)) > > > @@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct > > > iommu_domain *domain, > > > /* Update the pgtable base address register of the M4U HW */ > > > if (!data->m4u_dom) { > > > data->m4u_dom = dom; > > > - writel(dom->cfg.arm_v7s_cfg.ttbr[0], > > > + writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK, > > >data->base + REG_MMU_PT_BASE_ADDR); > > > } > > > > > > @@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, > > > struct of_phandle_args *args) > > > > > > static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > > > { > > > + enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat; > > > u32 regval; > > > int ret; > > > > > > @@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct > > > mtk_iommu_data *data) > > > } > > > > > > regval = F_MMU_TF_PROTECT_SEL(2, data); > > > - if (data->plat_data->m4u_plat == M4U_MT8173) > > > + if (m4u_plat == M4U_MT8173) > > > regval |= F_MMU_PREFETCH_RT_REPLACE_MOD; > > > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG); > > > > > > @@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct > > > mtk_iommu_data *data) > > > F_INT_PRETETCH_TRANSATION_FIFO_FAULT; > > > writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL); > > > > > > - if (data->plat_data->m4u_plat == M4U_MT8173) > > > + if (m4u_plat == M4U_MT8173) > > > 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 && data->plat_data->m4u_plat != M4U_MT8173) { > > > + if (data->enable_4GB && m4u_plat == M4U_MT2712) { > > > /* > > > * If 4GB mode is enabled, the validate PA range is from > > > * 0x1__ to 0x1__. here record bit[32:30]. > > > @@ -558,8 +560,11 @@ static int mtk_iommu_hw_init(const struct > > > mtk_iommu_data *data) > > > } > > > writel_relaxed(0, data->base + REG_MMU_DCM_DIS); > > > > > > - /* It's MISC control register whose default value is ok except > > >
Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support
On 08/12/2018 09:39, Yong Wu wrote: > The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use > the ARM Short-descriptor like mt8173, and most of the HW registers > are the same. > > Here list main differences between mt8183 and mt8173/mt2712: > 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two. > 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead. > 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB > mode". > 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent > the bit[33:32] in the physical address of the pgtable base, But the > standard ttbr0[1] means the S bit which is enabled defaultly, Hence, > we add a mask. > 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support. > 6) the larb-id in smi-common is remapped. M4U should enable > larbid_remapped support. > > Signed-off-by: Yong Wu > --- > drivers/iommu/mtk_iommu.c | 31 ++- > drivers/iommu/mtk_iommu.h | 1 + > drivers/memory/mtk-smi.c | 19 +++ > 3 files changed, 42 insertions(+), 9 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 8ab3b69..d91a554 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -36,6 +36,7 @@ > #include "mtk_iommu.h" > > #define REG_MMU_PT_BASE_ADDR 0x000 > +#define MMU_PT_ADDR_MASK GENMASK(31, 7) > > #define REG_MMU_INVALIDATE 0x020 > #define F_ALL_INVLD 0x2 > @@ -54,7 +55,7 @@ > #define REG_MMU_CTRL_REG 0x110 > #define F_MMU_PREFETCH_RT_REPLACE_MODBIT(4) > #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \ > - ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5) > + ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4) > /* It's named by F_MMU_TF_PROT_SEL in mt2712. */ > #define F_MMU_TF_PROTECT_SEL(prot, data) \ > (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data)) > @@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct iommu_domain > *domain, > /* Update the pgtable base address register of the M4U HW */ > if (!data->m4u_dom) { > data->m4u_dom = dom; > - writel(dom->cfg.arm_v7s_cfg.ttbr[0], > + writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK, > data->base + REG_MMU_PT_BASE_ADDR); > } > > @@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, struct > of_phandle_args *args) > > static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > { > + enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat; > u32 regval; > int ret; > > @@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data > *data) > } > > regval = F_MMU_TF_PROTECT_SEL(2, data); > - if (data->plat_data->m4u_plat == M4U_MT8173) > + if (m4u_plat == M4U_MT8173) > regval |= F_MMU_PREFETCH_RT_REPLACE_MOD; > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG); > > @@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct > mtk_iommu_data *data) > F_INT_PRETETCH_TRANSATION_FIFO_FAULT; > writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL); > > - if (data->plat_data->m4u_plat == M4U_MT8173) > + if (m4u_plat == M4U_MT8173) > 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 && data->plat_data->m4u_plat != M4U_MT8173) { > + if (data->enable_4GB && m4u_plat == M4U_MT2712) { > /* >* If 4GB mode is enabled, the validate PA range is from >* 0x1__ to 0x1__. here record bit[32:30]. > @@ -558,8 +560,11 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data > *data) > } > writel_relaxed(0, data->base + REG_MMU_DCM_DIS); > > - /* It's MISC control register whose default value is ok except mt8173.*/ > - if (data->plat_data->m4u_plat == M4U_MT8173) > + /* > + * It's MISC control register whose default value is ok > + * except mt8173 and mt8183. > + */ > + if (m4u_plat == M4U_MT8173 || m4u_plat == M4U_MT8183) > writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE); > > if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0, > @@ -713,6 +718,7 @@ static int __maybe_unused mtk_iommu_resume(struct device > *dev) > { > struct mtk_iommu_data *data = dev_get_drvdata(dev); > struct mtk_iommu_suspend_reg *reg = >reg; > + struct mtk_iommu_domain *m4u_dom = data->m4u_dom; > void __iomem *base = data->base; > int ret; > > @@ -728,8 +734,8 @@ static int __maybe_unused
Re: [PATCH v4 04/18] memory: mtk-smi: Use a struct for the platform data for smi-common
On 08/12/2018 09:39, Yong Wu wrote: > Use a struct as the platform special data instead of the enumeration. > > Also there is a minor change that moving the position of > "enum mtk_smi_gen" definition, this is because we expect define > "struct mtk_smi_common_plat" before it is referred. > > This is a preparing patch for mt8183. > > Signed-off-by: Yong Wu Reviewed-by: Matthias Brugger > --- > drivers/memory/mtk-smi.c | 35 --- > 1 file changed, 24 insertions(+), 11 deletions(-) > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > index 3b9ad0e..a5ddd42 100644 > --- a/drivers/memory/mtk-smi.c > +++ b/drivers/memory/mtk-smi.c > @@ -49,6 +49,15 @@ > #define SMI_LARB_NONSEC_CON(id) (0x380 + ((id) * 4)) > #define F_MMU_EN BIT(0) > > +enum mtk_smi_gen { > + MTK_SMI_GEN1, > + MTK_SMI_GEN2 > +}; > + > +struct mtk_smi_common_plat { > + enum mtk_smi_gen gen; > +}; > + > struct mtk_smi_larb_gen { > bool need_larbid; > int port_in_larb[MTK_LARB_NR_MAX + 1]; > @@ -61,6 +70,8 @@ struct mtk_smi { > struct clk *clk_apb, *clk_smi; > struct clk *clk_async; /*only needed by mt2701*/ > void __iomem*smi_ao_base; > + > + const struct mtk_smi_common_plat *plat; > }; > > struct mtk_smi_larb { /* larb: local arbiter */ > @@ -72,11 +83,6 @@ struct mtk_smi_larb { /* larb: local arbiter */ > u32 *mmu; > }; > > -enum mtk_smi_gen { > - MTK_SMI_GEN1, > - MTK_SMI_GEN2 > -}; > - > static int mtk_smi_enable(const struct mtk_smi *smi) > { > int ret; > @@ -351,18 +357,26 @@ static int mtk_smi_larb_remove(struct platform_device > *pdev) > } > }; > > +static const struct mtk_smi_common_plat mtk_smi_common_gen1 = { > + .gen = MTK_SMI_GEN1, > +}; > + > +static const struct mtk_smi_common_plat mtk_smi_common_gen2 = { > + .gen = MTK_SMI_GEN2, > +}; > + > static const struct of_device_id mtk_smi_common_of_ids[] = { > { > .compatible = "mediatek,mt8173-smi-common", > - .data = (void *)MTK_SMI_GEN2 > + .data = _smi_common_gen2, > }, > { > .compatible = "mediatek,mt2701-smi-common", > - .data = (void *)MTK_SMI_GEN1 > + .data = _smi_common_gen1, > }, > { > .compatible = "mediatek,mt2712-smi-common", > - .data = (void *)MTK_SMI_GEN2 > + .data = _smi_common_gen2, > }, > {} > }; > @@ -372,13 +386,13 @@ static int mtk_smi_common_probe(struct platform_device > *pdev) > struct device *dev = >dev; > struct mtk_smi *common; > struct resource *res; > - enum mtk_smi_gen smi_gen; > int ret; > > common = devm_kzalloc(dev, sizeof(*common), GFP_KERNEL); > if (!common) > return -ENOMEM; > common->dev = dev; > + common->plat = of_device_get_match_data(dev); > > common->clk_apb = devm_clk_get(dev, "apb"); > if (IS_ERR(common->clk_apb)) > @@ -394,8 +408,7 @@ static int mtk_smi_common_probe(struct platform_device > *pdev) >* clock into emi clock domain, but for mtk smi gen2, there's no smi ao >* base. >*/ > - smi_gen = (enum mtk_smi_gen)of_device_get_match_data(dev); > - if (smi_gen == MTK_SMI_GEN1) { > + if (common->plat->gen == MTK_SMI_GEN1) { > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > common->smi_ao_base = devm_ioremap_resource(dev, res); > if (IS_ERR(common->smi_ao_base)) > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 03/18] memory: mtk-smi: Use a general config_port interface
On 08/12/2018 09:39, Yong Wu wrote: > The config_port of mt2712 and mt8183 are the same. Use a general > config_port interface instead. > > In addition, in mt2712, larb8 and larb9 are the bdpsys larbs which > are not the normal larb, their register space are different from the > normal one. thus, we can not call the general config_port. In mt8183, > IPU0/1 and CCU connect with smi-common directly, they also are not > the normal larb. Hence, we add a "larb_special_mask" for these special > larbs. > > This is also a preparing patch for adding mt8183 SMI support. > > Signed-off-by: Yong Wu > --- > drivers/memory/mtk-smi.c | 12 +--- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > index 8f2d152..3b9ad0e 100644 > --- a/drivers/memory/mtk-smi.c > +++ b/drivers/memory/mtk-smi.c > @@ -53,6 +53,7 @@ struct mtk_smi_larb_gen { > bool need_larbid; > int port_in_larb[MTK_LARB_NR_MAX + 1]; > void (*config_port)(struct device *); > + unsigned int larb_special_mask; /* The special larbs mask. */ I'm not really happy with the name larb_special_mask but I can't think of anything else. The comment is not needed as it just rewords the name of the variable. Other then that (or even without changing anything): Reviewed-by: Matthias Brugger > }; > > struct mtk_smi { > @@ -176,17 +177,13 @@ void mtk_smi_larb_put(struct device *larbdev) > return -ENODEV; > } > > -static void mtk_smi_larb_config_port_mt2712(struct device *dev) > +static void mtk_smi_larb_config_port_gen2_general(struct device *dev) > { > struct mtk_smi_larb *larb = dev_get_drvdata(dev); > u32 reg; > int i; > > - /* > - * larb 8/9 is the bdpsys larb, the iommu_en is enabled defaultly. > - * Don't need to set it again. > - */ > - if (larb->larbid == 8 || larb->larbid == 9) > + if (BIT(larb->larbid) & larb->larb_gen->larb_special_mask) > return; > > for_each_set_bit(i, (unsigned long *)larb->mmu, 32) { > @@ -261,7 +258,8 @@ static void mtk_smi_larb_config_port_gen1(struct device > *dev) > > static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = { > .need_larbid = true, > - .config_port = mtk_smi_larb_config_port_mt2712, > + .config_port = mtk_smi_larb_config_port_gen2_general, > + .larb_special_mask = BIT(8) | BIT(9), /* bdpsys */ > }; > > static const struct of_device_id mtk_smi_larb_of_ids[] = { > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 02/18] iommu/mediatek: Use a struct as the platform data
On 08/12/2018 09:39, Yong Wu wrote: > Use a struct as the platform special data instead of the enumeration. > This is a prepare patch for adding mt8183 iommu support. > > Signed-off-by: Yong Wu > --- Reviewed-by: Matthias Brugger > drivers/iommu/mtk_iommu.c | 24 > drivers/iommu/mtk_iommu.h | 6 +- > 2 files changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 44bd5b9..9a2225b 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -54,7 +54,7 @@ > #define REG_MMU_CTRL_REG 0x110 > #define F_MMU_PREFETCH_RT_REPLACE_MODBIT(4) > #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \ > - ((data)->m4u_plat == M4U_MT2712 ? 4 : 5) > + ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5) > /* It's named by F_MMU_TF_PROT_SEL in mt2712. */ > #define F_MMU_TF_PROTECT_SEL(prot, data) \ > (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data)) > @@ -517,7 +517,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data > *data) > } > > regval = F_MMU_TF_PROTECT_SEL(2, data); > - if (data->m4u_plat == M4U_MT8173) > + if (data->plat_data->m4u_plat == M4U_MT8173) > regval |= F_MMU_PREFETCH_RT_REPLACE_MOD; > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG); > > @@ -538,14 +538,14 @@ static int mtk_iommu_hw_init(const struct > mtk_iommu_data *data) > F_INT_PRETETCH_TRANSATION_FIFO_FAULT; > writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL); > > - if (data->m4u_plat == M4U_MT8173) > + if (data->plat_data->m4u_plat == M4U_MT8173) > 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 && data->m4u_plat != M4U_MT8173) { > + if (data->enable_4GB && data->plat_data->m4u_plat != M4U_MT8173) { > /* >* If 4GB mode is enabled, the validate PA range is from >* 0x1__ to 0x1__. here record bit[32:30]. > @@ -556,7 +556,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data > *data) > writel_relaxed(0, data->base + REG_MMU_DCM_DIS); > > /* It's MISC control register whose default value is ok except mt8173.*/ > - if (data->m4u_plat == M4U_MT8173) > + if (data->plat_data->m4u_plat == M4U_MT8173) > writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE); > > if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0, > @@ -589,7 +589,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > if (!data) > return -ENOMEM; > data->dev = dev; > - data->m4u_plat = (enum mtk_iommu_plat)of_device_get_match_data(dev); > + data->plat_data = of_device_get_match_data(dev); > > /* Protect memory. HW will access here while translation fault.*/ > protect = devm_kzalloc(dev, MTK_PROTECT_PA_ALIGN * 2, GFP_KERNEL); > @@ -733,9 +733,17 @@ static int __maybe_unused mtk_iommu_resume(struct device > *dev) > SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume) > }; > > +static const struct mtk_iommu_plat_data mt2712_data = { > + .m4u_plat = M4U_MT2712, > +}; > + > +static const struct mtk_iommu_plat_data mt8173_data = { > + .m4u_plat = M4U_MT8173, > +}; > + > static const struct of_device_id mtk_iommu_of_ids[] = { > - { .compatible = "mediatek,mt2712-m4u", .data = (void *)M4U_MT2712}, > - { .compatible = "mediatek,mt8173-m4u", .data = (void *)M4U_MT8173}, > + { .compatible = "mediatek,mt2712-m4u", .data = _data}, > + { .compatible = "mediatek,mt8173-m4u", .data = _data}, > {} > }; > > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > index 778498b..333a0ef 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -41,6 +41,10 @@ enum mtk_iommu_plat { > M4U_MT8173, > }; > > +struct mtk_iommu_plat_data { > + enum mtk_iommu_plat m4u_plat; > +}; > + > struct mtk_iommu_domain; > > struct mtk_iommu_data { > @@ -57,7 +61,7 @@ struct mtk_iommu_data { > booltlb_flush_active; > > struct iommu_device iommu; > - enum mtk_iommu_plat m4u_plat; > + const struct mtk_iommu_plat_data *plat_data; > > struct list_headlist; > }; > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: question about broadcast invalidation and atc invalidation
Hi, On 21/12/2018 09:31, Zhongmiao wrote: > Hi Jean: > Now Soft need to make sure build atc inv cmd to smmu after Execute > "TLBI" (dvm sync). I don't think it's very reasonable. Why Smmu The > smmu automatically sends the inv atc command when smmu receive > broadcast invalidation ? I'd also like to see this done by hardware, but it's not straightforward. I see two difficulties with turning broadcast TLBI into ATC invalidation: * TLBI uses VMID and ASID to target the TLB entries to invalidate, but ATS uses RID and PASID. The SMMU would need to do a reverse lookup to convert VMID:ASID into RID:PASID, and there may be multiple RID:PASID pairs associated to one VMID:ASID. * ATS flow control. Each endpoint has a limited number of in-flight ATC invalidate requests. If the CPU is sending TLBI too fast, the SMMU would have to buffer them or back-pressure the interconnect, while waiting for in-flight ATC invalidate to complete. So for the moment the architecture doesn't support this. We use MMU notifiers to convert VMID:ASID into RID:PASID and wait for ATC invalidation to complete. Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [GIT PULL] dma-mapping updates for Linux 4.21
On Fri, Dec 21, 2018 at 10:45:49AM +0100, Christoph Hellwig wrote: > The following changes since commit c9d76d0655c06b8c1f944e46c4fd9e9cf4b331c0: > > dma-mapping: fix return type of dma_set_max_seg_size() (2018-11-27 08:39:52 > +0100) And that actually is the first commit already in the tree, sorry. Updated git-request-pull output attached: The following changes since commit ef78e5ec9214376c5cb989f5da70b02d0c117b66: ia64: export node_distance function (2018-11-26 18:30:40 -0800) are available in the Git repository at: git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-4.21 for you to fetch changes up to 8b1cce9f5832a8eda17d37a3c49fb7dd2d650f46: dma-mapping: fix inverted logic in dma_supported (2018-12-20 17:47:55 +0100) DMA mapping updates for Linux 4.21 A huge update this time, but a lot of that is just consolidating or removing code: - provide a common DMA_MAPPING_ERROR definition and avoid indirect calls for dma_map_* error checking - use direct calls for the DMA direct mapping case, avoiding huge retpoline overhead for high performance workloads - merge the swiotlb dma_map_ops into dma-direct - provide a generic remapping DMA consistent allocator for architectures that have devices that perform DMA that is not cache coherent. Based on the existing arm64 implementation and also used for csky now. - improve the dma-debug infrastructure, including dynamic allocation of entries (Robin Murphy) - default to providing chaining scatterlist everywhere, with opt-outs for the few architectures (alpha, parisc, most arm32 variants) that can't cope with it - misc sparc32 dma-related cleanups - remove the dma_mark_clean arch hook used by swiotlb on ia64 and replace it with the generic noncoherent infrastructure - fix the return type of dma_set_max_seg_size (Niklas Söderlund) - move the dummy dma ops for not DMA capable devices from arm64 to common code (Robin Murphy) - ensure dma_alloc_coherent returns zeroed memory to avoid kernel data leaks through userspace. We already did this for most common architectures, but this ensures we do it everywhere. dma_zalloc_coherent has been deprecated and can hopefully be removed after -rc1 with a coccinelle script. Christoph Hellwig (60): dma-direct: provide page based alloc/free helpers dma-direct: reject highmem pages from dma_alloc_from_contiguous dma-mapping: move the remap helpers to a separate file dma-mapping: move the arm64 noncoherent alloc/free support to common code dma-mapping: support highmem in the generic remap allocator dma-remap: support DMA_ATTR_NO_KERNEL_MAPPING csky: don't select DMA_NONCOHERENT_OPS csky: don't use GFP_DMA in atomic_pool_init csky: use the generic remapping dma alloc implementation dma-mapping: provide a generic DMA_MAPPING_ERROR dma-direct: remove the mapping_error dma_map_ops method arm: remove the mapping_error dma_map_ops method powerpc/iommu: remove the mapping_error dma_map_ops method mips/jazz: remove the mapping_error dma_map_ops method s390: remove the mapping_error dma_map_ops method sparc: remove the mapping_error dma_map_ops method parisc/ccio: remove the mapping_error dma_map_ops method parisc/sba_iommu: remove the mapping_error dma_map_ops method arm64: remove the dummy_dma_ops mapping_error method alpha: remove the mapping_error dma_map_ops method ia64/sba_iommu: improve internal map_page users ia64/sba_iommu: remove the mapping_error dma_map_ops method ia64/sn: remove the mapping_error dma_map_ops method x86/amd_gart: remove the mapping_error dma_map_ops method x86/calgary: remove the mapping_error dma_map_ops method iommu: remove the mapping_error dma_map_ops method iommu/intel: small map_page cleanup iommu/vt-d: remove the mapping_error dma_map_ops method iommu/dma-iommu: remove the mapping_error dma_map_ops method xen-swiotlb: remove the mapping_error dma_map_ops method dma-mapping: remove the mapping_error dma_map_ops method dma-mapping: return an error code from dma_mapping_error arch: switch the default on ARCH_HAS_SG_CHAIN sparc: remove not needed sbus_dma_ops methods sparc: factor the dma coherent mapping into helper sparc: remove the sparc32_dma_ops indirection sparc: remove not required includes from dma-mapping.h sparc: move the leon PCI memory space comment to sparc: merge 32-bit and 64-bit version of pci.h sparc: use DT node full_name in sparc_dma_alloc_resource dma-mapping: remove a pointless memset in dma_atomic_pool_init dma-mapping: simplify the dma_sync_single_range_for_{cpu,device} implementation dma-mapping: merge dma_unmap_page_attrs
[GIT PULL] dma-mapping updates for Linux 4.21
Hi Linus, an early pull request as requested. Besides the usual contextual conflicts in Kconfig files that can be solved by taking both updates and applying them manually there are some real conflicts this time: - the RISC-V tree has some updates to Documentation/features/io/sg-chain/arch-support.txt, which is removed in this tree. It should be put to rest. - the sparc64 tree has various updates to replace the name field with the full_name one in the OF device, including in code that gets refctored in the dma-mapping tree. I've applied an equivalent change to the refactored version, so take the dma-mapping tree version of arch/sparc/kernel/ioport.c. The following changes since commit c9d76d0655c06b8c1f944e46c4fd9e9cf4b331c0: dma-mapping: fix return type of dma_set_max_seg_size() (2018-11-27 08:39:52 +0100) are available in the Git repository at: git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-4.21 for you to fetch changes up to 8b1cce9f5832a8eda17d37a3c49fb7dd2d650f46: dma-mapping: fix inverted logic in dma_supported (2018-12-20 17:47:55 +0100) DMA mapping updates for Linux 4.21 A huge update this time, but a lot of that is just consolidating or removing code: - provide a common DMA_MAPPING_ERROR definition and avoid indirect calls for dma_map_* error checking - use direct calls for the DMA direct mapping case, avoiding huge retpoline overhead for high performance workloads - merge the swiotlb dma_map_ops into dma-direct - provide a generic remapping DMA consistent allocator for architectures that have devices that perform DMA that is not cache coherent. Based on the existing arm64 implementation and also used for csky now. - improve the dma-debug infrastructure, including dynamic allocation of entries (Robin Murphy) - default to providing chaining scatterlist everywhere, with opt-outs for the few architectures (alpha, parisc, most arm32 variants) that can't cope with it - misc sparc32 dma-related cleanups - remove the dma_mark_clean arch hook used by swiotlb on ia64 and replace it with the generic noncoherent infrastructure - fix the return type of dma_set_max_seg_size (Niklas Söderlund) - move the dummy dma ops for not DMA capable devices from arm64 to common code (Robin Murphy) - ensure dma_alloc_coherent returns zeroed memory to avoid kernel data leaks through userspace. We already did this for most common architectures, but this ensures we do it everywhere. dma_zalloc_coherent has been deprecated and can hopefully be removed after -rc1 with a coccinelle script. Christoph Hellwig (60): dma-direct: provide page based alloc/free helpers dma-direct: reject highmem pages from dma_alloc_from_contiguous dma-mapping: move the remap helpers to a separate file dma-mapping: move the arm64 noncoherent alloc/free support to common code dma-mapping: support highmem in the generic remap allocator dma-remap: support DMA_ATTR_NO_KERNEL_MAPPING csky: don't select DMA_NONCOHERENT_OPS csky: don't use GFP_DMA in atomic_pool_init csky: use the generic remapping dma alloc implementation dma-mapping: provide a generic DMA_MAPPING_ERROR dma-direct: remove the mapping_error dma_map_ops method arm: remove the mapping_error dma_map_ops method powerpc/iommu: remove the mapping_error dma_map_ops method mips/jazz: remove the mapping_error dma_map_ops method s390: remove the mapping_error dma_map_ops method sparc: remove the mapping_error dma_map_ops method parisc/ccio: remove the mapping_error dma_map_ops method parisc/sba_iommu: remove the mapping_error dma_map_ops method arm64: remove the dummy_dma_ops mapping_error method alpha: remove the mapping_error dma_map_ops method ia64/sba_iommu: improve internal map_page users ia64/sba_iommu: remove the mapping_error dma_map_ops method ia64/sn: remove the mapping_error dma_map_ops method x86/amd_gart: remove the mapping_error dma_map_ops method x86/calgary: remove the mapping_error dma_map_ops method iommu: remove the mapping_error dma_map_ops method iommu/intel: small map_page cleanup iommu/vt-d: remove the mapping_error dma_map_ops method iommu/dma-iommu: remove the mapping_error dma_map_ops method xen-swiotlb: remove the mapping_error dma_map_ops method dma-mapping: remove the mapping_error dma_map_ops method dma-mapping: return an error code from dma_mapping_error arch: switch the default on ARCH_HAS_SG_CHAIN sparc: remove not needed sbus_dma_ops methods sparc: factor the dma coherent mapping into helper sparc: remove the sparc32_dma_ops indirection sparc: remove not required includes from dma-mapping.h sparc:
Re: [PATCH v4 08/18] iommu/mediatek: Add larb-id remapped support
Hi Nicolas, Thanks for the review of this patchset. On Fri, 2018-12-21 at 11:35 +0800, Nicolas Boichat wrote: > On Sat, Dec 8, 2018 at 4:42 PM Yong Wu wrote: > > > > The larb-id may be remapped in the smi-common, this means the > > larb-id reported in the mtk_iommu_isr isn't the real larb-id, > > > > Take mt8183 as a example: > >M4U > > | > > - > > | SMI common | > > -0-7-5-6-1-2--3-4- <- Id remapped > > | | | | | | | | > > larb0 larb1 IPU0 IPU1 larb4 larb5 larb6 CCU > > disp vdec img cam venc imgcam > > As above, larb0 connects with the id 0 in smi-common. > > larb1 connects with the id 7 in smi-common. > > ... > > If the larb-id reported in the isr is 7, actually it's larb1(vdec). > > In order to output the right larb-id in the isr, we add a larb-id > > remapping relationship in this patch. > > > > This also is a preparing patch for mt8183. > > > > Signed-off-by: Yong Wu > > --- > > drivers/iommu/mtk_iommu.c | 3 +++ > > drivers/iommu/mtk_iommu.h | 4 > > 2 files changed, 7 insertions(+) > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > index eda062a..8ab3b69 100644 > > --- a/drivers/iommu/mtk_iommu.c > > +++ b/drivers/iommu/mtk_iommu.c > > @@ -220,6 +220,9 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id) > > fault_larb = F_MMU0_INT_ID_LARB_ID(regval); > > fault_port = F_MMU0_INT_ID_PORT_ID(regval); > > > > + if (data->plat_data->larbid_remap_enable) > > + fault_larb = data->plat_data->larbid_remapped[fault_larb]; > > + > > if (report_iommu_fault(>domain, data->dev, fault_iova, > >write ? IOMMU_FAULT_WRITE : > > IOMMU_FAULT_READ)) { > > dev_err_ratelimited( > > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > > index b8749ac..3877050 100644 > > --- a/drivers/iommu/mtk_iommu.h > > +++ b/drivers/iommu/mtk_iommu.h > > @@ -47,6 +47,10 @@ struct mtk_iommu_plat_data { > > > > /* HW will use the EMI clock if there isn't the "bclk". */ > > boolhas_bclk; > > + > > + /* The larb-id may be remapped in the smi-common. */ > > + boollarbid_remap_enable; > > + unsigned intlarbid_remapped[MTK_LARB_NR_MAX]; > > Wouldn't it be a little simpler if you just had > larbid_remap[MTK_LARB_NR_MAX] (no larbid_remap_enable), and just set > it to {0, 1, 2, 3, 4, 5, 6, 7} in platforms that don't need > complicated remapping? Actually I'd like the original way(Print the larb-id from the register directly if larb-id is not remapped). But this solution is also ok for me. > > Also, unsigned char/u8 array would be enough. OK. "unsigned char" is enough. Originally I think "int" or "long" may be better to access in ARM/ARM64. > > > }; > > > > struct mtk_iommu_domain; > > -- > > 1.9.1 > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 13/18] memory: mtk-smi: Add bus_sel for mt8183
On Fri, 2018-12-21 at 12:47 +0800, Nicolas Boichat wrote: > On Sat, Dec 8, 2018 at 4:43 PM Yong Wu wrote: > > > > There are 2 mmu cells in a M4U HW. we could adjust some larbs entering > > mmu0 or mmu1 to balance the bandwidth via the smi-common register > > SMI_BUS_SEL(0x220)(Each larb occupy 2 bits). > > > > In mt8183, For better performance, we switch larb1/2/5/7 to enter > > mmu1 while the others still keep enter mmu0. > > > > In mt8173 and mt2712, we don't get the performance issue, > > Keep its default value(0x0), that means all the larbs enter mmu0. > > > > Signed-off-by: Yong Wu > > --- > > drivers/memory/mtk-smi.c | 22 -- > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > > index ee6165e..88eb61a 100644 > > --- a/drivers/memory/mtk-smi.c > > +++ b/drivers/memory/mtk-smi.c > > @@ -49,6 +49,12 @@ > > #define SMI_LARB_NONSEC_CON(id)(0x380 + ((id) * 4)) > > #define F_MMU_EN BIT(0) > > > > +/* SMI COMMON */ > > +#define SMI_BUS_SEL0x220 > > +#define SMI_BUS_LARB_SHIFT(larbid) ((larbid) << 1) > > +/* All are MMU0 defaultly. Only specialize mmu1 here. */ > > +#define F_MMU1_LARB(larbid)(0x1 << SMI_BUS_LARB_SHIFT(larbid)) > > + > > enum mtk_smi_gen { > > MTK_SMI_GEN1, > > MTK_SMI_GEN2 > > @@ -57,6 +63,7 @@ enum mtk_smi_gen { > > struct mtk_smi_common_plat { > > enum mtk_smi_gen gen; > > bool has_gals; > > + u32 bus_sel; /* Balance some larbs to enter mmu0 or > > mmu1 */ > > }; > > > > struct mtk_smi_larb_gen { > > @@ -72,8 +79,8 @@ struct mtk_smi { > > struct clk *clk_apb, *clk_smi; > > struct clk *clk_gals0, *clk_gals1; > > struct clk *clk_async; /*only needed by > > mt2701*/ > > - void __iomem*smi_ao_base; > > - > > + void __iomem*smi_ao_base; /* only for gen1 */ > > + void __iomem*base;/* only for gen2 */ > > const struct mtk_smi_common_plat *plat; > > }; > > > > @@ -409,6 +416,8 @@ static int __maybe_unused mtk_smi_larb_suspend(struct > > device *dev) > > static const struct mtk_smi_common_plat mtk_smi_common_mt8183 = { > > .gen = MTK_SMI_GEN2, > > .has_gals = true, > > + .bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(5) | > > + F_MMU1_LARB(7), > > Maybe it's ok for now, but I wonder if this is something that should > be specified in device tree? Maybe different applications will want > different larb split between MMU0 and MMU1? Good question. Many Thanks. This value is recommended from the HW DE. In some SoCs, It may be different in different scenarios. and in some SoCs like mt8183, the bus_sel always use a fixed value. I guess it should not be in device tree if it may be changed in different applications. BTW, Is there some existed IOCTL command or the similar interface that can tell the kernel driver(SMI here) which scenario it is currently, like it is playing video or the camera is running. Currently the SMI driver is so simple, it doesn't know which application it is. All use the HW default value. of course, the default also is ok, only the bandwidth may be not so good. For the bus_sel of mt8183, I think it is ok right now. Sure, it can be improved if we can change it dynamically. > > > }; > > > > static const struct of_device_id mtk_smi_common_of_ids[] = { > > @@ -481,6 +490,11 @@ static int mtk_smi_common_probe(struct platform_device > > *pdev) > > ret = clk_prepare_enable(common->clk_async); > > if (ret) > > return ret; > > + } else { > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + common->base = devm_ioremap_resource(dev, res); > > + if (IS_ERR(common->base)) > > + return PTR_ERR(common->base); > > } > > pm_runtime_enable(dev); > > platform_set_drvdata(pdev, common); > > @@ -496,6 +510,7 @@ static int mtk_smi_common_remove(struct platform_device > > *pdev) > > static int __maybe_unused mtk_smi_common_resume(struct device *dev) > > { > > struct mtk_smi *common = dev_get_drvdata(dev); > > + u32 bus_sel = common->plat->bus_sel; > > int ret; > > > > ret = mtk_smi_clk_enable(common); > > @@ -503,6 +518,9 @@ static int __maybe_unused mtk_smi_common_resume(struct > > device *dev) > > dev_err(common->dev, "Failed to enable clock(%d).\n", ret); > > return ret; > > } > > + > > + if (common->plat->gen == MTK_SMI_GEN2 && bus_sel) > > + writel(bus_sel, common->base + SMI_BUS_SEL); > > return 0; > > } > > > > -- > > 1.9.1 > >
Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support
On Fri, 2018-12-21 at 12:43 +0800, Nicolas Boichat wrote: > On Sat, Dec 8, 2018 at 4:42 PM Yong Wu wrote: > > > > The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use > > the ARM Short-descriptor like mt8173, and most of the HW registers > > are the same. > > > > Here list main differences between mt8183 and mt8173/mt2712: > > 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two. > > 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead. > > 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB > > mode". > > 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent > > the bit[33:32] in the physical address of the pgtable base, But the > > standard ttbr0[1] means the S bit which is enabled defaultly, Hence, > > we add a mask. > > 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support. > > 6) the larb-id in smi-common is remapped. M4U should enable > > larbid_remapped support. > > > > Signed-off-by: Yong Wu > > --- > > drivers/iommu/mtk_iommu.c | 31 ++- > > drivers/iommu/mtk_iommu.h | 1 + > > drivers/memory/mtk-smi.c | 19 +++ > > 3 files changed, 42 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > index 8ab3b69..d91a554 100644 > > --- a/drivers/iommu/mtk_iommu.c > > +++ b/drivers/iommu/mtk_iommu.c > > @@ -36,6 +36,7 @@ > > #include "mtk_iommu.h" > > > > #define REG_MMU_PT_BASE_ADDR 0x000 > > +#define MMU_PT_ADDR_MASK GENMASK(31, 7) > > > > #define REG_MMU_INVALIDATE 0x020 > > #define F_ALL_INVLD0x2 > > @@ -54,7 +55,7 @@ > > #define REG_MMU_CTRL_REG 0x110 > > #define F_MMU_PREFETCH_RT_REPLACE_MOD BIT(4) > > #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \ > > - ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5) > > + ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4) > > Should the shift value be a member of plat_data instead? It's also ok. This TF_PROTECT_SEL MACRO looks a bit complex. I can use a new patch to refactor it. > > > /* It's named by F_MMU_TF_PROT_SEL in mt2712. */ > > #define F_MMU_TF_PROTECT_SEL(prot, data) \ > > (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data)) > > @@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct iommu_domain > > *domain, > > /* Update the pgtable base address register of the M4U HW */ > > if (!data->m4u_dom) { > > data->m4u_dom = dom; > > - writel(dom->cfg.arm_v7s_cfg.ttbr[0], > > + writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK, > >data->base + REG_MMU_PT_BASE_ADDR); > > } > > > > @@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, > > struct of_phandle_args *args) > > > > static int mtk_iommu_hw_init(const struct mtk_iommu_data *data) > > { > > + enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat; > > u32 regval; > > int ret; > > > > @@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct > > mtk_iommu_data *data) > > } > > > > regval = F_MMU_TF_PROTECT_SEL(2, data); > > - if (data->plat_data->m4u_plat == M4U_MT8173) > > + if (m4u_plat == M4U_MT8173) > > regval |= F_MMU_PREFETCH_RT_REPLACE_MOD; > > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG); > > > > @@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct > > mtk_iommu_data *data) > > F_INT_PRETETCH_TRANSATION_FIFO_FAULT; > > writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL); > > > > - if (data->plat_data->m4u_plat == M4U_MT8173) > > + if (m4u_plat == M4U_MT8173) > > 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 && data->plat_data->m4u_plat != M4U_MT8173) { > > + if (data->enable_4GB && m4u_plat == M4U_MT2712) { > > /* > > * If 4GB mode is enabled, the validate PA range is from > > * 0x1__ to 0x1__. here record bit[32:30]. > > @@ -558,8 +560,11 @@ static int mtk_iommu_hw_init(const struct > > mtk_iommu_data *data) > > } > > writel_relaxed(0, data->base + REG_MMU_DCM_DIS); > > > > - /* It's MISC control register whose default value is ok except > > mt8173.*/ > > - if (data->plat_data->m4u_plat == M4U_MT8173) > > + /* > > +* It's MISC control register whose default value is ok > > +* except mt8173 and mt8183. > > +*/ > > + if (m4u_plat == M4U_MT8173 || m4u_plat == M4U_MT8183) > >