Hi Lucas,
As you mentioned there hasn't been any further review comments, so
it is fair not to postpone this.
However, I have one new review comment myself that I would like to
see addressed in a v2 before I merge this for 5.3:
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.
>
> 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);
Please add comments here...
> 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);
... and here to explain why you can skip the cpu sync. The comment in ops_detach
can refer to the comment in ops_map, so there is no need to give the full
explanation in two places.
It is not obvious that you can skip the CPU sync, so an explanation is helpful.
Regards,
Hans
> if (!sgt->nents) {
> pr_err("failed to map scatterlist\n");
> mutex_unlock(lock);
>
