Hi Hans,
Am Dienstag, den 12.03.2019, 08:57 +0100 schrieb Hans Verkuil:
> Pawel and/or Marek, can you take a look at this?
>
> It looks sane to me, but I'd like to have a second opinion as well before
> merging this.
>
> On 2/28/19 8:19 AM, Lucas Stach wrote:
> > This is rougly equivalent to ca0e68e21aae (drm/prime: skip CPU sync
> > in map/unmap dma_buf). The contig memory allocated is already device
> > coherent memory, so there is no point in doing a CPU sync when
> > mapping it to another device. Also most importers currently cache
> > the mapping so the CPU sync would only happen on the first import.
> > With that in mind we are better off with not pretending to do a
> > cache synchronization at all.
> >
> > This gets rid of a lot of CPU overhead in uses where those dma-bufs
> > are regularily imported and detached again, like Weston is currently
> > doing in the DRM compositor.
>
> Lucas, one question: shouldn't the same be done for dma-sg and vmalloc?
I think we should do it eventually, as the importer side caching of the
mappings will undermine any cache maintenance we do in the dmabuf
map/unmap calls. But then the argument is less clear cut for dma-sg and
vmalloc as those buffers will most likely have valid allocations in the
cache. dc is a very clear special case, as the device coherent
allocations never have valid cache allocations, so dropping the
synchronization is very low risk to break any existing use-case.
Both dma-sg and vmalloc require a bit more thought to make the cache
maintenance both effective and low overhead, so this patch is only a
start by picking the low-hanging fruit.
Regards,
Lucas
> >
> > > > Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> > ---
> > drivers/media/common/videobuf2/videobuf2-dma-contig.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > index aff0ab7bf83d..d38f097c14ae 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > @@ -273,8 +273,8 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
> >
> > /* release the scatterlist cache */
> > if (attach->dma_dir != DMA_NONE)
> > - dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
> > - attach->dma_dir);
> > + dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents,
> > + attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
> > sg_free_table(sgt);
> > kfree(attach);
> > db_attach->priv = NULL;
> > @@ -305,8 +305,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
> > }
> >
> > /* mapping to the client with new direction */
> > - sgt->nents = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
> > - dma_dir);
> > + sgt->nents = dma_map_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents,
> > + dma_dir, DMA_ATTR_SKIP_CPU_SYNC);
> > if (!sgt->nents) {
> > pr_err("failed to map scatterlist\n");
> > mutex_unlock(lock);
> >
>
>
