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 <treding@xxxxxxxxxx>
> >
> > 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 <treding@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Attachment:
signature.asc
Description: PGP signature
