* [PATCH] drm/tegra: Stop using iommu_present()
@ 2022-04-05 14:19 Robin Murphy
2022-04-06 14:32 ` Dmitry Osipenko
0 siblings, 1 reply; 5+ messages in thread
From: Robin Murphy @ 2022-04-05 14:19 UTC (permalink / raw)
To: thierry.reding; +Cc: linux-tegra, iommu, dri-devel, jonathanh
Remove the pointless check. host1x_drm_wants_iommu() cannot return true
unless an IOMMU exists for the host1x platform device, which at the moment
means the iommu_present() test could never fail.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/gpu/drm/tegra/drm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 9464f522e257..bc4321561400 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1149,7 +1149,7 @@ static int host1x_drm_probe(struct host1x_device *dev)
goto put;
}
- if (host1x_drm_wants_iommu(dev) && iommu_present(&platform_bus_type)) {
+ if (host1x_drm_wants_iommu(dev)) {
tegra->domain = iommu_domain_alloc(&platform_bus_type);
if (!tegra->domain) {
err = -ENOMEM;
--
2.28.0.dirty
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/tegra: Stop using iommu_present()
2022-04-05 14:19 [PATCH] drm/tegra: Stop using iommu_present() Robin Murphy
@ 2022-04-06 14:32 ` Dmitry Osipenko
2022-04-06 18:06 ` Robin Murphy
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Osipenko @ 2022-04-06 14:32 UTC (permalink / raw)
To: Robin Murphy, thierry.reding; +Cc: linux-tegra, iommu, dri-devel, jonathanh
On 4/5/22 17:19, Robin Murphy wrote:
> Remove the pointless check. host1x_drm_wants_iommu() cannot return true
> unless an IOMMU exists for the host1x platform device, which at the moment
> means the iommu_present() test could never fail.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
> drivers/gpu/drm/tegra/drm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 9464f522e257..bc4321561400 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -1149,7 +1149,7 @@ static int host1x_drm_probe(struct host1x_device *dev)
> goto put;
> }
>
> - if (host1x_drm_wants_iommu(dev) && iommu_present(&platform_bus_type)) {
> + if (host1x_drm_wants_iommu(dev)) {
> tegra->domain = iommu_domain_alloc(&platform_bus_type);
> if (!tegra->domain) {
> err = -ENOMEM;
host1x_drm_wants_iommu() returns true if there is no IOMMU for the
host1x platform device of Tegra20/30 SoCs.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/tegra: Stop using iommu_present()
2022-04-06 14:32 ` Dmitry Osipenko
@ 2022-04-06 18:06 ` Robin Murphy
2022-04-07 17:51 ` Dmitry Osipenko
0 siblings, 1 reply; 5+ messages in thread
From: Robin Murphy @ 2022-04-06 18:06 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding; +Cc: linux-tegra, iommu, dri-devel, jonathanh
On 2022-04-06 15:32, Dmitry Osipenko wrote:
> On 4/5/22 17:19, Robin Murphy wrote:
>> Remove the pointless check. host1x_drm_wants_iommu() cannot return true
>> unless an IOMMU exists for the host1x platform device, which at the moment
>> means the iommu_present() test could never fail.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>> drivers/gpu/drm/tegra/drm.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
>> index 9464f522e257..bc4321561400 100644
>> --- a/drivers/gpu/drm/tegra/drm.c
>> +++ b/drivers/gpu/drm/tegra/drm.c
>> @@ -1149,7 +1149,7 @@ static int host1x_drm_probe(struct host1x_device *dev)
>> goto put;
>> }
>>
>> - if (host1x_drm_wants_iommu(dev) && iommu_present(&platform_bus_type)) {
>> + if (host1x_drm_wants_iommu(dev)) {
>> tegra->domain = iommu_domain_alloc(&platform_bus_type);
>> if (!tegra->domain) {
>> err = -ENOMEM;
>
> host1x_drm_wants_iommu() returns true if there is no IOMMU for the
> host1x platform device of Tegra20/30 SoCs.
Ah, apparently this is another example of what happens when I write
patches late on a Friday night...
So on second look, what we want to ascertain here is whether dev has an
IOMMU, but only if the host1x parent is not addressing-limited, either
because it can also use the IOMMU, or because all possible addresses are
small enough anyway, right? Are we specifically looking for the host1x
having a DMA-API-managed domain, or can that also end up using the
tegra->domain or another unmanaged domain too? I can't quite figure out
from the comments whether it's physical addresses, IOVAs, or both that
we're concerned with here.
Thanks,
Robin.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/tegra: Stop using iommu_present()
2022-04-06 18:06 ` Robin Murphy
@ 2022-04-07 17:51 ` Dmitry Osipenko
2022-04-08 17:53 ` Robin Murphy
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Osipenko @ 2022-04-07 17:51 UTC (permalink / raw)
To: Robin Murphy, thierry.reding; +Cc: linux-tegra, iommu, dri-devel, jonathanh
On 4/6/22 21:06, Robin Murphy wrote:
> On 2022-04-06 15:32, Dmitry Osipenko wrote:
>> On 4/5/22 17:19, Robin Murphy wrote:
>>> Remove the pointless check. host1x_drm_wants_iommu() cannot return true
>>> unless an IOMMU exists for the host1x platform device, which at the
>>> moment
>>> means the iommu_present() test could never fail.
>>>
>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>> ---
>>> drivers/gpu/drm/tegra/drm.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
>>> index 9464f522e257..bc4321561400 100644
>>> --- a/drivers/gpu/drm/tegra/drm.c
>>> +++ b/drivers/gpu/drm/tegra/drm.c
>>> @@ -1149,7 +1149,7 @@ static int host1x_drm_probe(struct
>>> host1x_device *dev)
>>> goto put;
>>> }
>>> - if (host1x_drm_wants_iommu(dev) &&
>>> iommu_present(&platform_bus_type)) {
>>> + if (host1x_drm_wants_iommu(dev)) {
>>> tegra->domain = iommu_domain_alloc(&platform_bus_type);
>>> if (!tegra->domain) {
>>> err = -ENOMEM;
>>
>> host1x_drm_wants_iommu() returns true if there is no IOMMU for the
>> host1x platform device of Tegra20/30 SoCs.
>
> Ah, apparently this is another example of what happens when I write
> patches late on a Friday night...
>
> So on second look, what we want to ascertain here is whether dev has an
> IOMMU, but only if the host1x parent is not addressing-limited, either
> because it can also use the IOMMU, or because all possible addresses are
> small enough anyway, right?
Yes
> Are we specifically looking for the host1x
> having a DMA-API-managed domain, or can that also end up using the
> tegra->domain or another unmanaged domain too?
We have host1x DMA that could have:
1. No IOMMU domain, depending on kernel/DT config
2. Managed domain, on newer SoCs
3. Unmanaged domain, on older SoCs
We have Tegra DRM devices which can:
1. Be attached to a shared unmanaged tegra->domain, on older SoCs
2. Have own managed domains, on newer SoCs
> I can't quite figure out
> from the comments whether it's physical addresses, IOVAs, or both that
> we're concerned with here.
Tegra DRM allocates buffers and submits jobs to h/w using host1x's
channel DMA. DRM framebuffers' addresses are inserted into host1x
command buffers by kernel driver and addresses beyond 32bit space need
to be treated specially, we don't support such addresses in upstream.
IOMMU AS is limited to 32bits on Tegra in upstream kernel for pre-T186
SoCs, it hides 64bit addresses from host1x. Post-T186 SoCs have extra
features that allow kernel driver not to bother about addresses.
For newer ARM64 SoCs there is assumption in the Tegra drivers that IOMMU
always presents, to simplify things.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/tegra: Stop using iommu_present()
2022-04-07 17:51 ` Dmitry Osipenko
@ 2022-04-08 17:53 ` Robin Murphy
0 siblings, 0 replies; 5+ messages in thread
From: Robin Murphy @ 2022-04-08 17:53 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding; +Cc: linux-tegra, iommu, dri-devel, jonathanh
On 2022-04-07 18:51, Dmitry Osipenko wrote:
> On 4/6/22 21:06, Robin Murphy wrote:
>> On 2022-04-06 15:32, Dmitry Osipenko wrote:
>>> On 4/5/22 17:19, Robin Murphy wrote:
>>>> Remove the pointless check. host1x_drm_wants_iommu() cannot return true
>>>> unless an IOMMU exists for the host1x platform device, which at the
>>>> moment
>>>> means the iommu_present() test could never fail.
>>>>
>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>>> ---
>>>> drivers/gpu/drm/tegra/drm.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
>>>> index 9464f522e257..bc4321561400 100644
>>>> --- a/drivers/gpu/drm/tegra/drm.c
>>>> +++ b/drivers/gpu/drm/tegra/drm.c
>>>> @@ -1149,7 +1149,7 @@ static int host1x_drm_probe(struct
>>>> host1x_device *dev)
>>>> goto put;
>>>> }
>>>> - if (host1x_drm_wants_iommu(dev) &&
>>>> iommu_present(&platform_bus_type)) {
>>>> + if (host1x_drm_wants_iommu(dev)) {
>>>> tegra->domain = iommu_domain_alloc(&platform_bus_type);
>>>> if (!tegra->domain) {
>>>> err = -ENOMEM;
>>>
>>> host1x_drm_wants_iommu() returns true if there is no IOMMU for the
>>> host1x platform device of Tegra20/30 SoCs.
>>
>> Ah, apparently this is another example of what happens when I write
>> patches late on a Friday night...
>>
>> So on second look, what we want to ascertain here is whether dev has an
>> IOMMU, but only if the host1x parent is not addressing-limited, either
>> because it can also use the IOMMU, or because all possible addresses are
>> small enough anyway, right?
>
> Yes
>
>> Are we specifically looking for the host1x
>> having a DMA-API-managed domain, or can that also end up using the
>> tegra->domain or another unmanaged domain too?
>
> We have host1x DMA that could have:
>
> 1. No IOMMU domain, depending on kernel/DT config
> 2. Managed domain, on newer SoCs
> 3. Unmanaged domain, on older SoCs
>
> We have Tegra DRM devices which can:
>
> 1. Be attached to a shared unmanaged tegra->domain, on older SoCs
> 2. Have own managed domains, on newer SoCs
>
>> I can't quite figure out
>> from the comments whether it's physical addresses, IOVAs, or both that
>> we're concerned with here.
>
> Tegra DRM allocates buffers and submits jobs to h/w using host1x's
> channel DMA. DRM framebuffers' addresses are inserted into host1x
> command buffers by kernel driver and addresses beyond 32bit space need
> to be treated specially, we don't support such addresses in upstream.
>
> IOMMU AS is limited to 32bits on Tegra in upstream kernel for pre-T186
> SoCs, it hides 64bit addresses from host1x. Post-T186 SoCs have extra
> features that allow kernel driver not to bother about addresses.
>
> For newer ARM64 SoCs there is assumption in the Tegra drivers that IOMMU
> always presents, to simplify things.
That summary helps a lot, thank you!
I was particularly worried about the case where the host1x has a
passthrough domain, which we'll assume is a DMA domain and leave in
place, but if all the SoCs with the 32-bit gather limitation are also
the ones with tegra-smmu, which doesn't support default domains anyway,
then it sounds like that's a non-issue.
I'll give this a bit more thought to make sure I really get it right,
and send a v2 next week.
Thanks,
Robin.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-04-08 17:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-05 14:19 [PATCH] drm/tegra: Stop using iommu_present() Robin Murphy
2022-04-06 14:32 ` Dmitry Osipenko
2022-04-06 18:06 ` Robin Murphy
2022-04-07 17:51 ` Dmitry Osipenko
2022-04-08 17:53 ` Robin Murphy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).