linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [media] v4l: vb2-dma-contig: add support for file access mode flags for DMABUF exporting
@ 2013-05-21  8:11 Philipp Zabel
  2013-09-11  9:56 ` Sylwester Nawrocki
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Zabel @ 2013-05-21  8:11 UTC (permalink / raw)
  To: linux-media
  Cc: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski,
	Hans Verkuil, Tomasz Stanislawski, Kyungmin Park, Philipp Zabel

Currently it is not possible for userspace to map a DMABUF exported buffer
with write permissions. This patch allows to also pass O_RDONLY/O_RDWR when
exporting the buffer, so that userspace may map it with write permissions.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 Documentation/DocBook/media/v4l/vidioc-expbuf.xml | 8 +++++---
 drivers/media/v4l2-core/videobuf2-core.c          | 8 ++++----
 drivers/media/v4l2-core/videobuf2-dma-contig.c    | 4 ++--
 include/media/videobuf2-core.h                    | 2 +-
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
index e287c8f..4165e7b 100644
--- a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
@@ -73,7 +73,8 @@ range from zero to the maximal number of valid planes for the currently active
 format. For the single-planar API, applications must set <structfield> plane
 </structfield> to zero.  Additional flags may be posted in the <structfield>
 flags </structfield> field.  Refer to a manual for open() for details.
-Currently only O_CLOEXEC is supported.  All other fields must be set to zero.
+Currently only O_CLOEXEC, O_RDONLY, O_WRONLY, and O_RDWR are supported.  All
+other fields must be set to zero.
 In the case of multi-planar API, every plane is exported separately using
 multiple <constant> VIDIOC_EXPBUF </constant> calls. </para>
 
@@ -170,8 +171,9 @@ multi-planar API. Otherwise this value must be set to zero. </entry>
 	    <entry>__u32</entry>
 	    <entry><structfield>flags</structfield></entry>
 	    <entry>Flags for the newly created file, currently only <constant>
-O_CLOEXEC </constant> is supported, refer to the manual of open() for more
-details.</entry>
+O_CLOEXEC </constant>, <constant>O_RDONLY</constant>, <constant>O_WRONLY
+</constant>, and <constant>O_RDWR</constant> are supported, refer to the manual
+of open() for more details.</entry>
 	  </row>
 	  <row>
 	    <entry>__s32</entry>
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 7d833ee..5f7ae44 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1785,8 +1785,8 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
 		return -EINVAL;
 	}
 
-	if (eb->flags & ~O_CLOEXEC) {
-		dprintk(1, "Queue does support only O_CLOEXEC flag\n");
+	if (eb->flags & ~(O_CLOEXEC | O_ACCMODE)) {
+		dprintk(1, "Queue does support only O_CLOEXEC and access mode flags\n");
 		return -EINVAL;
 	}
 
@@ -1809,14 +1809,14 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
 
 	vb_plane = &vb->planes[eb->plane];
 
-	dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv);
+	dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv, eb->flags & O_ACCMODE);
 	if (IS_ERR_OR_NULL(dbuf)) {
 		dprintk(1, "Failed to export buffer %d, plane %d\n",
 			eb->index, eb->plane);
 		return -EINVAL;
 	}
 
-	ret = dma_buf_fd(dbuf, eb->flags);
+	ret = dma_buf_fd(dbuf, eb->flags & ~O_ACCMODE);
 	if (ret < 0) {
 		dprintk(3, "buffer %d, plane %d failed to export (%d)\n",
 			eb->index, eb->plane, ret);
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index fd56f25..e443df5 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -393,7 +393,7 @@ static struct sg_table *vb2_dc_get_base_sgt(struct vb2_dc_buf *buf)
 	return sgt;
 }
 
-static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
+static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv, unsigned long flags)
 {
 	struct vb2_dc_buf *buf = buf_priv;
 	struct dma_buf *dbuf;
@@ -404,7 +404,7 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
 	if (WARN_ON(!buf->sgt_base))
 		return NULL;
 
-	dbuf = dma_buf_export(buf, &vb2_dc_dmabuf_ops, buf->size, 0);
+	dbuf = dma_buf_export(buf, &vb2_dc_dmabuf_ops, buf->size, flags);
 	if (IS_ERR(dbuf))
 		return NULL;
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index d88a098..41709a8 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -83,7 +83,7 @@ struct vb2_fileio_data;
 struct vb2_mem_ops {
 	void		*(*alloc)(void *alloc_ctx, unsigned long size, gfp_t gfp_flags);
 	void		(*put)(void *buf_priv);
-	struct dma_buf *(*get_dmabuf)(void *buf_priv);
+	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);
 
 	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
 					unsigned long size, int write);
-- 
1.8.2.rc2


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] [media] v4l: vb2-dma-contig: add support for file access mode flags for DMABUF exporting
  2013-05-21  8:11 [PATCH] [media] v4l: vb2-dma-contig: add support for file access mode flags for DMABUF exporting Philipp Zabel
@ 2013-09-11  9:56 ` Sylwester Nawrocki
  2013-10-08 10:06   ` Philipp Zabel
  0 siblings, 1 reply; 4+ messages in thread
From: Sylwester Nawrocki @ 2013-09-11  9:56 UTC (permalink / raw)
  To: Marek Szyprowski, Tomasz Stanislawski
  Cc: Philipp Zabel, linux-media, Mauro Carvalho Chehab, Pawel Osciak,
	Hans Verkuil, Kyungmin Park

Tomasz, Marek,

Can you review/ack this patch ? I can't see nothing wrong in it and
it has been sat on the ML for quite long. I would add it to my pull
request once it is reviewed.

Thanks,
Sylwester

On 05/21/2013 10:11 AM, Philipp Zabel wrote:
> Currently it is not possible for userspace to map a DMABUF exported buffer
> with write permissions. This patch allows to also pass O_RDONLY/O_RDWR when
> exporting the buffer, so that userspace may map it with write permissions.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  Documentation/DocBook/media/v4l/vidioc-expbuf.xml | 8 +++++---
>  drivers/media/v4l2-core/videobuf2-core.c          | 8 ++++----
>  drivers/media/v4l2-core/videobuf2-dma-contig.c    | 4 ++--
>  include/media/videobuf2-core.h                    | 2 +-
>  4 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
> index e287c8f..4165e7b 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
> @@ -73,7 +73,8 @@ range from zero to the maximal number of valid planes for the currently active
>  format. For the single-planar API, applications must set <structfield> plane
>  </structfield> to zero.  Additional flags may be posted in the <structfield>
>  flags </structfield> field.  Refer to a manual for open() for details.
> -Currently only O_CLOEXEC is supported.  All other fields must be set to zero.
> +Currently only O_CLOEXEC, O_RDONLY, O_WRONLY, and O_RDWR are supported.  All
> +other fields must be set to zero.
>  In the case of multi-planar API, every plane is exported separately using
>  multiple <constant> VIDIOC_EXPBUF </constant> calls. </para>
>  
> @@ -170,8 +171,9 @@ multi-planar API. Otherwise this value must be set to zero. </entry>
>  	    <entry>__u32</entry>
>  	    <entry><structfield>flags</structfield></entry>
>  	    <entry>Flags for the newly created file, currently only <constant>
> -O_CLOEXEC </constant> is supported, refer to the manual of open() for more
> -details.</entry>
> +O_CLOEXEC </constant>, <constant>O_RDONLY</constant>, <constant>O_WRONLY
> +</constant>, and <constant>O_RDWR</constant> are supported, refer to the manual
> +of open() for more details.</entry>
>  	  </row>
>  	  <row>
>  	    <entry>__s32</entry>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 7d833ee..5f7ae44 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1785,8 +1785,8 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
>  		return -EINVAL;
>  	}
>  
> -	if (eb->flags & ~O_CLOEXEC) {
> -		dprintk(1, "Queue does support only O_CLOEXEC flag\n");
> +	if (eb->flags & ~(O_CLOEXEC | O_ACCMODE)) {
> +		dprintk(1, "Queue does support only O_CLOEXEC and access mode flags\n");
>  		return -EINVAL;
>  	}
>  
> @@ -1809,14 +1809,14 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
>  
>  	vb_plane = &vb->planes[eb->plane];
>  
> -	dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv);
> +	dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv, eb->flags & O_ACCMODE);
>  	if (IS_ERR_OR_NULL(dbuf)) {
>  		dprintk(1, "Failed to export buffer %d, plane %d\n",
>  			eb->index, eb->plane);
>  		return -EINVAL;
>  	}
>  
> -	ret = dma_buf_fd(dbuf, eb->flags);
> +	ret = dma_buf_fd(dbuf, eb->flags & ~O_ACCMODE);
>  	if (ret < 0) {
>  		dprintk(3, "buffer %d, plane %d failed to export (%d)\n",
>  			eb->index, eb->plane, ret);
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> index fd56f25..e443df5 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -393,7 +393,7 @@ static struct sg_table *vb2_dc_get_base_sgt(struct vb2_dc_buf *buf)
>  	return sgt;
>  }
>  
> -static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
> +static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv, unsigned long flags)
>  {
>  	struct vb2_dc_buf *buf = buf_priv;
>  	struct dma_buf *dbuf;
> @@ -404,7 +404,7 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
>  	if (WARN_ON(!buf->sgt_base))
>  		return NULL;
>  
> -	dbuf = dma_buf_export(buf, &vb2_dc_dmabuf_ops, buf->size, 0);
> +	dbuf = dma_buf_export(buf, &vb2_dc_dmabuf_ops, buf->size, flags);
>  	if (IS_ERR(dbuf))
>  		return NULL;
>  
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index d88a098..41709a8 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -83,7 +83,7 @@ struct vb2_fileio_data;
>  struct vb2_mem_ops {
>  	void		*(*alloc)(void *alloc_ctx, unsigned long size, gfp_t gfp_flags);
>  	void		(*put)(void *buf_priv);
> -	struct dma_buf *(*get_dmabuf)(void *buf_priv);
> +	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);
>  
>  	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
>  					unsigned long size, int write);
> 


-- 
Sylwester Nawrocki
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] [media] v4l: vb2-dma-contig: add support for file access mode flags for DMABUF exporting
  2013-09-11  9:56 ` Sylwester Nawrocki
