diff mbox series

[v2,03/15] media: videobuf2: Allow exporting of a struct dmabuf

Message ID 20231109210309.638594-4-umang.jain@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series staging: vc04_services: bcm2835-isp support | expand

Commit Message

Umang Jain Nov. 9, 2023, 9:02 p.m. UTC 
From: Dave Stevenson <dave.stevenson@raspberrypi.org>

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@raspberrypi.org>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 .../media/common/videobuf2/videobuf2-core.c   | 36 +++++++++++++------
 include/media/videobuf2-core.h                | 15 ++++++++
 2 files changed, 40 insertions(+), 11 deletions(-)

Comments

Stefan Wahren Nov. 14, 2023, 7:41 p.m. UTC | #1 
Hi Umang,

Am 09.11.23 um 22:02 schrieb Umang Jain:
> From: Dave Stevenson <dave.stevenson@raspberrypi.org>
>
> 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@raspberrypi.org>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---

...

> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 4b6a9d2ea372..cba4e495f6a2 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -925,6 +925,21 @@ 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:         videobuf2 queue
> + * @type:      buffer type
> + * @index:     id number of the buffer
> + * @plane:     index of the plane to be exported, 0 for single plane queues
> + * @flags:     flags for newly created file, currently only O_CLOEXEC is
> + *             supported, refer to manual of open syscall for more details

i think "newly created file" could be confusing here

> + *
> + * Return: Returns the dmabuf pointer
> + */
> +struct dma_buf *vb2_core_expbuf_dmabuf(struct vb2_queue *q, unsigned int type,
> +				       unsigned int index, 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.
Laurent Pinchart Nov. 15, 2023, 7:31 p.m. UTC | #2 
On Tue, Nov 14, 2023 at 08:41:11PM +0100, Stefan Wahren wrote:
> Am 09.11.23 um 22:02 schrieb Umang Jain:
> > From: Dave Stevenson <dave.stevenson@raspberrypi.org>
> >
> > 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@raspberrypi.org>
> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > ---
> 
> ...
> 
> > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> > index 4b6a9d2ea372..cba4e495f6a2 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -925,6 +925,21 @@ 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:         videobuf2 queue
> > + * @type:      buffer type
> > + * @index:     id number of the buffer
> > + * @plane:     index of the plane to be exported, 0 for single plane queues
> > + * @flags:     flags for newly created file, currently only O_CLOEXEC is
> > + *             supported, refer to manual of open syscall for more details
> 
> i think "newly created file" could be confusing here

Would "newly created dmabuf file handle" be clearer and work for
everybody ?

> > + *
> > + * Return: Returns the dmabuf pointer
> > + */
> > +struct dma_buf *vb2_core_expbuf_dmabuf(struct vb2_queue *q, unsigned int type,
> > +				       unsigned int index, 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.
Stefan Wahren Nov. 15, 2023, 9 p.m. UTC | #3 
Am 15.11.23 um 20:31 schrieb Laurent Pinchart:
> On Tue, Nov 14, 2023 at 08:41:11PM +0100, Stefan Wahren wrote:
>> Am 09.11.23 um 22:02 schrieb Umang Jain:
>>> From: Dave Stevenson <dave.stevenson@raspberrypi.org>
>>>
>>> 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@raspberrypi.org>
>>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>>> ---
>> ...
>>
>>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>>> index 4b6a9d2ea372..cba4e495f6a2 100644
>>> --- a/include/media/videobuf2-core.h
>>> +++ b/include/media/videobuf2-core.h
>>> @@ -925,6 +925,21 @@ 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:         videobuf2 queue
>>> + * @type:      buffer type
>>> + * @index:     id number of the buffer
>>> + * @plane:     index of the plane to be exported, 0 for single plane queues
>>> + * @flags:     flags for newly created file, currently only O_CLOEXEC is
>>> + *             supported, refer to manual of open syscall for more details
>> i think "newly created file" could be confusing here
> Would "newly created dmabuf file handle" be clearer and work for
> everybody ?
i'm fine with it
>
>>> + *
>>> + * Return: Returns the dmabuf pointer
>>> + */
>>> +struct dma_buf *vb2_core_expbuf_dmabuf(struct vb2_queue *q, unsigned int type,
>>> +				       unsigned int index, 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.
diff mbox series

Patch

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 27aee92f3eea..8ba85c2db375 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2229,49 +2229,49 @@  static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off,
 	return -EINVAL;
 }
 
-int vb2_core_expbuf(struct vb2_queue *q, int *fd, unsigned int type,
-		unsigned int index, unsigned int plane, unsigned int flags)
+struct dma_buf *vb2_core_expbuf_dmabuf(struct vb2_queue *q, unsigned int type,
+				       unsigned int index, unsigned int plane,
+				       unsigned int flags)
 {
 	struct vb2_buffer *vb = NULL;
 	struct vb2_plane *vb_plane;
-	int ret;
 	struct dma_buf *dbuf;
 
 	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;
+		return ERR_PTR(-EINVAL);
 	}
 
 	if (type != q->type) {
 		dprintk(q, 1, "invalid buffer type\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	if (index >= q->num_buffers) {
 		dprintk(q, 1, "buffer index out of range\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	vb = q->bufs[index];
 
 	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];
@@ -2283,9 +2283,23 @@  int vb2_core_expbuf(struct vb2_queue *q, int *fd, unsigned int type,
 	if (IS_ERR_OR_NULL(dbuf)) {
 		dprintk(q, 1, "failed to export buffer %d, plane %d\n",
 			index, plane);
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 
+	return dbuf;
+}
+EXPORT_SYMBOL_GPL(vb2_core_expbuf_dmabuf);
+
+int vb2_core_expbuf(struct vb2_queue *q, int *fd, unsigned int type,
+		    unsigned int index, unsigned int plane, unsigned int flags)
+{
+	struct dma_buf *dbuf;
+	int ret;
+
+	dbuf = vb2_core_expbuf_dmabuf(q, type, index, 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 4b6a9d2ea372..cba4e495f6a2 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -925,6 +925,21 @@  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:         videobuf2 queue
+ * @type:      buffer type
+ * @index:     id number of the buffer
+ * @plane:     index of the plane to be exported, 0 for single plane queues
+ * @flags:     flags for newly created file, currently only O_CLOEXEC is
+ *             supported, refer to manual of open syscall for more details
+ *
+ * Return: Returns the dmabuf pointer
+ */
+struct dma_buf *vb2_core_expbuf_dmabuf(struct vb2_queue *q, unsigned int type,
+				       unsigned int index, 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.