[PATCH v3 04/11] iommu/mediatek: Get regionid from larb/port id

Yong Wu posted 11 patches 1 year, 7 months ago
There is a newer version of this series
[PATCH v3 04/11] iommu/mediatek: Get regionid from larb/port id
Posted by Yong Wu 1 year, 7 months ago
After commit f1ad5338a4d5 ("of: Fix "dma-ranges" handling for bus
controllers"), the dma-ranges is not allowed for dts leaf node.
but we still would like to separate to different masters
into different iova regions.

Thus we have to separate it by the HW larbid and portid. For example,
larb1/2 are in region2 and larb3 is in region3. The problem is that
some ports inside a larb are in region4 while some ports inside this
larb are in region5. Therefore I define a "iova_region_larb_msk" to help
record the information for each a port. Take a example for a larb:
 [1] = ~0: means all ports in this larb are in region1;
 [2] = BIT(3) | BIT(4): means port3/4 in this larb are region2;
 [3] = ~(BIT(3) | BIT(4)): means all the other ports except port3/4
                           in this larb are region3.

This method also avoids the users forget/abuse the iova regions.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/iommu/mtk_iommu.c | 47 ++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index ab53edcb221f..7e2cb3b8cac8 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -8,7 +8,6 @@
 #include <linux/clk.h>
 #include <linux/component.h>
 #include <linux/device.h>
-#include <linux/dma-direct.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -212,6 +211,11 @@ struct mtk_iommu_plat_data {
 	struct {
 		unsigned int	iova_region_nr;
 		const struct mtk_iommu_iova_region	*iova_region;
+		/*
+		 * Indicate the correspondence between larbs/ports and regions.
+		 * The index is same with iova_region.
+		 */
+		const u32	(*iova_region_larb_msk)[MTK_LARB_NR_MAX];
 	};
 
 	/*
@@ -529,30 +533,33 @@ static unsigned int mtk_iommu_get_bank_id(struct device *dev,
 static int mtk_iommu_get_iova_region_id(struct device *dev,
 					const struct mtk_iommu_plat_data *plat_data)
 {
-	const struct mtk_iommu_iova_region *rgn = plat_data->iova_region;
-	const struct bus_dma_region *dma_rgn = dev->dma_range_map;
-	int i, candidate = -1;
-	dma_addr_t dma_end;
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	unsigned int portidmsk = 0, larbid;
+	const u32 *rgn_larb_msk;
+	int i, region_id = -1;
 
-	if (!dma_rgn || plat_data->iova_region_nr == 1)
+	if (plat_data->iova_region_nr == 1)
 		return 0;
 
-	dma_end = dma_rgn->dma_start + dma_rgn->size - 1;
-	for (i = 0; i < plat_data->iova_region_nr; i++, rgn++) {
-		/* Best fit. */
-		if (dma_rgn->dma_start == rgn->iova_base &&
-		    dma_end == rgn->iova_base + rgn->size - 1)
-			return i;
-		/* ok if it is inside this region. */
-		if (dma_rgn->dma_start >= rgn->iova_base &&
-		    dma_end < rgn->iova_base + rgn->size)
-			candidate = i;
+	larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);
+	for (i = 0; i < fwspec->num_ids; i++)
+		portidmsk |= BIT(MTK_M4U_TO_PORT(fwspec->ids[i]));
+
+	for (i = 0; i < plat_data->iova_region_nr; i++) {
+		rgn_larb_msk = plat_data->iova_region_larb_msk[i];
+		if (!rgn_larb_msk)
+			continue;
+
+		if  ((rgn_larb_msk[larbid] & portidmsk) == portidmsk) {
+			region_id = i;
+			break;
+		}
 	}
 
-	if (candidate >= 0)
-		return candidate;
-	dev_err(dev, "Can NOT find the iommu domain id(%pad 0x%llx).\n",
-		&dma_rgn->dma_start, dma_rgn->size);
+	if (region_id >= 0)
+		return region_id;
+	dev_err(dev, "Can NOT find the region for larb(%d-%x).\n",
+		larbid, portidmsk);
 	return -EINVAL;
 }
 
