Re: [PATCH v5 07/10] media: videobuf2: Allow exporting of a struct dmabuf

[Hans Verkuil]

On 9/17/26 10:23, Jai Luthra wrote:
> From: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> 
> videobuf2 only allowed exporting a dmabuf as a file descriptor,
> but there are instances where having the struct dma_buf is
> useful within the kernel.
> 
> Split the current implementation into two, one step which
> exports a struct dma_buf, and the second which converts that
> into an fd.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> Reviewed-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
> Tested-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
> Signed-off-by: Jai Luthra <jai.luthra@xxxxxxxxxxxxxxxx>

Reviewed-by: Hans Verkuil <hverkuil+cisco@xxxxxxxxxx>

Regards,

	Hans

> ---
> Changes in v5:
> - Update documentation of return type to only ERR_PTR and not NULL
> Changes in v4:
> - Return a pointer to struct dmabuf instead of passing it as a double
>   pointer argument and returning an int for errors (Laurent)
> - Document the return type (Paul)
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 45 ++++++++++++++++---------
>  include/media/videobuf2-core.h                  | 16 +++++++++
>  2 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 5e318165659f..7b99d39d7cb1 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -2419,50 +2419,63 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long offset,
>  	return 0;
>  }
>  
> -int vb2_core_expbuf(struct vb2_queue *q, int *fd, struct vb2_buffer *vb,
> -		    unsigned int plane, unsigned int flags)
> +struct dma_buf *vb2_core_expbuf_dmabuf(struct vb2_queue *q,
> +				       struct vb2_buffer *vb,
> +				       unsigned int plane, unsigned int flags)
>  {
>  	struct vb2_plane *vb_plane;
> -	int ret;
> -	struct dma_buf *dbuf;
> +	struct dma_buf *dmabuf;
>  
>  	if (q->memory != VB2_MEMORY_MMAP) {
>  		dprintk(q, 1, "queue is not currently set up for mmap\n");
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	if (!q->mem_ops->get_dmabuf) {
>  		dprintk(q, 1, "queue does not support DMA buffer exporting\n");
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	if (flags & ~(O_CLOEXEC | O_ACCMODE)) {
> -		dprintk(q, 1, "queue does support only O_CLOEXEC and access mode flags\n");
> -		return -EINVAL;
> +		dprintk(q, 1, "queue supports only O_CLOEXEC and access mode flags\n");
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	if (plane >= vb->num_planes) {
>  		dprintk(q, 1, "buffer plane out of range\n");
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	if (vb2_fileio_is_active(q)) {
>  		dprintk(q, 1, "expbuf: file io in progress\n");
> -		return -EBUSY;
> +		return ERR_PTR(-EBUSY);
>  	}
>  
>  	vb_plane = &vb->planes[plane];
>  
> -	dbuf = call_ptr_memop(get_dmabuf,
> -			      vb,
> -			      vb_plane->mem_priv,
> -			      flags & O_ACCMODE);
> -	if (IS_ERR_OR_NULL(dbuf)) {
> +	dmabuf = call_ptr_memop(get_dmabuf, vb, vb_plane->mem_priv,
> +				flags & O_ACCMODE);
> +	if (IS_ERR_OR_NULL(dmabuf)) {
>  		dprintk(q, 1, "failed to export buffer %d, plane %d\n",
>  			vb->index, plane);
> -		return -EINVAL;
> +		if (!dmabuf)
> +			dmabuf = ERR_PTR(-EINVAL);
>  	}
>  
> +	return dmabuf;
> +}
> +EXPORT_SYMBOL_GPL(vb2_core_expbuf_dmabuf);
> +
> +int vb2_core_expbuf(struct vb2_queue *q, int *fd, struct vb2_buffer *vb,
> +		    unsigned int plane, unsigned int flags)
> +{
> +	struct dma_buf *dbuf;
> +	int ret;
> +
> +	dbuf = vb2_core_expbuf_dmabuf(q, vb, plane, flags);
> +	if (IS_ERR(dbuf))
> +		return PTR_ERR(dbuf);
> +
>  	ret = dma_buf_fd(dbuf, flags & ~O_ACCMODE);
>  	if (ret < 0) {
>  		dprintk(q, 3, "buffer %d, plane %d failed to export (%d)\n",
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index feda638d0064..45bfe8a2a12a 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -954,6 +954,22 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type);
>   */
>  int vb2_core_streamoff(struct vb2_queue *q, unsigned int type);
>  
> +/**
> + * vb2_core_expbuf_dmabuf() - Export a buffer as a dma_buf structure
> + * @q:		pointer to &struct vb2_queue with videobuf2 queue
> + * @vb:		pointer to &struct vb2_buffer
> + * @plane:	index of the plane to be exported, 0 for single plane queues
> + * @flags:	file flags for newly created file, as defined at
> + *		include/uapi/asm-generic/fcntl.h.
> + *		Currently, the only used flag is %O_CLOEXEC.
> + *		is supported, refer to manual of open syscall for more details.
> + *
> + * Return: returns a pointer to &struct dma_buf on success; an ERR_PTR otherwise.
> + */
> +struct dma_buf *vb2_core_expbuf_dmabuf(struct vb2_queue *q,
> +				       struct vb2_buffer *vb,
> +				       unsigned int plane, unsigned int flags);
> +
>  /**
>   * vb2_core_expbuf() - Export a buffer as a file descriptor.
>   * @q:		pointer to &struct vb2_queue with videobuf2 queue.
>