On Fri, 2021-10-22 at 16:03 +0200, Dafna Hirschfeld wrote:

Hi


On 23.09.21 13:58, Yong Wu wrote:

quoted

To simplify the code, Remove the power status checking in the
tlb_flush_all, remove this:
    if (pm_runtime_get_if_in_use(data->dev) <= 0)
	    continue;

After this patch, the mtk_iommu_tlb_flush_all will be called from
a) isr
b) pm runtime resume callback
c) tlb flush range fail case
d) iommu_create_device_direct_mappings
    -> iommu_flush_iotlb_all
In first three cases, the power and clock always are enabled; d) is
direct

Regarding case "c) tlb flush range fail case", I found out that this
often happens when the iommu is used while it is runtime suspended. 

Which SoC and branch are you testing on?

For example the mtk-vcodec encoder driver calls
"pm_runtime_resume_and_get" only when it starts
streaming but
buffers allocation is done in 'v4l2_reqbufs' before
"pm_runtime_resume_and_get" is called

This is the case I tried to fix in [14/33].
pm_runtime_get_if_in_use should return when the iommu device's pm is
not active when vcodec allocate buffer before pm_runtime_resume_and
get.

Do you have the devicelink patchset in your branch? if not, the vcodec
should call mtk_smi_larb_get to enable the power/clock for larbs, then
the iommu's device is active via devicelink between smi and iommu
device.

and then I see the warning "Partial TLB flush timed out, falling back
to full flush"
I am not sure how to fix that issue, but it seems that case 'c)'
might indicate that
power and clock are actually not enabled.

quoted

mapping, the tlb flush is unnecessay since we already have
tlb_flush_all
in the pm_runtime_resume callback. When the iommu's power status is
changed to active, the tlb always is clean.

In addition, there still are 2 reasons that don't add PM status
checking
in the tlb flush all:
a) Write tlb flush all register also is ok even though the HW has
no
power and clocks. Write ignore.
b) pm_runtime_get_if_in_use(m4udev) is 0 when the tlb_flush_all
is called frm pm_runtime_resume cb. From this point, we can not add
this code above in this tlb_flush_all.

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

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index e9e94944ed91..4a33b6c6b1db 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c

@@ -204,10 +204,14 @@ static struct mtk_iommu_domain

*to_mtk_domain(struct iommu_domain *dom)
  	return container_of(dom, struct mtk_iommu_domain, domain);
  }
  
-static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data
*data)
+static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
  {
  	unsigned long flags;
  
+	/*
+	 * No need get power status since the HW PM status nearly is
active
+	 * when entering here.
+	 */
  	spin_lock_irqsave(&data->tlb_lock, flags);
  	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
  		       data->base + data->plat_data->inv_sel_reg);

@@ -216,16 +220,6 @@ static void mtk_iommu_tlb_do_flush_all(struct

mtk_iommu_data *data)
  	spin_unlock_irqrestore(&data->tlb_lock, flags);
  }
  
-static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
-{
-	if (pm_runtime_get_if_in_use(data->dev) <= 0)
-		return;
-
-	mtk_iommu_tlb_do_flush_all(data);
-
-	pm_runtime_put(data->dev);
-}
-
  static void mtk_iommu_tlb_flush_range_sync(unsigned long iova,
size_t size,
  					   struct mtk_iommu_data *data)
  {

@@ -263,7 +257,7 @@ static void

mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
  		if (ret) {
  			dev_warn(data->dev,
  				 "Partial TLB flush timed out, falling
back to full flush\n");
-			mtk_iommu_tlb_do_flush_all(data);
+			mtk_iommu_tlb_flush_all(data);
  		}
  
  		if (has_pm)

@@ -993,7 +987,7 @@ static int __maybe_unused

mtk_iommu_runtime_resume(struct device *dev)
  	 *
  	 * Thus, Make sure the tlb always is clean after each PM
resume.
  	 */
-	mtk_iommu_tlb_do_flush_all(data);
+	mtk_iommu_tlb_flush_all(data);
  
  	/*
  	 * Uppon first resume, only enable the clk and return, since
the values of the