| Message ID | 20201111123838.15682-1-yong.wu@mediatek.com |
|---|---|
| Headers | show |
| Series | MT8192 IOMMU support | expand |
Hi Yong: Yong Wu <yong.wu@mediatek.com> 於 2020年11月11日 週三 下午8:53寫道： > > I am the author of MediaTek iommu driver, and will to maintain and > develop it further. > Add myself to cover these items. Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > MAINTAINERS | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e73636b75f29..462a87ee19c8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11056,6 +11056,15 @@ S: Maintained > F: Documentation/devicetree/bindings/i2c/i2c-mt65xx.txt > F: drivers/i2c/busses/i2c-mt65xx.c > > +MEDIATEK IOMMU DRIVER > +M: Yong Wu <yong.wu@mediatek.com> > +L: iommu@lists.linux-foundation.org > +L: linux-mediatek@lists.infradead.org (moderated for non-subscribers) > +S: Supported > +F: Documentation/devicetree/bindings/iommu/mediatek* > +F: drivers/iommu/mtk-iommu* > +F: include/dt-bindings/memory/mt*-larb-port.h > + > MEDIATEK JPEG DRIVER > M: Rick Chang <rick.chang@mediatek.com> > M: Bin Liu <bin.liu@mediatek.com> > -- > 2.18.0 > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Wed, Nov 11, 2020 at 08:38:17PM +0800, Yong Wu wrote: > Extend the max larb number definition as mt8192 has larb_nr over 16. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > Acked-by: Rob Herring <robh@kernel.org> > --- > Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 2 +- > include/dt-bindings/memory/mtk-smi-larb-port.h | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > Acked-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
On Wed, Nov 11, 2020 at 08:38:19PM +0800, Yong Wu wrote: > This patch adds decriptions for mt8192 IOMMU and SMI. > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation > table format. The M4U-SMI HW diagram is as below: > > EMI > | > M4U > | > ------------ > SMI Common > ------------ > | > +-------+------+------+----------------------+-------+ > | | | | ...... | | > | | | | | | > larb0 larb1 larb2 larb4 ...... larb19 larb20 > disp0 disp1 mdp vdec IPE IPE > > All the connections are HW fixed, SW can NOT adjust it. > > mt8192 M4U support 0~16GB iova range. we preassign different engines > into different iova ranges: > > domain-id module iova-range larbs > 0 disp 0 ~ 4G larb0/1 > 1 vcodec 4G ~ 8G larb4/5/7 > 2 cam/mdp 8G ~ 12G larb2/9/11/13/14/16/17/18/19/20 > 3 CCU0 0x4000_0000 ~ 0x43ff_ffff larb13: port 9/10 > 4 CCU1 0x4400_0000 ~ 0x47ff_ffff larb14: port 4/5 > > The iova range for CCU0/1(camera control unit) is HW requirement. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > Reviewed-by: Rob Herring <robh@kernel.org> > --- > .../bindings/iommu/mediatek,iommu.yaml | 18 +- > include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++++ > 2 files changed, 257 insertions(+), 1 deletion(-) > create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h > > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > index ba6626347381..0f26fe14c8e2 100644 > --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > @@ -76,6 +76,7 @@ properties: > - mediatek,mt8167-m4u # generation two > - mediatek,mt8173-m4u # generation two > - mediatek,mt8183-m4u # generation two > + - mediatek,mt8192-m4u # generation two > > - description: mt7623 generation one > items: > @@ -115,7 +116,11 @@ properties: > dt-binding/memory/mt6779-larb-port.h for mt6779, > dt-binding/memory/mt8167-larb-port.h for mt8167, > dt-binding/memory/mt8173-larb-port.h for mt8173, > - dt-binding/memory/mt8183-larb-port.h for mt8183. > + dt-binding/memory/mt8183-larb-port.h for mt8183, > + dt-binding/memory/mt8192-larb-port.h for mt8192. > + > + power-domains: > + maxItems: 1 > > required: > - compatible > @@ -133,11 +138,22 @@ allOf: > - mediatek,mt2701-m4u > - mediatek,mt2712-m4u > - mediatek,mt8173-m4u > + - mediatek,mt8192-m4u > > then: > required: > - clocks > > + - if: > + properties: > + compatible: > + enum: > + - mediatek,mt8192-m4u > + > + then: > + required: > + - power-domains > + > additionalProperties: false > > examples: > diff --git a/include/dt-bindings/memory/mt8192-larb-port.h b/include/dt-bindings/memory/mt8192-larb-port.h > new file mode 100644 > index 000000000000..7437fa62654e > --- /dev/null > +++ b/include/dt-bindings/memory/mt8192-larb-port.h > @@ -0,0 +1,240 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2020 MediaTek Inc. > + * > + * Author: Chao Hao <chao.hao@mediatek.com> > + * Author: Yong Wu <yong.wu@mediatek.com> > + */ > +#ifndef _DTS_IOMMU_PORT_MT8192_H_ > +#define _DTS_IOMMU_PORT_MT8192_H_ Not accurate header guard. Shoud be: _DT_BINDINGS_MEMORY_MT8192_LARB_PORT_H_ Probably you copied it from some other Mediatek headers - all of them have header guard pointing to different directory. Best regards, Krzysztof
On Wed, Nov 11, 2020 at 08:38:32PM +0800, Yong Wu wrote: > After extending v7s, our pagetable already support iova reach > 16GB(34bit). the master got the iova via dma_alloc_attrs may reach > 34bits, but its HW register still is 32bit. then how to set the > bit32/bit33 iova? this depend on a SMI larb setting(bank_sel). > > we separate whole 16GB iova to four banks: > bank: 0: 0~4G; 1: 4~8G; 2: 8-12G; 3: 12-16G; > The bank number is (iova >> 32). > > We will preassign which bank the larbs belong to. currently we don't > have a interface for master to adjust its bank number. > > Each a bank is a iova_region which is a independent iommu-domain. > the iova range for each iommu-domain can't cross 4G. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > Acked-by: Krzysztof Kozlowski <krzk@kernel.org> # memory part > --- > drivers/iommu/mtk_iommu.c | 12 +++++++++--- > drivers/memory/mtk-smi.c | 7 +++++++ > include/soc/mediatek/smi.h | 1 + > 3 files changed, 17 insertions(+), 3 deletions(-) > Acked-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
On Wed, Nov 11, 2020 at 8:40 PM Yong Wu <yong.wu@mediatek.com> wrote: > > In the lastest SoC, M4U has its special power domain. thus, If the engine > begin to work, it should help enable the power for M4U firstly. > Currently if the engine work, it always enable the power/clocks for > smi-larbs/smi-common. This patch adds device_link for smi-common and M4U. > then, if smi-common power is enabled, the M4U power also is powered on > automatically. > > Normally M4U connect with several smi-larbs and their smi-common always > are the same, In this patch it get smi-common dev from the first smi-larb > device(i==0), then add the device_link only while m4u has power-domain. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/mtk_iommu.c | 36 +++++++++++++++++++++++++++++++++--- > drivers/iommu/mtk_iommu.h | 1 + > 2 files changed, 34 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index cfdf5ce696fd..4ce7e0883e4d 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -20,6 +20,7 @@ > #include <linux/of_irq.h> > #include <linux/of_platform.h> > #include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > #include <linux/regmap.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > @@ -705,7 +706,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > return larb_nr; > > for (i = 0; i < larb_nr; i++) { > - struct device_node *larbnode; > + struct device_node *larbnode, *smicomm_node; > struct platform_device *plarbdev; > u32 id; > > @@ -731,6 +732,26 @@ static int mtk_iommu_probe(struct platform_device *pdev) > > component_match_add_release(dev, &match, release_of, > compare_of, larbnode); > + if (!i) { Maybe more of a style preference, but since you are actually comparing an integer, I prefer seeing i == 0. Also, might be nicer to do if (i != 0) continue; And de-indent the rest. > + smicomm_node = of_parse_phandle(larbnode, "mediatek,smi", 0); > + if (!smicomm_node) > + return -EINVAL; > + > + plarbdev = of_find_device_by_node(smicomm_node); > + of_node_put(smicomm_node); > + data->smicomm_dev = &plarbdev->dev; > + } > + } > + > + if (dev->pm_domain) { > + struct device_link *link; > + > + link = device_link_add(data->smicomm_dev, dev, > + DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME); > + if (!link) { > + dev_err(dev, "Unable link %s.\n", dev_name(data->smicomm_dev)); > + return -EINVAL; > + } > } > > platform_set_drvdata(pdev, data); > @@ -738,14 +759,14 @@ static int mtk_iommu_probe(struct platform_device *pdev) > ret = iommu_device_sysfs_add(&data->iommu, dev, NULL, > "mtk-iommu.%pa", &ioaddr); > if (ret) > - return ret; > + goto out_link_remove; > > iommu_device_set_ops(&data->iommu, &mtk_iommu_ops); > iommu_device_set_fwnode(&data->iommu, &pdev->dev.of_node->fwnode); > > ret = iommu_device_register(&data->iommu); > if (ret) > - return ret; > + goto out_sysfs_remove; Technically, this change is unrelated. > > spin_lock_init(&data->tlb_lock); > list_add_tail(&data->list, &m4ulist); > @@ -754,6 +775,13 @@ static int mtk_iommu_probe(struct platform_device *pdev) > bus_set_iommu(&platform_bus_type, &mtk_iommu_ops); > > return component_master_add_with_match(dev, &mtk_iommu_com_ops, match); > + > +out_sysfs_remove: > + iommu_device_sysfs_remove(&data->iommu); > +out_link_remove: > + if (dev->pm_domain) > + device_link_remove(data->smicomm_dev, dev); > + return ret; > } > > static int mtk_iommu_remove(struct platform_device *pdev) > @@ -767,6 +795,8 @@ static int mtk_iommu_remove(struct platform_device *pdev) > bus_set_iommu(&platform_bus_type, NULL); > > clk_disable_unprepare(data->bclk); > + if (pdev->dev.pm_domain) > + device_link_remove(data->smicomm_dev, &pdev->dev); > devm_free_irq(&pdev->dev, data->irq, data); > component_master_del(&pdev->dev, &mtk_iommu_com_ops); > return 0; > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > index d0c93652bdbe..5e03a029c4dc 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -68,6 +68,7 @@ struct mtk_iommu_data { > > struct iommu_device iommu; > const struct mtk_iommu_plat_data *plat_data; > + struct device *smicomm_dev; > > struct dma_iommu_mapping *mapping; /* For mtk_iommu_v1.c */ > > -- > 2.18.0 >
Hi Krzysztof, On Wed, 2020-11-11 at 22:33 +0100, Krzysztof Kozlowski wrote: > On Wed, Nov 11, 2020 at 08:38:19PM +0800, Yong Wu wrote: > > This patch adds decriptions for mt8192 IOMMU and SMI. > > > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation > > table format. The M4U-SMI HW diagram is as below: > > > > EMI > > | > > M4U > > | > > ------------ > > SMI Common > > ------------ > > | > > +-------+------+------+----------------------+-------+ > > | | | | ...... | | > > | | | | | | > > larb0 larb1 larb2 larb4 ...... larb19 larb20 > > disp0 disp1 mdp vdec IPE IPE > > > > All the connections are HW fixed, SW can NOT adjust it. > > > > mt8192 M4U support 0~16GB iova range. we preassign different engines > > into different iova ranges: > > > > domain-id module iova-range larbs > > 0 disp 0 ~ 4G larb0/1 > > 1 vcodec 4G ~ 8G larb4/5/7 > > 2 cam/mdp 8G ~ 12G larb2/9/11/13/14/16/17/18/19/20 > > 3 CCU0 0x4000_0000 ~ 0x43ff_ffff larb13: port 9/10 > > 4 CCU1 0x4400_0000 ~ 0x47ff_ffff larb14: port 4/5 > > > > The iova range for CCU0/1(camera control unit) is HW requirement. > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > Reviewed-by: Rob Herring <robh@kernel.org> > > --- [...] > > +#ifndef _DTS_IOMMU_PORT_MT8192_H_ > > +#define _DTS_IOMMU_PORT_MT8192_H_ > > Not accurate header guard. Shoud be: > _DT_BINDINGS_MEMORY_MT8192_LARB_PORT_H_ > > Probably you copied it from some other Mediatek headers - all of them > have header guard pointing to different directory. Thanks very much for your reviewing so many patches. This name like this when it was in the first version. Since it is only used when the consumer devices enable IOMMU, thus called it _IOMMU_PORT... I will use a new patch to rename all of them. > > Best regards, > Krzysztof
On Thu, 2020-11-12 at 09:10 +0800, Nicolas Boichat wrote: > On Wed, Nov 11, 2020 at 8:40 PM Yong Wu <yong.wu@mediatek.com> wrote: > > > > In the lastest SoC, M4U has its special power domain. thus, If the engine > > begin to work, it should help enable the power for M4U firstly. > > Currently if the engine work, it always enable the power/clocks for > > smi-larbs/smi-common. This patch adds device_link for smi-common and M4U. > > then, if smi-common power is enabled, the M4U power also is powered on > > automatically. > > > > Normally M4U connect with several smi-larbs and their smi-common always > > are the same, In this patch it get smi-common dev from the first smi-larb > > device(i==0), then add the device_link only while m4u has power-domain. > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > drivers/iommu/mtk_iommu.c | 36 +++++++++++++++++++++++++++++++++--- > > drivers/iommu/mtk_iommu.h | 1 + > > 2 files changed, 34 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > index cfdf5ce696fd..4ce7e0883e4d 100644 > > --- a/drivers/iommu/mtk_iommu.c > > +++ b/drivers/iommu/mtk_iommu.c > > @@ -20,6 +20,7 @@ > > #include <linux/of_irq.h> > > #include <linux/of_platform.h> > > #include <linux/platform_device.h> > > +#include <linux/pm_runtime.h> > > #include <linux/regmap.h> > > #include <linux/slab.h> > > #include <linux/spinlock.h> > > @@ -705,7 +706,7 @@ static int mtk_iommu_probe(struct platform_device *pdev) > > return larb_nr; > > > > for (i = 0; i < larb_nr; i++) { > > - struct device_node *larbnode; > > + struct device_node *larbnode, *smicomm_node; > > struct platform_device *plarbdev; > > u32 id; > > > > @@ -731,6 +732,26 @@ static int mtk_iommu_probe(struct platform_device *pdev) > > > > component_match_add_release(dev, &match, release_of, > > compare_of, larbnode); > > + if (!i) { > > Maybe more of a style preference, but since you are actually comparing > an integer, I prefer seeing i == 0. > > Also, might be nicer to do > > if (i != 0) > continue; > > And de-indent the rest. Thanks. will fix. > > > + smicomm_node = of_parse_phandle(larbnode, "mediatek,smi", 0); > > + if (!smicomm_node) > > + return -EINVAL; > > + > > + plarbdev = of_find_device_by_node(smicomm_node); > > + of_node_put(smicomm_node); > > + data->smicomm_dev = &plarbdev->dev; > > + } > > + } > > + > > + if (dev->pm_domain) { > > + struct device_link *link; > > + > > + link = device_link_add(data->smicomm_dev, dev, > > + DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME); > > + if (!link) { > > + dev_err(dev, "Unable link %s.\n", dev_name(data->smicomm_dev)); > > + return -EINVAL; > > + } > > } > > > > platform_set_drvdata(pdev, data); > > @@ -738,14 +759,14 @@ static int mtk_iommu_probe(struct platform_device *pdev) > > ret = iommu_device_sysfs_add(&data->iommu, dev, NULL, > > "mtk-iommu.%pa", &ioaddr); > > if (ret) > > - return ret; > > + goto out_link_remove; > > > > iommu_device_set_ops(&data->iommu, &mtk_iommu_ops); > > iommu_device_set_fwnode(&data->iommu, &pdev->dev.of_node->fwnode); > > > > ret = iommu_device_register(&data->iommu); > > if (ret) > > - return ret; > > + goto out_sysfs_remove; > > Technically, this change is unrelated. Sharp eye. Right. I thought it was small enough to squash here. I will use a new patch to fix this(add fixes tag, and no need add cc-stable I think.). > > > > > spin_lock_init(&data->tlb_lock); > > list_add_tail(&data->list, &m4ulist); > > @@ -754,6 +775,13 @@ static int mtk_iommu_probe(struct platform_device *pdev) > > bus_set_iommu(&platform_bus_type, &mtk_iommu_ops); > > > > return component_master_add_with_match(dev, &mtk_iommu_com_ops, match); > > + > > +out_sysfs_remove: > > + iommu_device_sysfs_remove(&data->iommu); > > +out_link_remove: > > + if (dev->pm_domain) > > + device_link_remove(data->smicomm_dev, dev); > > + return ret; > > } > > > > static int mtk_iommu_remove(struct platform_device *pdev) > > @@ -767,6 +795,8 @@ static int mtk_iommu_remove(struct platform_device *pdev) > > bus_set_iommu(&platform_bus_type, NULL); > > > > clk_disable_unprepare(data->bclk); > > + if (pdev->dev.pm_domain) > > + device_link_remove(data->smicomm_dev, &pdev->dev); > > devm_free_irq(&pdev->dev, data->irq, data); > > component_master_del(&pdev->dev, &mtk_iommu_com_ops); > > return 0; > > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > > index d0c93652bdbe..5e03a029c4dc 100644 > > --- a/drivers/iommu/mtk_iommu.h > > +++ b/drivers/iommu/mtk_iommu.h > > @@ -68,6 +68,7 @@ struct mtk_iommu_data { > > > > struct iommu_device iommu; > > const struct mtk_iommu_plat_data *plat_data; > > + struct device *smicomm_dev; > > > > struct dma_iommu_mapping *mapping; /* For mtk_iommu_v1.c */ > > > > -- > > 2.18.0 > > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Wed, 11 Nov 2020 20:38:15 +0800, Yong Wu wrote: > Convert MediaTek IOMMU to DT schema. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > .../bindings/iommu/mediatek,iommu.txt | 105 ----------- > .../bindings/iommu/mediatek,iommu.yaml | 167 ++++++++++++++++++ > 2 files changed, 167 insertions(+), 105 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/iommu/mediatek,iommu.txt > create mode 100644 Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
On Wed, Nov 11, 2020 at 08:38:14PM +0800, Yong Wu wrote: > This patch mainly adds support for mt8192 Multimedia IOMMU and SMI. > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation > table format. The M4U-SMI HW diagram is as below: > > EMI > | > M4U > | > ------------ > SMI Common > ------------ > | > +-------+------+------+----------------------+-------+ > | | | | ...... | | > | | | | | | > larb0 larb1 larb2 larb4 ...... larb19 larb20 > disp0 disp1 mdp vdec IPE IPE > > All the connections are HW fixed, SW can NOT adjust it. > > Comparing with the preview SoC, this patchset mainly adds two new functions: > a) add iova 34 bits support. > b) add multi domains support since several HW has the special iova > region requirement. > > change note: > v4: a) rebase on v5.10-rc1 > b) Move the smi part to a independent patchset. > c) Improve v7s code from Robin and Will. > d) Add a mediatek iommu entry patch in MAINTAINERS. Please can you post a v5 of this, adding the Acks from v4 and addressing the build failures reported by the kbuild robot on patch 20? Thanks, Will
On 2020-11-11 12:38, Yong Wu wrote: > Use the ias for the valid iova checking in arm_v7s_unmap. This is a > preparing patch for supporting iova 34bit for MediaTek. I can't help thinking of weird ways to generate better code here, but being consistent with what we already have on the map path is probably more valuable for now. Reviewed-by: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/io-pgtable-arm-v7s.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c > index a688f22cbe3b..e880745ab1e8 100644 > --- a/drivers/iommu/io-pgtable-arm-v7s.c > +++ b/drivers/iommu/io-pgtable-arm-v7s.c > @@ -717,7 +717,7 @@ static size_t arm_v7s_unmap(struct io_pgtable_ops *ops, unsigned long iova, > { > struct arm_v7s_io_pgtable *data = io_pgtable_ops_to_data(ops); > > - if (WARN_ON(upper_32_bits(iova))) > + if (WARN_ON(iova >= (1ULL << data->iop.cfg.ias))) > return 0; > > return __arm_v7s_unmap(data, gather, iova, size, 1, data->pgd); >
On 2020-11-11 12:38, Yong Wu wrote: > The current _ARM_V7S_LVL_BITS/ARM_V7S_LVL_SHIFT use a formula to calculate > the corresponding value for level1 and level2 to pretend the code sane. > Actually their level1 and level2 values are different from each other. > This patch only clear the two macro. No functional change. Grammar nit: to "clear" the macro sounds like you're making it empty or removing it entirely; I think you mean to say "clarify" here. English is the worst language sometimes... :) Reviewed-by: Robin Murphy <robin.murphy@arm.com> > Suggested-by: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/io-pgtable-arm-v7s.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c > index 4d0aa079470f..58cc201c10a3 100644 > --- a/drivers/iommu/io-pgtable-arm-v7s.c > +++ b/drivers/iommu/io-pgtable-arm-v7s.c > @@ -44,13 +44,11 @@ > > /* > * We have 32 bits total; 12 bits resolved at level 1, 8 bits at level 2, > - * and 12 bits in a page. With some carefully-chosen coefficients we can > - * hide the ugly inconsistencies behind these macros and at least let the > - * rest of the code pretend to be somewhat sane. > + * and 12 bits in a page. > */ > #define ARM_V7S_ADDR_BITS 32 > -#define _ARM_V7S_LVL_BITS(lvl) (16 - (lvl) * 4) > -#define ARM_V7S_LVL_SHIFT(lvl) (ARM_V7S_ADDR_BITS - (4 + 8 * (lvl))) > +#define _ARM_V7S_LVL_BITS(lvl) ((lvl) == 1 ? 12 : 8) > +#define ARM_V7S_LVL_SHIFT(lvl) ((lvl) == 1 ? 20 : 12) > #define ARM_V7S_TABLE_SHIFT 10 > > #define ARM_V7S_PTES_PER_LVL(lvl) (1 << _ARM_V7S_LVL_BITS(lvl)) >
On 2020-11-11 12:38, Yong Wu wrote: > The standard input iova bits is 32. MediaTek quad the lvl1 pagetable > (4 * lvl1). No change for lvl2 pagetable. Then the iova bits can reach > 34bit. Yay, I love how simple the actual change becomes now! Reviewed-by: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/io-pgtable-arm-v7s.c | 7 ++++--- > drivers/iommu/mtk_iommu.c | 2 +- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c > index 0b3c5b904ddc..5601dc8bf810 100644 > --- a/drivers/iommu/io-pgtable-arm-v7s.c > +++ b/drivers/iommu/io-pgtable-arm-v7s.c > @@ -45,9 +45,10 @@ > /* > * We have 32 bits total; 12 bits resolved at level 1, 8 bits at level 2, > * and 12 bits in a page. > + * MediaTek extend 2 bits to reach 34bits, 14 bits at lvl1 and 8 bits at lvl2. > */ > #define ARM_V7S_ADDR_BITS 32 > -#define _ARM_V7S_LVL_BITS(lvl, cfg) ((lvl) == 1 ? 12 : 8) > +#define _ARM_V7S_LVL_BITS(lvl, cfg) ((lvl) == 1 ? ((cfg)->ias - 20) : 8) > #define ARM_V7S_LVL_SHIFT(lvl) ((lvl) == 1 ? 20 : 12) > #define ARM_V7S_TABLE_SHIFT 10 > > @@ -61,7 +62,7 @@ > #define _ARM_V7S_IDX_MASK(lvl, cfg) (ARM_V7S_PTES_PER_LVL(lvl, cfg) - 1) > #define ARM_V7S_LVL_IDX(addr, lvl, cfg) ({ \ > int _l = lvl; \ > - ((u32)(addr) >> ARM_V7S_LVL_SHIFT(_l)) & _ARM_V7S_IDX_MASK(_l, cfg); \ > + ((addr) >> ARM_V7S_LVL_SHIFT(_l)) & _ARM_V7S_IDX_MASK(_l, cfg); \ > }) > > /* > @@ -754,7 +755,7 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg, > { > struct arm_v7s_io_pgtable *data; > > - if (cfg->ias > ARM_V7S_ADDR_BITS) > + if (cfg->ias > (arm_v7s_is_mtk_enabled(cfg) ? 34 : ARM_V7S_ADDR_BITS)) > return NULL; > > if (cfg->oas > (arm_v7s_is_mtk_enabled(cfg) ? 35 : ARM_V7S_ADDR_BITS)) > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index ec3c87d4b172..55f9b329e637 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -319,7 +319,7 @@ static int mtk_iommu_domain_finalise(struct mtk_iommu_domain *dom) > IO_PGTABLE_QUIRK_TLBI_ON_MAP | > IO_PGTABLE_QUIRK_ARM_MTK_EXT, > .pgsize_bitmap = mtk_iommu_ops.pgsize_bitmap, > - .ias = 32, > + .ias = 34, > .oas = 35, > .tlb = &mtk_iommu_flush_ops, > .iommu_dev = data->dev, >
On Thu, 2020-11-26 at 16:03 +0000, Robin Murphy wrote: > On 2020-11-11 12:38, Yong Wu wrote: > > The current _ARM_V7S_LVL_BITS/ARM_V7S_LVL_SHIFT use a formula to calculate > > the corresponding value for level1 and level2 to pretend the code sane. > > Actually their level1 and level2 values are different from each other. > > This patch only clear the two macro. No functional change. > > Grammar nit: to "clear" the macro sounds like you're making it empty or > removing it entirely; I think you mean to say "clarify" here. English is > the worst language sometimes... :) Thanks for the review. Feel free to tell me if some words is not fit:) I will use "clarify" in the title. > > Reviewed-by: Robin Murphy <robin.murphy@arm.com> > > > Suggested-by: Robin Murphy <robin.murphy@arm.com> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > drivers/iommu/io-pgtable-arm-v7s.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c > > index 4d0aa079470f..58cc201c10a3 100644 > > --- a/drivers/iommu/io-pgtable-arm-v7s.c > > +++ b/drivers/iommu/io-pgtable-arm-v7s.c > > @@ -44,13 +44,11 @@ > > > > /* > > * We have 32 bits total; 12 bits resolved at level 1, 8 bits at level 2, > > - * and 12 bits in a page. With some carefully-chosen coefficients we can > > - * hide the ugly inconsistencies behind these macros and at least let the > > - * rest of the code pretend to be somewhat sane. > > + * and 12 bits in a page. > > */ > > #define ARM_V7S_ADDR_BITS 32 > > -#define _ARM_V7S_LVL_BITS(lvl) (16 - (lvl) * 4) > > -#define ARM_V7S_LVL_SHIFT(lvl) (ARM_V7S_ADDR_BITS - (4 + 8 * (lvl))) > > +#define _ARM_V7S_LVL_BITS(lvl) ((lvl) == 1 ? 12 : 8) > > +#define ARM_V7S_LVL_SHIFT(lvl) ((lvl) == 1 ? 20 : 12) > > #define ARM_V7S_TABLE_SHIFT 10 > > > > #define ARM_V7S_PTES_PER_LVL(lvl) (1 << _ARM_V7S_LVL_BITS(lvl)) > >
This patch mainly adds support for mt8192 Multimedia IOMMU and SMI. mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation table format. The M4U-SMI HW diagram is as below: EMI | M4U | ------------ SMI Common ------------ | +-------+------+------+----------------------+-------+ | | | | ...... | | | | | | | | larb0 larb1 larb2 larb4 ...... larb19 larb20 disp0 disp1 mdp vdec IPE IPE All the connections are HW fixed, SW can NOT adjust it. Comparing with the preview SoC, this patchset mainly adds two new functions: a) add iova 34 bits support. b) add multi domains support since several HW has the special iova region requirement. change note: v4: a) rebase on v5.10-rc1 b) Move the smi part to a independent patchset. c) Improve v7s code from Robin and Will. d) Add a mediatek iommu entry patch in MAINTAINERS. v3: https://lore.kernel.org/linux-iommu/20200930070647.10188-1-yong.wu@mediatek.com/ a) Fix DT schema issue commented from Rob. b) Fix a v7s issue. Use "_lvl" instead of "_l" in the macro(ARM_V7S_PTES_PER_LVL) since it is called in ARM_V7S_LVL_IDX which has already used "_l". c) Fix a PM suspend issue: Avoid pm suspend in pm runtime case. v2: https://lore.kernel.org/linux-iommu/20200905080920.13396-1-yong.wu@mediatek.com/ a) Convert IOMMU/SMI dt-binding to DT schema. b) Fix some comment from Pi-Hsun and Nicolas. like use generic_iommu_put_resv_regions. c) Reword some comment, like add how to use domain-id. v1: https://lore.kernel.org/linux-iommu/20200711064846.16007-1-yong.wu@mediatek.com/ Yong Wu (24): dt-bindings: iommu: mediatek: Convert IOMMU to DT schema dt-bindings: memory: mediatek: Add a common larb-port header file dt-bindings: memory: mediatek: Extend LARB_NR_MAX to 32 dt-bindings: memory: mediatek: Add domain definition dt-bindings: mediatek: Add binding for mt8192 IOMMU iommu/mediatek: Use the common mtk-smi-larb-port.h iommu/io-pgtable-arm-v7s: Use ias to check the valid iova in unmap iommu/io-pgtable-arm-v7s: Extend PA34 for MediaTek iommu/io-pgtable-arm-v7s: Clear LVL_SHIFT/BITS macro instead of the formula iommu/io-pgtable-arm-v7s: Add cfg as a param in some macros iommu/io-pgtable-arm-v7s: Quad lvl1 pgtable for MediaTek iommu/mediatek: Move hw_init into attach_device iommu/mediatek: Add device link for smi-common and m4u iommu/mediatek: Add pm runtime callback iommu/mediatek: Add power-domain operation iommu/mediatek: Add iova reserved function iommu/mediatek: Add single domain iommu/mediatek: Support master use iova over 32bit iommu/mediatek: Support up to 34bit iova in tlb flush iommu/mediatek: Support report iova 34bit translation fault in ISR iommu/mediatek: Add support for multi domain iommu/mediatek: Adjust the structure iommu/mediatek: Add mt8192 support MAINTAINERS: Add entry for MediaTek IOMMU .../bindings/iommu/mediatek,iommu.txt | 105 ------- .../bindings/iommu/mediatek,iommu.yaml | 183 ++++++++++++ MAINTAINERS | 9 + drivers/iommu/io-pgtable-arm-v7s.c | 56 ++-- drivers/iommu/mtk_iommu.c | 271 +++++++++++++++--- drivers/iommu/mtk_iommu.h | 11 +- drivers/memory/mtk-smi.c | 8 + include/dt-bindings/memory/mt2712-larb-port.h | 2 +- include/dt-bindings/memory/mt6779-larb-port.h | 2 +- include/dt-bindings/memory/mt8167-larb-port.h | 2 +- include/dt-bindings/memory/mt8173-larb-port.h | 2 +- include/dt-bindings/memory/mt8183-larb-port.h | 2 +- include/dt-bindings/memory/mt8192-larb-port.h | 240 ++++++++++++++++ .../dt-bindings/memory/mtk-smi-larb-port.h | 22 ++ include/linux/io-pgtable.h | 4 +- include/soc/mediatek/smi.h | 3 +- 16 files changed, 735 insertions(+), 187 deletions(-) delete mode 100644 Documentation/devicetree/bindings/iommu/mediatek,iommu.txt create mode 100644 Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h create mode 100644 include/dt-bindings/memory/mtk-smi-larb-port.h