| Message ID | 20210813065324.29220-1-yong.wu@mediatek.com |
|---|---|
| Headers | show |
| Series | MT8195 IOMMU SUPPORT | expand |
On Fri, Aug 13, 2021 at 2:57 PM Yong Wu <yong.wu@mediatek.com> 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 <yong.wu@mediatek.com> > --- <snip>
On Fri, Aug 13, 2021 at 3:03 PM Yong Wu <yong.wu@mediatek.com> wrote: > > For MM IOMMU, We always add device link between smi-common and IOMMU HW. > In mt8195, we add smi-sub-common. Thus, if the node is sub-common, we still > need find again to get smi-common, then do device link. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/mtk_iommu.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index a4479916ad33..a72241724adb 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -845,6 +845,17 @@ static int mtk_iommu_mm_dts_parse(struct device *dev, > if (!smicomm_node) > return -EINVAL; > > + /* Find smi-common again if this is smi-sub-common */ > + if (of_property_read_bool(smicomm_node, "mediatek,smi_sub_common")) { > + of_node_put(smicomm_node); /* put the sub common */ > + > + smicomm_node = of_parse_phandle(smicomm_node, "mediatek,smi", 0); This only checks 1 level here, and does not check if the mediatek,smi of a sub-common node is not another sub-common node. So maybe add a check that the updated node here doesn't have mediatek,smi_sub_common property. > + if (!smicomm_node) { > + dev_err(dev, "sub-comm has no common.\n"); > + return -EINVAL; > + } > + } > + > plarbdev = of_find_device_by_node(smicomm_node); > of_node_put(smicomm_node); > data->smicomm_dev = &plarbdev->dev; > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Tue, 2021-08-24 at 15:35 +0800, Hsin-Yi Wang wrote: > On Fri, Aug 13, 2021 at 3:03 PM Yong Wu <yong.wu@mediatek.com> wrote: > > > > For MM IOMMU, We always add device link between smi-common and > > IOMMU HW. > > In mt8195, we add smi-sub-common. Thus, if the node is sub-common, > > we still > > need find again to get smi-common, then do device link. > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > drivers/iommu/mtk_iommu.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > index a4479916ad33..a72241724adb 100644 > > --- a/drivers/iommu/mtk_iommu.c > > +++ b/drivers/iommu/mtk_iommu.c > > @@ -845,6 +845,17 @@ static int mtk_iommu_mm_dts_parse(struct > > device *dev, > > if (!smicomm_node) > > return -EINVAL; > > > > + /* Find smi-common again if this is smi-sub-common */ > > + if (of_property_read_bool(smicomm_node, > > "mediatek,smi_sub_common")) { > > + of_node_put(smicomm_node); /* put the sub common */ > > + > > + smicomm_node = of_parse_phandle(smicomm_node, > > "mediatek,smi", 0); > > This only checks 1 level here, and does not check if the mediatek,smi > of a sub-common node is not another sub-common node. > So maybe add a check that the updated node here doesn't have > mediatek,smi_sub_common property. Yes. Currently there are only 2 levels. we could confirm if it is sub- common from if it has "mediatek,smi", then "mediatek,smi_sub_common" is unnecessary. Will fix in the next version. > > > + if (!smicomm_node) { > > + dev_err(dev, "sub-comm has no common.\n"); > > + return -EINVAL; > > + } > > + } > > + > > plarbdev = of_find_device_by_node(smicomm_node); > > of_node_put(smicomm_node); > > data->smicomm_dev = &plarbdev->dev; > > -- > > 2.18.0 > > _______________________________________________ > > Linux-mediatek mailing list > > Linux-mediatek@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Tue, 2021-08-24 at 15:10 +0800, Hsin-Yi Wang wrote: > On Fri, Aug 13, 2021 at 2:57 PM Yong Wu <yong.wu@mediatek.com> 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, &mtk_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 <yong.wu@mediatek.com> > > --- > > <snip>
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 <yong.wu@mediatek.com> > --- > 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(&data->tlb_lock, flags); >
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.wu@mediatek.com/ Thanks. > > Thanks, > Dafna > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > 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(&data->tlb_lock, flags); > >
This patchset add mt8195 iommu support. mt8195 have 3 IOMMU HWs. 2 IOMMU HW is for multimedia, and 1 IOMMU HW is for infra-master, like PCIe/USB. About the 2 MM IOMMU HW, something like this: IOMMU(VDO) IOMMU(VPP) | | SMI_COMMON(VDO) SMI_COMMON(VPP) --------------- ---------------- | | ... | | ... larb0 larb2 ... larb1 larb3 ... these two MM IOMMU HW share a pgtable. About the INFRA IOMMU, it don't have larbs, the master connects the iommu directly. It use a independent pgtable. Also, mt8195 IOMMU bank supports.Normally the IOMMU register size only is 0x1000. In this IOMMU HW, the register size is 5 * 0x1000. each 0x1000 is a bank. the banks' register look like this: ---------------------------------------- |bank0 | bank1 | bank2 | bank3 | bank4| ---------------------------------------- |global | |control| null |regs | ----------------------------------------- |bank |bank |bank |bank |bank | |regs |regs |regs |regs |regs | | | | | | | ----------------------------------------- All the banks share some global control registers, and each bank have its special bank registers, like pgtable base register, tlb operation registers, the fault status registers. In mt8195, we enable this bank feature for infra iommu, We put PCIe in bank0 and USB in bank4. they have independent pgtable. patch[1..19]: support mt8195 iommu. patch[20..29]: support bank feature. TODO: there is another APU_IOMMU in mt8195, this should depend on APU patches. thus, we need add that feature after that. Change note: v2: 1) Base on v5.14-rc1. 2) Fix build fail for arm32. 3) Fix dt-binding issue from Rob. 4) Fix the bank issue when tlb flush. v1 always use bank->base. 5) Adjust devlink with smi-common since the node may be smi-sub-common. 6) other changes: like reword some commit message(removing many "This patch..."); seperate serveral patches. v1: https://lore.kernel.org/linux-mediatek/20210630023504.18177-1-yong.wu@mediatek.com/ Base on v5.13-rc1. Yong Wu (29): dt-bindings: mediatek: mt8195: Add binding for MM IOMMU dt-bindings: mediatek: mt8195: Add binding for infra IOMMU iommu/mediatek: Fix 2 HW sharing pgtable issue iommu/mediatek: Adapt sharing and non-sharing pgtable case iommu/mediatek: Add 12G~16G support for multi domains iommu/mediatek: Add a flag DCM_DISABLE iommu/mediatek: Add a flag NON_STD_AXI iommu/mediatek: Remove for_each_m4u in tlb_sync_all iommu/mediatek: Add tlb_lock in tlb_flush_all iommu/mediatek: Remove the granule in the tlb flush iommu/mediatek: Always pm_runtime_get while tlb flush iommu/mediatek: Always enable output PA over 32bits in isr iommu/mediatek: Add SUB_COMMON_3BITS flag iommu/mediatek: Add IOMMU_TYPE flag iommu/mediatek: Contain MM IOMMU flow with the MM TYPE iommu/mediatek: Adjust device link when it is sub-common iommu/mediatek: Add infra iommu support iommu/mediatek: Add PCIe support iommu/mediatek: Add mt8195 support iommu/mediatek: Only adjust code about register base iommu/mediatek: Just move code position in hw_init iommu/mediatek: Add mtk_iommu_bank_data structure iommu/mediatek: Initialise bank HW for each a bank iommu/mediatek: Add bank_nr and bank_enable iommu/mediatek: Change the domid to iova_region_id iommu/mediatek: Get the proper bankid for multi banks iommu/mediatek: Initialise/Remove for multi bank dev iommu/mediatek: Backup/restore regsiters for multi banks iommu/mediatek: mt8195: Enable multi banks for infra iommu .../bindings/iommu/mediatek,iommu.yaml | 20 +- drivers/iommu/mtk_iommu.c | 792 ++++++++++++------ drivers/iommu/mtk_iommu.h | 55 +- .../dt-bindings/memory/mt8195-memory-port.h | 408 +++++++++ include/dt-bindings/memory/mtk-memory-port.h | 2 + 5 files changed, 1010 insertions(+), 267 deletions(-) create mode 100644 include/dt-bindings/memory/mt8195-memory-port.h