On Fri, Nov 29, 2019 at 10:10:38AM +0100, Daniel Vetter wrote: > On Thu, Nov 28, 2019 at 04:37:34PM +0100, Thierry Reding wrote: > > From: Thierry Reding <tred...@nvidia.com> > > > > Buffers that are imported from a DMA-BUF don't have pages allocated with > > them. At the same time an SG table for them can't be derived using the > > DMA API helpers because the necessary information doesn't exist. However > > there's already an SG table that was created during import, so this can > > simply be duplicated. > > > > Signed-off-by: Thierry Reding <tred...@nvidia.com> > > --- > > drivers/gpu/drm/tegra/gem.c | 43 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 43 insertions(+) > > > > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c > > index 746dae32c484..6dfad56eee2b 100644 > > --- a/drivers/gpu/drm/tegra/gem.c > > +++ b/drivers/gpu/drm/tegra/gem.c > > @@ -27,6 +27,29 @@ static void tegra_bo_put(struct host1x_bo *bo) > > drm_gem_object_put_unlocked(&obj->gem); > > } > > > > +/* XXX move this into lib/scatterlist.c? */ > > +static int sg_alloc_table_from_sg(struct sg_table *sgt, struct scatterlist > > *sg, > > + unsigned int nents, gfp_t gfp_mask) > > +{ > > + struct scatterlist *dst; > > + unsigned int i; > > + int err; > > + > > + err = sg_alloc_table(sgt, nents, gfp_mask); > > + if (err < 0) > > + return err; > > + > > + dst = sgt->sgl; > > + > > + for (i = 0; i < nents; i++) { > > + sg_set_page(dst, sg_page(sg), sg->length, 0); > > + dst = sg_next(dst); > > + sg = sg_next(sg); > > + } > > + > > + return 0; > > +} > > + > > static struct sg_table *tegra_bo_pin(struct device *dev, struct host1x_bo > > *bo, > > dma_addr_t *phys) > > { > > @@ -52,11 +75,31 @@ static struct sg_table *tegra_bo_pin(struct device > > *dev, struct host1x_bo *bo, > > return ERR_PTR(-ENOMEM); > > > > if (obj->pages) { > > + /* > > + * If the buffer object was allocated from the explicit IOMMU > > + * API code paths, construct an SG table from the pages. > > + */ > > err = sg_alloc_table_from_pages(sgt, obj->pages, obj->num_pages, > > 0, obj->gem.size, GFP_KERNEL); > > if (err < 0) > > goto free; > > + } else if (obj->sgt) { > > + /* > > + * If the buffer object already has an SG table but no pages > > + * were allocated for it, it means the buffer was imported and > > + * the SG table needs to be copied to avoid overwriting any > > + * other potential users of the original SG table. > > + */ > > + err = sg_alloc_table_from_sg(sgt, obj->sgt->sgl, > > obj->sgt->nents, > > + GFP_KERNEL); > > Why duplicate this instead of just handing out obj->sgt, and then in unpin > making sure you don't release it? You could also only map/unmap the > dma_buf here in your pin/unpin, but that's a pile of work plus the mapping > is cached anyway so won't change a thing.
The problem with just handing out obj->sgt is that these buffers may be used by several of the host1x engines in the same job. This means that they may end up getting dma_map()'ed by multiple devices. dma_map_*() stores the DMA addresses for the buffer in the SG entries, so subsequent calls would effectively overwrite the earlier mappings, so we need a new SG table for each device. Thierry > -Daniel > > > + if (err < 0) > > + goto free; > > } else { > > + /* > > + * If the buffer object had no pages allocated and if it was > > + * not imported, it had to be allocated with the DMA API, so > > + * the DMA API helper can be used. > > + */ > > err = dma_get_sgtable(dev, sgt, obj->vaddr, obj->iova, > > obj->gem.size); > > if (err < 0) > > -- > > 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
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel