mbox series

[v6,00/10] MT6779 IOMMU SUPPORT

Message ID 20200703044127.27438-1-chao.hao@mediatek.com
Headers show
Series MT6779 IOMMU SUPPORT | expand

Message

Chao Hao July 3, 2020, 4:41 a.m. UTC
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

Comments

Yingjoe Chen July 4, 2020, 1:16 a.m. UTC | #1
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
Matthias Brugger July 6, 2020, 3:17 p.m. UTC | #2
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
Matthias Brugger July 6, 2020, 3:18 p.m. UTC | #3
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)) {
>
Matthias Brugger July 6, 2020, 3:20 p.m. UTC | #4
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;
>
Matthias Brugger July 6, 2020, 3:22 p.m. UTC | #5
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 |
>
Chao Hao July 8, 2020, 10:44 a.m. UTC | #6
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
Matthias Brugger July 10, 2020, 1:49 p.m. UTC | #7
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 {
>
Joerg Roedel July 10, 2020, 2:13 p.m. UTC | #8
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.
Yong Wu July 11, 2020, 7:11 a.m. UTC | #9
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/
Joerg Roedel July 13, 2020, 1:29 p.m. UTC | #10
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