@ 2013-10-08 10:06   ` Philipp Zabel
  2013-10-19 19:03     ` Sylwester Nawrocki
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Zabel @ 2013-10-08 10:06 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: Marek Szyprowski, Tomasz Stanislawski, linux-media,
	Mauro Carvalho Chehab, Pawel Osciak, Hans Verkuil, Kyungmin Park,
	kernel

Am Mittwoch, den 11.09.2013, 11:56 +0200 schrieb Sylwester Nawrocki:
> Tomasz, Marek,
> 
> Can you review/ack this patch ? I can't see nothing wrong in it and
> it has been sat on the ML for quite long. I would add it to my pull
> request once it is reviewed.
> 
> Thanks,
> Sylwester

Ping?

This patch is needed to map videobuf2 exported dmabuf fds writeable from
userspace, for example for timestamp software rendering into frames
passing from a v4l2 capture device to a v4l2 output device.

regards
Philipp

> On 05/21/2013 10:11 AM, Philipp Zabel wrote:
> > Currently it is not possible for userspace to map a DMABUF exported buffer
> > with write permissions. This patch allows to also pass O_RDONLY/O_RDWR when
> > exporting the buffer, so that userspace may map it with write permissions.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  Documentation/DocBook/media/v4l/vidioc-expbuf.xml | 8 +++++---
> >  drivers/media/v4l2-core/videobuf2-core.c          | 8 ++++----
> >  drivers/media/v4l2-core/videobuf2-dma-contig.c    | 4 ++--
> >  include/media/videobuf2-core.h                    | 2 +-
> >  4 files changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
> > index e287c8f..4165e7b 100644
> > --- a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
> > +++ b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
> > @@ -73,7 +73,8 @@ range from zero to the maximal number of valid planes for the currently active
> >  format. For the single-planar API, applications must set <structfield> plane
> >  </structfield> to zero.  Additional flags may be posted in the <structfield>
> >  flags </structfield> field.  Refer to a manual for open() for details.
> > -Currently only O_CLOEXEC is supported.  All other fields must be set to zero.
> > +Currently only O_CLOEXEC, O_RDONLY, O_WRONLY, and O_RDWR are supported.  All
> > +other fields must be set to zero.
> >  In the case of multi-planar API, every plane is exported separately using
> >  multiple <constant> VIDIOC_EXPBUF </constant> calls. </para>
> >  
> > @@ -170,8 +171,9 @@ multi-planar API. Otherwise this value must be set to zero. </entry>
> >  	    <entry>__u32</entry>
> >  	    <entry><structfield>flags</structfield></entry>
> >  	    <entry>Flags for the newly created file, currently only <constant>
> > -O_CLOEXEC </constant> is supported, refer to the manual of open() for more
> > -details.</entry>
> > +O_CLOEXEC </constant>, <constant>O_RDONLY</constant>, <constant>O_WRONLY
> > +</constant>, and <constant>O_RDWR</constant> are supported, refer to the manual
> > +of open() for more details.</entry>
> >  	  </row>
> >  	  <row>
> >  	    <entry>__s32</entry>
> > diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> > index 7d833ee..5f7ae44 100644
> > --- a/drivers/media/v4l2-core/videobuf2-core.c
> > +++ b/drivers/media/v4l2-core/videobuf2-core.c
> > @@ -1785,8 +1785,8 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
> >  		return -EINVAL;
> >  	}
> >  
> > -	if (eb->flags & ~O_CLOEXEC) {
> > -		dprintk(1, "Queue does support only O_CLOEXEC flag\n");
> > +	if (eb->flags & ~(O_CLOEXEC | O_ACCMODE)) {
> > +		dprintk(1, "Queue does support only O_CLOEXEC and access mode flags\n");
> >  		return -EINVAL;
> >  	}
> >  
> > @@ -1809,14 +1809,14 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
> >  
> >  	vb_plane = &vb->planes[eb->plane];
> >  
> > -	dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv);
> > +	dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv, eb->flags & O_ACCMODE);
> >  	if (IS_ERR_OR_NULL(dbuf)) {
> >  		dprintk(1, "Failed to export buffer %d, plane %d\n",
> >  			eb->index, eb->plane);
> >  		return -EINVAL;
> >  	}
> >  
> > -	ret = dma_buf_fd(dbuf, eb->flags);
> > +	ret = dma_buf_fd(dbuf, eb->flags & ~O_ACCMODE);
> >  	if (ret < 0) {
> >  		dprintk(3, "buffer %d, plane %d failed to export (%d)\n",
> >  			eb->index, eb->plane, ret);
> > diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > index fd56f25..e443df5 100644
> > --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> > @@ -393,7 +393,7 @@ static struct sg_table *vb2_dc_get_base_sgt(struct vb2_dc_buf *buf)
> >  	return sgt;
> >  }
> >  
> > -static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
> > +static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv, unsigned long flags)
> >  {
> >  	struct vb2_dc_buf *buf = buf_priv;
> >  	struct dma_buf *dbuf;
> > @@ -404,7 +404,7 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
> >  	if (WARN_ON(!buf->sgt_base))
> >  		return NULL;
> >  
> > -	dbuf = dma_buf_export(buf, &vb2_dc_dmabuf_ops, buf->size, 0);
> > +	dbuf = dma_buf_export(buf, &vb2_dc_dmabuf_ops, buf->size, flags);
> >  	if (IS_ERR(dbuf))
> >  		return NULL;
> >  
> > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> > index d88a098..41709a8 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -83,7 +83,7 @@ struct vb2_fileio_data;
> >  struct vb2_mem_ops {
> >  	void		*(*alloc)(void *alloc_ctx, unsigned long size, gfp_t gfp_flags);
> >  	void		(*put)(void *buf_priv);
> > -	struct dma_buf *(*get_dmabuf)(void *buf_priv);
> > +	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);
> >  
> >  	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
> >  					unsigned long size, int write);
> > 
> 
> 