-- 
2.18.0
Re: [PATCH v3 04/11] iommu/mediatek: Get regionid from larb/port id
Posted by AngeloGioacchino Del Regno 1 year, 7 months ago
Il 14/02/23 04:11, Yong Wu ha scritto:
> After commit f1ad5338a4d5 ("of: Fix "dma-ranges" handling for bus
> controllers"), the dma-ranges is not allowed for dts leaf node.
> but we still would like to separate to different masters
> into different iova regions.
> 
> Thus we have to separate it by the HW larbid and portid. For example,
> larb1/2 are in region2 and larb3 is in region3. The problem is that
> some ports inside a larb are in region4 while some ports inside this
> larb are in region5. Therefore I define a "iova_region_larb_msk" to help
> record the information for each a port. Take a example for a larb:
>   [1] = ~0: means all ports in this larb are in region1;
>   [2] = BIT(3) | BIT(4): means port3/4 in this larb are region2;
>   [3] = ~(BIT(3) | BIT(4)): means all the other ports except port3/4
>                             in this larb are region3.
> 
> This method also avoids the users forget/abuse the iova regions.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>   drivers/iommu/mtk_iommu.c | 47 ++++++++++++++++++++++-----------------
>   1 file changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index ab53edcb221f..7e2cb3b8cac8 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -8,7 +8,6 @@
>   #include <linux/clk.h>
>   #include <linux/component.h>
>   #include <linux/device.h>
> -#include <linux/dma-direct.h>
>   #include <linux/err.h>
>   #include <linux/interrupt.h>
>   #include <linux/io.h>
> @@ -212,6 +211,11 @@ struct mtk_iommu_plat_data {
>   	struct {
>   		unsigned int	iova_region_nr;
>   		const struct mtk_iommu_iova_region	*iova_region;
> +		/*
> +		 * Indicate the correspondence between larbs/ports and regions.
> +		 * The index is same with iova_region.

* The index is the same as iova_region and larb port numbers are
* described as bit positions.
* For example, storing BIT(0) at index 2 means "port 0 is in region 2".

That's most probably the best short explanation that we can give, so that nobody
goes crazy with understanding this one.

After fixing that comment, you totally deserve my

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Cheers!
Re: [PATCH v3 04/11] iommu/mediatek: Get regionid from larb/port id
Posted by AngeloGioacchino Del Regno 1 year, 7 months ago
Il 14/02/23 10:07, AngeloGioacchino Del Regno ha scritto:
> Il 14/02/23 04:11, Yong Wu ha scritto:
>> After commit f1ad5338a4d5 ("of: Fix "dma-ranges" handling for bus
>> controllers"), the dma-ranges is not allowed for dts leaf node.
>> but we still would like to separate to different masters
>> into different iova regions.
>>
>> Thus we have to separate it by the HW larbid and portid. For example,
>> larb1/2 are in region2 and larb3 is in region3. The problem is that
>> some ports inside a larb are in region4 while some ports inside this
>> larb are in region5. Therefore I define a "iova_region_larb_msk" to help
>> record the information for each a port. Take a example for a larb:
>>   [1] = ~0: means all ports in this larb are in region1;
>>   [2] = BIT(3) | BIT(4): means port3/4 in this larb are region2;
>>   [3] = ~(BIT(3) | BIT(4)): means all the other ports except port3/4
>>                             in this larb are region3.
>>
>> This method also avoids the users forget/abuse the iova regions.
>>
>> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
>> ---
>>   drivers/iommu/mtk_iommu.c | 47 ++++++++++++++++++++++-----------------
>>   1 file changed, 27 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>> index ab53edcb221f..7e2cb3b8cac8 100644
>> --- a/drivers/iommu/mtk_iommu.c
>> +++ b/drivers/iommu/mtk_iommu.c
>> @@ -8,7 +8,6 @@
>>   #include <linux/clk.h>
>>   #include <linux/component.h>
>>   #include <linux/device.h>
>> -#include <linux/dma-direct.h>
>>   #include <linux/err.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/io.h>
>> @@ -212,6 +211,11 @@ struct mtk_iommu_plat_data {
>>       struct {
>>           unsigned int    iova_region_nr;
>>           const struct mtk_iommu_iova_region    *iova_region;
>> +        /*
>> +         * Indicate the correspondence between larbs/ports and regions.
>> +         * The index is same with iova_region.
> 
> * The index is the same as iova_region and larb port numbers are
> * described as bit positions.
> * For example, storing BIT(0) at index 2 means "port 0 is in region 2".
> 

Sorry, I'm reiterating because the proposed comment is wrong.

/*
  * Indicates the correspondance between larbs, ports and regions.
  *
  * The index is the same as iova_region and larb port numbers are
  * described as bit positions.
  * For example, storing BIT(0) at index 2,1 means "larb 1, port0 is in region 2".
  *              [2] = { [1] = BIT(0) }
  */


> That's most probably the best short explanation that we can give, so that nobody
> goes crazy with understanding this one.
> 
> After fixing that comment, you totally deserve my
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> Cheers!
> 
>