Message ID | 20210111111914.22211-1-yong.wu@mediatek.com |
---|---|
Headers | show |
Series | MT8192 IOMMU support | expand |
On Mon, Jan 11, 2021 at 07:18:48PM +0800, Yong Wu wrote: > If group->default_domain exists, avoid reallocate it. > > In some iommu drivers, there may be several devices share a group. Avoid > realloc the default domain for this case. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/iommu.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 3d099a31ddca..f4b87e6abe80 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -266,7 +266,8 @@ int iommu_probe_device(struct device *dev) > * support default domains, so the return value is not yet > * checked. > */ > - iommu_alloc_default_domain(group, dev); > + if (!group->default_domain) > + iommu_alloc_default_domain(group, dev); I don't really get what this achieves, since iommu_alloc_default_domain() looks like this: static int iommu_alloc_default_domain(struct iommu_group *group, struct device *dev) { unsigned int type; if (group->default_domain) return 0; ... in which case, it should be fine? Will
On Mon, Jan 11, 2021 at 07:18:41PM +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. This is looking good and I'd really like to see it merged, especially as it has changes to the io-pgtable code. Please could you post a new version ASAP to address the comments on patches 6 and 7? Will
On Tue, 2021-01-26 at 22:23 +0000, Will Deacon wrote: > On Mon, Jan 11, 2021 at 07:18:48PM +0800, Yong Wu wrote: > > If group->default_domain exists, avoid reallocate it. > > > > In some iommu drivers, there may be several devices share a group. Avoid > > realloc the default domain for this case. > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > drivers/iommu/iommu.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > index 3d099a31ddca..f4b87e6abe80 100644 > > --- a/drivers/iommu/iommu.c > > +++ b/drivers/iommu/iommu.c > > @@ -266,7 +266,8 @@ int iommu_probe_device(struct device *dev) > > * support default domains, so the return value is not yet > > * checked. > > */ > > - iommu_alloc_default_domain(group, dev); > > + if (!group->default_domain) > > + iommu_alloc_default_domain(group, dev); > > I don't really get what this achieves, since iommu_alloc_default_domain() > looks like this: > > static int iommu_alloc_default_domain(struct iommu_group *group, > struct device *dev) > { > unsigned int type; > > if (group->default_domain) > return 0; > > ... > > in which case, it should be fine? oh. sorry, I overlooked this. the current code is enough. I will remove this patch. and send the next version in this week. Thanks very much. > > Will
On Wed, Jan 27, 2021 at 05:39:16PM +0800, Yong Wu wrote: > On Tue, 2021-01-26 at 22:23 +0000, Will Deacon wrote: > > On Mon, Jan 11, 2021 at 07:18:48PM +0800, Yong Wu wrote: > > > If group->default_domain exists, avoid reallocate it. > > > > > > In some iommu drivers, there may be several devices share a group. Avoid > > > realloc the default domain for this case. > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > > --- > > > drivers/iommu/iommu.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > > index 3d099a31ddca..f4b87e6abe80 100644 > > > --- a/drivers/iommu/iommu.c > > > +++ b/drivers/iommu/iommu.c > > > @@ -266,7 +266,8 @@ int iommu_probe_device(struct device *dev) > > > * support default domains, so the return value is not yet > > > * checked. > > > */ > > > - iommu_alloc_default_domain(group, dev); > > > + if (!group->default_domain) > > > + iommu_alloc_default_domain(group, dev); > > > > I don't really get what this achieves, since iommu_alloc_default_domain() > > looks like this: > > > > static int iommu_alloc_default_domain(struct iommu_group *group, > > struct device *dev) > > { > > unsigned int type; > > > > if (group->default_domain) > > return 0; > > > > ... > > > > in which case, it should be fine? > > oh. sorry, I overlooked this. the current code is enough. > I will remove this patch. and send the next version in this week. > Thanks very much. Actually, looking at this again, if we're dropping this patch and patch 6 just needs the kfree() moving about, then there's no need to repost. The issue that Robin and Paul are discussing can be handled separately. Will
On Thu, Jan 28, 2021 at 09:10:20PM +0000, Will Deacon wrote: > On Wed, Jan 27, 2021 at 05:39:16PM +0800, Yong Wu wrote: > > On Tue, 2021-01-26 at 22:23 +0000, Will Deacon wrote: > > > On Mon, Jan 11, 2021 at 07:18:48PM +0800, Yong Wu wrote: > > > > If group->default_domain exists, avoid reallocate it. > > > > > > > > In some iommu drivers, there may be several devices share a group. Avoid > > > > realloc the default domain for this case. > > > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > > > --- > > > > drivers/iommu/iommu.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > > > index 3d099a31ddca..f4b87e6abe80 100644 > > > > --- a/drivers/iommu/iommu.c > > > > +++ b/drivers/iommu/iommu.c > > > > @@ -266,7 +266,8 @@ int iommu_probe_device(struct device *dev) > > > > * support default domains, so the return value is not yet > > > > * checked. > > > > */ > > > > - iommu_alloc_default_domain(group, dev); > > > > + if (!group->default_domain) > > > > + iommu_alloc_default_domain(group, dev); > > > > > > I don't really get what this achieves, since iommu_alloc_default_domain() > > > looks like this: > > > > > > static int iommu_alloc_default_domain(struct iommu_group *group, > > > struct device *dev) > > > { > > > unsigned int type; > > > > > > if (group->default_domain) > > > return 0; > > > > > > ... > > > > > > in which case, it should be fine? > > > > oh. sorry, I overlooked this. the current code is enough. > > I will remove this patch. and send the next version in this week. > > Thanks very much. > > Actually, looking at this again, if we're dropping this patch and patch 6 > just needs the kfree() moving about, then there's no need to repost. The > issue that Robin and Paul are discussing can be handled separately. Argh, except that this version of the patches doesn't apply :) So after all that, please go ahead and post a v7 ASAP based on this branch: https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/mtk Will
On 2021-01-28 21:14, Will Deacon wrote: > On Thu, Jan 28, 2021 at 09:10:20PM +0000, Will Deacon wrote: >> On Wed, Jan 27, 2021 at 05:39:16PM +0800, Yong Wu wrote: >>> On Tue, 2021-01-26 at 22:23 +0000, Will Deacon wrote: >>>> On Mon, Jan 11, 2021 at 07:18:48PM +0800, Yong Wu wrote: >>>>> If group->default_domain exists, avoid reallocate it. >>>>> >>>>> In some iommu drivers, there may be several devices share a group. Avoid >>>>> realloc the default domain for this case. >>>>> >>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> >>>>> --- >>>>> drivers/iommu/iommu.c | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >>>>> index 3d099a31ddca..f4b87e6abe80 100644 >>>>> --- a/drivers/iommu/iommu.c >>>>> +++ b/drivers/iommu/iommu.c >>>>> @@ -266,7 +266,8 @@ int iommu_probe_device(struct device *dev) >>>>> * support default domains, so the return value is not yet >>>>> * checked. >>>>> */ >>>>> - iommu_alloc_default_domain(group, dev); >>>>> + if (!group->default_domain) >>>>> + iommu_alloc_default_domain(group, dev); >>>> >>>> I don't really get what this achieves, since iommu_alloc_default_domain() >>>> looks like this: >>>> >>>> static int iommu_alloc_default_domain(struct iommu_group *group, >>>> struct device *dev) >>>> { >>>> unsigned int type; >>>> >>>> if (group->default_domain) >>>> return 0; >>>> >>>> ... >>>> >>>> in which case, it should be fine? >>> >>> oh. sorry, I overlooked this. the current code is enough. >>> I will remove this patch. and send the next version in this week. >>> Thanks very much. >> >> Actually, looking at this again, if we're dropping this patch and patch 6 >> just needs the kfree() moving about, then there's no need to repost. The >> issue that Robin and Paul are discussing can be handled separately. FWIW patch #6 gets dropped as well now, since Rob has applied the standalone fix (89c7cb1608ac). Robin. > Argh, except that this version of the patches doesn't apply :) > > So after all that, please go ahead and post a v7 ASAP based on this branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/mtk > > Will >
On Thu, 2021-01-28 at 21:14 +0000, Will Deacon wrote: > On Thu, Jan 28, 2021 at 09:10:20PM +0000, Will Deacon wrote: > > On Wed, Jan 27, 2021 at 05:39:16PM +0800, Yong Wu wrote: > > > On Tue, 2021-01-26 at 22:23 +0000, Will Deacon wrote: > > > > On Mon, Jan 11, 2021 at 07:18:48PM +0800, Yong Wu wrote: > > > > > If group->default_domain exists, avoid reallocate it. > > > > > > > > > > In some iommu drivers, there may be several devices share a group. Avoid > > > > > realloc the default domain for this case. > > > > > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > > > > --- > > > > > drivers/iommu/iommu.c | 3 ++- > > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > > > > index 3d099a31ddca..f4b87e6abe80 100644 > > > > > --- a/drivers/iommu/iommu.c > > > > > +++ b/drivers/iommu/iommu.c > > > > > @@ -266,7 +266,8 @@ int iommu_probe_device(struct device *dev) > > > > > * support default domains, so the return value is not yet > > > > > * checked. > > > > > */ > > > > > - iommu_alloc_default_domain(group, dev); > > > > > + if (!group->default_domain) > > > > > + iommu_alloc_default_domain(group, dev); > > > > > > > > I don't really get what this achieves, since iommu_alloc_default_domain() > > > > looks like this: > > > > > > > > static int iommu_alloc_default_domain(struct iommu_group *group, > > > > struct device *dev) > > > > { > > > > unsigned int type; > > > > > > > > if (group->default_domain) > > > > return 0; > > > > > > > > ... > > > > > > > > in which case, it should be fine? > > > > > > oh. sorry, I overlooked this. the current code is enough. > > > I will remove this patch. and send the next version in this week. > > > Thanks very much. > > > > Actually, looking at this again, if we're dropping this patch and patch 6 > > just needs the kfree() moving about, then there's no need to repost. The > > issue that Robin and Paul are discussing can be handled separately. > > Argh, except that this version of the patches doesn't apply :) > > So after all that, please go ahead and post a v7 ASAP based on this branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/mtk After confirm with Tomasz, He still need some time to take a look at v6. thus I need wait some time to send v7 after his feedback. Thanks for your comment. and Thanks Tomasz for the review. > > Will
On Mon, Jan 11, 2021 at 07:18:41PM +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: > v6:a) base on v5.11-rc1. and tlb v4: > https://lore.kernel.org/linux-mediatek/20210107122909.16317-1-yong.wu@mediatek.com/T/#t I've queued this up apart from patches 6 and 7. Thanks, Will
On Tue, Feb 02, 2021 at 10:03:45AM +0800, Yong Wu wrote: > On Mon, 2021-02-01 at 14:54 +0000, Will Deacon wrote: > > On Mon, Jan 11, 2021 at 07:18:41PM +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: > > > v6:a) base on v5.11-rc1. and tlb v4: > > > https://lore.kernel.org/linux-mediatek/20210107122909.16317-1-yong.wu@mediatek.com/T/#t > > > > I've queued this up apart from patches 6 and 7. > > Thanks very much for the applying. I'd like to show there is a little > conflict with a smi change[1] in /include/soc/mediatek/smi.h. > > This is the detailed conflict: > > --- a/include/soc/mediatek/smi.h > +++ b/include/soc/mediatek/smi.h > @@ -9,7 +9,7 @@ > #include <linux/bitops.h> > #include <linux/device.h> > > -#ifdef CONFIG_MTK_SMI > +#if IS_ENABLED(CONFIG_MTK_SMI) <---The smi patch change here. > > #define MTK_LARB_NR_MAX 16 <---This iommu patchset delete this line. > > > This code is simple. Please feel free to tell me how to do this if this > is not convenient to merge. Thanks, but this should be trivial to resolve, so I don't think we need to worry about it. Will