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 <chao....@mediatek.com>
> > ---
> > 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];
> >
