Re: [PATCH v2 11/29] iommu/mediatek: Always pm_runtime_get while tlb flush
On Thu, 2021-09-30 at 13:26 +0200, Dafna Hirschfeld wrote:
>
> On 13.08.21 08:53, Yong Wu wrote:
> > Prepare for 2 HWs that sharing pgtable in different power-domains.
> >
> > The previous SoC don't have PM. Only mt8192 has power-domain,
> > and it is display's power-domain which nearly always is enabled.
>
> hi, I see that in mt1873.dtsi, many devices that uses the iommu have
> the
> 'power-domains' property.
Sorry, I didn't clarify this clear. I mean the iommu device don't have
this property rather than the other device.
>
> >
> > When there are 2 M4U HWs, it may has problem.
> > In this function, we get the pm_status via the m4u dev, but it
> > don't
> > reflect the real power-domain status of the HW since there may be
> > other
> > HW also use that power-domain.
> >
> > Currently we could not get the real power-domain status, thus
> > always
> > pm_runtime_get here.
> >
> > Prepare for mt8195, thus, no need fix tags here.
> >
> > This patch may drop the performance, we expect the user could
> > pm_runtime_get_sync before dma_alloc_attrs which need tlb ops.
> >
>
> Could you explain this sentence a bit? should the user call
> pm_runtime_get_sync
> before calling dma_alloc_attrs?
In v3, I have removed this patch. Use [1] instead.
[1]
https://lore.kernel.org/linux-mediatek/20210923115840.17813-13-yong...@mediatek.com/
Thanks.
>
> Thanks,
> Dafna
>
> > Signed-off-by: Yong Wu
> > ---
> > drivers/iommu/mtk_iommu.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index add23a36a5e2..abc721a1da21 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -238,8 +238,11 @@ static void
> > mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> >
> > for_each_m4u(data, head) {
> > if (has_pm) {
> > - if (pm_runtime_get_if_in_use(data->dev) <= 0)
> > + ret = pm_runtime_resume_and_get(data->dev);
> > + if (ret < 0) {
> > + dev_err(data->dev, "tlb flush: pm get
> > fail %d.\n", ret);
> > continue;
> > + }
> > }
> >
> > spin_lock_irqsave(>tlb_lock, flags);
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 11/29] iommu/mediatek: Always pm_runtime_get while tlb flush
On 13.08.21 08:53, Yong Wu wrote:
Prepare for 2 HWs that sharing pgtable in different power-domains.
The previous SoC don't have PM. Only mt8192 has power-domain,
and it is display's power-domain which nearly always is enabled.
hi, I see that in mt1873.dtsi, many devices that uses the iommu have the
'power-domains' property.
When there are 2 M4U HWs, it may has problem.
In this function, we get the pm_status via the m4u dev, but it don't
reflect the real power-domain status of the HW since there may be other
HW also use that power-domain.
Currently we could not get the real power-domain status, thus always
pm_runtime_get here.
Prepare for mt8195, thus, no need fix tags here.
This patch may drop the performance, we expect the user could
pm_runtime_get_sync before dma_alloc_attrs which need tlb ops.
Could you explain this sentence a bit? should the user call pm_runtime_get_sync
before calling dma_alloc_attrs?
Thanks,
Dafna
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index add23a36a5e2..abc721a1da21 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -238,8 +238,11 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long
iova, size_t size,
for_each_m4u(data, head) {
if (has_pm) {
- if (pm_runtime_get_if_in_use(data->dev) <= 0)
+ ret = pm_runtime_resume_and_get(data->dev);
+ if (ret < 0) {
+ dev_err(data->dev, "tlb flush: pm get fail
%d.\n", ret);
continue;
+ }
}
spin_lock_irqsave(>tlb_lock, flags);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 11/29] iommu/mediatek: Always pm_runtime_get while tlb flush
On Tue, 2021-08-24 at 15:10 +0800, Hsin-Yi Wang wrote:
> On Fri, Aug 13, 2021 at 2:57 PM Yong Wu wrote:
> >
> > Prepare for 2 HWs that sharing pgtable in different power-domains.
> >
> > The previous SoC don't have PM. Only mt8192 has power-domain,
> > and it is display's power-domain which nearly always is enabled.
> >
> > When there are 2 M4U HWs, it may has problem.
> > In this function, we get the pm_status via the m4u dev, but it
> > don't
> > reflect the real power-domain status of the HW since there may be
> > other
> > HW also use that power-domain.
> >
> > Currently we could not get the real power-domain status, thus
> > always
> > pm_runtime_get here.
> >
> > Prepare for mt8195, thus, no need fix tags here.
> >
> > This patch may drop the performance, we expect the user could
> > pm_runtime_get_sync before dma_alloc_attrs which need tlb ops.
>
> Can you check if there are existing users that need to add this
> change?
The issue may exist in our most users. Our users mainly are in v4l2.
normally their flow like this:
a) VIDIOC_REQBUFS: call dma_alloc_attrs or dma_buf_map_attachment.
b) VIDIOC_STREAMON.
c) VIDIOC_QBUF: device_run: pm_runtime_get_sync.
Requesting they call pm_runtime_get before dma_alloc_attrs looks not
reasonable. It seems that they should not care about this.
This patch mainly make sure the flow is right. Locally I have a TODO to
try get the real power-domain status here, the sample code like below:
static struct notifier_block mtk_penpd_notifier;
/* Register the genpd notifier. */
mtk_penpd_notifier.notifier_call = mtk_iommu_pd_callback;
ret = dev_pm_genpd_add_notifier(dev, _penpd_notifier);
/* Then get the real power domain status in the notifier */
static int mtk_iommu_pd_callback(struct notifier_block *nb,
unsigned long flags, void *data)
{
if (flags == GENPD_NOTIFY_ON)
/* the real power domain is power on */
else if (flags == GENPD_NOTIFY_PRE_OFF)
/* the real power domain are going to power off. Take it as
power off.
* Skip the tlb ops after receivice this flag.
*/
}
How about this? or any other suggestion to get the real power-domain
rather than the iommu device's power domain status.
Thanks.
>
>
> >
> > Signed-off-by: Yong Wu
> > ---
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 11/29] iommu/mediatek: Always pm_runtime_get while tlb flush
On Fri, Aug 13, 2021 at 2:57 PM Yong Wu wrote: > > Prepare for 2 HWs that sharing pgtable in different power-domains. > > The previous SoC don't have PM. Only mt8192 has power-domain, > and it is display's power-domain which nearly always is enabled. > > When there are 2 M4U HWs, it may has problem. > In this function, we get the pm_status via the m4u dev, but it don't > reflect the real power-domain status of the HW since there may be other > HW also use that power-domain. > > Currently we could not get the real power-domain status, thus always > pm_runtime_get here. > > Prepare for mt8195, thus, no need fix tags here. > > This patch may drop the performance, we expect the user could > pm_runtime_get_sync before dma_alloc_attrs which need tlb ops. Can you check if there are existing users that need to add this change? > > Signed-off-by: Yong Wu > --- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 11/29] iommu/mediatek: Always pm_runtime_get while tlb flush
Prepare for 2 HWs that sharing pgtable in different power-domains.
The previous SoC don't have PM. Only mt8192 has power-domain,
and it is display's power-domain which nearly always is enabled.
When there are 2 M4U HWs, it may has problem.
In this function, we get the pm_status via the m4u dev, but it don't
reflect the real power-domain status of the HW since there may be other
HW also use that power-domain.
Currently we could not get the real power-domain status, thus always
pm_runtime_get here.
Prepare for mt8195, thus, no need fix tags here.
This patch may drop the performance, we expect the user could
pm_runtime_get_sync before dma_alloc_attrs which need tlb ops.
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index add23a36a5e2..abc721a1da21 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -238,8 +238,11 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long
iova, size_t size,
for_each_m4u(data, head) {
if (has_pm) {
- if (pm_runtime_get_if_in_use(data->dev) <= 0)
+ ret = pm_runtime_resume_and_get(data->dev);
+ if (ret < 0) {
+ dev_err(data->dev, "tlb flush: pm get fail
%d.\n", ret);
continue;
+ }
}
spin_lock_irqsave(>tlb_lock, flags);
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