^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] [media] v4l: vb2-dma-contig: add support for file access mode flags for DMABUF exporting
  2013-10-08 10:06   ` Philipp Zabel
@ 2013-10-19 19:03     ` Sylwester Nawrocki
  0 siblings, 0 replies; 4+ messages in thread
From: Sylwester Nawrocki @ 2013-10-19 19:03 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Sylwester Nawrocki, Marek Szyprowski, Tomasz Stanislawski,
	linux-media, Mauro Carvalho Chehab, Pawel Osciak, Hans Verkuil,
	Kyungmin Park, kernel

Hi Philipp,

On 10/08/2013 12:06 PM, Philipp Zabel wrote:
> Ping?
>
> This patch is needed to map videobuf2 exported dmabuf fds writeable from
> userspace, for example for timestamp software rendering into frames
> passing from a v4l2 capture device to a v4l2 output device.

I have added this patch to my tree for v3.13.

Thanks,
Sylwester

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-19 19:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-21  8:11 [PATCH] [media] v4l: vb2-dma-contig: add support for file access mode flags for DMABUF exporting Philipp Zabel
2013-09-11  9:56 ` Sylwester Nawrocki
2013-10-08 10:06   ` Philipp Zabel
2013-10-19 19:03     ` Sylwester Nawrocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).