Re: [PATCH v3 12/33] iommu/mediatek: Always tlb_flush_all when each PM resume

From: Yong Wu <yong.wu@mediatek.com>
To: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Cc: youlin.pei@mediatek.com, anan.sun@mediatek.com,
	srv_heupstream@mediatek.com,
	Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
	Robin Murphy <robin.murphy@arm.com>,
	sebastian.reichel@collabora.com, yen-chang.chen@mediatek.com,
	Fabien Parent <fparent@baylibre.com>,
	iommu@lists.linux-foundation.org,
	Rob Herring <robh+dt@kernel.org>,
	linux-mediatek@lists.infradead.org, yf.wang@mediatek.com,
	Hsin-Yi Wang <hsinyi@chromium.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Collabora Kernel ML <kernel@collabora.com>,
	Will Deacon <will@kernel.org>,
	mingyuan.ma@mediatek.com, linux-arm-kernel@lists.infradead.org,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>
Subject: Re: [PATCH v3 12/33] iommu/mediatek: Always tlb_flush_all when each PM resume
Date: Mon, 6 Dec 2021 16:28:00 +0800	[thread overview]
Message-ID: <9fd624a9f0752b92f309c0bb8524547f250830cb.camel@mediatek.com> (raw)
In-Reply-To: <58b13d3a-e89b-d2e3-278d-af8859d901b9@collabora.com>

On Tue, 2021-11-30 at 13:33 +0200, Dafna Hirschfeld wrote:
> 
> On 30.11.21 09:39, Yong Wu wrote:
> > On Sat, 2021-11-27 at 12:11 +0200, Dafna Hirschfeld wrote:
> > > 
> > > On 10.11.21 09:50, Yong Wu wrote:
> > > > On Wed, 2021-11-10 at 07:29 +0200, Dafna Hirschfeld wrote:
> > > > > 
> > > > > On 10.11.21 04:20, Yong Wu wrote:
> > > > > > On Tue, 2021-11-09 at 14:21 +0200, Dafna Hirschfeld wrote:
> > > > > > > Hi
> > > > > > > This patch is needed in order to update the tlb when a
> > > > > > > device
> > > > > > > is
> > > > > > > powered on.
> > > > > > > Could you send this patch alone without the whole series
> > > > > > > so
> > > > > > > it
> > > > > > > get
> > > > > > > accepted easier?
> > > > > > 
> > > > > > Which SoC are you testing on? In previous SoC, the IOMMU HW
> > > > > > don't
> > > > > > have
> > > > > > power-domain, and we have a "has_pm"[1] in the tlb function
> > > > > > for
> > > > > > that
> > > > > > case. The "has_pm" should be always 0 for the previous SoC
> > > > > > like
> > > > > > mt8173,
> > > > > > it should always tlb synchronize.
> > > > > > 
> > > > > > thus, Could you help share more about your issue? In which
> > > > > > case
> > > > > > it
> > > > > > lack
> > > > > > the necessary tlb operation. At least, We need confirm if
> > > > > > it
> > > > > > needs
> > > > > > a
> > > > > > "Fixes" tags if sending this patch alone.
> > > > > 
> > > > > Hi,
> > > > > I work with the mtk-vcodec driver on mt8173. As you wrote,
> > > > > the
> > > > > iommu
> > > > > doesn't
> > > > > have a power-domain and so when allocating buffers before the
> > > > > device
> > > > > is powered
> > > > > on, there is the warning
> > > > > "Partial TLB flush timed out, falling back to full flush"
> > > > > flooding the log buf.
> > > > 
> > > > oh. Thanks very much for your information. Get it now.
> > > > 
> > > > This issue should be introduced by the:
> > > > 
> > > > b34ea31fe013 ("iommu/mediatek: Always enable the clk on
> > > > resume")
> > > 
> > > Hi, reverting this commit didn't solve those warnings,
> > > I think this is because in the function mtk_iommu_attach_device
> > > the first call to pm_runtime_resume_and_get does not turn the
> > > clks on
> > > since m4u_dom is not yet initialize. And then
> > > mtk_iommu_attach_device
> > > calls pm_runtime_put right after mtk_iommu_hw_init is called
> > > (where the clks are turned on)
> > 
> > oh. Right. this is also related with the patch of "Add power-domain
> > operation".
> > 
> > The current problem is that there is a redundant log of "Partial
> > TLB
> > flush timed out" in mt8173. We need fix this issue firstly. Are you
> > going to prepare the patches again? If not, I could help this. You
> > could help confirm them if you are free.
> 
> Hi,
> I already sent a patch last week: 
> https://lore.kernel.org/linux-iommu/afb46ad6ca9477a2bf71233858406caa6ccb1588.camel@mediatek.com/T/
> could you please review it?

I have replied them before you send this reply. Have you got them?

https://lore.kernel.org/linux-iommu/afb46ad6ca9477a2bf71233858406caa6ccb1588.camel@mediatek.com/

https://lore.kernel.org/linux-iommu/6abef78f6447c626b737fd35688f421c29871f43.camel@mediatek.com/

Thanks.

> 
> Thanks,
> Dafna
> 
> 
> 
> > 
> > Thanks.
> > 
> > > 
> > > thanks,
> > > Dafna
> > > 
> > > 
> > > > 
> > > > tlb failed due to the bclk is not enabled. Could you help try
> > > > that
> > > > after reverting this?
> > > > 
> > > > > 
> > > > > Sebastian Reichel suggested to remove the 'if(has_pm)' check
> > > > > to
> > > > > avoid
> > > > > this warning,
> > > > > and avoid flushing the tlb if the device is off:
> > > > > 
> > > > > [1] http://ix.io/3Eyr
> > > > > 
> > > > > This fixes the warning, but then the tlb is not flushed in
> > > > > sync,
> > > > > Therefore the tlb should be flushed when the device is
> > > > > resumed.
> > > > > 
> > > > > So the two patches (the one suggested in the link [1] and
> > > > > this
> > > > > patch)
> > > > > should be sent together as a 2-patch series.
> > > > 
> > > > then this is reasonable. You could help this into a new
> > > > patchset if
> > > > you
> > > > are free(add Fixes tag).
> > > > 
> > > > Thanks.
> > > > 
> > > > > 
> > > > > Thanks,
> > > > > Dafna
> > > > > 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu