Re: [PATCH 3/9] drm/tegra: gem: Remove premature import restrictions

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3046 bytes --]

On Fri, Nov 29, 2019 at 10:12:44AM +0100, Daniel Vetter wrote:
> On Thu, Nov 28, 2019 at 04:37:35PM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > It's not known at import time whether or not all users of a DMA-BUF will
> > be able to deal with non-contiguous memory. Each user needs to verify at
> > map-time whether it can access the buffer.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> 
> I'm not seeing any other check for nents ... does this mean that there's
> not actually any block that requires contig mem?

All the blocks require contiguous memory. However, they are all behind
an IOMMU and in practice will always end up mapping the buffers through
the IOMMU. Techically this check should now be in tegra_dc_pin(), which
is called by the ->prepare_fb() callback. I didn't add it because there
are no practical use-cases where this happens, although I guess you
could come up with a kernel and DTB combination where this is actually
possible by jumping through some hoops.

This fix here is to make Tegra DRM interoperation with Nouveau work
again since that's currently broken after moving to the IOMMU-backed DMA
API as an alternative to explicit IOMMU usage. With explicit IOMMU usage
(that's the if corresponding to the else removed below) the IOMMU domain
was shared between the display controllers at the driver level, so it
was fine to make this determination in the else branch because this was
the case where no IOMMU was in play. After the move to the DMA API, this
else branch is also taken when the DMA API is backed by an IOMMU and at
it is unfortunately not known at import time which display controller
ends up scanning out the DMA BUF, nor if that display controller is
behind an IOMMU. We only know that when the actual mapping takes place,
so we'd need to look at sgt->nents after dma_map_sg() in in
tegra_dc_pin().

I'll add that check there, just in case anyone manages to conjure up
such a configuration.

Thierry

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/tegra/gem.c | 7 -------
> >  1 file changed, 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> > index 6dfad56eee2b..bc15b430156d 100644
> > --- a/drivers/gpu/drm/tegra/gem.c
> > +++ b/drivers/gpu/drm/tegra/gem.c
> > @@ -440,13 +440,6 @@ static struct tegra_bo *tegra_bo_import(struct drm_device *drm,
> >  		err = tegra_bo_iommu_map(tegra, bo);
> >  		if (err < 0)
> >  			goto detach;
> > -	} else {
> > -		if (bo->sgt->nents > 1) {
> > -			err = -EINVAL;
> > -			goto detach;
> > -		}
> > -
> > -		bo->iova = sg_dma_address(bo->sgt->sgl);
> >  	}
> >  
> >  	bo->gem.import_attach = attach;
> > -- 
> > 2.23.0
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel