Re: [PATCH 1/2] media: videobuf2-dma-sg: fix vmap and vunmap callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Marek,

Can you review this? It looks good to me, but I wonder if videobuf2-dma-config.c
has a similar problem. That looks to be mapping as well, but there is no vunmap.

Michael, I have a comment below:

On 26/10/2022 20:42, Michael Grzeschik wrote:
> For dmabuf import users to be able to use the vaddr from another
> videobuf2-dma-sg source, the exporter needs to set a proper vaddr on
> vb2_dma_sg_dmabuf_ops_vmap callback.
> 
> This patch adds vm_map_ram on map if buf->vaddr was not set, and also
> adds the vb2_dma_sg_dmabuf_ops_vunmap function to remove the mapping
> afterwards.
> 
> Cc: stable <stable@xxxxxxxxxx>
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> ---
>  drivers/media/common/videobuf2/videobuf2-dma-sg.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index fa69158a65b1fd..8d6e154bbbc8b0 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -496,11 +496,25 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
>  {
>  	struct vb2_dma_sg_buf *buf = dbuf->priv;
>  
> +	if (!buf->vaddr)
> +		buf->vaddr = vm_map_ram(buf->pages, buf->num_pages, -1);

The comments before the vm_map_ram function state that it should be used for
up to 256 KB only, and video buffers are definitely much larger. It recommends
using vmap in that case. Any reason for not switching to vmap()?

Regards,

	Hans

> +
>  	iosys_map_set_vaddr(map, buf->vaddr);
>  
>  	return 0;
>  }
>  
> +static void vb2_dma_sg_dmabuf_ops_vunmap(struct dma_buf *dbuf,
> +				      struct iosys_map *map)
> +{
> +	struct vb2_dma_sg_buf *buf = dbuf->priv;
> +
> +	if (buf->vaddr)
> +		vm_unmap_ram(buf->vaddr, buf->num_pages);
> +
> +	buf->vaddr = NULL;
> +}
> +
>  static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
>  	struct vm_area_struct *vma)
>  {
> @@ -515,6 +529,7 @@ static const struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
>  	.begin_cpu_access = vb2_dma_sg_dmabuf_ops_begin_cpu_access,
>  	.end_cpu_access = vb2_dma_sg_dmabuf_ops_end_cpu_access,
>  	.vmap = vb2_dma_sg_dmabuf_ops_vmap,
> +	.vunmap = vb2_dma_sg_dmabuf_ops_vunmap,
>  	.mmap = vb2_dma_sg_dmabuf_ops_mmap,
>  	.release = vb2_dma_sg_dmabuf_ops_release,
>  };




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux