Re: [PATCH] drm/tegra: Stop using iommu_present()

[Robin Murphy <robin.murphy@arm.com>]

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.