[PATCH] gpu: host1x: Avoid trying to use GART on Tegra20

[PATCH] gpu: host1x: Avoid trying to use GART on Tegra20

[Robin Murphy]

Since commit c7e3ca515e78 ("iommu/tegra: gart: Do not register with
bus") quite some time ago, the GART driver has effectively disabled
itself to avoid issues with the GPU driver expecting it to work in ways
that it doesn't. As of commit 57365a04c921 ("iommu: Move bus setup to
IOMMU device registration") that bodge no longer works, but really the
GPU driver should be responsible for its own behaviour anyway. Make the
workaround explicit.

Reported-by: Jon Hunter <jonathanh@nvidia.com>
Suggested-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/gpu/host1x/dev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index a13fd9441edc..1cae8eea92cf 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -352,6 +352,10 @@ static struct iommu_domain *host1x_iommu_attach(struct host1x *host)
 	if (!host1x_wants_iommu(host) || domain)
 		return domain;
 
+	/* Our IOMMU usage policy doesn't currently play well with GART */
+	if (of_machine_is_compatible("nvidia,tegra20"))
+		return NULL;
+
 	host->group = iommu_group_get(host->dev);
 	if (host->group) {
 		struct iommu_domain_geometry *geometry;
-- 
2.36.1.dirty

Re: [PATCH] gpu: host1x: Avoid trying to use GART on Tegra20

[Jon Hunter]

Hi Robin,

On 19/10/2022 18:23, Robin Murphy wrote:
> Since commit c7e3ca515e78 ("iommu/tegra: gart: Do not register with
> bus") quite some time ago, the GART driver has effectively disabled
> itself to avoid issues with the GPU driver expecting it to work in ways
> that it doesn't. As of commit 57365a04c921 ("iommu: Move bus setup to
> IOMMU device registration") that bodge no longer works, but really the
> GPU driver should be responsible for its own behaviour anyway. Make the
> workaround explicit.
> 
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Suggested-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>   drivers/gpu/host1x/dev.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
> index a13fd9441edc..1cae8eea92cf 100644
> --- a/drivers/gpu/host1x/dev.c
> +++ b/drivers/gpu/host1x/dev.c
> @@ -352,6 +352,10 @@ static struct iommu_domain *host1x_iommu_attach(struct host1x *host)
>   	if (!host1x_wants_iommu(host) || domain)
>   		return domain;
>   
> +	/* Our IOMMU usage policy doesn't currently play well with GART */
> +	if (of_machine_is_compatible("nvidia,tegra20"))
> +		return NULL;
> +
>   	host->group = iommu_group_get(host->dev);
>   	if (host->group) {
>   		struct iommu_domain_geometry *geometry;


Thanks for sending. I gave this a quick test, but I still see ...

[    2.901739] tegra-gr2d 54140000.gr2d: failed to attach to domain: -19
[    2.908373] drm drm: failed to initialize 54140000.gr2d: -19

Cheers
Jon

-- 
nvpublic

Re: [PATCH] gpu: host1x: Avoid trying to use GART on Tegra20

[Robin Murphy]

On 2022-10-20 13:25, Jon Hunter wrote:
> Hi Robin,
> 
> On 19/10/2022 18:23, Robin Murphy wrote:
>> Since commit c7e3ca515e78 ("iommu/tegra: gart: Do not register with
>> bus") quite some time ago, the GART driver has effectively disabled
>> itself to avoid issues with the GPU driver expecting it to work in ways
>> that it doesn't. As of commit 57365a04c921 ("iommu: Move bus setup to
>> IOMMU device registration") that bodge no longer works, but really the
>> GPU driver should be responsible for its own behaviour anyway. Make the
>> workaround explicit.
>>
>> Reported-by: Jon Hunter <jonathanh@nvidia.com>
>> Suggested-by: Dmitry Osipenko <digetx@gmail.com>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   drivers/gpu/host1x/dev.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
>> index a13fd9441edc..1cae8eea92cf 100644
>> --- a/drivers/gpu/host1x/dev.c
>> +++ b/drivers/gpu/host1x/dev.c
>> @@ -352,6 +352,10 @@ static struct iommu_domain 
>> *host1x_iommu_attach(struct host1x *host)
>>       if (!host1x_wants_iommu(host) || domain)
>>           return domain;
>> +    /* Our IOMMU usage policy doesn't currently play well with GART */
>> +    if (of_machine_is_compatible("nvidia,tegra20"))
>> +        return NULL;
>> +
>>       host->group = iommu_group_get(host->dev);
>>       if (host->group) {
>>           struct iommu_domain_geometry *geometry;
> 
> 
> Thanks for sending. I gave this a quick test, but I still see ...
> 
> [    2.901739] tegra-gr2d 54140000.gr2d: failed to attach to domain: -19
> [    2.908373] drm drm: failed to initialize 54140000.gr2d: -19

Urgh, of course it's the same-but-different logic in host1x_drm_probe() 
that matters for that one. Am I allowed to mention how much these 
drivers drive me to despair?

v2 coming soon...

Thanks,
Robin.

Re: [PATCH] gpu: host1x: Avoid trying to use GART on Tegra20

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3160 bytes --]

On Thu, Oct 20, 2022 at 01:42:28PM +0100, Robin Murphy wrote:
> On 2022-10-20 13:25, Jon Hunter wrote:
> > Hi Robin,
> > 
> > On 19/10/2022 18:23, Robin Murphy wrote:
> > > Since commit c7e3ca515e78 ("iommu/tegra: gart: Do not register with
> > > bus") quite some time ago, the GART driver has effectively disabled
> > > itself to avoid issues with the GPU driver expecting it to work in ways
> > > that it doesn't. As of commit 57365a04c921 ("iommu: Move bus setup to
> > > IOMMU device registration") that bodge no longer works, but really the
> > > GPU driver should be responsible for its own behaviour anyway. Make the
> > > workaround explicit.
> > > 
> > > Reported-by: Jon Hunter <jonathanh@nvidia.com>
> > > Suggested-by: Dmitry Osipenko <digetx@gmail.com>
> > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > > ---
> > >   drivers/gpu/host1x/dev.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
> > > index a13fd9441edc..1cae8eea92cf 100644
> > > --- a/drivers/gpu/host1x/dev.c
> > > +++ b/drivers/gpu/host1x/dev.c
> > > @@ -352,6 +352,10 @@ static struct iommu_domain
> > > *host1x_iommu_attach(struct host1x *host)
> > >       if (!host1x_wants_iommu(host) || domain)
> > >           return domain;
> > > +    /* Our IOMMU usage policy doesn't currently play well with GART */
> > > +    if (of_machine_is_compatible("nvidia,tegra20"))
> > > +        return NULL;
> > > +
> > >       host->group = iommu_group_get(host->dev);
> > >       if (host->group) {
> > >           struct iommu_domain_geometry *geometry;
> > 
> > 
> > Thanks for sending. I gave this a quick test, but I still see ...
> > 
> > [    2.901739] tegra-gr2d 54140000.gr2d: failed to attach to domain: -19
> > [    2.908373] drm drm: failed to initialize 54140000.gr2d: -19
> 
> Urgh, of course it's the same-but-different logic in host1x_drm_probe() that
> matters for that one. Am I allowed to mention how much these drivers drive
> me to despair?

Yeah, the whole situation around these drivers has been difficult over
the years. In fact there's a current regression in certain corner cases
because some the IOMMU internals keep changing.

I'm not sure if anybody else (or why nobody else) is seeing similar
issues. Perhaps we're the only users left where direct IOMMU API is
used. Or used in a problematic way, which is primarily due to a need to
share IOMMU domains on certain older devices where they are very scarce.

Frankly, this whole mess has been going on for so long that I no longer
remember why we never switched to using the DMA API exclusively. It may
be because we don't support those DMA domains on 32-bit ARM (or on pre-
Tegra186, more generally), which in turn is due to the lack of early
direct mapping support. The iommu-addresses series is meant to fix that
on newer chips. On older chips we likely won't be able to do anything on
that front anymore, but there may be a way to hack something up in the
tegra-smmu driver to avoid faults...

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]