[PATCH 00/10] Integration of videobuf2 with dmabuf

[PATCH 00/10] Integration of videobuf2 with dmabuf

[Tomasz Stanislawski]

Hello everyone,
This patchset is an incremental patch to patchset created by Sumit
Semwal [1].  The patches are dedicated to help find a better solution for
support of buffer sharing by V4L2 API.  It is expected to start discussion on
final installment for dma-buf in vb2-dma-contig allocator.  Current version of
the patches contain little documentation. It is going to be fixed after
achieving consensus about design for buffer exporting.  Moreover the API
between vb2-core and the allocator should be revised.

The amount of changes to vb2-dma-contig.c was significant making the difference
patch very difficult to read.  Therefore the patch was split into two parts.
One removes old file, the next patch creates the version of the file.

The patchset contains extension for DMA API and its implementation for ARM
architecture. Therefore the patchset should be applied on the top of:

http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/3.2-dma-v5

After applying patches from [2] and [1].

v1: List of changes since [1].
- support for DMA api extension dma_get_pages, the function is used to retrieve pages
 used to create DMA mapping.
- small fixes/code cleanup to videobuf2
- added prepare and finish callbacks to vb2 allocators, it is used keep consistency between dma-cpu acess to the memory (by Marek Szyprowski)
- support for exporting of DMABUF buffer in V4L2 and Videobuf2, originated from [3].
- support for dma-buf exporting in vb2-dma-contig allocator
- support for DMABUF for s5p-tv and s5p-fimc (capture interface) drivers, originated from [3]
- changed handling for userptr buffers (by Marek Szyprowski, Andrzej Pietrasiewicz)
- let mmap method to use dma_mmap_writecombine call (by Marek Szyprowski)

[1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968
[2] https://lkml.org/lkml/2011/12/26/29
[3] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/36354/focus=36355


Marek Szyprowski (2):
  [media] media: vb2: remove plane argument from call_memop and cleanup
    mempriv usage
  media: vb2: add prepare/finish callbacks to allocators

Tomasz Stanislawski (8):
  arm: dma: support for dma_get_pages
  v4l: vb2: fixes for DMABUF support
  v4l: add buffer exporting via dmabuf
  v4l: vb2: add buffer exporting via dmabuf
  v4l: vb2: remove dma-contig allocator
  v4l: vb2-dma-contig: code refactoring, support for DMABUF exporting
  v4l: fimc: integrate capture i-face with dmabuf
  v4l: s5p-tv: mixer: integrate with dmabuf

 arch/arm/include/asm/dma-mapping.h          |    8 +
 arch/arm/mm/dma-mapping.c                   |   44 ++
 drivers/media/video/s5p-fimc/fimc-capture.c |   11 +-
 drivers/media/video/s5p-tv/mixer_video.c    |   11 +-
 drivers/media/video/v4l2-compat-ioctl32.c   |    1 +
 drivers/media/video/v4l2-ioctl.c            |   11 +
 drivers/media/video/videobuf2-core.c        |  114 ++++-
 drivers/media/video/videobuf2-dma-contig.c  |  754 +++++++++++++++++++++------
 include/linux/dma-mapping.h                 |    2 +
 include/linux/videodev2.h                   |    1 +
 include/media/v4l2-ioctl.h                  |    1 +
 include/media/videobuf2-core.h              |   10 +-
 12 files changed, 789 insertions(+), 179 deletions(-)

-- 
1.7.5.4

[PATCH 01/10] arm: dma: support for dma_get_pages

[Tomasz Stanislawski]

This patch provides reliable mechanism for obtaining pages associated with a
given dma_mapping.  This is a proof-of-concept patch.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/include/asm/dma-mapping.h |    8 ++++++
 arch/arm/mm/dma-mapping.c          |   44 ++++++++++++++++++++++++++++++++++++
 include/linux/dma-mapping.h        |    2 +
 3 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index ca7a378..79b6c3d 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -196,6 +196,14 @@ static inline int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
 	return ops->mmap(dev, vma, cpu_addr, dma_addr, size, attrs);
 }
 
+static inline int dma_get_pages(struct device *dev, void *cpu_addr,
+	dma_addr_t dma_addr, struct page **pages, size_t n_pages)
+{
+	const struct dma_map_ops *ops = get_dma_ops(dev);
+	BUG_ON(!ops);
+	return ops->get_pages(dev, cpu_addr, dma_addr, pages, n_pages);
+}
+
 static inline void *dma_alloc_writecombine(struct device *dev, size_t size,
 				       dma_addr_t *dma_handle, gfp_t flag)
 {
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 2287b01..93a3508 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -116,10 +116,14 @@ static void arm_dma_sync_single_for_device(struct device *dev,
 
 static int arm_dma_set_mask(struct device *dev, u64 dma_mask);
 
+static int arm_dma_get_pages(struct device *dev, void *cpu_addr,
+	dma_addr_t dma_addr, struct page **pages, size_t n_pages);
+
 struct dma_map_ops arm_dma_ops = {
 	.alloc			= arm_dma_alloc,
 	.free			= arm_dma_free,
 	.mmap			= arm_dma_mmap,
+	.get_pages		= arm_dma_get_pages,
 	.map_page		= arm_dma_map_page,
 	.unmap_page		= arm_dma_unmap_page,
 	.map_sg			= arm_dma_map_sg,
@@ -531,6 +535,25 @@ int arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 }
 
 /*
+ * Get pages for the DMA-coherent memory.
+ */
+static int arm_dma_get_pages(struct device *dev, void *cpu_addr,
+	dma_addr_t dma_addr, struct page **pages, size_t n_pages)
+{
+#ifdef CONFIG_MMU
+	int i;
+	unsigned long pfn = dma_to_pfn(dev, dma_addr);
+
+	for (i = 0; i < n_pages; ++i)
+		pages[i] = pfn_to_page(pfn + i);
+
+	return n_pages;
+#else
+	return -ENXIO;
+#endif	/* CONFIG_MMU */
+}
+
+/*
  * free a page as defined by the above mapping.
  * Must not be called with IRQs disabled.
  */
@@ -1033,6 +1056,26 @@ static int arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
 	return 0;
 }
 
+static int arm_iommu_get_pages(struct device *dev, void *cpu_addr,
+	dma_addr_t dma_addr, struct page **pages, size_t n_pages)
+{
+	struct arm_vmregion *c;
+	int n_valid_pages;
+
+	c = arm_vmregion_find(&consistent_head, (unsigned long)cpu_addr);
+
+	if (!c)
+		return -ENXIO;
+
+	n_valid_pages = (c->vm_end - c->vm_start) >> PAGE_SHIFT;
+	if (n_valid_pages < n_pages)
+		n_pages = n_valid_pages;
+
+	memcpy(pages, c->priv, n_pages * sizeof pages[0]);
+
+	return n_pages;
+}
+
 /*
  * free a page as defined by the above mapping.
  * Must not be called with IRQs disabled.
@@ -1271,6 +1314,7 @@ struct dma_map_ops iommu_ops = {
 	.alloc		= arm_iommu_alloc_attrs,
 	.free		= arm_iommu_free_attrs,
 	.mmap		= arm_iommu_mmap_attrs,
+	.get_pages	= arm_iommu_get_pages,
 
 	.map_page		= arm_iommu_map_page,
 	.unmap_page		= arm_iommu_unmap_page,
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index b903a20..409d3a9 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -17,6 +17,8 @@ struct dma_map_ops {
 			      struct dma_attrs *attrs);
 	int (*mmap)(struct device *, struct vm_area_struct *,
 			  void *, dma_addr_t, size_t, struct dma_attrs *attrs);
+	int (*get_pages)(struct device *dev, void *vaddr, dma_addr_t dma_addr,
+			struct page **pages, size_t n_pages);
 
 	dma_addr_t (*map_page)(struct device *dev, struct page *page,
 			       unsigned long offset, size_t size,
-- 
1.7.5.4

[PATCH 02/10] [media] media: vb2: remove plane argument from call_memop and cleanup mempriv usage

[Tomasz Stanislawski]

From: Marek Szyprowski <m.szyprowski@samsung.com>

This patch removes unused 'plane' argument from call_memop macro.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Pawel Osciak <pawel@osciak.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/video/videobuf2-core.c |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 6cd2f97..4c3a82e 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -30,7 +30,7 @@ module_param(debug, int, 0644);
 			printk(KERN_DEBUG "vb2: " fmt, ## arg);		\
 	} while (0)
 
-#define call_memop(q, plane, op, args...)				\
+#define call_memop(q, op, args...)					\
 	(((q)->mem_ops->op) ?						\
 		((q)->mem_ops->op(args)) : 0)
 
@@ -52,7 +52,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 
 	/* Allocate memory for all planes in this buffer */
 	for (plane = 0; plane < vb->num_planes; ++plane) {
-		mem_priv = call_memop(q, plane, alloc, q->alloc_ctx[plane],
+		mem_priv = call_memop(q, alloc, q->alloc_ctx[plane],
 				      q->plane_sizes[plane]);
 		if (IS_ERR_OR_NULL(mem_priv))
 			goto free;
@@ -66,7 +66,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 free:
 	/* Free already allocated memory if one of the allocations failed */
 	for (; plane > 0; --plane)
-		call_memop(q, plane, put, vb->planes[plane - 1].mem_priv);
+		call_memop(q, put, vb->planes[plane - 1].mem_priv);
 
 	return -ENOMEM;
 }
@@ -80,7 +80,7 @@ static void __vb2_buf_mem_free(struct vb2_buffer *vb)
 	unsigned int plane;
 
 	for (plane = 0; plane < vb->num_planes; ++plane) {
-		call_memop(q, plane, put, vb->planes[plane].mem_priv);
+		call_memop(q, put, vb->planes[plane].mem_priv);
 		vb->planes[plane].mem_priv = NULL;
 		dprintk(3, "Freed plane %d of buffer %d\n",
 				plane, vb->v4l2_buf.index);
@@ -100,7 +100,7 @@ static void __vb2_buf_userptr_put(struct vb2_buffer *vb)
 		void *mem_priv = vb->planes[plane].mem_priv;
 
 		if (mem_priv) {
-			call_memop(q, plane, put_userptr, mem_priv);
+			call_memop(q, put_userptr, mem_priv);
 			vb->planes[plane].mem_priv = NULL;
 		}
 	}
@@ -328,7 +328,7 @@ static bool __buffer_in_use(struct vb2_queue *q, struct vb2_buffer *vb)
 		 * case anyway. If num_users() returns more than 1,
 		 * we are not the only user of the plane's memory.
 		 */
-		if (mem_priv && call_memop(q, plane, num_users, mem_priv) > 1)
+		if (mem_priv && call_memop(q, num_users, mem_priv) > 1)
 			return true;
 	}
 	return false;
@@ -793,7 +793,7 @@ void *vb2_plane_vaddr(struct vb2_buffer *vb, unsigned int plane_no)
 	if (plane_no > vb->num_planes)
 		return NULL;
 
-	return call_memop(q, plane_no, vaddr, vb->planes[plane_no].mem_priv);
+	return call_memop(q, vaddr, vb->planes[plane_no].mem_priv);
 
 }
 EXPORT_SYMBOL_GPL(vb2_plane_vaddr);
@@ -816,7 +816,7 @@ void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned int plane_no)
 	if (plane_no > vb->num_planes)
 		return NULL;
 
-	return call_memop(q, plane_no, cookie, vb->planes[plane_no].mem_priv);
+	return call_memop(q, cookie, vb->planes[plane_no].mem_priv);
 }
 EXPORT_SYMBOL_GPL(vb2_plane_cookie);
 
@@ -966,8 +966,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 
 		/* Release previously acquired memory if present */
 		if (vb->planes[plane].mem_priv)
-			call_memop(q, plane, put_userptr,
-					vb->planes[plane].mem_priv);
+			call_memop(q, put_userptr, vb->planes[plane].mem_priv);
 
 		vb->planes[plane].mem_priv = NULL;
 		vb->v4l2_planes[plane].m.userptr = 0;
@@ -1011,8 +1010,7 @@ err:
 	/* In case of errors, release planes that were already acquired */
 	for (plane = 0; plane < vb->num_planes; ++plane) {
 		if (vb->planes[plane].mem_priv)
-			call_memop(q, plane, put_userptr,
-				   vb->planes[plane].mem_priv);
+			call_memop(q, put_userptr, vb->planes[plane].mem_priv);
 		vb->planes[plane].mem_priv = NULL;
 		vb->v4l2_planes[plane].m.userptr = 0;
 		vb->v4l2_planes[plane].length = 0;
-- 
1.7.5.4

[PATCH 03/10] media: vb2: add prepare/finish callbacks to allocators

[Tomasz Stanislawski]

From: Marek Szyprowski <m.szyprowski@samsung.com>

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/video/videobuf2-core.c |   11 +++++++++++
 include/media/videobuf2-core.h       |    2 ++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 4c3a82e..cb85874 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -836,6 +836,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	unsigned long flags;
+	int plane;
 
 	if (vb->state != VB2_BUF_STATE_ACTIVE)
 		return;
@@ -846,6 +847,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	dprintk(4, "Done processing on buffer %d, state: %d\n",
 			vb->v4l2_buf.index, vb->state);
 
+	/* sync buffers */
+	for (plane = 0; plane < vb->num_planes; ++plane)
+		call_memop(q, finish, vb->planes[plane].mem_priv);
+
 	/* Add the buffer to the done buffers list */
 	spin_lock_irqsave(&q->done_lock, flags);
 	vb->state = state;
@@ -1136,9 +1141,15 @@ err:
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
+	int plane;
 
 	vb->state = VB2_BUF_STATE_ACTIVE;
 	atomic_inc(&q->queued_count);
+
+	/* sync buffers */
+	for (plane = 0; plane < vb->num_planes; ++plane)
+		call_memop(q, prepare, vb->planes[plane].mem_priv);
+
 	q->ops->buf_queue(vb);
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 35607f7..d8b8171 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -76,6 +76,8 @@ struct vb2_fileio_data;
  */
 struct vb2_mem_ops {
 	void		*(*alloc)(void *alloc_ctx, unsigned long size);
+	void		(*prepare)(void *buf_priv);
+	void		(*finish)(void *buf_priv);
 	void		(*put)(void *buf_priv);
 
 	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
-- 
1.7.5.4

Re: [PATCH 03/10] media: vb2: add prepare/finish callbacks to allocators

[Pawel Osciak]

Hi Tomasz,

On Mon, Jan 23, 2012 at 05:51, Tomasz Stanislawski
<t.stanislaws@samsung.com> wrote:
> From: Marek Szyprowski <m.szyprowski@samsung.com>
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/media/video/videobuf2-core.c |   11 +++++++++++
>  include/media/videobuf2-core.h       |    2 ++
>  2 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
> index 4c3a82e..cb85874 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -836,6 +836,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>  {
>        struct vb2_queue *q = vb->vb2_queue;
>        unsigned long flags;
> +       int plane;
>
>        if (vb->state != VB2_BUF_STATE_ACTIVE)
>                return;
> @@ -846,6 +847,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>        dprintk(4, "Done processing on buffer %d, state: %d\n",
>                        vb->v4l2_buf.index, vb->state);
>
> +       /* sync buffers */
> +       for (plane = 0; plane < vb->num_planes; ++plane)
> +               call_memop(q, finish, vb->planes[plane].mem_priv);
> +
>        /* Add the buffer to the done buffers list */
>        spin_lock_irqsave(&q->done_lock, flags);
>        vb->state = state;
> @@ -1136,9 +1141,15 @@ err:
>  static void __enqueue_in_driver(struct vb2_buffer *vb)
>  {
>        struct vb2_queue *q = vb->vb2_queue;
> +       int plane;
>
>        vb->state = VB2_BUF_STATE_ACTIVE;
>        atomic_inc(&q->queued_count);
> +
> +       /* sync buffers */
> +       for (plane = 0; plane < vb->num_planes; ++plane)
> +               call_memop(q, prepare, vb->planes[plane].mem_priv);
> +
>        q->ops->buf_queue(vb);
>  }
>
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 35607f7..d8b8171 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -76,6 +76,8 @@ struct vb2_fileio_data;
>  */
>  struct vb2_mem_ops {
>        void            *(*alloc)(void *alloc_ctx, unsigned long size);
> +       void            (*prepare)(void *buf_priv);
> +       void            (*finish)(void *buf_priv);
>        void            (*put)(void *buf_priv);
>
>        void            *(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
> --
> 1.7.5.4
>

Those callbacks need to be documented in struct vb2_mem_ops
documentation in code. Apart from that the patch looks good.

-- 
Best regards,
Pawel Osciak

[PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Tomasz Stanislawski]

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/videobuf2-core.c |   21 +++++++++------------
 include/media/videobuf2-core.h       |    6 +++---
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index cb85874..59bb1bc 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb)
 		void *mem_priv = vb->planes[plane].mem_priv;
 
 		if (mem_priv) {
-			call_memop(q, plane, detach_dmabuf, mem_priv);
+			call_memop(q, detach_dmabuf, mem_priv);
 			dma_buf_put(vb->planes[plane].dbuf);
 			vb->planes[plane].dbuf = NULL;
 			vb->planes[plane].mem_priv = NULL;
@@ -907,6 +907,8 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
 		if (b->memory == V4L2_MEMORY_DMABUF) {
 			for (plane = 0; plane < vb->num_planes; ++plane) {
 				v4l2_planes[plane].m.fd = b->m.planes[plane].m.fd;
+				v4l2_planes[plane].length =
+					b->m.planes[plane].length;
 			}
 		}
 	} else {
@@ -1055,15 +1057,10 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		if (IS_ERR_OR_NULL(dbuf)) {
 			dprintk(1, "qbuf: invalid dmabuf fd for "
 				"plane %d\n", plane);
-			ret = PTR_ERR(dbuf);
+			ret = -EINVAL;
 			goto err;
 		}
 
-		/* this doesn't get filled in until __fill_vb2_buffer(),
-		 * since it isn't known until after dma_buf_get()..
-		 */
-		planes[plane].length = dbuf->size;
-
 		/* Skip the plane if already verified */
 		if (dbuf == vb->planes[plane].dbuf) {
 			dma_buf_put(dbuf);
@@ -1075,7 +1072,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 
 		/* Release previously acquired memory if present */
 		if (vb->planes[plane].mem_priv) {
-			call_memop(q, plane, detach_dmabuf,
+			call_memop(q, detach_dmabuf,
 				vb->planes[plane].mem_priv);
 			dma_buf_put(vb->planes[plane].dbuf);
 		}
@@ -1083,8 +1080,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		vb->planes[plane].mem_priv = NULL;
 
 		/* Acquire each plane's memory */
-		mem_priv = q->mem_ops->attach_dmabuf(
-				q->alloc_ctx[plane], dbuf);
+		mem_priv = q->mem_ops->attach_dmabuf(q->alloc_ctx[plane],
+			dbuf, planes[plane].length, write);
 		if (IS_ERR(mem_priv)) {
 			dprintk(1, "qbuf: failed acquiring dmabuf "
 				"memory for plane %d\n", plane);
@@ -1102,7 +1099,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	 */
 	for (plane = 0; plane < vb->num_planes; ++plane) {
 		ret = q->mem_ops->map_dmabuf(
-				vb->planes[plane].mem_priv, write);
+			vb->planes[plane].mem_priv);
 		if (ret) {
 			dprintk(1, "qbuf: failed mapping dmabuf "
 				"memory for plane %d\n", plane);
@@ -1497,7 +1494,7 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
 	 */
 	if (q->memory == V4L2_MEMORY_DMABUF)
 		for (plane = 0; plane < vb->num_planes; ++plane)
-			call_memop(q, plane, unmap_dmabuf,
+			call_memop(q, unmap_dmabuf,
 				vb->planes[plane].mem_priv);
 
 	switch (vb->state) {
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index d8b8171..412c6a4 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -88,10 +88,10 @@ struct vb2_mem_ops {
 	 * in the vb2 core, and vb2_mem_ops really just need to get/put the
 	 * sglist (and make sure that the sglist fits it's needs..)
 	 */
-	void		*(*attach_dmabuf)(void *alloc_ctx,
-					  struct dma_buf *dbuf);
+	void		*(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf,
+				unsigned long size, int write);
 	void		(*detach_dmabuf)(void *buf_priv);
-	int		(*map_dmabuf)(void *buf_priv, int write);
+	int		(*map_dmabuf)(void *buf_priv);
 	void		(*unmap_dmabuf)(void *buf_priv);
 
 	void		*(*vaddr)(void *buf_priv);
-- 
1.7.5.4

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Mauro Carvalho Chehab]

Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Please better describe this patch. What is it supposing to fix?

> ---
>  drivers/media/video/videobuf2-core.c |   21 +++++++++------------
>  include/media/videobuf2-core.h       |    6 +++---
>  2 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
> index cb85874..59bb1bc 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb)
>  		void *mem_priv = vb->planes[plane].mem_priv;
>  
>  		if (mem_priv) {
> -			call_memop(q, plane, detach_dmabuf, mem_priv);
> +			call_memop(q, detach_dmabuf, mem_priv);

Huh? You're not removing the "plane" parameter on this patch, but, instead,
on a previous patch.

No patch is allowed to break compilation, as it breaks git bisect.

>  			dma_buf_put(vb->planes[plane].dbuf);
>  			vb->planes[plane].dbuf = NULL;
>  			vb->planes[plane].mem_priv = NULL;
> @@ -907,6 +907,8 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
>  		if (b->memory == V4L2_MEMORY_DMABUF) {
>  			for (plane = 0; plane < vb->num_planes; ++plane) {
>  				v4l2_planes[plane].m.fd = b->m.planes[plane].m.fd;
> +				v4l2_planes[plane].length =
> +					b->m.planes[plane].length;
>  			}
>  		}
>  	} else {
> @@ -1055,15 +1057,10 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>  		if (IS_ERR_OR_NULL(dbuf)) {
>  			dprintk(1, "qbuf: invalid dmabuf fd for "
>  				"plane %d\n", plane);
> -			ret = PTR_ERR(dbuf);
> +			ret = -EINVAL;
>  			goto err;
>  		}
>  
> -		/* this doesn't get filled in until __fill_vb2_buffer(),
> -		 * since it isn't known until after dma_buf_get()..
> -		 */
> -		planes[plane].length = dbuf->size;
> -
>  		/* Skip the plane if already verified */
>  		if (dbuf == vb->planes[plane].dbuf) {
>  			dma_buf_put(dbuf);
> @@ -1075,7 +1072,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>  
>  		/* Release previously acquired memory if present */
>  		if (vb->planes[plane].mem_priv) {
> -			call_memop(q, plane, detach_dmabuf,
> +			call_memop(q, detach_dmabuf,
>  				vb->planes[plane].mem_priv);
>  			dma_buf_put(vb->planes[plane].dbuf);
>  		}
> @@ -1083,8 +1080,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>  		vb->planes[plane].mem_priv = NULL;
>  
>  		/* Acquire each plane's memory */
> -		mem_priv = q->mem_ops->attach_dmabuf(
> -				q->alloc_ctx[plane], dbuf);
> +		mem_priv = q->mem_ops->attach_dmabuf(q->alloc_ctx[plane],
> +			dbuf, planes[plane].length, write);
>  		if (IS_ERR(mem_priv)) {
>  			dprintk(1, "qbuf: failed acquiring dmabuf "
>  				"memory for plane %d\n", plane);
> @@ -1102,7 +1099,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>  	 */
>  	for (plane = 0; plane < vb->num_planes; ++plane) {
>  		ret = q->mem_ops->map_dmabuf(
> -				vb->planes[plane].mem_priv, write);
> +			vb->planes[plane].mem_priv);
>  		if (ret) {
>  			dprintk(1, "qbuf: failed mapping dmabuf "
>  				"memory for plane %d\n", plane);
> @@ -1497,7 +1494,7 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
>  	 */
>  	if (q->memory == V4L2_MEMORY_DMABUF)
>  		for (plane = 0; plane < vb->num_planes; ++plane)
> -			call_memop(q, plane, unmap_dmabuf,
> +			call_memop(q, unmap_dmabuf,
>  				vb->planes[plane].mem_priv);
>  
>  	switch (vb->state) {
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index d8b8171..412c6a4 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -88,10 +88,10 @@ struct vb2_mem_ops {
>  	 * in the vb2 core, and vb2_mem_ops really just need to get/put the
>  	 * sglist (and make sure that the sglist fits it's needs..)
>  	 */
> -	void		*(*attach_dmabuf)(void *alloc_ctx,
> -					  struct dma_buf *dbuf);
> +	void		*(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf,
> +				unsigned long size, int write);
>  	void		(*detach_dmabuf)(void *buf_priv);
> -	int		(*map_dmabuf)(void *buf_priv, int write);
> +	int		(*map_dmabuf)(void *buf_priv);
>  	void		(*unmap_dmabuf)(void *buf_priv);
>  
>  	void		*(*vaddr)(void *buf_priv);

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Tomasz Stanislawski]

Hi Mauro,

On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote:
> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>
> Please better describe this patch. What is it supposing to fix?

Usually compilation error or bugs discovered in previous vb2-dma-contig 
patches adding support for dma-buf.

>
>> ---
>>   drivers/media/video/videobuf2-core.c |   21 +++++++++------------
>>   include/media/videobuf2-core.h       |    6 +++---
>>   2 files changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
>> index cb85874..59bb1bc 100644
>> --- a/drivers/media/video/videobuf2-core.c
>> +++ b/drivers/media/video/videobuf2-core.c
>> @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb)
>>   		void *mem_priv = vb->planes[plane].mem_priv;
>>
>>   		if (mem_priv) {
>> -			call_memop(q, plane, detach_dmabuf, mem_priv);
>> +			call_memop(q, detach_dmabuf, mem_priv);
>
> Huh? You're not removing the "plane" parameter on this patch, but, instead,
> on a previous patch.
>
> No patch is allowed to break compilation, as it breaks git bisect.

I agree that patches should not break compilation if patches are 
accepted to the mainline. There is a big compilation failure at patch 07 
where videobuf2-dma-contig.c disappears.

Note that these are proof-of-concept patches for support of dma-buf 
exporting/importing dma-buf in V4L2. It would be a waste of time 
polished the patches if they are going to be rejected due to design flaws.

Regards,
Tomasz Stanislawski

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Mauro Carvalho Chehab]

Em 23-01-2012 12:32, Tomasz Stanislawski escreveu:
> Hi Mauro,
> 
> On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote:
>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>
>> Please better describe this patch. What is it supposing to fix?
> 
> Usually compilation error or bugs discovered in previous
> vb2-dma-contig patches adding support for dma-buf.
>
>>
>>> ---
>>>   drivers/media/video/videobuf2-core.c |   21 +++++++++------------
>>>   include/media/videobuf2-core.h       |    6 +++---
>>>   2 files changed, 12 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
>>> index cb85874..59bb1bc 100644
>>> --- a/drivers/media/video/videobuf2-core.c
>>> +++ b/drivers/media/video/videobuf2-core.c
>>> @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb)
>>>           void *mem_priv = vb->planes[plane].mem_priv;
>>>
>>>           if (mem_priv) {
>>> -            call_memop(q, plane, detach_dmabuf, mem_priv);
>>> +            call_memop(q, detach_dmabuf, mem_priv);
>>
>> Huh? You're not removing the "plane" parameter on this patch, but, instead,
>> on a previous patch.
>>
>> No patch is allowed to break compilation, as it breaks git bisect.
> 
> I agree that patches should not break compilation if patches are accepted to
> the mainline. There is a big compilation failure at patch 07 where videobuf2-dma-contig.c disappears.
> 
> Note that these are proof-of-concept patches for support of dma-buf exporting/importing dma-buf in V4L2.
> It would be a waste of time polished the patches if they are going to be rejected due to design flaws.

It is a waste of time for the reviewers to see a patch like this one,
as:
	- No description of what is inside the patch is provided;
	- changes that should be happening inside the other patches are
	  mixed here.

It is also a waste of your time to submit a patch that will need to be later
polished, as you'll need to work with the same thing twice.

So, please fix your patch workflow. As a general rule, you should
compile every patch you're applying and fix the breakages on them.

Also, if you found bugs that need to be fixed and that aren't 
directly related to your current task, those should generate
their own patches, and submitted in separate, in order to be
applied sooner upstream and to stable.

Failing to do that will mean that important fixes for upstream
will be missed.

Regards,
Mauro

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Tomasz Stanislawski]

Hi Mauro,

On 01/23/2012 03:52 PM, Mauro Carvalho Chehab wrote:
> Em 23-01-2012 12:32, Tomasz Stanislawski escreveu:
>> Hi Mauro,
>>
>> On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote:
>>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>>
>>> Please better describe this patch. What is it supposing to fix?
>>
>> Usually compilation error or bugs discovered in previous
>> vb2-dma-contig patches adding support for dma-buf.
>>
>>>
>>>> ---
>>>>    drivers/media/video/videobuf2-core.c |   21 +++++++++------------
>>>>    include/media/videobuf2-core.h       |    6 +++---
>>>>    2 files changed, 12 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
>>>> index cb85874..59bb1bc 100644
>>>> --- a/drivers/media/video/videobuf2-core.c
>>>> +++ b/drivers/media/video/videobuf2-core.c
>>>> @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb)
>>>>            void *mem_priv = vb->planes[plane].mem_priv;
>>>>
>>>>            if (mem_priv) {
>>>> -            call_memop(q, plane, detach_dmabuf, mem_priv);
>>>> +            call_memop(q, detach_dmabuf, mem_priv);
>>>
>>> Huh? You're not removing the "plane" parameter on this patch, but, instead,
>>> on a previous patch.
>>>
>>> No patch is allowed to break compilation, as it breaks git bisect.
>>
>> I agree that patches should not break compilation if patches are accepted to
>> the mainline. There is a big compilation failure at patch 07 where videobuf2-dma-contig.c disappears.
>>
>> Note that these are proof-of-concept patches for support of dma-buf exporting/importing dma-buf in V4L2.
>> It would be a waste of time polished the patches if they are going to be rejected due to design flaws.
>
> It is a waste of time for the reviewers to see a patch like this one,
> as:
> 	- No description of what is inside the patch is provided;

Ok. I should have given more details about the patch. I am sorry for 
missing it. My kernel tree failed to compile after applying patches from

[1] 
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968

I had to generate this patch to compile the code and test it. Most of 
the fixes refer to Sumit's code and I think he will take care of those bugs.

I admit that I focused on other patches. Like DMA extension, exporting 
in vb2-core and vb2-dma-contig. Sorry for putting so little attention to 
bugfixing patch.

> 	- changes that should be happening inside the other patches are
> 	  mixed here.

Right. I missed call_memop here.

>
> It is also a waste of your time to submit a patch that will need to be later
> polished, as you'll need to work with the same thing twice.

The problem is that those patches were not intended to be accepted. The 
were intended for discussion. The other problem is that there are many 
people waiting for those patches. The dma-buf was already accepted to 
the mainline. Me and Sumit are trying to help V4L2 to catch-up. The 
dma-buf and its support in vb2-core seams to change very dynamically. 
Polishing the patch takes much time. If the dma-buf API changes the 
design of vb2-core may have to be changed. Therefore time spent on 
polishing would be lost. I am sorry for patch flaws. All of them would 
be fixed when the design is stabilized.

>
> So, please fix your patch workflow. As a general rule, you should
> compile every patch you're applying and fix the breakages on them.
>
> Also, if you found bugs that need to be fixed and that aren't
> directly related to your current task, those should generate
> their own patches, and submitted in separate, in order to be
> applied sooner upstream and to stable.

I wanted to post the complete set of patches that produce compilable 
kernel. Therefore most important bugs/issues had to be fixed and 
attached to the patchset. Some of the issues in [1] were mentioned by 
Laurent and Sakari. I hope Sumit will take care of those problems.

>
> Failing to do that will mean that important fixes for upstream
> will be missed.

Ok. It will be fixed.

>
> Regards,
> Mauro
>

Regards,
Tomasz Stanislawski

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Mauro Carvalho Chehab]

Em 23-01-2012 13:25, Tomasz Stanislawski escreveu:
> Hi Mauro,
> 
> On 01/23/2012 03:52 PM, Mauro Carvalho Chehab wrote:
>> Em 23-01-2012 12:32, Tomasz Stanislawski escreveu:
>>> Hi Mauro,
>>>
>>> On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote:
>>>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>>>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>>>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>>>
>>>> Please better describe this patch. What is it supposing to fix?
>>>
>>> Usually compilation error or bugs discovered in previous
>>> vb2-dma-contig patches adding support for dma-buf.
>>>
>>>>
>>>>> ---
>>>>>    drivers/media/video/videobuf2-core.c |   21 +++++++++------------
>>>>>    include/media/videobuf2-core.h       |    6 +++---
>>>>>    2 files changed, 12 insertions(+), 15 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
>>>>> index cb85874..59bb1bc 100644
>>>>> --- a/drivers/media/video/videobuf2-core.c
>>>>> +++ b/drivers/media/video/videobuf2-core.c
>>>>> @@ -119,7 +119,7 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb)
>>>>>            void *mem_priv = vb->planes[plane].mem_priv;
>>>>>
>>>>>            if (mem_priv) {
>>>>> -            call_memop(q, plane, detach_dmabuf, mem_priv);
>>>>> +            call_memop(q, detach_dmabuf, mem_priv);
>>>>
>>>> Huh? You're not removing the "plane" parameter on this patch, but, instead,
>>>> on a previous patch.
>>>>
>>>> No patch is allowed to break compilation, as it breaks git bisect.
>>>
>>> I agree that patches should not break compilation if patches are accepted to
>>> the mainline. There is a big compilation failure at patch 07 where videobuf2-dma-contig.c disappears.
>>>
>>> Note that these are proof-of-concept patches for support of dma-buf exporting/importing dma-buf in V4L2.
>>> It would be a waste of time polished the patches if they are going to be rejected due to design flaws.
>>
>> It is a waste of time for the reviewers to see a patch like this one,
>> as:
>>     - No description of what is inside the patch is provided;
> 
> Ok. I should have given more details about the patch. I am sorry for missing it. My kernel tree failed to compile after applying patches from
> 
> [1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968
> 
> I had to generate this patch to compile the code and test it. Most of the fixes refer to Sumit's code and I think he will take care of those bugs.

Ok.

> 
> I admit that I focused on other patches. Like DMA extension, exporting in vb2-core and vb2-dma-contig. Sorry for putting so little attention to bugfixing patch.
> 
>>     - changes that should be happening inside the other patches are
>>       mixed here.
> 
> Right. I missed call_memop here.
> 
>>
>> It is also a waste of your time to submit a patch that will need to be later
>> polished, as you'll need to work with the same thing twice.
> 
> The problem is that those patches were not intended to be accepted. The were intended for discussion. 

The patch subjects were not marked with RFC. Please prefix the subject with something like
	git send-email --subject-prefix " PATCH RFC"

When submitting such patches.

> The other problem is that there are many people waiting for those patches. The dma-buf was already 
> accepted to the mainline. Me and Sumit are trying to help V4L2 to catch-up. The dma-buf and its support
> in vb2-core seams to change very dynamically. Polishing the patch takes much time. If the dma-buf API 
> changes the design of vb2-core may have to be changed. Therefore time spent on polishing would be lost. 

Ok.

> I am sorry for patch flaws. All of them would be fixed when the design is stabilized.

No problem.

> 
>>
>> So, please fix your patch workflow. As a general rule, you should
>> compile every patch you're applying and fix the breakages on them.
>>
>> Also, if you found bugs that need to be fixed and that aren't
>> directly related to your current task, those should generate
>> their own patches, and submitted in separate, in order to be
>> applied sooner upstream and to stable.
> 
> I wanted to post the complete set of patches that produce compilable kernel. 
> herefore most important bugs/issues had to be fixed and attached to the patchset.
> Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will
> take care of those problems.

Ok. My main concern was not with the driver bits, but with:

1) if fixes are needed at the vb2 core, to ensure that they'll go
   upstream earlier;

2) The userspace API changes to properly support for dma buffers.

If you're not ready to discuss (2), that's ok, but I'd like to follow
the discussions for it with care, not only for reviewing the actual
patches, but also since I want to be sure that it will address the
needs for xawtv and for the Xorg v4l driver. 

>> Failing to do that will mean that important fixes for upstream
>> will be missed.
> 
> Ok. It will be fixed.

Thanks!

Mauro

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Tomasz Stanislawski]

Hi Mauro,
On 01/23/2012 05:06 PM, Mauro Carvalho Chehab wrote:
> Em 23-01-2012 13:25, Tomasz Stanislawski escreveu:
>> Hi Mauro,
>>
>> On 01/23/2012 03:52 PM, Mauro Carvalho Chehab wrote:
>>> Em 23-01-2012 12:32, Tomasz Stanislawski escreveu:
>>>> Hi Mauro,
>>>>
>>>> On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote:
>>>>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>>>>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>>>>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>>>>
>>>>> Please better describe this patch. What is it supposing to fix?

[snip]

>>> It is also a waste of your time to submit a patch that will need to be later
>>> polished, as you'll need to work with the same thing twice.
>>
>> The problem is that those patches were not intended to be accepted. The were intended for discussion.
>
> The patch subjects were not marked with RFC. Please prefix the subject with something like
> 	git send-email --subject-prefix " PATCH RFC"
>
> When submitting such patches.

Right!!!
Sorry, I forgot about it. Probably, if I had added those 3 letter we 
would have avoided this whole misunderstanding about the purpose of the 
patches :)

>
>> The other problem is that there are many people waiting for those patches. The dma-buf was already
>> accepted to the mainline. Me and Sumit are trying to help V4L2 to catch-up. The dma-buf and its support
>> in vb2-core seams to change very dynamically. Polishing the patch takes much time. If the dma-buf API
>> changes the design of vb2-core may have to be changed. Therefore time spent on polishing would be lost.
>
> Ok.
>
>> I am sorry for patch flaws. All of them would be fixed when the design is stabilized.
>
> No problem.
>
>>
>>>
>>> So, please fix your patch workflow. As a general rule, you should
>>> compile every patch you're applying and fix the breakages on them.
>>>
>>> Also, if you found bugs that need to be fixed and that aren't
>>> directly related to your current task, those should generate
>>> their own patches, and submitted in separate, in order to be
>>> applied sooner upstream and to stable.
>>
>> I wanted to post the complete set of patches that produce compilable kernel.
>> herefore most important bugs/issues had to be fixed and attached to the patchset.
>> Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will
>> take care of those problems.
>
> Ok. My main concern was not with the driver bits, but with:
>
> 1) if fixes are needed at the vb2 core, to ensure that they'll go
>     upstream earlier;
>

First, we should select patches not related to DMABUF.

The fixes related to DMABUF could be postponed because they will be 
applied in new version for Sumit's patches.

Next videobuf2-dma-contig.c file is patched.

Fixes to handling of mmap and userptr would go first with all DMABUF 
related features removed. Lack of DMABUF related callbacks will not 
break backward compatibility.

Next, DMABUF support is added to vb2-core.

Finally, that DMABUF exporting/importing patches would be applied.

> 2) The userspace API changes to properly support for dma buffers.
>
> If you're not ready to discuss (2), that's ok, but I'd like to follow
> the discussions for it with care, not only for reviewing the actual
> patches, but also since I want to be sure that it will address the
> needs for xawtv and for the Xorg v4l driver.
>

The support of dmabuf could be easily added to framebuffer API.
I expect that it would not be difficult to add it to Xv.
The selection API could be used to control scaling and composing
of video stream into framebuffer or a texture for composing manager.

Regards,
Tomasz Stanislawski

>>> Failing to do that will mean that important fixes for upstream
>>> will be missed.
>>
>> Ok. It will be fixed.
>
> Thanks!
>
> Mauro
>

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Mauro Carvalho Chehab]

Em 23-01-2012 14:37, Tomasz Stanislawski escreveu:
> Hi Mauro,
> On 01/23/2012 05:06 PM, Mauro Carvalho Chehab wrote:
>> Em 23-01-2012 13:25, Tomasz Stanislawski escreveu:
>>> Hi Mauro,
>>>
>>> On 01/23/2012 03:52 PM, Mauro Carvalho Chehab wrote:
>>>> Em 23-01-2012 12:32, Tomasz Stanislawski escreveu:
>>>>> Hi Mauro,
>>>>>
>>>>> On 01/23/2012 03:22 PM, Mauro Carvalho Chehab wrote:
>>>>>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>>>>>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>>>>>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>>>>>
>>>>>> Please better describe this patch. What is it supposing to fix?
> 
> [snip]
> 
>>>> It is also a waste of your time to submit a patch that will need to be later
>>>> polished, as you'll need to work with the same thing twice.
>>>
>>> The problem is that those patches were not intended to be accepted. The were intended for discussion.
>>
>> The patch subjects were not marked with RFC. Please prefix the subject with something like
>>     git send-email --subject-prefix " PATCH RFC"
>>
>> When submitting such patches.
> 
> Right!!!
> Sorry, I forgot about it. Probably, if I had added those 3 letter we would have avoided this whole misunderstanding about the purpose of the patches :)
> 
>>
>>> The other problem is that there are many people waiting for those patches. The dma-buf was already
>>> accepted to the mainline. Me and Sumit are trying to help V4L2 to catch-up. The dma-buf and its support
>>> in vb2-core seams to change very dynamically. Polishing the patch takes much time. If the dma-buf API
>>> changes the design of vb2-core may have to be changed. Therefore time spent on polishing would be lost.
>>
>> Ok.
>>
>>> I am sorry for patch flaws. All of them would be fixed when the design is stabilized.
>>
>> No problem.
>>
>>>
>>>>
>>>> So, please fix your patch workflow. As a general rule, you should
>>>> compile every patch you're applying and fix the breakages on them.
>>>>
>>>> Also, if you found bugs that need to be fixed and that aren't
>>>> directly related to your current task, those should generate
>>>> their own patches, and submitted in separate, in order to be
>>>> applied sooner upstream and to stable.
>>>
>>> I wanted to post the complete set of patches that produce compilable kernel.
>>> herefore most important bugs/issues had to be fixed and attached to the patchset.
>>> Some of the issues in [1] were mentioned by Laurent and Sakari. I hope Sumit will
>>> take care of those problems.
>>
>> Ok. My main concern was not with the driver bits, but with:
>>
>> 1) if fixes are needed at the vb2 core, to ensure that they'll go
>>     upstream earlier;
>>
> 
> First, we should select patches not related to DMABUF.
> 
> The fixes related to DMABUF could be postponed because they will be applied in new version for Sumit's patches.
> 
> Next videobuf2-dma-contig.c file is patched.
> 
> Fixes to handling of mmap and userptr would go first with all DMABUF related features removed. Lack of DMABUF related callbacks will not break backward compatibility.
> 
> Next, DMABUF support is added to vb2-core.
> 
> Finally, that DMABUF exporting/importing patches would be applied.

Sounds like a plan.

Regards,
Mauro

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Semwal, Sumit]

Hi Tomasz,
On Mon, Jan 23, 2012 at 8:55 PM, Tomasz Stanislawski
<t.stanislaws@samsung.com> wrote:
> Hi Mauro,
>
>
<snip>
>
> Ok. I should have given more details about the patch. I am sorry for missing
> it. My kernel tree failed to compile after applying patches from
>
> [1]
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968
>
> I had to generate this patch to compile the code and test it. Most of the
> fixes refer to Sumit's code and I think he will take care of those bugs.
Is your kernel tree a mainline kernel? I am pretty sure I posted out
the RFC after compile testing.

>
<snip>
>
>
> I wanted to post the complete set of patches that produce compilable kernel.
> Therefore most important bugs/issues had to be fixed and attached to the
> patchset. Some of the issues in [1] were mentioned by Laurent and Sakari. I
> hope Sumit will take care of those problems.
I must've misunderstood when you said 'I would like to take care of
these patches'. Please let me know if you'd like me to submit next
version of my RFC separately with fixes for these issues, or would you
manage that as part of your RFC patch series submission.
>
>>
>> Failing to do that will mean that important fixes for upstream
>> will be missed.
>
>
> Ok. It will be fixed.
>
>>
>> Regards,
>> Mauro
>>
>
> Regards,
> Tomasz Stanislawski
>
Best regards,
~Sumit.

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Tomasz Stanislawski]

Hi Sumit,
On 01/25/2012 06:35 AM, Semwal, Sumit wrote:
> Hi Tomasz,
> On Mon, Jan 23, 2012 at 8:55 PM, Tomasz Stanislawski
> <t.stanislaws@samsung.com>  wrote:
>> Hi Mauro,
>>
>>
> <snip>
>>
>> Ok. I should have given more details about the patch. I am sorry for missing
>> it. My kernel tree failed to compile after applying patches from
>>
>> [1]
>> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968
>>
>> I had to generate this patch to compile the code and test it. Most of the
>> fixes refer to Sumit's code and I think he will take care of those bugs.
> Is your kernel tree a mainline kernel? I am pretty sure I posted out
> the RFC after compile testing.

Our development kernel often contains patches that are not posted to 
opensource. The tree presented in the cover letter contains only patches 
that were approved for opensource submission.

Some of the patches that are not merged into the mainline may break 
compilation if patches from the mailing list are applied on the top. The 
example is 'media: vb2: remove plane argument from call_memop and 
cleanup mempriv usage'. I had to add fixes to compile the code. Moreover 
I had to test a working application that makes use of DMABUF 
exporting/importing via V4L2 API. So I had to fix other issues that are 
not only compilation related.

As I remember we agreed that I had to post an incremental patchset.
Therefore all needed fixes had to be present in the tree.

The fixes were posted in this patchset to keep the whole work together.
I expect that you already prepared a patch fixing majority of issues 
from this patch. Many of them were mentioned in Pawel's and Laurent's 
and Sakari's reviews. If you find fixes in this patch useful you can 
merge them into next version of RFC 'v4l: DMA buffer sharing support as 
a user'.

>
>>
> <snip>
>>
>>
>> I wanted to post the complete set of patches that produce compilable kernel.
>> Therefore most important bugs/issues had to be fixed and attached to the
>> patchset. Some of the issues in [1] were mentioned by Laurent and Sakari. I
>> hope Sumit will take care of those problems.
> I must've misunderstood when you said 'I would like to take care of
> these patches'. Please let me know if you'd like me to submit next
> version of my RFC separately with fixes for these issues, or would you
> manage that as part of your RFC patch series submission.

This patchset is an RFC. It was my big mistake that I forgot to add this 
to the title of the patchset. I am not going to post the patch with 
fixes to your part any more. It would be great if you merged it into new 
version of 'DMA buffer sharing support as a user'.

IMO, some parts should go as separate threads:
- extension to DMA subsystem, introduction of dma_get_pages. This would 
probably go to DMA mailing list.
- redesign of dma-contig allocator (w/o dmabuf exporting/importing)
- buffer importing via dmabuf in V4L2 and vb2-dma-contig
- buffer exporting via dmabuf in V4L2 and vb2-dma-contig

BTW. Could you state your opinion on presented solution for dma-buf 
exporting in vb2-core and vb2-dma-contig allocator?

Regards,
Tomasz Stanislawski

>>
>>>
>>> Failing to do that will mean that important fixes for upstream
>>> will be missed.
>>
>>
>> Ok. It will be fixed.
>>
>>>
>>> Regards,
>>> Mauro
>>>
>>
>> Regards,
>> Tomasz Stanislawski
>>
> Best regards,
> ~Sumit.

Re: [PATCH 04/10] v4l: vb2: fixes for DMABUF support

[Semwal, Sumit]

Hi Tomasz,
On Wed, Jan 25, 2012 at 4:04 PM, Tomasz Stanislawski
<t.stanislaws@samsung.com> wrote:
>
> Hi Sumit,
>
> On 01/25/2012 06:35 AM, Semwal, Sumit wrote:
>>
>> Hi Tomasz,
>> On Mon, Jan 23, 2012 at 8:55 PM, Tomasz Stanislawski
>> <t.stanislaws@samsung.com>  wrote:
>>>
>>> Hi Mauro,
>>>
>>>
>> <snip>
>>>
>>>
>>> Ok. I should have given more details about the patch. I am sorry for
>>> missing
>>> it. My kernel tree failed to compile after applying patches from
>>>
>>> [1]
>>>
>>> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968
>>>
>>> I had to generate this patch to compile the code and test it. Most of the
>>> fixes refer to Sumit's code and I think he will take care of those bugs.
>>
>> Is your kernel tree a mainline kernel? I am pretty sure I posted out
>> the RFC after compile testing.
>
>
> Our development kernel often contains patches that are not posted to
> opensource. The tree presented in the cover letter contains only patches
> that were approved for opensource submission.
Right; I understand that - I just wanted to make sure you didn't hit
some problem with my patches that I didn't. Thanks for confirming that
it was your dev kernel.
>
> Some of the patches that are not merged into the mainline may break
> compilation if patches from the mailing list are applied on the top. The
> example is 'media: vb2: remove plane argument from call_memop and cleanup
> mempriv usage'. I had to add fixes to compile the code. Moreover I had to
> test a working application that makes use of DMABUF exporting/importing via
> V4L2 API. So I had to fix other issues that are not only compilation
> related.
>
I understand.
> As I remember we agreed that I had to post an incremental patchset.
> Therefore all needed fixes had to be present in the tree.
>
> The fixes were posted in this patchset to keep the whole work together.
> I expect that you already prepared a patch fixing majority of issues from
> this patch. Many of them were mentioned in Pawel's and Laurent's and
> Sakari's reviews. If you find fixes in this patch useful you can merge them
> into next version of RFC 'v4l: DMA buffer sharing support as a user'.
>
>
OK - this makes it quite clear; I will re-work my RFC then.

>>
>>>
>> <snip>
>>>
>>>
>>>
>>> I wanted to post the complete set of patches that produce compilable
>>> kernel.
>>> Therefore most important bugs/issues had to be fixed and attached to the
>>> patchset. Some of the issues in [1] were mentioned by Laurent and Sakari.
>>> I
>>> hope Sumit will take care of those problems.
>>
>> I must've misunderstood when you said 'I would like to take care of
>> these patches'. Please let me know if you'd like me to submit next
>> version of my RFC separately with fixes for these issues, or would you
>> manage that as part of your RFC patch series submission.
>
>
> This patchset is an RFC. It was my big mistake that I forgot to add this to
> the title of the patchset. I am not going to post the patch with fixes to
> your part any more. It would be great if you merged it into new version of
> 'DMA buffer sharing support as a user'.
>
That's OK with me.

> IMO, some parts should go as separate threads:
> - extension to DMA subsystem, introduction of dma_get_pages. This would
> probably go to DMA mailing list.
> - redesign of dma-contig allocator (w/o dmabuf exporting/importing)
> - buffer importing via dmabuf in V4L2 and vb2-dma-contig
> - buffer exporting via dmabuf in V4L2 and vb2-dma-contig
>
> BTW. Could you state your opinion on presented solution for dma-buf
> exporting in vb2-core and vb2-dma-contig allocator?
>
I agree with your ordering of these parts; Also, with this ordering, I
guess I should pay more attention to parts 1. (extension to DMA...)
and 2. (redesign of dma-contig allocator...) - which I hope you are
going to do? I can then base out my next version of RFC on these.

I was OoO for past couple of days, hence missed all the action :) - I
will try and go through your approach, and comment as soon as I can.
Hopefully in a couple of days.
> Regards,
> Tomasz Stanislawski
>
>
<snip>
Best regards,
~Sumit.

[PATCH 05/10] v4l: add buffer exporting via dmabuf

[Tomasz Stanislawski]

This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
mmap and return a file descriptor on success.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/v4l2-compat-ioctl32.c |    1 +
 drivers/media/video/v4l2-ioctl.c          |   11 +++++++++++
 include/linux/videodev2.h                 |    1 +
 include/media/v4l2-ioctl.h                |    1 +
 4 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
index c68531b..0f18b5e 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -954,6 +954,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
 	case VIDIOC_S_FBUF32:
 	case VIDIOC_OVERLAY32:
 	case VIDIOC_QBUF32:
+	case VIDIOC_EXPBUF:
 	case VIDIOC_DQBUF32:
 	case VIDIOC_STREAMON32:
 	case VIDIOC_STREAMOFF32:
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index e1da8fc..cb29e00 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -207,6 +207,7 @@ static const char *v4l2_ioctls[] = {
 	[_IOC_NR(VIDIOC_S_FBUF)]           = "VIDIOC_S_FBUF",
 	[_IOC_NR(VIDIOC_OVERLAY)]          = "VIDIOC_OVERLAY",
 	[_IOC_NR(VIDIOC_QBUF)]             = "VIDIOC_QBUF",
+	[_IOC_NR(VIDIOC_EXPBUF)]           = "VIDIOC_EXPBUF",
 	[_IOC_NR(VIDIOC_DQBUF)]            = "VIDIOC_DQBUF",
 	[_IOC_NR(VIDIOC_STREAMON)]         = "VIDIOC_STREAMON",
 	[_IOC_NR(VIDIOC_STREAMOFF)]        = "VIDIOC_STREAMOFF",
@@ -932,6 +933,16 @@ static long __video_do_ioctl(struct file *file,
 			dbgbuf(cmd, vfd, p);
 		break;
 	}
+	case VIDIOC_EXPBUF:
+	{
+		unsigned int *p = arg;
+
+		if (!ops->vidioc_expbuf)
+			break;
+
+		ret = ops->vidioc_expbuf(file, fh, *p);
+		break;
+	}
 	case VIDIOC_DQBUF:
 	{
 		struct v4l2_buffer *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 3c0ade1..448fbed 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -2183,6 +2183,7 @@ struct v4l2_create_buffers {
 #define VIDIOC_S_FBUF		 _IOW('V', 11, struct v4l2_framebuffer)
 #define VIDIOC_OVERLAY		 _IOW('V', 14, int)
 #define VIDIOC_QBUF		_IOWR('V', 15, struct v4l2_buffer)
+#define VIDIOC_EXPBUF		_IOWR('V', 16, __u32)
 #define VIDIOC_DQBUF		_IOWR('V', 17, struct v4l2_buffer)
 #define VIDIOC_STREAMON		 _IOW('V', 18, int)
 #define VIDIOC_STREAMOFF	 _IOW('V', 19, int)
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 4d1c74a..8201546 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -120,6 +120,7 @@ struct v4l2_ioctl_ops {
 	int (*vidioc_reqbufs) (struct file *file, void *fh, struct v4l2_requestbuffers *b);
 	int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer *b);
 	int (*vidioc_qbuf)    (struct file *file, void *fh, struct v4l2_buffer *b);
+	int (*vidioc_expbuf)  (struct file *file, void *fh, __u32 offset);
 	int (*vidioc_dqbuf)   (struct file *file, void *fh, struct v4l2_buffer *b);
 
 	int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b);
-- 
1.7.5.4

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Mauro Carvalho Chehab]

Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
> This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
> mmap and return a file descriptor on success.

This requires more discussions. 

The usecase for this new API seems to replace the features previously provided
by the overlay mode. There, not only the buffer were exposed to userspace, but
some control were provided, in order to control the overlay window.

Please start a separate thread about that, explaining how are you imagining that
a V4L2 application would use such ioctl.

Regards,
Mauro

> 
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/video/v4l2-compat-ioctl32.c |    1 +
>  drivers/media/video/v4l2-ioctl.c          |   11 +++++++++++
>  include/linux/videodev2.h                 |    1 +
>  include/media/v4l2-ioctl.h                |    1 +
>  4 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
> index c68531b..0f18b5e 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -954,6 +954,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
>  	case VIDIOC_S_FBUF32:
>  	case VIDIOC_OVERLAY32:
>  	case VIDIOC_QBUF32:
> +	case VIDIOC_EXPBUF:
>  	case VIDIOC_DQBUF32:
>  	case VIDIOC_STREAMON32:
>  	case VIDIOC_STREAMOFF32:
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index e1da8fc..cb29e00 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -207,6 +207,7 @@ static const char *v4l2_ioctls[] = {
>  	[_IOC_NR(VIDIOC_S_FBUF)]           = "VIDIOC_S_FBUF",
>  	[_IOC_NR(VIDIOC_OVERLAY)]          = "VIDIOC_OVERLAY",
>  	[_IOC_NR(VIDIOC_QBUF)]             = "VIDIOC_QBUF",
> +	[_IOC_NR(VIDIOC_EXPBUF)]           = "VIDIOC_EXPBUF",
>  	[_IOC_NR(VIDIOC_DQBUF)]            = "VIDIOC_DQBUF",
>  	[_IOC_NR(VIDIOC_STREAMON)]         = "VIDIOC_STREAMON",
>  	[_IOC_NR(VIDIOC_STREAMOFF)]        = "VIDIOC_STREAMOFF",
> @@ -932,6 +933,16 @@ static long __video_do_ioctl(struct file *file,
>  			dbgbuf(cmd, vfd, p);
>  		break;
>  	}
> +	case VIDIOC_EXPBUF:
> +	{
> +		unsigned int *p = arg;
> +
> +		if (!ops->vidioc_expbuf)
> +			break;
> +
> +		ret = ops->vidioc_expbuf(file, fh, *p);
> +		break;
> +	}
>  	case VIDIOC_DQBUF:
>  	{
>  		struct v4l2_buffer *p = arg;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 3c0ade1..448fbed 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -2183,6 +2183,7 @@ struct v4l2_create_buffers {
>  #define VIDIOC_S_FBUF		 _IOW('V', 11, struct v4l2_framebuffer)
>  #define VIDIOC_OVERLAY		 _IOW('V', 14, int)
>  #define VIDIOC_QBUF		_IOWR('V', 15, struct v4l2_buffer)
> +#define VIDIOC_EXPBUF		_IOWR('V', 16, __u32)
>  #define VIDIOC_DQBUF		_IOWR('V', 17, struct v4l2_buffer)
>  #define VIDIOC_STREAMON		 _IOW('V', 18, int)
>  #define VIDIOC_STREAMOFF	 _IOW('V', 19, int)
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index 4d1c74a..8201546 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -120,6 +120,7 @@ struct v4l2_ioctl_ops {
>  	int (*vidioc_reqbufs) (struct file *file, void *fh, struct v4l2_requestbuffers *b);
>  	int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer *b);
>  	int (*vidioc_qbuf)    (struct file *file, void *fh, struct v4l2_buffer *b);
> +	int (*vidioc_expbuf)  (struct file *file, void *fh, __u32 offset);
>  	int (*vidioc_dqbuf)   (struct file *file, void *fh, struct v4l2_buffer *b);
>  
>  	int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b);

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Mauro,

On Monday 23 January 2012 15:32:40 Mauro Carvalho Chehab wrote:
> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
> > This patch adds extension to V4L2 api. It allow to export a mmap buffer
> > as file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer
> > offset used by mmap and return a file descriptor on success.
> 
> This requires more discussions.
> 
> The usecase for this new API seems to replace the features previously
> provided by the overlay mode. There, not only the buffer were exposed to
> userspace, but some control were provided, in order to control the overlay
> window.
> 
> Please start a separate thread about that, explaining how are you imagining
> that a V4L2 application would use such ioctl.

I think this is currently just a proof of concept. I'm sure Tomasz will 
discuss the V4L2 API extension on the linux-media list when the code will be 
stabilized.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Tomasz Stanislawski]

Hi Mauro.
On 01/23/2012 03:32 PM, Mauro Carvalho Chehab wrote:
> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>> This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
>> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
>> mmap and return a file descriptor on success.
>
> This requires more discussions.
>
> The usecase for this new API seems to replace the features previously provided
> by the overlay mode. There, not only the buffer were exposed to userspace, but
> some control were provided, in order to control the overlay window.

This ioctl was introduced to support exporting of V4L2 buffers via 
dma-buf interface. This framework was little common with overlay mode. 
Could you describe what overlay mode feature is replaced by VIDIOC_EXPBUF?

>
> Please start a separate thread about that, explaining how are you imagining that
> a V4L2 application would use such ioctl.

This patch is essential for full implementation of support for DMABUF 
framework in V4L2. Therefore the patch cannot be moved to separate thread.

Regrads,
Tomasz Stanislawski

>
> Regards,
> Mauro



>
>>
>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>> ---
>>   drivers/media/video/v4l2-compat-ioctl32.c |    1 +
>>   drivers/media/video/v4l2-ioctl.c          |   11 +++++++++++
>>   include/linux/videodev2.h                 |    1 +
>>   include/media/v4l2-ioctl.h                |    1 +
>>   4 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
>> index c68531b..0f18b5e 100644
>> --- a/drivers/media/video/v4l2-compat-ioctl32.c
>> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
>> @@ -954,6 +954,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
>>   	case VIDIOC_S_FBUF32:
>>   	case VIDIOC_OVERLAY32:
>>   	case VIDIOC_QBUF32:
>> +	case VIDIOC_EXPBUF:
>>   	case VIDIOC_DQBUF32:
>>   	case VIDIOC_STREAMON32:
>>   	case VIDIOC_STREAMOFF32:
>> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
>> index e1da8fc..cb29e00 100644
>> --- a/drivers/media/video/v4l2-ioctl.c
>> +++ b/drivers/media/video/v4l2-ioctl.c
>> @@ -207,6 +207,7 @@ static const char *v4l2_ioctls[] = {
>>   	[_IOC_NR(VIDIOC_S_FBUF)]           = "VIDIOC_S_FBUF",
>>   	[_IOC_NR(VIDIOC_OVERLAY)]          = "VIDIOC_OVERLAY",
>>   	[_IOC_NR(VIDIOC_QBUF)]             = "VIDIOC_QBUF",
>> +	[_IOC_NR(VIDIOC_EXPBUF)]           = "VIDIOC_EXPBUF",
>>   	[_IOC_NR(VIDIOC_DQBUF)]            = "VIDIOC_DQBUF",
>>   	[_IOC_NR(VIDIOC_STREAMON)]         = "VIDIOC_STREAMON",
>>   	[_IOC_NR(VIDIOC_STREAMOFF)]        = "VIDIOC_STREAMOFF",
>> @@ -932,6 +933,16 @@ static long __video_do_ioctl(struct file *file,
>>   			dbgbuf(cmd, vfd, p);
>>   		break;
>>   	}
>> +	case VIDIOC_EXPBUF:
>> +	{
>> +		unsigned int *p = arg;
>> +
>> +		if (!ops->vidioc_expbuf)
>> +			break;
>> +
>> +		ret = ops->vidioc_expbuf(file, fh, *p);
>> +		break;
>> +	}
>>   	case VIDIOC_DQBUF:
>>   	{
>>   		struct v4l2_buffer *p = arg;
>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>> index 3c0ade1..448fbed 100644
>> --- a/include/linux/videodev2.h
>> +++ b/include/linux/videodev2.h
>> @@ -2183,6 +2183,7 @@ struct v4l2_create_buffers {
>>   #define VIDIOC_S_FBUF		 _IOW('V', 11, struct v4l2_framebuffer)
>>   #define VIDIOC_OVERLAY		 _IOW('V', 14, int)
>>   #define VIDIOC_QBUF		_IOWR('V', 15, struct v4l2_buffer)
>> +#define VIDIOC_EXPBUF		_IOWR('V', 16, __u32)
>>   #define VIDIOC_DQBUF		_IOWR('V', 17, struct v4l2_buffer)
>>   #define VIDIOC_STREAMON		 _IOW('V', 18, int)
>>   #define VIDIOC_STREAMOFF	 _IOW('V', 19, int)
>> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
>> index 4d1c74a..8201546 100644
>> --- a/include/media/v4l2-ioctl.h
>> +++ b/include/media/v4l2-ioctl.h
>> @@ -120,6 +120,7 @@ struct v4l2_ioctl_ops {
>>   	int (*vidioc_reqbufs) (struct file *file, void *fh, struct v4l2_requestbuffers *b);
>>   	int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer *b);
>>   	int (*vidioc_qbuf)    (struct file *file, void *fh, struct v4l2_buffer *b);
>> +	int (*vidioc_expbuf)  (struct file *file, void *fh, __u32 offset);
>>   	int (*vidioc_dqbuf)   (struct file *file, void *fh, struct v4l2_buffer *b);
>>
>>   	int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b);
>

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Mauro Carvalho Chehab]

Em 23-01-2012 12:42, Tomasz Stanislawski escreveu:
> Hi Mauro.
> On 01/23/2012 03:32 PM, Mauro Carvalho Chehab wrote:
>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>> This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
>>> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
>>> mmap and return a file descriptor on success.
>>
>> This requires more discussions.
>>
>> The usecase for this new API seems to replace the features previously provided
>> by the overlay mode. There, not only the buffer were exposed to userspace, but
>> some control were provided, in order to control the overlay window.
> 
> This ioctl was introduced to support exporting of V4L2 buffers via dma-buf interface. This framework was little common with overlay mode. Could you describe what overlay mode feature is replaced by VIDIOC_EXPBUF?

The V4L2 API doesn't just export "raw" buffers. It provides a logic to control
the streams, with includes buffer settings, buffer queue/dequeue, buffer meta-data
(like timestamps), etc.

I would expect to see something similar for the dma buffers.

With regards to the overlay mode, this is the old way to export DMA buffers between
a video capture driver and a graphics adapter driver. A dma-buf interface will
superseed the video overlay mode, as it will provide more features. Yet, care
should be taken when writing the userspace interface, in order to be sure that all
features needed will be provided there.

>>
>> Please start a separate thread about that, explaining how are you imagining that
>> a V4L2 application would use such ioctl.
> 
> This patch is essential for full implementation of support for DMABUF framework in V4L2. Therefore the patch cannot be moved to separate thread.

I'm not proposing to move the patch to a separate thread. All I'm saying
is that the API extensions for dmabuf requires its own separate discussions.

I couldn't guess, just from your patches, what ioctl's a V4L2 application
like tvtime or xawtv would use the DMABUF.

> 
> Regrads,
> Tomasz Stanislawski
> 
>>
>> Regards,
>> Mauro
> 
> 
> 
>>
>>>
>>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>> ---
>>>   drivers/media/video/v4l2-compat-ioctl32.c |    1 +
>>>   drivers/media/video/v4l2-ioctl.c          |   11 +++++++++++
>>>   include/linux/videodev2.h                 |    1 +
>>>   include/media/v4l2-ioctl.h                |    1 +
>>>   4 files changed, 14 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
>>> index c68531b..0f18b5e 100644
>>> --- a/drivers/media/video/v4l2-compat-ioctl32.c
>>> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
>>> @@ -954,6 +954,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
>>>       case VIDIOC_S_FBUF32:
>>>       case VIDIOC_OVERLAY32:
>>>       case VIDIOC_QBUF32:
>>> +    case VIDIOC_EXPBUF:
>>>       case VIDIOC_DQBUF32:
>>>       case VIDIOC_STREAMON32:
>>>       case VIDIOC_STREAMOFF32:
>>> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
>>> index e1da8fc..cb29e00 100644
>>> --- a/drivers/media/video/v4l2-ioctl.c
>>> +++ b/drivers/media/video/v4l2-ioctl.c
>>> @@ -207,6 +207,7 @@ static const char *v4l2_ioctls[] = {
>>>       [_IOC_NR(VIDIOC_S_FBUF)]           = "VIDIOC_S_FBUF",
>>>       [_IOC_NR(VIDIOC_OVERLAY)]          = "VIDIOC_OVERLAY",
>>>       [_IOC_NR(VIDIOC_QBUF)]             = "VIDIOC_QBUF",
>>> +    [_IOC_NR(VIDIOC_EXPBUF)]           = "VIDIOC_EXPBUF",
>>>       [_IOC_NR(VIDIOC_DQBUF)]            = "VIDIOC_DQBUF",
>>>       [_IOC_NR(VIDIOC_STREAMON)]         = "VIDIOC_STREAMON",
>>>       [_IOC_NR(VIDIOC_STREAMOFF)]        = "VIDIOC_STREAMOFF",
>>> @@ -932,6 +933,16 @@ static long __video_do_ioctl(struct file *file,
>>>               dbgbuf(cmd, vfd, p);
>>>           break;
>>>       }
>>> +    case VIDIOC_EXPBUF:
>>> +    {
>>> +        unsigned int *p = arg;
>>> +
>>> +        if (!ops->vidioc_expbuf)
>>> +            break;
>>> +
>>> +        ret = ops->vidioc_expbuf(file, fh, *p);
>>> +        break;
>>> +    }
>>>       case VIDIOC_DQBUF:
>>>       {
>>>           struct v4l2_buffer *p = arg;
>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>> index 3c0ade1..448fbed 100644
>>> --- a/include/linux/videodev2.h
>>> +++ b/include/linux/videodev2.h
>>> @@ -2183,6 +2183,7 @@ struct v4l2_create_buffers {
>>>   #define VIDIOC_S_FBUF         _IOW('V', 11, struct v4l2_framebuffer)
>>>   #define VIDIOC_OVERLAY         _IOW('V', 14, int)
>>>   #define VIDIOC_QBUF        _IOWR('V', 15, struct v4l2_buffer)
>>> +#define VIDIOC_EXPBUF        _IOWR('V', 16, __u32)
>>>   #define VIDIOC_DQBUF        _IOWR('V', 17, struct v4l2_buffer)
>>>   #define VIDIOC_STREAMON         _IOW('V', 18, int)
>>>   #define VIDIOC_STREAMOFF     _IOW('V', 19, int)
>>> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
>>> index 4d1c74a..8201546 100644
>>> --- a/include/media/v4l2-ioctl.h
>>> +++ b/include/media/v4l2-ioctl.h
>>> @@ -120,6 +120,7 @@ struct v4l2_ioctl_ops {
>>>       int (*vidioc_reqbufs) (struct file *file, void *fh, struct v4l2_requestbuffers *b);
>>>       int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer *b);
>>>       int (*vidioc_qbuf)    (struct file *file, void *fh, struct v4l2_buffer *b);
>>> +    int (*vidioc_expbuf)  (struct file *file, void *fh, __u32 offset);
>>>       int (*vidioc_dqbuf)   (struct file *file, void *fh, struct v4l2_buffer *b);
>>>
>>>       int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b);
>>
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Tomasz Stanislawski]

Hi Mauro,

On 01/23/2012 04:04 PM, Mauro Carvalho Chehab wrote:
> Em 23-01-2012 12:42, Tomasz Stanislawski escreveu:
>> Hi Mauro.
>> On 01/23/2012 03:32 PM, Mauro Carvalho Chehab wrote:
>>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>>> This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
>>>> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
>>>> mmap and return a file descriptor on success.
>>>
>>> This requires more discussions.
>>>
>>> The usecase for this new API seems to replace the features previously provided
>>> by the overlay mode. There, not only the buffer were exposed to userspace, but
>>> some control were provided, in order to control the overlay window.
>>
>> This ioctl was introduced to support exporting of V4L2 buffers via dma-buf interface. This framework was little common with overlay mode. Could you describe what overlay mode feature is replaced by VIDIOC_EXPBUF?
>
> The V4L2 API doesn't just export "raw" buffers. It provides a logic to control
> the streams, with includes buffer settings, buffer queue/dequeue, buffer meta-data
> (like timestamps), etc.

The DMABUF buffers are handled by vb2-core. It provides control for 
queuing and passing streaming and metadata management (like timestamps) 
to the driver.

>
> I would expect to see something similar for the dma buffers.

Those features may be introduced to dma-buf. As I understand 
queue/dequeue refers to passing ownership between a CPU and a driver. It 
is handled in vb2-core. Passing buffer between multiple APIs like V4L2 
and DRM will be probably handled in the userspace. Currently the dma-buf 
provides only the mechanism for mapping the same memory by multiple devices.

>
> With regards to the overlay mode, this is the old way to export DMA buffers between
> a video capture driver and a graphics adapter driver. A dma-buf interface will
> superseed the video overlay mode, as it will provide more features. Yet, care
> should be taken when writing the userspace interface, in order to be sure that all
> features needed will be provided there.
>

The s5p-tv and s5p-fimc do not have support for OVERLAY mode. As I know 
vb2-core has no support for the mode, either. What kind of features 
present in OVERLAYS are needed in dmabuf? Note that dmabuf do not have 
be used only for buffers with video data.

>>>
>>> Please start a separate thread about that, explaining how are you imagining that
>>> a V4L2 application would use such ioctl.

I will post a simple application that does buffer sharing between two 
V4L2 devices (camera and TV output).

>>
>> This patch is essential for full implementation of support for DMABUF framework in V4L2. Therefore the patch cannot be moved to separate thread.
>
> I'm not proposing to move the patch to a separate thread. All I'm saying
> is that the API extensions for dmabuf requires its own separate discussions.

I agree. However DMA patches plays important role in this PoC patchset 
so I decided to keep patches to together. Moreover I wanted this code to 
compile successfully.

I prefer to have a good reason for adding extension before proposing it 
on the mailing list. The DMA buffer sharing seams to be a right reason 
for adding dma_get_pages but comments for V4L2/Linaro people is needed.

>
> I couldn't guess, just from your patches, what ioctl's a V4L2 application
> like tvtime or xawtv would use the DMABUF.

DMABUF is dedicated for application that use streaming between at least 
two devices. Especially if those devices are controlled by different 
APIs, like DRM and V4L2. It would be probably used in the middle-ware 
like gstreamer or OpenMAX.

Regards,
Tomasz Stanislawski

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Mauro Carvalho Chehab]

Em 23-01-2012 13:56, Tomasz Stanislawski escreveu:
> Hi Mauro,
> 
> On 01/23/2012 04:04 PM, Mauro Carvalho Chehab wrote:
>> Em 23-01-2012 12:42, Tomasz Stanislawski escreveu:
>>> Hi Mauro.
>>> On 01/23/2012 03:32 PM, Mauro Carvalho Chehab wrote:
>>>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>>>> This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
>>>>> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
>>>>> mmap and return a file descriptor on success.
>>>>
>>>> This requires more discussions.
>>>>
>>>> The usecase for this new API seems to replace the features previously provided
>>>> by the overlay mode. There, not only the buffer were exposed to userspace, but
>>>> some control were provided, in order to control the overlay window.
>>>
>>> This ioctl was introduced to support exporting of V4L2 buffers via dma-buf interface. This framework was little common with overlay mode. Could you describe what overlay mode feature is replaced by VIDIOC_EXPBUF?
>>
>> The V4L2 API doesn't just export "raw" buffers. It provides a logic to control
>> the streams, with includes buffer settings, buffer queue/dequeue, buffer meta-data
>> (like timestamps), etc.
> 
> The DMABUF buffers are handled by vb2-core. It provides control for queuing and passing streaming and metadata management (like timestamps) to the driver.
> 
>>
>> I would expect to see something similar for the dma buffers.
> 
> Those features may be introduced to dma-buf. As I understand queue/dequeue refers to passing 
> ownership between a CPU and a driver. It is handled in vb2-core. Passing buffer between multiple 
> APIs like V4L2 and DRM will be probably handled in the userspace. Currently the dma-buf provides 
> only the mechanism for mapping the same memory by multiple devices.

I'm not sure if the dma-buf itself should have such meta data, but the V4L2 API 
likely needs it.

>>
>> With regards to the overlay mode, this is the old way to export DMA buffers between
>> a video capture driver and a graphics adapter driver. A dma-buf interface will
>> superseed the video overlay mode, as it will provide more features. Yet, care
>> should be taken when writing the userspace interface, in order to be sure that all
>> features needed will be provided there.
>>
> 
> The s5p-tv and s5p-fimc do not have support for OVERLAY mode. As I know vb2-core 
> has no support for the mode, either.

True. It was decided that overlay is an old design, and a dma-buffer
oriented approach would be needed. So, the decision were to not implement
anything there, until a proper dma-buf support were not added.

> What kind of features present in OVERLAYS are 
> needed in dmabuf? Note that dmabuf do not have be used only for buffers with video data.

That's a good point. Basically, Ovelay mode is supported by
those 3 ioctl's:

#define VIDIOC_G_FBUF            _IOR('V', 10, struct v4l2_framebuffer)
#define VIDIOC_S_FBUF            _IOW('V', 11, struct v4l2_framebuffer)
#define VIDIOC_OVERLAY           _IOW('V', 14, int)

With use these structs:

struct v4l2_pix_format {
        __u32                   width;
        __u32                   height;
        __u32                   pixelformat;
        enum v4l2_field         field;
       	__u32                   bytesperline;
        __u32                   sizeimage;
        enum v4l2_colorspace    colorspace;
        __u32                   priv;
};

struct v4l2_framebuffer {
        __u32                   capability;
        __u32                   flags;

        void                    *base;		/* Should be replaced by the DMA buf specifics */
        struct v4l2_pix_format  fmt;
};
/*  Flags for the 'capability' field. Read only */
#define V4L2_FBUF_CAP_EXTERNOVERLAY     0x0001
#define V4L2_FBUF_CAP_CHROMAKEY         0x0002
#define V4L2_FBUF_CAP_LIST_CLIPPING     0x0004
#define V4L2_FBUF_CAP_BITMAP_CLIPPING   0x0008
#define V4L2_FBUF_CAP_LOCAL_ALPHA       0x0010
#define V4L2_FBUF_CAP_GLOBAL_ALPHA      0x0020
#define V4L2_FBUF_CAP_LOCAL_INV_ALPHA   0x0040
#define V4L2_FBUF_CAP_SRC_CHROMAKEY     0x0080
/*  Flags for the 'flags' field. */
#define V4L2_FBUF_FLAG_PRIMARY          0x0001
#define V4L2_FBUF_FLAG_OVERLAY          0x0002
#define V4L2_FBUF_FLAG_CHROMAKEY        0x0004
#define V4L2_FBUF_FLAG_LOCAL_ALPHA	0x0008
#define V4L2_FBUF_FLAG_GLOBAL_ALPHA     0x0010
#define V4L2_FBUF_FLAG_LOCAL_INV_ALPHA  0x0020
#define V4L2_FBUF_FLAG_SRC_CHROMAKEY    0x0040

It should be noticed that devices that support OVERLAY can provide
data on both dma-buffer sharing and via the standard MMAP/read() mode at
the same time, each with a different video format. So, the VIDIOC_S_FBUF
ioctl needs to set the pixel format, and image size for the overlay,
while the other ioctl's set it for the MMAP (or read) mode.

Buffer queue/dequeue happens internally, and can be started/stopped via
a VIDIOC_OVERLAY call.

>>>>
>>>> Please start a separate thread about that, explaining how are you imagining that
>>>> a V4L2 application would use such ioctl.
> 
> I will post a simple application that does buffer sharing between two V4L2 devices (camera and TV output).

Ok.

>>>
>>> This patch is essential for full implementation of support for DMABUF framework in V4L2. Therefore the patch cannot be moved to separate thread.
>>
>> I'm not proposing to move the patch to a separate thread. All I'm saying
>> is that the API extensions for dmabuf requires its own separate discussions.
> 
> I agree. However DMA patches plays important role in this PoC patchset so I decided to keep patches to together. Moreover I wanted this code to compile successfully.
> 
> I prefer to have a good reason for adding extension before proposing it on the mailing list. The DMA buffer sharing seams to be a right reason for adding dma_get_pages but comments for V4L2/Linaro people is needed.
> 
>>
>> I couldn't guess, just from your patches, what ioctl's a V4L2 application
>> like tvtime or xawtv would use the DMABUF.
> 
> DMABUF is dedicated for application that use streaming between at least two devices. 
> Especially if those devices are controlled by different APIs, like DRM and V4L2. 
> It would be probably used in the middle-ware like gstreamer or OpenMAX.

This is what the X11 v4l extension driver does: it shares DMA buffers between V4L2 
and DRM. The extension currently relies on XV extension, simply because this is what
were available at the time the extension was written. I didn't have any time yet
to port it to use something more modern.

It is probably a good idea for you to take a look on it, when writing the API bits.
Its source is available at:

	http://cgit.freedesktop.org/xorg/driver/xf86-video-v4l/

Regards,
Mauro

V4L2 Overlay mode replacement by dma-buf - was: Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Mauro Carvalho Chehab]

Em 23-01-2012 14:42, Mauro Carvalho Chehab escreveu:
> Em 23-01-2012 13:56, Tomasz Stanislawski escreveu:
>> Hi Mauro,
>>
>> On 01/23/2012 04:04 PM, Mauro Carvalho Chehab wrote:
>>> Em 23-01-2012 12:42, Tomasz Stanislawski escreveu:
>>>> Hi Mauro.
>>>> On 01/23/2012 03:32 PM, Mauro Carvalho Chehab wrote:
>>>>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>>>>> This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
>>>>>> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
>>>>>> mmap and return a file descriptor on success.
>>>>>
>>>>> This requires more discussions.
>>>>>
>>>>> The usecase for this new API seems to replace the features previously provided
>>>>> by the overlay mode. There, not only the buffer were exposed to userspace, but
>>>>> some control were provided, in order to control the overlay window.
>>>>
>>>> This ioctl was introduced to support exporting of V4L2 buffers via dma-buf interface. This framework was little common with overlay mode. Could you describe what overlay mode feature is replaced by VIDIOC_EXPBUF?
>>>
>>> The V4L2 API doesn't just export "raw" buffers. It provides a logic to control
>>> the streams, with includes buffer settings, buffer queue/dequeue, buffer meta-data
>>> (like timestamps), etc.
>>
>> The DMABUF buffers are handled by vb2-core. It provides control for queuing and passing streaming and metadata management (like timestamps) to the driver.
>>
>>>
>>> I would expect to see something similar for the dma buffers.
>>
>> Those features may be introduced to dma-buf. As I understand queue/dequeue refers to passing 
>> ownership between a CPU and a driver. It is handled in vb2-core. Passing buffer between multiple 
>> APIs like V4L2 and DRM will be probably handled in the userspace. Currently the dma-buf provides 
>> only the mechanism for mapping the same memory by multiple devices.
> 
> I'm not sure if the dma-buf itself should have such meta data, but the V4L2 API 
> likely needs it.
> 
>>>
>>> With regards to the overlay mode, this is the old way to export DMA buffers between
>>> a video capture driver and a graphics adapter driver. A dma-buf interface will
>>> superseed the video overlay mode, as it will provide more features. Yet, care
>>> should be taken when writing the userspace interface, in order to be sure that all
>>> features needed will be provided there.
>>>
>>
>> The s5p-tv and s5p-fimc do not have support for OVERLAY mode. As I know vb2-core 
>> has no support for the mode, either.
> 
> True. It was decided that overlay is an old design, and a dma-buffer
> oriented approach would be needed. So, the decision were to not implement
> anything there, until a proper dma-buf support were not added.
> 
>> What kind of features present in OVERLAYS are 
>> needed in dmabuf? Note that dmabuf do not have be used only for buffers with video data.
> 
> That's a good point. Basically, Ovelay mode is supported by
> those 3 ioctl's:
> 
> #define VIDIOC_G_FBUF            _IOR('V', 10, struct v4l2_framebuffer)
> #define VIDIOC_S_FBUF            _IOW('V', 11, struct v4l2_framebuffer)
> #define VIDIOC_OVERLAY           _IOW('V', 14, int)
> 
> With use these structs:
> 
> struct v4l2_pix_format {
>         __u32                   width;
>         __u32                   height;
>         __u32                   pixelformat;
>         enum v4l2_field         field;
>        	__u32                   bytesperline;
>         __u32                   sizeimage;
>         enum v4l2_colorspace    colorspace;
>         __u32                   priv;
> };
> 
> struct v4l2_framebuffer {
>         __u32                   capability;
>         __u32                   flags;
> 
>         void                    *base;		/* Should be replaced by the DMA buf specifics */
>         struct v4l2_pix_format  fmt;
> };
> /*  Flags for the 'capability' field. Read only */
> #define V4L2_FBUF_CAP_EXTERNOVERLAY     0x0001
> #define V4L2_FBUF_CAP_CHROMAKEY         0x0002
> #define V4L2_FBUF_CAP_LIST_CLIPPING     0x0004
> #define V4L2_FBUF_CAP_BITMAP_CLIPPING   0x0008
> #define V4L2_FBUF_CAP_LOCAL_ALPHA       0x0010
> #define V4L2_FBUF_CAP_GLOBAL_ALPHA      0x0020
> #define V4L2_FBUF_CAP_LOCAL_INV_ALPHA   0x0040
> #define V4L2_FBUF_CAP_SRC_CHROMAKEY     0x0080
> /*  Flags for the 'flags' field. */
> #define V4L2_FBUF_FLAG_PRIMARY          0x0001
> #define V4L2_FBUF_FLAG_OVERLAY          0x0002
> #define V4L2_FBUF_FLAG_CHROMAKEY        0x0004
> #define V4L2_FBUF_FLAG_LOCAL_ALPHA	0x0008
> #define V4L2_FBUF_FLAG_GLOBAL_ALPHA     0x0010
> #define V4L2_FBUF_FLAG_LOCAL_INV_ALPHA  0x0020
> #define V4L2_FBUF_FLAG_SRC_CHROMAKEY    0x0040
> 
> It should be noticed that devices that support OVERLAY can provide
> data on both dma-buffer sharing and via the standard MMAP/read() mode at
> the same time, each with a different video format. So, the VIDIOC_S_FBUF
> ioctl needs to set the pixel format, and image size for the overlay,
> while the other ioctl's set it for the MMAP (or read) mode.
> 
> Buffer queue/dequeue happens internally, and can be started/stopped via
> a VIDIOC_OVERLAY call.
> 
>>>>>
>>>>> Please start a separate thread about that, explaining how are you imagining that
>>>>> a V4L2 application would use such ioctl.
>>
>> I will post a simple application that does buffer sharing between two V4L2 devices (camera and TV output).
> 
> Ok.
> 
>>>>
>>>> This patch is essential for full implementation of support for DMABUF framework in V4L2. Therefore the patch cannot be moved to separate thread.
>>>
>>> I'm not proposing to move the patch to a separate thread. All I'm saying
>>> is that the API extensions for dmabuf requires its own separate discussions.
>>
>> I agree. However DMA patches plays important role in this PoC patchset so I decided to keep patches to together. Moreover I wanted this code to compile successfully.
>>
>> I prefer to have a good reason for adding extension before proposing it on the mailing list. The DMA buffer sharing seams to be a right reason for adding dma_get_pages but comments for V4L2/Linaro people is needed.
>>
>>>
>>> I couldn't guess, just from your patches, what ioctl's a V4L2 application
>>> like tvtime or xawtv would use the DMABUF.
>>
>> DMABUF is dedicated for application that use streaming between at least two devices. 
>> Especially if those devices are controlled by different APIs, like DRM and V4L2. 
>> It would be probably used in the middle-ware like gstreamer or OpenMAX.
> 
> This is what the X11 v4l extension driver does: it shares DMA buffers between V4L2 
> and DRM. The extension currently relies on XV extension, simply because this is what
> were available at the time the extension was written. I didn't have any time yet
> to port it to use something more modern.
> 
> It is probably a good idea for you to take a look on it, when writing the API bits.
> Its source is available at:
> 
> 	http://cgit.freedesktop.org/xorg/driver/xf86-video-v4l/


(I moved the comment from the other thread to this one, and renamed it,
 in order to have the DMA buf API discussion at the same place)

>> 2) The userspace API changes to properly support for dma buffers.
>>
>> If you're not ready to discuss (2), that's ok, but I'd like to follow
>> the discussions for it with care, not only for reviewing the actual
>> patches, but also since I want to be sure that it will address the
>> needs for xawtv and for the Xorg v4l driver.
>>
> 
> The support of dmabuf could be easily added to framebuffer API.
> I expect that it would not be difficult to add it to Xv.

A texture based API is likely needed, at least for it to work with
modern PC GPU's.

> The selection API could be used to control scaling and composing
> of video stream into framebuffer or a texture for composing manager.

The current approach for Overlay mode is to put everything related to
the overlay settings at VIDIOC_G_FBUF/VIDIOC_S_FBUF. The rationale was
that the same video node supporting Overlay were also supporting MMAP
and/or read() mode.

A dmabuf-based overlay mode can either keep the same strategy, using
an structure very similar to struct v4l2_framebuffer, or, alternatively,
a new video node could be required, for the "overlay2" caps property.

Regards,
Mauro

Re: V4L2 Overlay mode replacement by dma-buf - was: Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Clark, Rob]

On Mon, Jan 23, 2012 at 10:57 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
>
>>> 2) The userspace API changes to properly support for dma buffers.
>>>
>>> If you're not ready to discuss (2), that's ok, but I'd like to follow
>>> the discussions for it with care, not only for reviewing the actual
>>> patches, but also since I want to be sure that it will address the
>>> needs for xawtv and for the Xorg v4l driver.
>>>
>>
>> The support of dmabuf could be easily added to framebuffer API.
>> I expect that it would not be difficult to add it to Xv.

You might want to have a look at my dri2video proposal a while back.
I plan some minor changes to make the api for multi-planar formats
look a bit more like how addfb2 ended up (ie. array of handles,
offsets, and pitches), but you could get the basic idea from:

http://patchwork.freedesktop.org/patch/7939/

> A texture based API is likely needed, at least for it to work with
> modern PC GPU's.

I suspect we will end up w/ an eglImage extension to go dmabuf fd <->
eglImage, and perhaps handle barriers and userspace mappings.  That
should, I think, be the best approach to best hide/abstract all the
GPU crazy games from the rest of the world.

BR,
-R

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Mauro,

On Monday 23 January 2012 17:42:45 Mauro Carvalho Chehab wrote:
> Em 23-01-2012 13:56, Tomasz Stanislawski escreveu:
> > On 01/23/2012 04:04 PM, Mauro Carvalho Chehab wrote:
> >> Em 23-01-2012 12:42, Tomasz Stanislawski escreveu:
> >>> On 01/23/2012 03:32 PM, Mauro Carvalho Chehab wrote:
> >>>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
> >>>>> This patch adds extension to V4L2 api. It allow to export a mmap
> >>>>> buffer as file descriptor. New ioctl VIDIOC_EXPBUF is added. It
> >>>>> takes a buffer offset used by mmap and return a file descriptor on
> >>>>> success.
> >>>> 
> >>>> This requires more discussions.
> >>>> 
> >>>> The usecase for this new API seems to replace the features previously
> >>>> provided by the overlay mode. There, not only the buffer were exposed
> >>>> to userspace, but some control were provided, in order to control the
> >>>> overlay window.
> >>> 
> >>> This ioctl was introduced to support exporting of V4L2 buffers via
> >>> dma-buf interface. This framework was little common with overlay mode.
> >>> Could you describe what overlay mode feature is replaced by
> >>> VIDIOC_EXPBUF?
> >> 
> >> The V4L2 API doesn't just export "raw" buffers. It provides a logic to
> >> control the streams, with includes buffer settings, buffer
> >> queue/dequeue, buffer meta-data (like timestamps), etc.
> > 
> > The DMABUF buffers are handled by vb2-core. It provides control for
> > queuing and passing streaming and metadata management (like timestamps)
> > to the driver.
> > 
> >> I would expect to see something similar for the dma buffers.
> > 
> > Those features may be introduced to dma-buf. As I understand
> > queue/dequeue refers to passing ownership between a CPU and a driver. It
> > is handled in vb2-core. Passing buffer between multiple APIs like V4L2
> > and DRM will be probably handled in the userspace. Currently the dma-buf
> > provides only the mechanism for mapping the same memory by multiple
> > devices.
> 
> I'm not sure if the dma-buf itself should have such meta data, but the V4L2
> API likely needs it.
> 
> >> With regards to the overlay mode, this is the old way to export DMA
> >> buffers between a video capture driver and a graphics adapter driver. A
> >> dma-buf interface will superseed the video overlay mode, as it will
> >> provide more features. Yet, care should be taken when writing the
> >> userspace interface, in order to be sure that all features needed will
> >> be provided there.
> > 
> > The s5p-tv and s5p-fimc do not have support for OVERLAY mode. As I know
> > vb2-core has no support for the mode, either.
> 
> True. It was decided that overlay is an old design, and a dma-buffer
> oriented approach would be needed. So, the decision were to not implement
> anything there, until a proper dma-buf support were not added.
> 
> > What kind of features present in OVERLAYS are
> > needed in dmabuf? Note that dmabuf do not have be used only for buffers
> > with video data.
> 
> That's a good point. Basically, Ovelay mode is supported by
> those 3 ioctl's:
> 
> #define VIDIOC_G_FBUF            _IOR('V', 10, struct v4l2_framebuffer)
> #define VIDIOC_S_FBUF            _IOW('V', 11, struct v4l2_framebuffer)
> #define VIDIOC_OVERLAY           _IOW('V', 14, int)
> 
> With use these structs:
> 
> struct v4l2_pix_format {
>         __u32                   width;
>         __u32                   height;
>         __u32                   pixelformat;
>         enum v4l2_field         field;
>        	__u32                   bytesperline;
>         __u32                   sizeimage;
>         enum v4l2_colorspace    colorspace;
>         __u32                   priv;
> };
> 
> struct v4l2_framebuffer {
>         __u32                   capability;
>         __u32                   flags;
> 
>         void                    *base;		/* Should be replaced by the 
DMA
> buf specifics */ struct v4l2_pix_format  fmt;
> };
> /*  Flags for the 'capability' field. Read only */
> #define V4L2_FBUF_CAP_EXTERNOVERLAY     0x0001
> #define V4L2_FBUF_CAP_CHROMAKEY         0x0002
> #define V4L2_FBUF_CAP_LIST_CLIPPING     0x0004
> #define V4L2_FBUF_CAP_BITMAP_CLIPPING   0x0008
> #define V4L2_FBUF_CAP_LOCAL_ALPHA       0x0010
> #define V4L2_FBUF_CAP_GLOBAL_ALPHA      0x0020
> #define V4L2_FBUF_CAP_LOCAL_INV_ALPHA   0x0040
> #define V4L2_FBUF_CAP_SRC_CHROMAKEY     0x0080
> /*  Flags for the 'flags' field. */
> #define V4L2_FBUF_FLAG_PRIMARY          0x0001
> #define V4L2_FBUF_FLAG_OVERLAY          0x0002
> #define V4L2_FBUF_FLAG_CHROMAKEY        0x0004
> #define V4L2_FBUF_FLAG_LOCAL_ALPHA	0x0008
> #define V4L2_FBUF_FLAG_GLOBAL_ALPHA     0x0010
> #define V4L2_FBUF_FLAG_LOCAL_INV_ALPHA  0x0020
> #define V4L2_FBUF_FLAG_SRC_CHROMAKEY    0x0040
> 
> It should be noticed that devices that support OVERLAY can provide
> data on both dma-buffer sharing and via the standard MMAP/read() mode at
> the same time, each with a different video format. So, the VIDIOC_S_FBUF
> ioctl needs to set the pixel format, and image size for the overlay,
> while the other ioctl's set it for the MMAP (or read) mode.
> 
> Buffer queue/dequeue happens internally, and can be started/stopped via
> a VIDIOC_OVERLAY call.

The approach taken by dma-buf is different. First of all, please remember that 
dma-buf is not restricted to capturing directly to the frame buffer memory as 
V4L2 overlay support does, but can also be used to capture to GL textures and 
to pass data to a DSP (or anoother image processor). For those reasons, 
sequencing needs to be handled by userspace. Buffer queuing and dequeuing will 
use the usual VIDIOC_QBUF and VIDIOC_DQBUF ioctls. The only difference is that 
the queue will use a new memory type specific to dma-buf, and vb2 will handle 
buffer mapping and unmapping internally. This should be transparent to the 
driver.

For those reasons, the VIDIOC_G_FBUF, VIDIOC_S_FBUF and VIDIOC_OVERLAY ioctls 
are not needed anymore. Applications in dma-buf systems will use 
VIDIOC_G_FMT/VIDIOC_S_FMT instead of VIDIOC_G_FBUF and VIDIOC_S_BUF (and 
probably the selection API to get/set the compose rectangle), and 
VIDIOC_STREAMON, VIDIOC_STREAMOFF, VIDIOC_DQBUF and VIDIOC_QBUF instead of 
VIDIOC_OVERLAY.

What we will then need is a way to get/set the "overlay" capabilities and 
flags, but those should not be restricted to "overlay" mode anymore. Alpha-
blending and color-keying can be useful when capturing to memory, not only 
when capturing directly to the frame buffer.

> >>>> Please start a separate thread about that, explaining how are you
> >>>> imagining that a V4L2 application would use such ioctl.
> > 
> > I will post a simple application that does buffer sharing between two
> > V4L2 devices (camera and TV output).
> 
> Ok.
> 
> >>> This patch is essential for full implementation of support for DMABUF
> >>> framework in V4L2. Therefore the patch cannot be moved to separate
> >>> thread.
> >> 
> >> I'm not proposing to move the patch to a separate thread. All I'm saying
> >> is that the API extensions for dmabuf requires its own separate
> >> discussions.
> > 
> > I agree. However DMA patches plays important role in this PoC patchset so
> > I decided to keep patches to together. Moreover I wanted this code to
> > compile successfully.
> > 
> > I prefer to have a good reason for adding extension before proposing it
> > on the mailing list. The DMA buffer sharing seams to be a right reason
> > for adding dma_get_pages but comments for V4L2/Linaro people is needed.
> > 
> >> I couldn't guess, just from your patches, what ioctl's a V4L2
> >> application like tvtime or xawtv would use the DMABUF.
> > 
> > DMABUF is dedicated for application that use streaming between at least
> > two devices. Especially if those devices are controlled by different
> > APIs, like DRM and V4L2. It would be probably used in the middle-ware
> > like gstreamer or OpenMAX.
> 
> This is what the X11 v4l extension driver does: it shares DMA buffers
> between V4L2 and DRM. The extension currently relies on XV extension,
> simply because this is what were available at the time the extension was
> written. I didn't have any time yet to port it to use something more
> modern.
> 
> It is probably a good idea for you to take a look on it, when writing the
> API bits. Its source is available at:
> 
> 	http://cgit.freedesktop.org/xorg/driver/xf86-video-v4l/

-- 
Regards,

Laurent Pinchart

Re: [Linaro-mm-sig] [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Subash Patel]

Hello Thomasz,

Instead of adding another IOCTL to query the file-descriptor in 
user-space, why dont we extend the existing ones in v4l2/vb2?

When the memory type set is V4L2_MEMORY_DMABUF, call to VIDIOC_REQBUFS 
/VIDIOC_QUERYBUF from driver can take/return the fd. We will need to add 
another attribute to struct v4l2_requestbuffers for this.

struct v4l2_requestbuffers {
	...
	__u32 buf_fd;
};

The application requesting buffer would set it to -1, and will receive 
the fd's when it calls the vb2_querybuf. In the same way, application 
which is trying to attach to already allocated buffer will set this to 
valid fd when it calls vidioc_reqbuf, and in vb2_reqbuf depending on the 
memory type, this can be checked and used to attach with the dma_buf for 
the respective buffer.

Ofcourse, this requires changes in vb2_reqbufs and vb2_querybuf similar 
to what you did in vb2_expbufs.

Will there be any issues in such an approach?

Regards,
Subash

On 01/23/2012 07:21 PM, Tomasz Stanislawski wrote:
> This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
> mmap and return a file descriptor on success.
>
> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
> ---
>   drivers/media/video/v4l2-compat-ioctl32.c |    1 +
>   drivers/media/video/v4l2-ioctl.c          |   11 +++++++++++
>   include/linux/videodev2.h                 |    1 +
>   include/media/v4l2-ioctl.h                |    1 +
>   4 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
> index c68531b..0f18b5e 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -954,6 +954,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
>   	case VIDIOC_S_FBUF32:
>   	case VIDIOC_OVERLAY32:
>   	case VIDIOC_QBUF32:
> +	case VIDIOC_EXPBUF:
>   	case VIDIOC_DQBUF32:
>   	case VIDIOC_STREAMON32:
>   	case VIDIOC_STREAMOFF32:
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index e1da8fc..cb29e00 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -207,6 +207,7 @@ static const char *v4l2_ioctls[] = {
>   	[_IOC_NR(VIDIOC_S_FBUF)]           = "VIDIOC_S_FBUF",
>   	[_IOC_NR(VIDIOC_OVERLAY)]          = "VIDIOC_OVERLAY",
>   	[_IOC_NR(VIDIOC_QBUF)]             = "VIDIOC_QBUF",
> +	[_IOC_NR(VIDIOC_EXPBUF)]           = "VIDIOC_EXPBUF",
>   	[_IOC_NR(VIDIOC_DQBUF)]            = "VIDIOC_DQBUF",
>   	[_IOC_NR(VIDIOC_STREAMON)]         = "VIDIOC_STREAMON",
>   	[_IOC_NR(VIDIOC_STREAMOFF)]        = "VIDIOC_STREAMOFF",
> @@ -932,6 +933,16 @@ static long __video_do_ioctl(struct file *file,
>   			dbgbuf(cmd, vfd, p);
>   		break;
>   	}
> +	case VIDIOC_EXPBUF:
> +	{
> +		unsigned int *p = arg;
> +
> +		if (!ops->vidioc_expbuf)
> +			break;
> +
> +		ret = ops->vidioc_expbuf(file, fh, *p);
> +		break;
> +	}
>   	case VIDIOC_DQBUF:
>   	{
>   		struct v4l2_buffer *p = arg;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 3c0ade1..448fbed 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -2183,6 +2183,7 @@ struct v4l2_create_buffers {
>   #define VIDIOC_S_FBUF		 _IOW('V', 11, struct v4l2_framebuffer)
>   #define VIDIOC_OVERLAY		 _IOW('V', 14, int)
>   #define VIDIOC_QBUF		_IOWR('V', 15, struct v4l2_buffer)
> +#define VIDIOC_EXPBUF		_IOWR('V', 16, __u32)
>   #define VIDIOC_DQBUF		_IOWR('V', 17, struct v4l2_buffer)
>   #define VIDIOC_STREAMON		 _IOW('V', 18, int)
>   #define VIDIOC_STREAMOFF	 _IOW('V', 19, int)
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index 4d1c74a..8201546 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -120,6 +120,7 @@ struct v4l2_ioctl_ops {
>   	int (*vidioc_reqbufs) (struct file *file, void *fh, struct v4l2_requestbuffers *b);
>   	int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer *b);
>   	int (*vidioc_qbuf)    (struct file *file, void *fh, struct v4l2_buffer *b);
> +	int (*vidioc_expbuf)  (struct file *file, void *fh, __u32 offset);
>   	int (*vidioc_dqbuf)   (struct file *file, void *fh, struct v4l2_buffer *b);
>
>   	int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b);

Re: [Linaro-mm-sig] [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Tomasz Stanislawski]

Hi,

On 01/24/2012 12:07 PM, Subash Patel wrote:
> Hello Thomasz,
>
> Instead of adding another IOCTL to query the file-descriptor in
> user-space, why dont we extend the existing ones in v4l2/vb2?
>
> When the memory type set is V4L2_MEMORY_DMABUF, call to VIDIOC_REQBUFS
> /VIDIOC_QUERYBUF from driver can take/return the fd. We will need to add
> another attribute to struct v4l2_requestbuffers for this.
>
> struct v4l2_requestbuffers {
> ...
> __u32 buf_fd;
> };
>
> The application requesting buffer would set it to -1, and will receive
> the fd's when it calls the vb2_querybuf. In the same way, application
> which is trying to attach to already allocated buffer will set this to
> valid fd when it calls vidioc_reqbuf, and in vb2_reqbuf depending on the
> memory type, this can be checked and used to attach with the dma_buf for
> the respective buffer.
>
> Ofcourse, this requires changes in vb2_reqbufs and vb2_querybuf similar
> to what you did in vb2_expbufs.
>
> Will there be any issues in such an approach?

I like your idea but IMO there are some issues which this approach.

The VIDIOC_REQBUF is used to create a collection of buffers (i.e. 5 
frames). Every frame is a separate buffer therefore it should have 
separate dmabuf file descriptor. This way you should add buffer index to 
v4l2_request_buffers. Of course but one could reuse count field but 
there is still problem with multiplane buffers (see below).

Please note that dmabuf file could be create only for buffers with MMAP 
memory type. The DMABUF memory type for VIDIOC_REQBUFS indicate that a 
buffer is imported from DMABUF file descriptor. Similar way like content 
of USERPTR buffers is taken from user pointer.

Therefore only MMAP buffers can be exported as DMABUF file descriptor.

If VIDIOC_REQUBUF is used for buffer exporting, how to request a bunch 
of buffers that are dedicated to obtain memory from DMABUF file descriptors?

The second problem is VIDIOC_QUERYBUF. According to V4L2 spec, the 
memory type field is read only. The driver returns the same memory type 
as it was used in VIDIOC_REQBUFS. Therefore VIDIOC_QUERYBUF can only be 
used for MMAP buffers to obtain memoffset. For DMABUF it may return 
fd=-1 or most recently used file descriptor. As I remember, it was not 
defined yet.

The third reason are multiplane buffers. This API was introduced if V4L2 
buffer (IMO it should be called a frame) consist of multiple memory 
buffers. Every plane can be mmapped separately. Therefore it should be 
possible to export every plane as separate DMABUF descriptor.

After some research it was found that memoffset is good identifier of a 
plane in a buffers. This id is also available in the userspace without 
any API extensions.

VIDIOC_EXPBUF was used to export a buffer by id, similar way as mmap 
uses this identifier to map a buffer to userspace. It seams to be the 
simplest solution.

Fourth reason, VIDIOC_REQBUF means that the application request a 
buffer. DMABUF framework is used to export existing buffers. Note that 
memory may not be pinned to a buffer for some APIs like DRM. I think 
that is should stated explicitly that application wants to export not 
request a buffer. Using VIDIOC_REQBUF seams to be an API abuse.

What is your opinion?

Regards,
Tomasz Stanislawski

>
> Regards,
> Subash
>
> On 01/23/2012 07:21 PM, Tomasz Stanislawski wrote:
>> This patch adds extension to V4L2 api. It allow to export a mmap
>> buffer as file
>> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset
>> used by
>> mmap and return a file descriptor on success.
>>
>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>> ---
>> drivers/media/video/v4l2-compat-ioctl32.c | 1 +
>> drivers/media/video/v4l2-ioctl.c | 11 +++++++++++
>> include/linux/videodev2.h | 1 +
>> include/media/v4l2-ioctl.h | 1 +
>> 4 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
>> b/drivers/media/video/v4l2-compat-ioctl32.c
>> index c68531b..0f18b5e 100644
>> --- a/drivers/media/video/v4l2-compat-ioctl32.c
>> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
>> @@ -954,6 +954,7 @@ long v4l2_compat_ioctl32(struct file *file,
>> unsigned int cmd, unsigned long arg)
>> case VIDIOC_S_FBUF32:
>> case VIDIOC_OVERLAY32:
>> case VIDIOC_QBUF32:
>> + case VIDIOC_EXPBUF:
>> case VIDIOC_DQBUF32:
>> case VIDIOC_STREAMON32:
>> case VIDIOC_STREAMOFF32:
>> diff --git a/drivers/media/video/v4l2-ioctl.c
>> b/drivers/media/video/v4l2-ioctl.c
>> index e1da8fc..cb29e00 100644
>> --- a/drivers/media/video/v4l2-ioctl.c
>> +++ b/drivers/media/video/v4l2-ioctl.c
>> @@ -207,6 +207,7 @@ static const char *v4l2_ioctls[] = {
>> [_IOC_NR(VIDIOC_S_FBUF)] = "VIDIOC_S_FBUF",
>> [_IOC_NR(VIDIOC_OVERLAY)] = "VIDIOC_OVERLAY",
>> [_IOC_NR(VIDIOC_QBUF)] = "VIDIOC_QBUF",
>> + [_IOC_NR(VIDIOC_EXPBUF)] = "VIDIOC_EXPBUF",
>> [_IOC_NR(VIDIOC_DQBUF)] = "VIDIOC_DQBUF",
>> [_IOC_NR(VIDIOC_STREAMON)] = "VIDIOC_STREAMON",
>> [_IOC_NR(VIDIOC_STREAMOFF)] = "VIDIOC_STREAMOFF",
>> @@ -932,6 +933,16 @@ static long __video_do_ioctl(struct file *file,
>> dbgbuf(cmd, vfd, p);
>> break;
>> }
>> + case VIDIOC_EXPBUF:
>> + {
>> + unsigned int *p = arg;
>> +
>> + if (!ops->vidioc_expbuf)
>> + break;
>> +
>> + ret = ops->vidioc_expbuf(file, fh, *p);
>> + break;
>> + }
>> case VIDIOC_DQBUF:
>> {
>> struct v4l2_buffer *p = arg;
>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>> index 3c0ade1..448fbed 100644
>> --- a/include/linux/videodev2.h
>> +++ b/include/linux/videodev2.h
>> @@ -2183,6 +2183,7 @@ struct v4l2_create_buffers {
>> #define VIDIOC_S_FBUF _IOW('V', 11, struct v4l2_framebuffer)
>> #define VIDIOC_OVERLAY _IOW('V', 14, int)
>> #define VIDIOC_QBUF _IOWR('V', 15, struct v4l2_buffer)
>> +#define VIDIOC_EXPBUF _IOWR('V', 16, __u32)
>> #define VIDIOC_DQBUF _IOWR('V', 17, struct v4l2_buffer)
>> #define VIDIOC_STREAMON _IOW('V', 18, int)
>> #define VIDIOC_STREAMOFF _IOW('V', 19, int)
>> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
>> index 4d1c74a..8201546 100644
>> --- a/include/media/v4l2-ioctl.h
>> +++ b/include/media/v4l2-ioctl.h
>> @@ -120,6 +120,7 @@ struct v4l2_ioctl_ops {
>> int (*vidioc_reqbufs) (struct file *file, void *fh, struct
>> v4l2_requestbuffers *b);
>> int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer
>> *b);
>> int (*vidioc_qbuf) (struct file *file, void *fh, struct v4l2_buffer *b);
>> + int (*vidioc_expbuf) (struct file *file, void *fh, __u32 offset);
>> int (*vidioc_dqbuf) (struct file *file, void *fh, struct v4l2_buffer *b);
>>
>> int (*vidioc_create_bufs)(struct file *file, void *fh, struct
>> v4l2_create_buffers *b);
>

Re: [Linaro-mm-sig] [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Subash Patel]

Hello Tomasz,

Sorry for a late reply. Please find my answers inline.

On 01/24/2012 05:36 PM, Tomasz Stanislawski wrote:
> Hi,
>
> On 01/24/2012 12:07 PM, Subash Patel wrote:
>> Hello Thomasz,
>>
>> Instead of adding another IOCTL to query the file-descriptor in
>> user-space, why dont we extend the existing ones in v4l2/vb2?
>>
>> When the memory type set is V4L2_MEMORY_DMABUF, call to VIDIOC_REQBUFS
>> /VIDIOC_QUERYBUF from driver can take/return the fd. We will need to add
>> another attribute to struct v4l2_requestbuffers for this.
>>
>> struct v4l2_requestbuffers {
>> ...
>> __u32 buf_fd;
>> };
>>
>> The application requesting buffer would set it to -1, and will receive
>> the fd's when it calls the vb2_querybuf. In the same way, application
>> which is trying to attach to already allocated buffer will set this to
>> valid fd when it calls vidioc_reqbuf, and in vb2_reqbuf depending on the
>> memory type, this can be checked and used to attach with the dma_buf for
>> the respective buffer.
>>
>> Ofcourse, this requires changes in vb2_reqbufs and vb2_querybuf similar
>> to what you did in vb2_expbufs.
>>
>> Will there be any issues in such an approach?
>
> I like your idea but IMO there are some issues which this approach.
>
> The VIDIOC_REQBUF is used to create a collection of buffers (i.e. 5
> frames). Every frame is a separate buffer therefore it should have
> separate dmabuf file descriptor. This way you should add buffer index to
> v4l2_request_buffers. Of course but one could reuse count field but
> there is still problem with multiplane buffers (see below).
I agree.
>
> Please note that dmabuf file could be create only for buffers with MMAP
> memory type. The DMABUF memory type for VIDIOC_REQBUFS indicate that a
> buffer is imported from DMABUF file descriptor. Similar way like content
> of USERPTR buffers is taken from user pointer.
>
> Therefore only MMAP buffers can be exported as DMABUF file descriptor.
I think for time being, mmap is best way we can share buffers between 
IP's, like MM and GPU.
>
> If VIDIOC_REQUBUF is used for buffer exporting, how to request a bunch
> of buffers that are dedicated to obtain memory from DMABUF file
> descriptors?
I am not sure if I understand this question. But what I meant is, 
VIDIOC_REQBUF with memory type V4L2_MEMORY_DMABUF will ask the driver to 
allocate MMAP type memory and create the dmabuf handles for those. In 
the next call to VIDIOC_QUERYBUF (which you do on each of the buffers 
infact), driver will return the dmabuf file descriptors to user space. 
This can be used by the user space to mmap(when dmabuf supports) or pass 
it onto another process to share the buffers.

The recipient user process can now pass these dmabuf file descriptors in 
VIDIOC_REQBUF itself to its driver (say another v4l2 based). In the 
driver, when the user space calls VIDIOC_QUERYBUF for those buffers, we 
can call dmabuf's attach() to actually link to the buffer. Does it make 
sense?
>
> The second problem is VIDIOC_QUERYBUF. According to V4L2 spec, the
> memory type field is read only. The driver returns the same memory type
> as it was used in VIDIOC_REQBUFS. Therefore VIDIOC_QUERYBUF can only be
> used for MMAP buffers to obtain memoffset. For DMABUF it may return
> fd=-1 or most recently used file descriptor. As I remember, it was not
> defined yet.
>
I was thinking why not the memory type move out from enum to an int? In 
that case, we can send ORed types, like 
V4L2_MEMORY_MMAP|V4L2_MEMORY_DMABUF etc. Does it help?

> The third reason are multiplane buffers. This API was introduced if V4L2
> buffer (IMO it should be called a frame) consist of multiple memory
> buffers. Every plane can be mmapped separately. Therefore it should be
> possible to export every plane as separate DMABUF descriptor.
>
Do we require to export every plane as separate dmabuf? I think dmabuf 
should cover entire multi-plane buffer instead of individual planes. How 
to calculate individual plane offsets should be the property of the 
driver depending on its need for it.

> After some research it was found that memoffset is good identifier of a
> plane in a buffers. This id is also available in the userspace without
> any API extensions.
>
> VIDIOC_EXPBUF was used to export a buffer by id, similar way as mmap
> uses this identifier to map a buffer to userspace. It seams to be the
> simplest solution.
>
Ok. Then dont you think when dmabuf supports mmaping the buffers, 
VIDIOC_REQBUF will be useless? VIDIOC_EXPBUF will provide that 
functionality as well.

> Fourth reason, VIDIOC_REQBUF means that the application request a
> buffer. DMABUF framework is used to export existing buffers. Note that
> memory may not be pinned to a buffer for some APIs like DRM. I think
> that is should stated explicitly that application wants to export not
> request a buffer. Using VIDIOC_REQBUF seams to be an API abuse.
I agree that memory may not be static like V4L2 in case of DRM. But this 
is still an issue even with a new IOCTL :) VIDIOC_REQBUF might not be 
abused as far I feel. It is extended to a) request buffers if not 
allocated b) request, as well as pass the information of already 
allocated buffers if any. Both ways, we are using it for the same 
purpose, requesting the buffers from corresponding driver.
>
> What is your opinion?
My concern is what will happen to applications which are already written 
with existing V4L2 IOCTL flows on new framework? i.e., if I have 
developed an application for say, just camera using s5p-fimc, and 
s5p-fimc driver eventually moves to dmabuf. If the application wont move 
into new buffer sharing framework, will things work in first place, and 
second will driver state move even if the new IOCTL is not made (driver 
can question QBUF without QUERYBUF and/or EXPBUF)?

Regards,
Subash
>
> Regards,
> Tomasz Stanislawski
>
>>
>> Regards,
>> Subash
>>
>> On 01/23/2012 07:21 PM, Tomasz Stanislawski wrote:
>>> This patch adds extension to V4L2 api. It allow to export a mmap
>>> buffer as file
>>> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset
>>> used by
>>> mmap and return a file descriptor on success.
>>>
>>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>> ---
>>> drivers/media/video/v4l2-compat-ioctl32.c | 1 +
>>> drivers/media/video/v4l2-ioctl.c | 11 +++++++++++
>>> include/linux/videodev2.h | 1 +
>>> include/media/v4l2-ioctl.h | 1 +
>>> 4 files changed, 14 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
>>> b/drivers/media/video/v4l2-compat-ioctl32.c
>>> index c68531b..0f18b5e 100644
>>> --- a/drivers/media/video/v4l2-compat-ioctl32.c
>>> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
>>> @@ -954,6 +954,7 @@ long v4l2_compat_ioctl32(struct file *file,
>>> unsigned int cmd, unsigned long arg)
>>> case VIDIOC_S_FBUF32:
>>> case VIDIOC_OVERLAY32:
>>> case VIDIOC_QBUF32:
>>> + case VIDIOC_EXPBUF:
>>> case VIDIOC_DQBUF32:
>>> case VIDIOC_STREAMON32:
>>> case VIDIOC_STREAMOFF32:
>>> diff --git a/drivers/media/video/v4l2-ioctl.c
>>> b/drivers/media/video/v4l2-ioctl.c
>>> index e1da8fc..cb29e00 100644
>>> --- a/drivers/media/video/v4l2-ioctl.c
>>> +++ b/drivers/media/video/v4l2-ioctl.c
>>> @@ -207,6 +207,7 @@ static const char *v4l2_ioctls[] = {
>>> [_IOC_NR(VIDIOC_S_FBUF)] = "VIDIOC_S_FBUF",
>>> [_IOC_NR(VIDIOC_OVERLAY)] = "VIDIOC_OVERLAY",
>>> [_IOC_NR(VIDIOC_QBUF)] = "VIDIOC_QBUF",
>>> + [_IOC_NR(VIDIOC_EXPBUF)] = "VIDIOC_EXPBUF",
>>> [_IOC_NR(VIDIOC_DQBUF)] = "VIDIOC_DQBUF",
>>> [_IOC_NR(VIDIOC_STREAMON)] = "VIDIOC_STREAMON",
>>> [_IOC_NR(VIDIOC_STREAMOFF)] = "VIDIOC_STREAMOFF",
>>> @@ -932,6 +933,16 @@ static long __video_do_ioctl(struct file *file,
>>> dbgbuf(cmd, vfd, p);
>>> break;
>>> }
>>> + case VIDIOC_EXPBUF:
>>> + {
>>> + unsigned int *p = arg;
>>> +
>>> + if (!ops->vidioc_expbuf)
>>> + break;
>>> +
>>> + ret = ops->vidioc_expbuf(file, fh, *p);
>>> + break;
>>> + }
>>> case VIDIOC_DQBUF:
>>> {
>>> struct v4l2_buffer *p = arg;
>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>> index 3c0ade1..448fbed 100644
>>> --- a/include/linux/videodev2.h
>>> +++ b/include/linux/videodev2.h
>>> @@ -2183,6 +2183,7 @@ struct v4l2_create_buffers {
>>> #define VIDIOC_S_FBUF _IOW('V', 11, struct v4l2_framebuffer)
>>> #define VIDIOC_OVERLAY _IOW('V', 14, int)
>>> #define VIDIOC_QBUF _IOWR('V', 15, struct v4l2_buffer)
>>> +#define VIDIOC_EXPBUF _IOWR('V', 16, __u32)
>>> #define VIDIOC_DQBUF _IOWR('V', 17, struct v4l2_buffer)
>>> #define VIDIOC_STREAMON _IOW('V', 18, int)
>>> #define VIDIOC_STREAMOFF _IOW('V', 19, int)
>>> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
>>> index 4d1c74a..8201546 100644
>>> --- a/include/media/v4l2-ioctl.h
>>> +++ b/include/media/v4l2-ioctl.h
>>> @@ -120,6 +120,7 @@ struct v4l2_ioctl_ops {
>>> int (*vidioc_reqbufs) (struct file *file, void *fh, struct
>>> v4l2_requestbuffers *b);
>>> int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer
>>> *b);
>>> int (*vidioc_qbuf) (struct file *file, void *fh, struct v4l2_buffer *b);
>>> + int (*vidioc_expbuf) (struct file *file, void *fh, __u32 offset);
>>> int (*vidioc_dqbuf) (struct file *file, void *fh, struct v4l2_buffer
>>> *b);
>>>
>>> int (*vidioc_create_bufs)(struct file *file, void *fh, struct
>>> v4l2_create_buffers *b);
>>
>

Re: [Linaro-mm-sig] [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Subash,

On Friday 27 January 2012 14:40:34 Subash Patel wrote:
> On 01/24/2012 05:36 PM, Tomasz Stanislawski wrote:
> > On 01/24/2012 12:07 PM, Subash Patel wrote:
> >> 
> >> Instead of adding another IOCTL to query the file-descriptor in
> >> user-space, why dont we extend the existing ones in v4l2/vb2?
> >> 
> >> When the memory type set is V4L2_MEMORY_DMABUF, call to VIDIOC_REQBUFS
> >> /VIDIOC_QUERYBUF from driver can take/return the fd. We will need to add
> >> another attribute to struct v4l2_requestbuffers for this.
> >> 
> >> struct v4l2_requestbuffers {
> >> ...
> >> __u32 buf_fd;
> >> };
> >> 
> >> The application requesting buffer would set it to -1, and will receive
> >> the fd's when it calls the vb2_querybuf. In the same way, application
> >> which is trying to attach to already allocated buffer will set this to
> >> valid fd when it calls vidioc_reqbuf, and in vb2_reqbuf depending on the
> >> memory type, this can be checked and used to attach with the dma_buf for
> >> the respective buffer.
> >> 
> >> Ofcourse, this requires changes in vb2_reqbufs and vb2_querybuf similar
> >> to what you did in vb2_expbufs.
> >> 
> >> Will there be any issues in such an approach?
> > 
> > I like your idea but IMO there are some issues which this approach.
> > 
> > The VIDIOC_REQBUF is used to create a collection of buffers (i.e. 5
> > frames). Every frame is a separate buffer therefore it should have
> > separate dmabuf file descriptor. This way you should add buffer index to
> > v4l2_request_buffers. Of course but one could reuse count field but
> > there is still problem with multiplane buffers (see below).
> 
> I agree.
> 
> > Please note that dmabuf file could be create only for buffers with MMAP
> > memory type. The DMABUF memory type for VIDIOC_REQBUFS indicate that a
> > buffer is imported from DMABUF file descriptor. Similar way like content
> > of USERPTR buffers is taken from user pointer.
> > 
> > Therefore only MMAP buffers can be exported as DMABUF file descriptor.
> 
> I think for time being, mmap is best way we can share buffers between
> IP's, like MM and GPU.
> 
> > If VIDIOC_REQUBUF is used for buffer exporting, how to request a bunch
> > of buffers that are dedicated to obtain memory from DMABUF file
> > descriptors?
> 
> I am not sure if I understand this question. But what I meant is,
> VIDIOC_REQBUF with memory type V4L2_MEMORY_DMABUF will ask the driver to
> allocate MMAP type memory and create the dmabuf handles for those. In
> the next call to VIDIOC_QUERYBUF (which you do on each of the buffers
> infact), driver will return the dmabuf file descriptors to user space.
> This can be used by the user space to mmap(when dmabuf supports) or pass
> it onto another process to share the buffers.

That would be nice, but VIDIOC_REQBUFS(DMABUF) is already used to allocate 
buffers that will be imported using dma-buf. We need to handle both the 
importer use case and the exporter use case, we can't handle both with just 
VIDIOC_REQBUFS(DMABUF).

> The recipient user process can now pass these dmabuf file descriptors in
> VIDIOC_REQBUF itself to its driver (say another v4l2 based). In the
> driver, when the user space calls VIDIOC_QUERYBUF for those buffers, we
> can call dmabuf's attach() to actually link to the buffer. Does it make
> sense?
> 
> > The second problem is VIDIOC_QUERYBUF. According to V4L2 spec, the
> > memory type field is read only. The driver returns the same memory type
> > as it was used in VIDIOC_REQBUFS. Therefore VIDIOC_QUERYBUF can only be
> > used for MMAP buffers to obtain memoffset. For DMABUF it may return
> > fd=-1 or most recently used file descriptor. As I remember, it was not
> > defined yet.
> 
> I was thinking why not the memory type move out from enum to an int? In
> that case, we can send ORed types, like
> V4L2_MEMORY_MMAP|V4L2_MEMORY_DMABUF etc. Does it help?

That would break our API, so we can't do that (although we could still OR enum 
types).

> > The third reason are multiplane buffers. This API was introduced if V4L2
> > buffer (IMO it should be called a frame) consist of multiple memory
> > buffers. Every plane can be mmapped separately. Therefore it should be
> > possible to export every plane as separate DMABUF descriptor.
> 
> Do we require to export every plane as separate dmabuf? I think dmabuf
> should cover entire multi-plane buffer instead of individual planes.

It depends on the drivers you use. Some drivers require planes to be 
contiguous in memory, in which case a single dma-buf will likely be used. 
Other drivers can allocate the planes in separate memory regions. Several dma-
buf objects then make sense.

> How to calculate individual plane offsets should be the property of the
> driver depending on its need for it.
>
> > After some research it was found that memoffset is good identifier of a
> > plane in a buffers. This id is also available in the userspace without
> > any API extensions.
> > 
> > VIDIOC_EXPBUF was used to export a buffer by id, similar way as mmap
> > uses this identifier to map a buffer to userspace. It seams to be the
> > simplest solution.
> 
> Ok. Then dont you think when dmabuf supports mmaping the buffers,
> VIDIOC_REQBUF will be useless? VIDIOC_EXPBUF will provide that functionality
> as well.

VIDIOC_REQBUFS will be used to allocate the buffers, and VIDIOC_EXPBUF will be 
used to export them. Those are independent operations.

> > Fourth reason, VIDIOC_REQBUF means that the application request a
> > buffer. DMABUF framework is used to export existing buffers. Note that
> > memory may not be pinned to a buffer for some APIs like DRM. I think
> > that is should stated explicitly that application wants to export not
> > request a buffer. Using VIDIOC_REQBUF seams to be an API abuse.
> 
> I agree that memory may not be static like V4L2 in case of DRM. But this
> is still an issue even with a new IOCTL :) VIDIOC_REQBUF might not be
> abused as far I feel. It is extended to a) request buffers if not
> allocated b) request, as well as pass the information of already
> allocated buffers if any. Both ways, we are using it for the same
> purpose, requesting the buffers from corresponding driver.
> 
> > What is your opinion?
> 
> My concern is what will happen to applications which are already written
> with existing V4L2 IOCTL flows on new framework? i.e., if I have developed
> an application for say, just camera using s5p-fimc, and s5p-fimc driver
> eventually moves to dmabuf. If the application wont move into new buffer
> sharing framework, will things work in first place, and second will driver
> state move even if the new IOCTL is not made (driver can question QBUF
> without QUERYBUF and/or EXPBUF)?

Things will not break. Your existing applications will still use MMAP and/or 
USERPTR buffers. DMABUF support will add dma-buf import and export 
capabilities, but it won't remove existing features.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Tomasz Stanislawski]

Hi Everyone,

I would like to present a simple test application used for testing 
DMABUF support in V4L2. It is used to show how support for DMABUF may 
look like in V4L2.

The test application creates a simple pipeline between two V4L2 devices. 
One of them is a capture device. The second one is an output device.

The buffers are exchanged using DMABUF mechanism. The application takes 
additional argument to setup capture's size and rotation and compose 
window on output device (controlled using VIDIOC_S_CROP).

The application was tested on s5p-tv as output and s5p-fimc as capture.
It is written in GNU99 standard

Regards,
Tomasz Stanislawski

----


#include <errno.h>
#include <fcntl.h>
#include <linux/videodev2.h>
#include <math.h>
#include <poll.h>
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>

#define ERRSTR strerror(errno)

#define BYE_ON(cond, ...) \
do { \
	if (cond) { \
		int errsv = errno; \
		fprintf(stderr, "ERROR(%s:%d) : ", \
			__FILE__, __LINE__); \
		errno = errsv; \
		fprintf(stderr,  __VA_ARGS__); \
		abort(); \
	} \
} while(0)

#define BUFFER_CNT	5

int main(int argc, char *argv[])
{
	int ret;

	BYE_ON(argc < 3, "bad args:\n\t%s input-node output-node "
		"[w,h,rot] [left,top,w,h]\n", argv[0]);

	int f_in, f_out;

	f_in = open(argv[1], O_RDWR);
	BYE_ON(f_in < 0, "open failed: %s\n", ERRSTR);

	f_out = open(argv[2], O_RDWR);
	BYE_ON(f_out < 0, "open failed: %s\n", ERRSTR);

	/* configure desired image size */
	struct v4l2_format fmt;
	memset(&fmt, 0, sizeof fmt);
	fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
	fmt.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_RGB565;
	int rotation = 0;
	if (argc >= 4) {
		ret = sscanf(argv[3], "%u,%u,%d",
			&fmt.fmt.pix_mp.width,
			&fmt.fmt.pix_mp.height,
			&rotation);
		BYE_ON(ret < 2, "incorrect sensor size and rotation\n");
	}

	if (rotation) {
		struct v4l2_control ctrl = {
			.id = V4L2_CID_ROTATE,
			.value = rotation,
		};
		ret = ioctl(f_in, VIDIOC_S_CTRL, &ctrl);
		BYE_ON(ret < 0, "VIDIOC_S_CTRL failed: %s\n", ERRSTR);
	}

	/* set format struct */
	ret = ioctl(f_in, VIDIOC_S_FMT, &fmt);
	BYE_ON(ret < 0, "VIDIOC_S_FMT failed: %s\n", ERRSTR);

	/* get format struct */
	ret = ioctl(f_in, VIDIOC_G_FMT, &fmt);
	BYE_ON(ret < 0, "VIDIOC_G_FMT failed: %s\n", ERRSTR);
	printf("G_FMT(f_in): width = %u, height = %u, 4cc = %.4s\n",
		fmt.fmt.pix.width, fmt.fmt.pix.height,
		(char*)&fmt.fmt.pix.pixelformat);

	if (argc >= 5) {
		struct v4l2_crop crop;
		crop.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
		ret = sscanf(argv[4], "%d,%d,%d,%d",
			&crop.c.left, &crop.c.top,
			&crop.c.width, &crop.c.height);
		BYE_ON(ret != 4, "incorrect cropping format\n");
		ret = ioctl(f_out, VIDIOC_S_CROP, &crop);
		BYE_ON(ret < 0, "VIDIOC_S_CROP failed: %s\n", ERRSTR);
	}

	/* pass input format to output */
	fmt.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
	ret = ioctl(f_out, VIDIOC_S_FMT, &fmt);
	BYE_ON(ret < 0, "VIDIOC_S_FMT failed: %s\n", ERRSTR);

	/* get format struct */
	ret = ioctl(f_out, VIDIOC_G_FMT, &fmt);
	BYE_ON(ret < 0, "VIDIOC_G_FMT failed: %s\n", ERRSTR);
	printf("G_FMT(f_out): width = %u, height = %u, 4cc = %.4s\n",
		fmt.fmt.pix.width, fmt.fmt.pix.height,
		(char*)&fmt.fmt.pix.pixelformat);

	/* allocate buffers */
	struct v4l2_requestbuffers rqbufs;
	rqbufs.count = BUFFER_CNT;
	rqbufs.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
	rqbufs.memory = V4L2_MEMORY_MMAP;

	ret = ioctl(f_in, VIDIOC_REQBUFS, &rqbufs);
	BYE_ON(ret < 0, "VIDIOC_REQBUFS failed: %s\n", ERRSTR);
	BYE_ON(rqbufs.count < BUFFER_CNT, "failed to get %d buffers\n", 
BUFFER_CNT);

	rqbufs.count = BUFFER_CNT;
	rqbufs.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
	rqbufs.memory = V4L2_MEMORY_DMABUF;

	ret = ioctl(f_out, VIDIOC_REQBUFS, &rqbufs);
	BYE_ON(ret < 0, "VIDIOC_REQBUFS failed: %s\n", ERRSTR);
	BYE_ON(rqbufs.count < BUFFER_CNT, "failed to get %d buffers\n", 
BUFFER_CNT);

	int fd[BUFFER_CNT];
	/* buffers initalization */
	for (int i = 0; i < BUFFER_CNT; ++i) {
		struct v4l2_plane plane;
		struct v4l2_buffer buf;

		memset(&buf, 0, sizeof buf);
		memset(&plane, 0, sizeof plane);
		buf.index = i;
		buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
		buf.memory = V4L2_MEMORY_MMAP;
		buf.m.planes = &plane;
		buf.length = 1;

		/* get buffer properties from a driver */
		ret = ioctl(f_in, VIDIOC_QUERYBUF, &buf);
		BYE_ON(ret < 0, "VIDIOC_QUERYBUF for buffer %d failed: %s\n",
			buf.index, ERRSTR);
		BYE_ON(buf.memory != V4L2_MEMORY_MMAP, "bad memory type\n");
		BYE_ON(buf.length != 1, "non singular plane format\n");

		fd[i] = ioctl(f_in, VIDIOC_EXPBUF, &plane.m.mem_offset);
		BYE_ON(fd[i] < 0, "VIDIOC_EXPBUF failed; %s\n", ERRSTR);

		if (i < BUFFER_CNT / 2) {
			/* enqueue low-index buffers to input */
			ret = ioctl(f_in, VIDIOC_QBUF, &buf);
		} else {
			/* enqueue high-index buffers to output */
			buf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
			buf.memory = V4L2_MEMORY_DMABUF;
			plane.m.fd = fd[i];
			ret = ioctl(f_out, VIDIOC_QBUF, &buf);
		}
		BYE_ON(ret < 0, "VIDIOC_QBUF for buffer %d failed: %s\n",
			buf.index, ERRSTR);
	}

	/* start streaming */
	int type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
	ret = ioctl(f_in, VIDIOC_STREAMON, &type);
	BYE_ON(ret < 0, "STREAMON failed: %s\n", ERRSTR);

	type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
	ret = ioctl(f_out, VIDIOC_STREAMON, &type);
	BYE_ON(ret < 0, "STREAMON failed: %s\n", ERRSTR);

	/* setup polling */
	struct pollfd fds[2] = {
		{ .fd = f_in, .events = POLLIN },
		{ .fd = f_out, .events = POLLOUT },
	};

	while ((ret = poll(fds, 2, 5000)) > 0) {
		struct v4l2_buffer buf;
		struct v4l2_plane plane;

		memset(&buf, 0, sizeof buf);
		memset(&plane, 0, sizeof plane);
		buf.m.planes = &plane;
		buf.length = 1;

		if (fds[0].revents & POLLIN) {
			/* dequeue buffer */
			buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
			buf.memory = V4L2_MEMORY_MMAP;
			ret = ioctl(f_in, VIDIOC_DQBUF, &buf);
			BYE_ON(ret, "VIDIOC_DQBUF failed: %s\n", ERRSTR);

			/* enqueue buffer */
			buf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
			buf.memory = V4L2_MEMORY_DMABUF;
			plane.m.fd = fd[buf.index];
			ret = ioctl(f_out, VIDIOC_QBUF, &buf);
			BYE_ON(ret, "VIDIOC_QBUF failed: %s\n", ERRSTR);
		}
		if (fds[1].revents & POLLOUT) {
			/* dequeue buffer */
			buf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
			buf.memory = V4L2_MEMORY_DMABUF;
			ret = ioctl(f_out, VIDIOC_DQBUF, &buf);
			BYE_ON(ret, "VIDIOC_DQBUF failed: %s\n", ERRSTR);

			/* enqueue buffer */
			buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
			buf.memory = V4L2_MEMORY_MMAP;
			ret = ioctl(f_in, VIDIOC_QBUF, &buf);
			BYE_ON(ret, "VIDIOC_QBUF failed: %s\n", ERRSTR);
		}
	}

	BYE_ON(ret == 0, "polling timeout\n");
	BYE_ON(1, "polling stopped: %s\n", ERRSTR);

	return 0;
}

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Pawel Osciak]

Hi Tomasz,
I like the direction in which you are going with the userspace
handling. This is almost exactly as I envisioned it. I have one
comment though:

On Thu, Jan 26, 2012 at 01:48, Tomasz Stanislawski
<t.stanislaws@samsung.com> wrote:

[snip]

>        /* setup polling */
>        struct pollfd fds[2] = {
>                { .fd = f_in, .events = POLLIN },
>                { .fd = f_out, .events = POLLOUT },
>        };
>
>        while ((ret = poll(fds, 2, 5000)) > 0) {
>                struct v4l2_buffer buf;
>                struct v4l2_plane plane;
>
>                memset(&buf, 0, sizeof buf);
>                memset(&plane, 0, sizeof plane);
>                buf.m.planes = &plane;
>                buf.length = 1;
>
>                if (fds[0].revents & POLLIN) {
>                        /* dequeue buffer */
>                        buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>                        buf.memory = V4L2_MEMORY_MMAP;
>                        ret = ioctl(f_in, VIDIOC_DQBUF, &buf);
>                        BYE_ON(ret, "VIDIOC_DQBUF failed: %s\n", ERRSTR);
>
>                        /* enqueue buffer */
>                        buf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
>                        buf.memory = V4L2_MEMORY_DMABUF;
>                        plane.m.fd = fd[buf.index];
>                        ret = ioctl(f_out, VIDIOC_QBUF, &buf);
>                        BYE_ON(ret, "VIDIOC_QBUF failed: %s\n", ERRSTR);
>                }

This passes fd, so the OUTPUT driver will get the correct buffer from dmabuf.

>                if (fds[1].revents & POLLOUT) {
>                        /* dequeue buffer */
>                        buf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
>                        buf.memory = V4L2_MEMORY_DMABUF;
>                        ret = ioctl(f_out, VIDIOC_DQBUF, &buf);
>                        BYE_ON(ret, "VIDIOC_DQBUF failed: %s\n", ERRSTR);
>
>                        /* enqueue buffer */
>                        buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>                        buf.memory = V4L2_MEMORY_MMAP;
>                        ret = ioctl(f_in, VIDIOC_QBUF, &buf);
>                        BYE_ON(ret, "VIDIOC_QBUF failed: %s\n", ERRSTR);
>                }

This, however, relies on the indexes to be equal for the same
buffers/planes in both drivers. I don't see why we should restrict
ourselves to that. In fact, we must not. You should have a reverse
mapping of fd->index for the INPUT device and use the fd returned in
buf by DQBUF from OUTPUT device to look-up the correct index to be
passed to the INPUT device.

>        }
>
>        BYE_ON(ret == 0, "polling timeout\n");
>        BYE_ON(1, "polling stopped: %s\n", ERRSTR);
>
>        return 0;
> }



-- 
Best regards,
Pawel Osciak

Re: [PATCH 05/10] v4l: add buffer exporting via dmabuf

[Pawel Osciak]

Hi Tomasz,

On Mon, Jan 23, 2012 at 05:51, Tomasz Stanislawski
<t.stanislaws@samsung.com> wrote:
> This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
> mmap and return a file descriptor on success.
>
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/video/v4l2-compat-ioctl32.c |    1 +
>  drivers/media/video/v4l2-ioctl.c          |   11 +++++++++++
>  include/linux/videodev2.h                 |    1 +
>  include/media/v4l2-ioctl.h                |    1 +
>  4 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
> index c68531b..0f18b5e 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -954,6 +954,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
>        case VIDIOC_S_FBUF32:
>        case VIDIOC_OVERLAY32:
>        case VIDIOC_QBUF32:
> +       case VIDIOC_EXPBUF:
>        case VIDIOC_DQBUF32:
>        case VIDIOC_STREAMON32:
>        case VIDIOC_STREAMOFF32:
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index e1da8fc..cb29e00 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -207,6 +207,7 @@ static const char *v4l2_ioctls[] = {
>        [_IOC_NR(VIDIOC_S_FBUF)]           = "VIDIOC_S_FBUF",
>        [_IOC_NR(VIDIOC_OVERLAY)]          = "VIDIOC_OVERLAY",
>        [_IOC_NR(VIDIOC_QBUF)]             = "VIDIOC_QBUF",
> +       [_IOC_NR(VIDIOC_EXPBUF)]           = "VIDIOC_EXPBUF",
>        [_IOC_NR(VIDIOC_DQBUF)]            = "VIDIOC_DQBUF",
>        [_IOC_NR(VIDIOC_STREAMON)]         = "VIDIOC_STREAMON",
>        [_IOC_NR(VIDIOC_STREAMOFF)]        = "VIDIOC_STREAMOFF",
> @@ -932,6 +933,16 @@ static long __video_do_ioctl(struct file *file,
>                        dbgbuf(cmd, vfd, p);
>                break;
>        }
> +       case VIDIOC_EXPBUF:
> +       {
> +               unsigned int *p = arg;
> +
> +               if (!ops->vidioc_expbuf)
> +                       break;
> +
> +               ret = ops->vidioc_expbuf(file, fh, *p);
> +               break;
> +       }

Personally, I believe we shouldn't just limit this to one u32
argument. Granted, right now this indeed is enough for MMAP buffers,
but maybe we should be careful here and not make future additions
impossible? Perhaps, in the future, a use case surfaces that requires
a different argument than an offset? Or maybe new memory types are
added to V4L2 and this prevents us from supporting them? Or, perhaps,
dmabuf adds new functionality that could be controlled from the
userspace (some flags or something for example) and we wouldn't be
able to support them here?

I just feel this is not symmetric to the other parts of V4L2 API and
does not leave us prepared for any future additions to V4L2 and/or
dmabuf. I feel we should use a structure here instead and keep it
symmetric with REQBUFS and other ioctls (with regards to memory types,
union for offset/etc.), and also add a few reserved fields.

[snip]

-- 
Best regards,
Pawel Osciak

[PATCH 06/10] v4l: vb2: add buffer exporting via dmabuf

[Tomasz Stanislawski]

This patch adds extension to videobuf2-core. It allow to export a mmap buffer
as a file descriptor.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/videobuf2-core.c |   60 ++++++++++++++++++++++++++++++++++
 include/media/videobuf2-core.h       |    2 +
 2 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 59bb1bc..29cf6ed 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -1522,6 +1522,66 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
 }
 EXPORT_SYMBOL_GPL(vb2_dqbuf);
 
+static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off,
+			unsigned int *_buffer, unsigned int *_plane);
+
+/**
+ * vb2_expbuf() - Export a buffer as a file descriptor
+ * @q:		videobuf2 queue
+ * @b:		buffer structure passed from userspace to vidioc_expbuf handler
+ *		in driver
+ *
+ * The return values from this function are intended to be directly returned
+ * from vidioc_expbuf handler in driver.
+ */
+int vb2_expbuf(struct vb2_queue *q, unsigned int offset)
+{
+	struct vb2_buffer *vb = NULL;
+	struct vb2_plane *vb_plane;
+	unsigned int buffer, plane;
+	int ret;
+	struct dma_buf *dbuf;
+
+	if (q->memory != V4L2_MEMORY_MMAP) {
+		dprintk(1, "Queue is not currently set up for mmap\n");
+		return -EINVAL;
+	}
+
+	if (!q->mem_ops->get_dmabuf) {
+		dprintk(1, "Queue does not support DMA buffer exporting\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Find the plane corresponding to the offset passed by userspace.
+	 */
+	ret = __find_plane_by_offset(q, offset, &buffer, &plane);
+	if (ret) {
+		dprintk(1, "invalid offset %d\n", offset);
+		return ret;
+	}
+
+	vb = q->bufs[buffer];
+	vb_plane = &vb->planes[plane];
+
+	dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv);
+	if (IS_ERR_OR_NULL(dbuf)) {
+		dprintk(1, "Failed to export buffer %d, plane %d\n",
+			buffer, plane);
+		return -EINVAL;
+	}
+
+	ret = dma_buf_fd(dbuf);
+	if (ret < 0)
+		dprintk(3, "buffer %d, plane %d failed to export (%d)\n",
+			buffer, plane, ret);
+	else
+		dprintk(3, "buffer %d, plane %d exported as %d descriptor\n",
+			buffer, plane, ret);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(vb2_expbuf);
+
 /**
  * __vb2_queue_cancel() - cancel and stop (pause) streaming
  *
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 412c6a4..3d43954 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -79,6 +79,7 @@ struct vb2_mem_ops {
 	void		(*prepare)(void *buf_priv);
 	void		(*finish)(void *buf_priv);
 	void		(*put)(void *buf_priv);
+	struct dma_buf *(*get_dmabuf)(void *buf_priv);
 
 	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
 					unsigned long size, int write);
@@ -348,6 +349,7 @@ int vb2_queue_init(struct vb2_queue *q);
 void vb2_queue_release(struct vb2_queue *q);
 
 int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b);
+int vb2_expbuf(struct vb2_queue *q, unsigned int offset);
 int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking);
 
 int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type);
-- 
1.7.5.4

[PATCH 07/10] v4l: vb2: remove dma-contig allocator

[Tomasz Stanislawski]

This is temporary patch. The dma-contig changes were significant
and the difference patch would be very difficult to read.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/videobuf2-dma-contig.c |  308 ----------------------------
 1 files changed, 0 insertions(+), 308 deletions(-)
 delete mode 100644 drivers/media/video/videobuf2-dma-contig.c

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
deleted file mode 100644
index ea2699f..0000000
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ /dev/null
@@ -1,308 +0,0 @@
-/*
- * videobuf2-dma-contig.c - DMA contig memory allocator for videobuf2
- *
- * Copyright (C) 2010 Samsung Electronics
- *
- * Author: Pawel Osciak <pawel@osciak.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation.
- */
-
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/dma-mapping.h>
-#include <linux/scatterlist.h>
-#include <linux/dma-buf.h>
-
-#include <media/videobuf2-core.h>
-#include <media/videobuf2-memops.h>
-
-struct vb2_dc_conf {
-	struct device		*dev;
-};
-
-struct vb2_dc_buf {
-	struct vb2_dc_conf		*conf;
-	void				*vaddr;
-	dma_addr_t			dma_addr;
-	unsigned long			size;
-	struct vm_area_struct		*vma;
-	struct dma_buf_attachment	*db_attach;
-	atomic_t			refcount;
-	struct vb2_vmarea_handler	handler;
-};
-
-static void vb2_dma_contig_put(void *buf_priv);
-
-static void *vb2_dma_contig_alloc(void *alloc_ctx, unsigned long size)
-{
-	struct vb2_dc_conf *conf = alloc_ctx;
-	struct vb2_dc_buf *buf;
-	/* TODO: add db_attach processing while adding DMABUF as exporter */
-
-	buf = kzalloc(sizeof *buf, GFP_KERNEL);
-	if (!buf)
-		return ERR_PTR(-ENOMEM);
-
-	buf->vaddr = dma_alloc_coherent(conf->dev, size, &buf->dma_addr,
-					GFP_KERNEL);
-	if (!buf->vaddr) {
-		dev_err(conf->dev, "dma_alloc_coherent of size %ld failed\n",
-			size);
-		kfree(buf);
-		return ERR_PTR(-ENOMEM);
-	}
-
-	buf->conf = conf;
-	buf->size = size;
-
-	buf->handler.refcount = &buf->refcount;
-	buf->handler.put = vb2_dma_contig_put;
-	buf->handler.arg = buf;
-
-	atomic_inc(&buf->refcount);
-
-	return buf;
-}
-
-static void vb2_dma_contig_put(void *buf_priv)
-{
-	struct vb2_dc_buf *buf = buf_priv;
-
-	if (atomic_dec_and_test(&buf->refcount)) {
-		dma_free_coherent(buf->conf->dev, buf->size, buf->vaddr,
-				  buf->dma_addr);
-		kfree(buf);
-	}
-}
-
-static void *vb2_dma_contig_cookie(void *buf_priv)
-{
-	struct vb2_dc_buf *buf = buf_priv;
-
-	return &buf->dma_addr;
-}
-
-static void *vb2_dma_contig_vaddr(void *buf_priv)
-{
-	struct vb2_dc_buf *buf = buf_priv;
-	if (!buf)
-		return 0;
-
-	return buf->vaddr;
-}
-
-static unsigned int vb2_dma_contig_num_users(void *buf_priv)
-{
-	struct vb2_dc_buf *buf = buf_priv;
-
-	return atomic_read(&buf->refcount);
-}
-
-static int vb2_dma_contig_mmap(void *buf_priv, struct vm_area_struct *vma)
-{
-	struct vb2_dc_buf *buf = buf_priv;
-
-	if (!buf) {
-		printk(KERN_ERR "No buffer to map\n");
-		return -EINVAL;
-	}
-
-	return vb2_mmap_pfn_range(vma, buf->dma_addr, buf->size,
-				  &vb2_common_vm_ops, &buf->handler);
-}
-
-static void *vb2_dma_contig_get_userptr(void *alloc_ctx, unsigned long vaddr,
-					unsigned long size, int write)
-{
-	struct vb2_dc_buf *buf;
-	struct vm_area_struct *vma;
-	dma_addr_t dma_addr = 0;
-	int ret;
-
-	buf = kzalloc(sizeof *buf, GFP_KERNEL);
-	if (!buf)
-		return ERR_PTR(-ENOMEM);
-
-	ret = vb2_get_contig_userptr(vaddr, size, &vma, &dma_addr);
-	if (ret) {
-		printk(KERN_ERR "Failed acquiring VMA for vaddr 0x%08lx\n",
-				vaddr);
-		kfree(buf);
-		return ERR_PTR(ret);
-	}
-
-	buf->size = size;
-	buf->dma_addr = dma_addr;
-	buf->vma = vma;
-
-	return buf;
-}
-
-static void vb2_dma_contig_put_userptr(void *mem_priv)
-{
-	struct vb2_dc_buf *buf = mem_priv;
-
-	if (!buf)
-		return;
-
-	vb2_put_vma(buf->vma);
-	kfree(buf);
-}
-
-static int vb2_dma_contig_map_dmabuf(void *mem_priv,
-					enum dma_data_direction direction)
-{
-	struct vb2_dc_buf *buf = mem_priv;
-	struct dma_buf *dmabuf;
-	struct sg_table *sg;
-
-	if (!buf || !buf->db_attach) {
-		printk(KERN_ERR "No dma buffer to pin\n");
-		return -EINVAL;
-	}
-
-	WARN_ON(buf->dma_addr);
-
-	if (direction == DMA_NONE) {
-		printk(KERN_ERR "Incorrect DMA direction\n");
-		return -EINVAL;
-	}
-
-	dmabuf = buf->db_attach->dmabuf;
-
-	/* get the associated scatterlist for this buffer */
-	sg = dma_buf_map_attachment(buf->db_attach, direction);
-
-	if (!sg) {
-		printk(KERN_ERR "Error getting dmabuf scatterlist\n");
-		return -EINVAL;
-	}
-
-	/*
-	 *  convert sglist to paddr:
-	 *  Assumption: for dma-contig, dmabuf would map to single entry
-	 *  Will return an error if it has more than one.
-	 */
-	if (sg->nents > 1) {
-		printk(KERN_ERR
-			"dmabuf scatterlist has more than 1 entry\n");
-		return -EINVAL;
-	}
-
-	buf->dma_addr = sg_dma_address(sg->sgl);
-	/* TODO: check the buffer size as per S_FMT */
-	buf->size = sg_dma_len(sg->sgl);
-
-	/* save this scatterlist in dmabuf for put_scatterlist */
-	dmabuf->priv = sg;
-
-	return 0;
-}
-
-static void vb2_dma_contig_unmap_dmabuf(void *mem_priv)
-{
-	struct vb2_dc_buf *buf = mem_priv;
-	struct dma_buf *dmabuf;
-	struct sg_table *sg;
-
-	if (!buf || !buf->db_attach)
-		return;
-
-	WARN_ON(!buf->dma_addr);
-
-	dmabuf = buf->db_attach->dmabuf;
-	sg = dmabuf->priv;
-
-	/* Put the sg for this buffer */
-	dma_buf_unmap_attachment(buf->db_attach, sg);
-
-	buf->dma_addr = 0;
-	buf->size = 0;
-}
-
-static void *vb2_dma_contig_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf)
-{
-	struct vb2_dc_conf *conf = alloc_ctx;
-	struct vb2_dc_buf *buf;
-	struct dma_buf_attachment *dba;
-
-	buf = kzalloc(sizeof *buf, GFP_KERNEL);
-	if (!buf)
-		return ERR_PTR(-ENOMEM);
-
-	/* create attachment for the dmabuf with the user device */
-	dba = dma_buf_attach(dbuf, conf->dev);
-	if (IS_ERR(dba)) {
-		printk(KERN_ERR "failed to attach dmabuf\n");
-		kfree(buf);
-		return dba;
-	}
-
-	buf->conf = conf;
-	buf->size = dba->dmabuf->size;
-	buf->db_attach = dba;
-	buf->dma_addr = 0; /* dma_addr is available only after map */
-
-	return buf;
-}
-
-static void vb2_dma_contig_detach_dmabuf(void *mem_priv)
-{
-	struct vb2_dc_buf *buf = mem_priv;
-
-	if (!buf)
-		return;
-
-	if (buf->dma_addr) {
-		vb2_dma_contig_unmap_dmabuf(buf);
-	}
-
-	/* detach this attachment */
-	dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
-	buf->db_attach = NULL;
-
-	kfree(buf);
-}
-
-const struct vb2_mem_ops vb2_dma_contig_memops = {
-	.alloc		= vb2_dma_contig_alloc,
-	.put		= vb2_dma_contig_put,
-	.cookie		= vb2_dma_contig_cookie,
-	.vaddr		= vb2_dma_contig_vaddr,
-	.mmap		= vb2_dma_contig_mmap,
-	.get_userptr	= vb2_dma_contig_get_userptr,
-	.put_userptr	= vb2_dma_contig_put_userptr,
-	.map_dmabuf	= vb2_dma_contig_map_dmabuf,
-	.unmap_dmabuf	= vb2_dma_contig_unmap_dmabuf,
-	.attach_dmabuf	= vb2_dma_contig_attach_dmabuf,
-	.detach_dmabuf	= vb2_dma_contig_detach_dmabuf,
-	.num_users	= vb2_dma_contig_num_users,
-};
-EXPORT_SYMBOL_GPL(vb2_dma_contig_memops);
-
-void *vb2_dma_contig_init_ctx(struct device *dev)
-{
-	struct vb2_dc_conf *conf;
-
-	conf = kzalloc(sizeof *conf, GFP_KERNEL);
-	if (!conf)
-		return ERR_PTR(-ENOMEM);
-
-	conf->dev = dev;
-
-	return conf;
-}
-EXPORT_SYMBOL_GPL(vb2_dma_contig_init_ctx);
-
-void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
-{
-	kfree(alloc_ctx);
-}
-EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
-
-MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2");
-MODULE_AUTHOR("Pawel Osciak <pawel@osciak.com>");
-MODULE_LICENSE("GPL");
-- 
1.7.5.4

Re: [PATCH 07/10] v4l: vb2: remove dma-contig allocator

[Mauro Carvalho Chehab]

Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
> This is temporary patch. The dma-contig changes were significant
> and the difference patch would be very difficult to read.

NACK.

This breaks git bisect.

> 
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/video/videobuf2-dma-contig.c |  308 ----------------------------
>  1 files changed, 0 insertions(+), 308 deletions(-)
>  delete mode 100644 drivers/media/video/videobuf2-dma-contig.c
> 
> diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
> deleted file mode 100644
> index ea2699f..0000000
> --- a/drivers/media/video/videobuf2-dma-contig.c
> +++ /dev/null
> @@ -1,308 +0,0 @@
> -/*
> - * videobuf2-dma-contig.c - DMA contig memory allocator for videobuf2
> - *
> - * Copyright (C) 2010 Samsung Electronics
> - *
> - * Author: Pawel Osciak <pawel@osciak.com>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation.
> - */
> -
> -#include <linux/module.h>
> -#include <linux/slab.h>
> -#include <linux/dma-mapping.h>
> -#include <linux/scatterlist.h>
> -#include <linux/dma-buf.h>
> -
> -#include <media/videobuf2-core.h>
> -#include <media/videobuf2-memops.h>
> -
> -struct vb2_dc_conf {
> -	struct device		*dev;
> -};
> -
> -struct vb2_dc_buf {
> -	struct vb2_dc_conf		*conf;
> -	void				*vaddr;
> -	dma_addr_t			dma_addr;
> -	unsigned long			size;
> -	struct vm_area_struct		*vma;
> -	struct dma_buf_attachment	*db_attach;
> -	atomic_t			refcount;
> -	struct vb2_vmarea_handler	handler;
> -};
> -
> -static void vb2_dma_contig_put(void *buf_priv);
> -
> -static void *vb2_dma_contig_alloc(void *alloc_ctx, unsigned long size)
> -{
> -	struct vb2_dc_conf *conf = alloc_ctx;
> -	struct vb2_dc_buf *buf;
> -	/* TODO: add db_attach processing while adding DMABUF as exporter */
> -
> -	buf = kzalloc(sizeof *buf, GFP_KERNEL);
> -	if (!buf)
> -		return ERR_PTR(-ENOMEM);
> -
> -	buf->vaddr = dma_alloc_coherent(conf->dev, size, &buf->dma_addr,
> -					GFP_KERNEL);
> -	if (!buf->vaddr) {
> -		dev_err(conf->dev, "dma_alloc_coherent of size %ld failed\n",
> -			size);
> -		kfree(buf);
> -		return ERR_PTR(-ENOMEM);
> -	}
> -
> -	buf->conf = conf;
> -	buf->size = size;
> -
> -	buf->handler.refcount = &buf->refcount;
> -	buf->handler.put = vb2_dma_contig_put;
> -	buf->handler.arg = buf;
> -
> -	atomic_inc(&buf->refcount);
> -
> -	return buf;
> -}
> -
> -static void vb2_dma_contig_put(void *buf_priv)
> -{
> -	struct vb2_dc_buf *buf = buf_priv;
> -
> -	if (atomic_dec_and_test(&buf->refcount)) {
> -		dma_free_coherent(buf->conf->dev, buf->size, buf->vaddr,
> -				  buf->dma_addr);
> -		kfree(buf);
> -	}
> -}
> -
> -static void *vb2_dma_contig_cookie(void *buf_priv)
> -{
> -	struct vb2_dc_buf *buf = buf_priv;
> -
> -	return &buf->dma_addr;
> -}
> -
> -static void *vb2_dma_contig_vaddr(void *buf_priv)
> -{
> -	struct vb2_dc_buf *buf = buf_priv;
> -	if (!buf)
> -		return 0;
> -
> -	return buf->vaddr;
> -}
> -
> -static unsigned int vb2_dma_contig_num_users(void *buf_priv)
> -{
> -	struct vb2_dc_buf *buf = buf_priv;
> -
> -	return atomic_read(&buf->refcount);
> -}
> -
> -static int vb2_dma_contig_mmap(void *buf_priv, struct vm_area_struct *vma)
> -{
> -	struct vb2_dc_buf *buf = buf_priv;
> -
> -	if (!buf) {
> -		printk(KERN_ERR "No buffer to map\n");
> -		return -EINVAL;
> -	}
> -
> -	return vb2_mmap_pfn_range(vma, buf->dma_addr, buf->size,
> -				  &vb2_common_vm_ops, &buf->handler);
> -}
> -
> -static void *vb2_dma_contig_get_userptr(void *alloc_ctx, unsigned long vaddr,
> -					unsigned long size, int write)
> -{
> -	struct vb2_dc_buf *buf;
> -	struct vm_area_struct *vma;
> -	dma_addr_t dma_addr = 0;
> -	int ret;
> -
> -	buf = kzalloc(sizeof *buf, GFP_KERNEL);
> -	if (!buf)
> -		return ERR_PTR(-ENOMEM);
> -
> -	ret = vb2_get_contig_userptr(vaddr, size, &vma, &dma_addr);
> -	if (ret) {
> -		printk(KERN_ERR "Failed acquiring VMA for vaddr 0x%08lx\n",
> -				vaddr);
> -		kfree(buf);
> -		return ERR_PTR(ret);
> -	}
> -
> -	buf->size = size;
> -	buf->dma_addr = dma_addr;
> -	buf->vma = vma;
> -
> -	return buf;
> -}
> -
> -static void vb2_dma_contig_put_userptr(void *mem_priv)
> -{
> -	struct vb2_dc_buf *buf = mem_priv;
> -
> -	if (!buf)
> -		return;
> -
> -	vb2_put_vma(buf->vma);
> -	kfree(buf);
> -}
> -
> -static int vb2_dma_contig_map_dmabuf(void *mem_priv,
> -					enum dma_data_direction direction)
> -{
> -	struct vb2_dc_buf *buf = mem_priv;
> -	struct dma_buf *dmabuf;
> -	struct sg_table *sg;
> -
> -	if (!buf || !buf->db_attach) {
> -		printk(KERN_ERR "No dma buffer to pin\n");
> -		return -EINVAL;
> -	}
> -
> -	WARN_ON(buf->dma_addr);
> -
> -	if (direction == DMA_NONE) {
> -		printk(KERN_ERR "Incorrect DMA direction\n");
> -		return -EINVAL;
> -	}
> -
> -	dmabuf = buf->db_attach->dmabuf;
> -
> -	/* get the associated scatterlist for this buffer */
> -	sg = dma_buf_map_attachment(buf->db_attach, direction);
> -
> -	if (!sg) {
> -		printk(KERN_ERR "Error getting dmabuf scatterlist\n");
> -		return -EINVAL;
> -	}
> -
> -	/*
> -	 *  convert sglist to paddr:
> -	 *  Assumption: for dma-contig, dmabuf would map to single entry
> -	 *  Will return an error if it has more than one.
> -	 */
> -	if (sg->nents > 1) {
> -		printk(KERN_ERR
> -			"dmabuf scatterlist has more than 1 entry\n");
> -		return -EINVAL;
> -	}
> -
> -	buf->dma_addr = sg_dma_address(sg->sgl);
> -	/* TODO: check the buffer size as per S_FMT */
> -	buf->size = sg_dma_len(sg->sgl);
> -
> -	/* save this scatterlist in dmabuf for put_scatterlist */
> -	dmabuf->priv = sg;
> -
> -	return 0;
> -}
> -
> -static void vb2_dma_contig_unmap_dmabuf(void *mem_priv)
> -{
> -	struct vb2_dc_buf *buf = mem_priv;
> -	struct dma_buf *dmabuf;
> -	struct sg_table *sg;
> -
> -	if (!buf || !buf->db_attach)
> -		return;
> -
> -	WARN_ON(!buf->dma_addr);
> -
> -	dmabuf = buf->db_attach->dmabuf;
> -	sg = dmabuf->priv;
> -
> -	/* Put the sg for this buffer */
> -	dma_buf_unmap_attachment(buf->db_attach, sg);
> -
> -	buf->dma_addr = 0;
> -	buf->size = 0;
> -}
> -
> -static void *vb2_dma_contig_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf)
> -{
> -	struct vb2_dc_conf *conf = alloc_ctx;
> -	struct vb2_dc_buf *buf;
> -	struct dma_buf_attachment *dba;
> -
> -	buf = kzalloc(sizeof *buf, GFP_KERNEL);
> -	if (!buf)
> -		return ERR_PTR(-ENOMEM);
> -
> -	/* create attachment for the dmabuf with the user device */
> -	dba = dma_buf_attach(dbuf, conf->dev);
> -	if (IS_ERR(dba)) {
> -		printk(KERN_ERR "failed to attach dmabuf\n");
> -		kfree(buf);
> -		return dba;
> -	}
> -
> -	buf->conf = conf;
> -	buf->size = dba->dmabuf->size;
> -	buf->db_attach = dba;
> -	buf->dma_addr = 0; /* dma_addr is available only after map */
> -
> -	return buf;
> -}
> -
> -static void vb2_dma_contig_detach_dmabuf(void *mem_priv)
> -{
> -	struct vb2_dc_buf *buf = mem_priv;
> -
> -	if (!buf)
> -		return;
> -
> -	if (buf->dma_addr) {
> -		vb2_dma_contig_unmap_dmabuf(buf);
> -	}
> -
> -	/* detach this attachment */
> -	dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
> -	buf->db_attach = NULL;
> -
> -	kfree(buf);
> -}
> -
> -const struct vb2_mem_ops vb2_dma_contig_memops = {
> -	.alloc		= vb2_dma_contig_alloc,
> -	.put		= vb2_dma_contig_put,
> -	.cookie		= vb2_dma_contig_cookie,
> -	.vaddr		= vb2_dma_contig_vaddr,
> -	.mmap		= vb2_dma_contig_mmap,
> -	.get_userptr	= vb2_dma_contig_get_userptr,
> -	.put_userptr	= vb2_dma_contig_put_userptr,
> -	.map_dmabuf	= vb2_dma_contig_map_dmabuf,
> -	.unmap_dmabuf	= vb2_dma_contig_unmap_dmabuf,
> -	.attach_dmabuf	= vb2_dma_contig_attach_dmabuf,
> -	.detach_dmabuf	= vb2_dma_contig_detach_dmabuf,
> -	.num_users	= vb2_dma_contig_num_users,
> -};
> -EXPORT_SYMBOL_GPL(vb2_dma_contig_memops);
> -
> -void *vb2_dma_contig_init_ctx(struct device *dev)
> -{
> -	struct vb2_dc_conf *conf;
> -
> -	conf = kzalloc(sizeof *conf, GFP_KERNEL);
> -	if (!conf)
> -		return ERR_PTR(-ENOMEM);
> -
> -	conf->dev = dev;
> -
> -	return conf;
> -}
> -EXPORT_SYMBOL_GPL(vb2_dma_contig_init_ctx);
> -
> -void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
> -{
> -	kfree(alloc_ctx);
> -}
> -EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
> -
> -MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2");
> -MODULE_AUTHOR("Pawel Osciak <pawel@osciak.com>");
> -MODULE_LICENSE("GPL");

[PATCH 08/10] v4l: vb2-dma-contig: code refactoring, support for DMABUF exporting

[Tomasz Stanislawski]

Signed-off-by: Pawel Osciak <pawel@osciak.com>
	[author of the original file]
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
	[implemetation of mmap, finish/prepare]
Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
	[implementation of userprt handling]
Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
	[PoC for importing dmabuf buffer]
Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
       [buffer exporting using dmabuf, code refactoring]
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/videobuf2-dma-contig.c |  762 ++++++++++++++++++++++++++++
 1 files changed, 762 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/videobuf2-dma-contig.c

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
new file mode 100644
index 0000000..d95b23a
--- /dev/null
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -0,0 +1,762 @@
+/*
+ * videobuf2-dma-contig.c - DMA contig memory allocator for videobuf2
+ *
+ * Copyright (C) 2010 Samsung Electronics
+ *
+ * Author: Pawel Osciak <pawel@osciak.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/dma-buf.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/scatterlist.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+
+#include <media/videobuf2-core.h>
+#include <media/videobuf2-memops.h>
+
+struct vb2_dc_buf {
+	struct device			*dev;
+	void				*vaddr;
+	unsigned long			size;
+	dma_addr_t			dma_addr;
+	struct sg_table			*dma_sgt;
+	enum dma_data_direction		dma_dir;
+
+	/* MMAP related */
+	struct vb2_vmarea_handler	handler;
+	atomic_t			refcount;
+	struct dma_buf			*dma_buf;
+	struct sg_table			*sgt_base;
+
+	/* USERPTR related */
+	struct vm_area_struct		*vma;
+
+	/* DMABUF related */
+	struct dma_buf_attachment	*db_attach;
+};
+
+/*********************************************/
+/*        scatterlist table functions        */
+/*********************************************/
+
+static struct sg_table *vb2_dc_pages_to_sgt(struct page **pages,
+	unsigned long n_pages, size_t offset, size_t offset2)
+{
+	struct sg_table *sgt;
+	int i, j; /* loop counters */
+	int cur_page, chunks;
+	int ret;
+	struct scatterlist *s;
+
+	sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
+	if (!sgt)
+		return ERR_PTR(-ENOMEM);
+
+	/* compute number of chunks */
+	chunks = 1;
+	for (i = 1; i < n_pages; ++i)
+		if (pages[i] != pages[i - 1] + 1)
+			++chunks;
+
+	ret = sg_alloc_table(sgt, chunks, GFP_KERNEL);
+	if (ret) {
+		kfree(sgt);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	/* merging chunks and putting them into the scatterlist */
+	cur_page = 0;
+	for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
+		size_t size = PAGE_SIZE;
+
+		for (j = cur_page + 1; j < n_pages; ++j) {
+			if (pages[j] != pages[j - 1] + 1)
+				break;
+			size += PAGE_SIZE;
+		}
+
+		/* cut offset if chunk starts at the first page */
+		if (cur_page == 0)
+			size -= offset;
+		/* cut offset2 if chunk ends at the last page */
+		if (j == n_pages)
+			size -= offset2;
+
+		sg_set_page(s, pages[cur_page], size, offset);
+		offset = 0;
+		cur_page = j;
+	}
+
+	return sgt;
+}
+
+static void vb2_dc_release_sgtable(struct sg_table *sgt)
+{
+	sg_free_table(sgt);
+	kfree(sgt);
+}
+
+static void vb2_dc_put_sgtable(struct sg_table *sgt, int dirty)
+{
+	struct scatterlist *s;
+	int i, j;
+
+	for_each_sg(sgt->sgl, s, sgt->nents, i) {
+		struct page *page = sg_page(s);
+		int n_pages = PAGE_ALIGN(s->offset + s->length) >> PAGE_SHIFT;
+
+		for (j = 0; j < n_pages; ++j, ++page) {
+			if (dirty)
+				set_page_dirty_lock(page);
+			put_page(page);
+		}
+	}
+
+	vb2_dc_release_sgtable(sgt);
+}
+
+static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
+{
+	struct scatterlist *s;
+	dma_addr_t expected = sg_dma_address(sgt->sgl);
+	int i;
+	unsigned long size = 0;
+
+	for_each_sg(sgt->sgl, s, sgt->nents, i) {
+		if (sg_dma_address(s) != expected)
+			break;
+		expected = sg_dma_address(s) + sg_dma_len(s);
+		size += sg_dma_len(s);
+	}
+	return size;
+}
+
+/*********************************************/
+/*         callbacks for all buffers         */
+/*********************************************/
+
+static void *vb2_dc_cookie(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+
+	return &buf->dma_addr;
+}
+
+static void *vb2_dc_vaddr(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+
+	return buf->vaddr;
+}
+
+static unsigned int vb2_dc_num_users(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+
+	return atomic_read(&buf->refcount);
+}
+
+static void vb2_dc_prepare(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	if (!sgt)
+		return;
+
+	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+}
+
+static void vb2_dc_finish(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	if (!sgt)
+		return;
+
+	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+}
+
+/*********************************************/
+/*        callbacks for MMAP buffers         */
+/*********************************************/
+
+static void vb2_dc_put(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+
+	if (!atomic_dec_and_test(&buf->refcount))
+		return;
+
+	if (buf->dma_buf)
+		dma_buf_put(buf->dma_buf);
+	vb2_dc_release_sgtable(buf->sgt_base);
+	dma_free_coherent(buf->dev, buf->size, buf->vaddr,
+		buf->dma_addr);
+	kfree(buf);
+}
+
+static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
+{
+	struct device *dev = alloc_ctx;
+	struct vb2_dc_buf *buf;
+	int ret;
+	int n_pages;
+	struct page **pages = NULL;
+
+	buf = kzalloc(sizeof *buf, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	buf->dev = dev;
+	buf->size = size;
+	buf->vaddr = dma_alloc_coherent(buf->dev, buf->size, &buf->dma_addr,
+		GFP_KERNEL);
+
+	ret = -ENOMEM;
+	if (!buf->vaddr) {
+		dev_err(dev, "dma_alloc_coherent of size %ld failed\n",
+			size);
+		goto fail_buf;
+	}
+
+	WARN_ON((unsigned long)buf->vaddr & ~PAGE_MASK);
+	WARN_ON(buf->dma_addr & ~PAGE_MASK);
+
+	n_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+
+	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
+	if (!pages) {
+		printk(KERN_ERR "failed to alloc page table\n");
+		goto fail_dma;
+	}
+
+	ret = dma_get_pages(dev, buf->vaddr, buf->dma_addr, pages, n_pages);
+	if (ret < 0) {
+		printk(KERN_ERR "failed to get buffer pages from DMA API\n");
+		goto fail_pages;
+	}
+	if (ret != n_pages) {
+		ret = -EFAULT;
+		printk(KERN_ERR "failed to get all pages from DMA API\n");
+		goto fail_pages;
+	}
+
+	buf->sgt_base = vb2_dc_pages_to_sgt(pages, n_pages, 0, 0);
+	if (IS_ERR(buf->sgt_base)) {
+		ret = PTR_ERR(buf->sgt_base);
+		printk(KERN_ERR "failed to prepare sg table\n");
+		goto fail_pages;
+	}
+
+	/* pages are no longer needed */
+	kfree(pages);
+
+	buf->handler.refcount = &buf->refcount;
+	buf->handler.put = vb2_dc_put;
+	buf->handler.arg = buf;
+
+	atomic_inc(&buf->refcount);
+
+	return buf;
+
+fail_pages:
+	kfree(pages);
+
+fail_dma:
+	dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
+
+fail_buf:
+	kfree(buf);
+
+	return ERR_PTR(ret);
+}
+
+static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+	int ret;
+
+	/*
+	 * dma_mmap_* uses vm_pgoff as in-buffer offset, but we want to
+	 * map whole buffer
+	 */
+	vma->vm_pgoff = 0;
+
+	ret = dma_mmap_writecombine(buf->dev, vma, buf->vaddr,
+		buf->dma_addr, buf->size);
+
+	if (ret) {
+		printk(KERN_ERR "Remapping memory failed, error: %d\n", ret);
+		return ret;
+	}
+
+	vma->vm_flags		|= VM_DONTEXPAND | VM_RESERVED;
+	vma->vm_private_data	= &buf->handler;
+	vma->vm_ops		= &vb2_common_vm_ops;
+
+	vma->vm_ops->open(vma);
+
+	printk(KERN_DEBUG "%s: mapped dma addr 0x%08lx at 0x%08lx, size %ld\n",
+		__func__, (unsigned long)buf->dma_addr, vma->vm_start,
+		buf->size);
+
+	return 0;
+}
+
+/*********************************************/
+/*         DMABUF ops for exporters          */
+/*********************************************/
+
+struct vb2_dc_attachment {
+	struct sg_table sgt;
+	enum dma_data_direction dir;
+};
+
+static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
+	struct dma_buf_attachment *dbuf_attach)
+{
+	/* nothing to be done */
+	return 0;
+}
+
+static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
+	struct dma_buf_attachment *db_attach)
+{
+	struct vb2_dc_attachment *attach = db_attach->priv;
+	struct sg_table *sgt;
+
+	if (!attach)
+		return;
+
+	sgt = &attach->sgt;
+
+	dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->nents, attach->dir);
+	sg_free_table(sgt);
+	kfree(attach);
+	db_attach->priv = NULL;
+}
+
+static struct sg_table *vb2_dc_dmabuf_ops_map(
+	struct dma_buf_attachment *db_attach, enum dma_data_direction dir)
+{
+	struct dma_buf *dbuf = db_attach->dmabuf;
+	struct vb2_dc_buf *buf = dbuf->priv;
+	struct vb2_dc_attachment *attach = db_attach->priv;
+	struct sg_table *sgt;
+	struct scatterlist *rd, *wr;
+	int i, ret;
+
+	/* return previously mapped sg table */
+	if (attach)
+		return &attach->sgt;
+
+	attach = kzalloc(sizeof *attach, GFP_KERNEL);
+	if (!attach)
+		return ERR_PTR(-ENOMEM);
+
+	sgt = &attach->sgt;
+	attach->dir = dir;
+
+	/* copying the buf->base_sgt to attachment */
+	ret = sg_alloc_table(sgt, buf->sgt_base->orig_nents, GFP_KERNEL);
+	if (ret) {
+		kfree(attach);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	rd = buf->sgt_base->sgl;
+	wr = sgt->sgl;
+	for (i = 0; i < sgt->orig_nents; ++i) {
+		sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
+		rd = sg_next(rd);
+		wr = sg_next(wr);
+	}
+
+	/* mapping new sglist to the client */
+	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dir);
+	if (ret <= 0) {
+		printk(KERN_ERR "failed to map scatterlist\n");
+		sg_free_table(sgt);
+		kfree(attach);
+		return ERR_PTR(-EIO);
+	}
+
+	db_attach->priv = attach;
+
+	return sgt;
+}
+
+static void vb2_dc_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
+	struct sg_table *sgt)
+{
+	/* nothing to be done here */
+}
+
+static void vb2_dc_dmabuf_ops_release(struct dma_buf *dbuf)
+{
+	/* drop reference obtained in vb2_dc_get_dmabuf */
+	vb2_dc_put(dbuf->priv);
+}
+
+static struct dma_buf_ops vb2_dc_dmabuf_ops = {
+	.attach = vb2_dc_dmabuf_ops_attach,
+	.detach = vb2_dc_dmabuf_ops_detach,
+	.map_dma_buf = vb2_dc_dmabuf_ops_map,
+	.unmap_dma_buf = vb2_dc_dmabuf_ops_unmap,
+	.release = vb2_dc_dmabuf_ops_release,
+};
+
+static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+	struct dma_buf *dbuf;
+
+	if (buf->dma_buf)
+		return buf->dma_buf;
+
+	/* dmabuf keeps reference to vb2 buffer */
+	atomic_inc(&buf->refcount);
+	dbuf = dma_buf_export(buf, &vb2_dc_dmabuf_ops, buf->size, 0);
+	if (IS_ERR(dbuf)) {
+		atomic_dec(&buf->refcount);
+		return NULL;
+	}
+
+	buf->dma_buf = dbuf;
+
+	return dbuf;
+}
+
+/*********************************************/
+/*       callbacks for USERPTR buffers       */
+/*********************************************/
+
+static inline int vma_is_io(struct vm_area_struct *vma)
+{
+	return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
+}
+
+static int vb2_dc_get_pages(unsigned long start, struct page **pages,
+	int n_pages, struct vm_area_struct **copy_vma, int write)
+{
+	struct vm_area_struct *vma;
+	int n = 0; /* number of get pages */
+	int ret = -EFAULT;
+
+	/* entering critical section for mm access */
+	down_read(&current->mm->mmap_sem);
+
+	vma = find_vma(current->mm, start);
+	if (!vma) {
+		printk(KERN_ERR "no vma for address %lu\n", start);
+		goto cleanup;
+	}
+
+	if (vma_is_io(vma)) {
+		unsigned long pfn;
+
+		if (vma->vm_end - start < n_pages * PAGE_SIZE) {
+			printk(KERN_ERR "vma is too small\n");
+			goto cleanup;
+		}
+
+		for (n = 0; n < n_pages; ++n, start += PAGE_SIZE) {
+			ret = follow_pfn(vma, start, &pfn);
+			if (ret) {
+				printk(KERN_ERR "no page for address %lu\n",
+					start);
+				goto cleanup;
+			}
+			pages[n] = pfn_to_page(pfn);
+			get_page(pages[n]);
+		}
+	} else {
+		n = get_user_pages(current, current->mm, start & PAGE_MASK,
+			n_pages, write, 1, pages, NULL);
+		if (n != n_pages) {
+			printk(KERN_ERR "got only %d of %d user pages\n",
+				n, n_pages);
+			goto cleanup;
+		}
+	}
+
+	*copy_vma = vb2_get_vma(vma);
+	if (!*copy_vma) {
+		printk(KERN_ERR "failed to copy vma\n");
+		ret = -ENOMEM;
+		goto cleanup;
+	}
+
+	/* leaving critical section for mm access */
+	up_read(&current->mm->mmap_sem);
+
+	return 0;
+
+cleanup:
+	up_read(&current->mm->mmap_sem);
+
+	/* putting user pages if used, can be done wothout the lock */
+	while (n)
+		put_page(pages[--n]);
+
+	return ret;
+}
+
+static void vb2_dc_put_userptr(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	dma_unmap_sg(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
+	vb2_dc_put_sgtable(sgt, !vma_is_io(buf->vma));
+	vb2_put_vma(buf->vma);
+	kfree(buf);
+}
+
+static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
+	unsigned long size, int write)
+{
+	struct vb2_dc_buf *buf;
+	unsigned long start, end, offset, offset2;
+	struct page **pages;
+	int n_pages;
+	int ret = 0;
+	struct sg_table *sgt;
+	unsigned long contig_size;
+
+	buf = kzalloc(sizeof *buf, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	buf->dev = alloc_ctx;
+	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+	start = (unsigned long)vaddr & PAGE_MASK;
+	offset = (unsigned long)vaddr & ~PAGE_MASK;
+	end = PAGE_ALIGN((unsigned long)vaddr + size);
+	offset2 = end - (unsigned long)vaddr - size;
+	n_pages = (end - start) >> PAGE_SHIFT;
+
+	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
+	if (!pages) {
+		ret = -ENOMEM;
+		printk(KERN_ERR "failed to allocate pages table\n");
+		goto fail_buf;
+	}
+
+	/* extract page list from userspace mapping */
+	ret = vb2_dc_get_pages(start, pages, n_pages, &buf->vma, write);
+	if (ret) {
+		printk(KERN_ERR "failed to get user pages\n");
+		goto fail_pages;
+	}
+
+	sgt = vb2_dc_pages_to_sgt(pages, n_pages, offset, offset2);
+	if (!sgt) {
+		printk(KERN_ERR "failed to create scatterlist table\n");
+		ret = -ENOMEM;
+		goto fail_get_pages;
+	}
+
+	/* pages are no longer needed */
+	kfree(pages);
+	pages = NULL;
+
+	sgt->nents = dma_map_sg(buf->dev, sgt->sgl, sgt->orig_nents,
+		buf->dma_dir);
+	if (sgt->nents <= 0) {
+		printk(KERN_ERR "failed to map scatterlist\n");
+		ret = -EIO;
+		goto fail_sgt;
+	}
+
+	contig_size = vb2_dc_get_contiguous_size(sgt);
+	if (contig_size < size) {
+		printk(KERN_ERR "contiguous mapping is too small %lu/%lu\n",
+			contig_size, size);
+		ret = -EFAULT;
+		goto fail_map_sg;
+	}
+
+	buf->dma_addr = sg_dma_address(sgt->sgl);
+	buf->size = size;
+	buf->dma_sgt = sgt;
+
+	atomic_inc(&buf->refcount);
+
+	return buf;
+
+fail_map_sg:
+	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+
+fail_sgt:
+	vb2_dc_put_sgtable(sgt, 0);
+
+fail_get_pages:
+	while (pages && n_pages)
+		put_page(pages[--n_pages]);
+	vb2_put_vma(buf->vma);
+
+fail_pages:
+	kfree(pages); /* kfree is NULL-proof */
+
+fail_buf:
+	kfree(buf);
+
+	return ERR_PTR(ret);
+}
+
+/*********************************************/
+/*       callbacks for DMABUF buffers        */
+/*********************************************/
+
+static int vb2_dc_map_dmabuf(void *mem_priv)
+{
+	struct vb2_dc_buf *buf = mem_priv;
+	struct sg_table *sgt;
+	unsigned long contig_size;
+	int ret = 0;
+
+	if (WARN_ON(!buf->db_attach)) {
+		printk(KERN_ERR "trying to pin a non attached buffer\n");
+		return -EINVAL;
+	}
+
+	if (WARN_ON(buf->dma_sgt)) {
+		printk(KERN_ERR "dmabuf buffer is already pinned\n");
+		return 0;
+	}
+
+	/* get the associated scatterlist for this buffer */
+	sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
+	if (IS_ERR_OR_NULL(sgt)) {
+		printk(KERN_ERR "Error getting dmabuf scatterlist\n");
+		return -EINVAL;
+	}
+
+	/* checking if dmabuf is big enough to store contiguous chunk */
+	contig_size = vb2_dc_get_contiguous_size(sgt);
+	if (contig_size < buf->size) {
+		printk(KERN_ERR "contiguous chunk of dmabuf is too small\n");
+		ret = -EFAULT;
+		goto fail_map;
+	}
+
+	buf->dma_addr = sg_dma_address(sgt->sgl);
+	buf->dma_sgt = sgt;
+
+	return 0;
+
+fail_map:
+	dma_buf_unmap_attachment(buf->db_attach, sgt);
+
+	return ret;
+}
+
+static void vb2_dc_unmap_dmabuf(void *mem_priv)
+{
+	struct vb2_dc_buf *buf = mem_priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	if (WARN_ON(!buf->db_attach)) {
+		printk(KERN_ERR "trying to unpin a not attached buffer\n");
+		return;
+	}
+
+	if (WARN_ON(!sgt)) {
+		printk(KERN_ERR "dmabuf buffer is already unpinned\n");
+		return;
+	}
+
+	dma_buf_unmap_attachment(buf->db_attach, sgt);
+
+	buf->dma_addr = 0;
+	buf->dma_sgt = NULL;
+}
+
+static void vb2_dc_detach_dmabuf(void *mem_priv)
+{
+	struct vb2_dc_buf *buf = mem_priv;
+
+	if (buf->dma_addr)
+		vb2_dc_unmap_dmabuf(buf);
+
+	/* detach this attachment */
+	dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
+	kfree(buf);
+}
+
+static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
+	unsigned long size, int write)
+{
+	struct vb2_dc_buf *buf;
+	struct dma_buf_attachment *dba;
+
+	if (dbuf->size < size)
+		return ERR_PTR(-EFAULT);
+
+	buf = kzalloc(sizeof *buf, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	buf->dev = alloc_ctx;
+	/* create attachment for the dmabuf with the user device */
+	dba = dma_buf_attach(dbuf, buf->dev);
+	if (IS_ERR(dba)) {
+		printk(KERN_ERR "failed to attach dmabuf\n");
+		kfree(buf);
+		return dba;
+	}
+
+	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->size = size;
+	buf->db_attach = dba;
+
+	return buf;
+}
+
+/*********************************************/
+/*       DMA CONTIG exported functions       */
+/*********************************************/
+
+const struct vb2_mem_ops vb2_dma_contig_memops = {
+	.alloc		= vb2_dc_alloc,
+	.put		= vb2_dc_put,
+	.get_dmabuf	= vb2_dc_get_dmabuf,
+	.cookie		= vb2_dc_cookie,
+	.vaddr		= vb2_dc_vaddr,
+	.mmap		= vb2_dc_mmap,
+	.get_userptr	= vb2_dc_get_userptr,
+	.put_userptr	= vb2_dc_put_userptr,
+	.prepare	= vb2_dc_prepare,
+	.finish		= vb2_dc_finish,
+	.map_dmabuf	= vb2_dc_map_dmabuf,
+	.unmap_dmabuf	= vb2_dc_unmap_dmabuf,
+	.attach_dmabuf	= vb2_dc_attach_dmabuf,
+	.detach_dmabuf	= vb2_dc_detach_dmabuf,
+	.num_users	= vb2_dc_num_users,
+};
+EXPORT_SYMBOL_GPL(vb2_dma_contig_memops);
+
+void *vb2_dma_contig_init_ctx(struct device *dev)
+{
+	return dev;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_contig_init_ctx);
+
+void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
+{
+}
+EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
+
+MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2");
+MODULE_AUTHOR("Pawel Osciak <pawel@osciak.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.5.4

Re: [PATCH 08/10] v4l: vb2-dma-contig: code refactoring, support for DMABUF exporting

[Mauro Carvalho Chehab]

Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
> Signed-off-by: Pawel Osciak <pawel@osciak.com>
> 	[author of the original file]
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> 	[implemetation of mmap, finish/prepare]
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> 	[implementation of userprt handling]
> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> 	[PoC for importing dmabuf buffer]
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
>        [buffer exporting using dmabuf, code refactoring]
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Please provide a diff against the existing driver, in order to allow reviewing
what has changed.

> ---
>  drivers/media/video/videobuf2-dma-contig.c |  762 ++++++++++++++++++++++++++++
>  1 files changed, 762 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/media/video/videobuf2-dma-contig.c
> 
> diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
> new file mode 100644
> index 0000000..d95b23a
> --- /dev/null
> +++ b/drivers/media/video/videobuf2-dma-contig.c
> @@ -0,0 +1,762 @@
> +/*
> + * videobuf2-dma-contig.c - DMA contig memory allocator for videobuf2
> + *
> + * Copyright (C) 2010 Samsung Electronics
> + *
> + * Author: Pawel Osciak <pawel@osciak.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/scatterlist.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +
> +#include <media/videobuf2-core.h>
> +#include <media/videobuf2-memops.h>
> +
> +struct vb2_dc_buf {
> +	struct device			*dev;
> +	void				*vaddr;
> +	unsigned long			size;
> +	dma_addr_t			dma_addr;
> +	struct sg_table			*dma_sgt;
> +	enum dma_data_direction		dma_dir;
> +
> +	/* MMAP related */
> +	struct vb2_vmarea_handler	handler;
> +	atomic_t			refcount;
> +	struct dma_buf			*dma_buf;
> +	struct sg_table			*sgt_base;
> +
> +	/* USERPTR related */
> +	struct vm_area_struct		*vma;
> +
> +	/* DMABUF related */
> +	struct dma_buf_attachment	*db_attach;
> +};
> +
> +/*********************************************/
> +/*        scatterlist table functions        */
> +/*********************************************/
> +
> +static struct sg_table *vb2_dc_pages_to_sgt(struct page **pages,
> +	unsigned long n_pages, size_t offset, size_t offset2)
> +{
> +	struct sg_table *sgt;
> +	int i, j; /* loop counters */
> +	int cur_page, chunks;
> +	int ret;
> +	struct scatterlist *s;
> +
> +	sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
> +	if (!sgt)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* compute number of chunks */
> +	chunks = 1;
> +	for (i = 1; i < n_pages; ++i)
> +		if (pages[i] != pages[i - 1] + 1)
> +			++chunks;
> +
> +	ret = sg_alloc_table(sgt, chunks, GFP_KERNEL);
> +	if (ret) {
> +		kfree(sgt);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	/* merging chunks and putting them into the scatterlist */
> +	cur_page = 0;
> +	for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
> +		size_t size = PAGE_SIZE;
> +
> +		for (j = cur_page + 1; j < n_pages; ++j) {
> +			if (pages[j] != pages[j - 1] + 1)
> +				break;
> +			size += PAGE_SIZE;
> +		}
> +
> +		/* cut offset if chunk starts at the first page */
> +		if (cur_page == 0)
> +			size -= offset;
> +		/* cut offset2 if chunk ends at the last page */
> +		if (j == n_pages)
> +			size -= offset2;
> +
> +		sg_set_page(s, pages[cur_page], size, offset);
> +		offset = 0;
> +		cur_page = j;
> +	}
> +
> +	return sgt;
> +}
> +
> +static void vb2_dc_release_sgtable(struct sg_table *sgt)
> +{
> +	sg_free_table(sgt);
> +	kfree(sgt);
> +}
> +
> +static void vb2_dc_put_sgtable(struct sg_table *sgt, int dirty)
> +{
> +	struct scatterlist *s;
> +	int i, j;
> +
> +	for_each_sg(sgt->sgl, s, sgt->nents, i) {
> +		struct page *page = sg_page(s);
> +		int n_pages = PAGE_ALIGN(s->offset + s->length) >> PAGE_SHIFT;
> +
> +		for (j = 0; j < n_pages; ++j, ++page) {
> +			if (dirty)
> +				set_page_dirty_lock(page);
> +			put_page(page);
> +		}
> +	}
> +
> +	vb2_dc_release_sgtable(sgt);
> +}
> +
> +static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
> +{
> +	struct scatterlist *s;
> +	dma_addr_t expected = sg_dma_address(sgt->sgl);
> +	int i;
> +	unsigned long size = 0;
> +
> +	for_each_sg(sgt->sgl, s, sgt->nents, i) {
> +		if (sg_dma_address(s) != expected)
> +			break;
> +		expected = sg_dma_address(s) + sg_dma_len(s);
> +		size += sg_dma_len(s);
> +	}
> +	return size;
> +}
> +
> +/*********************************************/
> +/*         callbacks for all buffers         */
> +/*********************************************/
> +
> +static void *vb2_dc_cookie(void *buf_priv)
> +{
> +	struct vb2_dc_buf *buf = buf_priv;
> +
> +	return &buf->dma_addr;
> +}
> +
> +static void *vb2_dc_vaddr(void *buf_priv)
> +{
> +	struct vb2_dc_buf *buf = buf_priv;
> +
> +	return buf->vaddr;
> +}
> +
> +static unsigned int vb2_dc_num_users(void *buf_priv)
> +{
> +	struct vb2_dc_buf *buf = buf_priv;
> +
> +	return atomic_read(&buf->refcount);
> +}
> +
> +static void vb2_dc_prepare(void *buf_priv)
> +{
> +	struct vb2_dc_buf *buf = buf_priv;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	if (!sgt)
> +		return;
> +
> +	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> +}
> +
> +static void vb2_dc_finish(void *buf_priv)
> +{
> +	struct vb2_dc_buf *buf = buf_priv;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	if (!sgt)
> +		return;
> +
> +	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> +}
> +
> +/*********************************************/
> +/*        callbacks for MMAP buffers         */
> +/*********************************************/
> +
> +static void vb2_dc_put(void *buf_priv)
> +{
> +	struct vb2_dc_buf *buf = buf_priv;
> +
> +	if (!atomic_dec_and_test(&buf->refcount))
> +		return;
> +
> +	if (buf->dma_buf)
> +		dma_buf_put(buf->dma_buf);
> +	vb2_dc_release_sgtable(buf->sgt_base);
> +	dma_free_coherent(buf->dev, buf->size, buf->vaddr,
> +		buf->dma_addr);
> +	kfree(buf);
> +}
> +
> +static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
> +{
> +	struct device *dev = alloc_ctx;
> +	struct vb2_dc_buf *buf;
> +	int ret;
> +	int n_pages;
> +	struct page **pages = NULL;
> +
> +	buf = kzalloc(sizeof *buf, GFP_KERNEL);
> +	if (!buf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	buf->dev = dev;
> +	buf->size = size;
> +	buf->vaddr = dma_alloc_coherent(buf->dev, buf->size, &buf->dma_addr,
> +		GFP_KERNEL);
> +
> +	ret = -ENOMEM;
> +	if (!buf->vaddr) {
> +		dev_err(dev, "dma_alloc_coherent of size %ld failed\n",
> +			size);
> +		goto fail_buf;
> +	}
> +
> +	WARN_ON((unsigned long)buf->vaddr & ~PAGE_MASK);
> +	WARN_ON(buf->dma_addr & ~PAGE_MASK);
> +
> +	n_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> +
> +	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
> +	if (!pages) {
> +		printk(KERN_ERR "failed to alloc page table\n");
> +		goto fail_dma;
> +	}
> +
> +	ret = dma_get_pages(dev, buf->vaddr, buf->dma_addr, pages, n_pages);
> +	if (ret < 0) {
> +		printk(KERN_ERR "failed to get buffer pages from DMA API\n");
> +		goto fail_pages;
> +	}
> +	if (ret != n_pages) {
> +		ret = -EFAULT;
> +		printk(KERN_ERR "failed to get all pages from DMA API\n");
> +		goto fail_pages;
> +	}
> +
> +	buf->sgt_base = vb2_dc_pages_to_sgt(pages, n_pages, 0, 0);
> +	if (IS_ERR(buf->sgt_base)) {
> +		ret = PTR_ERR(buf->sgt_base);
> +		printk(KERN_ERR "failed to prepare sg table\n");
> +		goto fail_pages;
> +	}
> +
> +	/* pages are no longer needed */
> +	kfree(pages);
> +
> +	buf->handler.refcount = &buf->refcount;
> +	buf->handler.put = vb2_dc_put;
> +	buf->handler.arg = buf;
> +
> +	atomic_inc(&buf->refcount);
> +
> +	return buf;
> +
> +fail_pages:
> +	kfree(pages);
> +
> +fail_dma:
> +	dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
> +
> +fail_buf:
> +	kfree(buf);
> +
> +	return ERR_PTR(ret);
> +}
> +
> +static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma)
> +{
> +	struct vb2_dc_buf *buf = buf_priv;
> +	int ret;
> +
> +	/*
> +	 * dma_mmap_* uses vm_pgoff as in-buffer offset, but we want to
> +	 * map whole buffer
> +	 */
> +	vma->vm_pgoff = 0;
> +
> +	ret = dma_mmap_writecombine(buf->dev, vma, buf->vaddr,
> +		buf->dma_addr, buf->size);
> +
> +	if (ret) {
> +		printk(KERN_ERR "Remapping memory failed, error: %d\n", ret);
> +		return ret;
> +	}
> +
> +	vma->vm_flags		|= VM_DONTEXPAND | VM_RESERVED;
> +	vma->vm_private_data	= &buf->handler;
> +	vma->vm_ops		= &vb2_common_vm_ops;
> +
> +	vma->vm_ops->open(vma);
> +
> +	printk(KERN_DEBUG "%s: mapped dma addr 0x%08lx at 0x%08lx, size %ld\n",
> +		__func__, (unsigned long)buf->dma_addr, vma->vm_start,
> +		buf->size);
> +
> +	return 0;
> +}
> +
> +/*********************************************/
> +/*         DMABUF ops for exporters          */
> +/*********************************************/
> +
> +struct vb2_dc_attachment {
> +	struct sg_table sgt;
> +	enum dma_data_direction dir;
> +};
> +
> +static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
> +	struct dma_buf_attachment *dbuf_attach)
> +{
> +	/* nothing to be done */
> +	return 0;
> +}
> +
> +static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
> +	struct dma_buf_attachment *db_attach)
> +{
> +	struct vb2_dc_attachment *attach = db_attach->priv;
> +	struct sg_table *sgt;
> +
> +	if (!attach)
> +		return;
> +
> +	sgt = &attach->sgt;
> +
> +	dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->nents, attach->dir);
> +	sg_free_table(sgt);
> +	kfree(attach);
> +	db_attach->priv = NULL;
> +}
> +
> +static struct sg_table *vb2_dc_dmabuf_ops_map(
> +	struct dma_buf_attachment *db_attach, enum dma_data_direction dir)
> +{
> +	struct dma_buf *dbuf = db_attach->dmabuf;
> +	struct vb2_dc_buf *buf = dbuf->priv;
> +	struct vb2_dc_attachment *attach = db_attach->priv;
> +	struct sg_table *sgt;
> +	struct scatterlist *rd, *wr;
> +	int i, ret;
> +
> +	/* return previously mapped sg table */
> +	if (attach)
> +		return &attach->sgt;
> +
> +	attach = kzalloc(sizeof *attach, GFP_KERNEL);
> +	if (!attach)
> +		return ERR_PTR(-ENOMEM);
> +
> +	sgt = &attach->sgt;
> +	attach->dir = dir;
> +
> +	/* copying the buf->base_sgt to attachment */
> +	ret = sg_alloc_table(sgt, buf->sgt_base->orig_nents, GFP_KERNEL);
> +	if (ret) {
> +		kfree(attach);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	rd = buf->sgt_base->sgl;
> +	wr = sgt->sgl;
> +	for (i = 0; i < sgt->orig_nents; ++i) {
> +		sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
> +		rd = sg_next(rd);
> +		wr = sg_next(wr);
> +	}
> +
> +	/* mapping new sglist to the client */
> +	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dir);
> +	if (ret <= 0) {
> +		printk(KERN_ERR "failed to map scatterlist\n");
> +		sg_free_table(sgt);
> +		kfree(attach);
> +		return ERR_PTR(-EIO);
> +	}
> +
> +	db_attach->priv = attach;
> +
> +	return sgt;
> +}
> +
> +static void vb2_dc_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
> +	struct sg_table *sgt)
> +{
> +	/* nothing to be done here */
> +}
> +
> +static void vb2_dc_dmabuf_ops_release(struct dma_buf *dbuf)
> +{
> +	/* drop reference obtained in vb2_dc_get_dmabuf */
> +	vb2_dc_put(dbuf->priv);
> +}
> +
> +static struct dma_buf_ops vb2_dc_dmabuf_ops = {
> +	.attach = vb2_dc_dmabuf_ops_attach,
> +	.detach = vb2_dc_dmabuf_ops_detach,
> +	.map_dma_buf = vb2_dc_dmabuf_ops_map,
> +	.unmap_dma_buf = vb2_dc_dmabuf_ops_unmap,
> +	.release = vb2_dc_dmabuf_ops_release,
> +};
> +
> +static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
> +{
> +	struct vb2_dc_buf *buf = buf_priv;
> +	struct dma_buf *dbuf;
> +
> +	if (buf->dma_buf)
> +		return buf->dma_buf;
> +
> +	/* dmabuf keeps reference to vb2 buffer */
> +	atomic_inc(&buf->refcount);
> +	dbuf = dma_buf_export(buf, &vb2_dc_dmabuf_ops, buf->size, 0);
> +	if (IS_ERR(dbuf)) {
> +		atomic_dec(&buf->refcount);
> +		return NULL;
> +	}
> +
> +	buf->dma_buf = dbuf;
> +
> +	return dbuf;
> +}
> +
> +/*********************************************/
> +/*       callbacks for USERPTR buffers       */
> +/*********************************************/
> +
> +static inline int vma_is_io(struct vm_area_struct *vma)
> +{
> +	return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
> +}
> +
> +static int vb2_dc_get_pages(unsigned long start, struct page **pages,
> +	int n_pages, struct vm_area_struct **copy_vma, int write)
> +{
> +	struct vm_area_struct *vma;
> +	int n = 0; /* number of get pages */
> +	int ret = -EFAULT;
> +
> +	/* entering critical section for mm access */
> +	down_read(&current->mm->mmap_sem);
> +
> +	vma = find_vma(current->mm, start);
> +	if (!vma) {
> +		printk(KERN_ERR "no vma for address %lu\n", start);
> +		goto cleanup;
> +	}
> +
> +	if (vma_is_io(vma)) {
> +		unsigned long pfn;
> +
> +		if (vma->vm_end - start < n_pages * PAGE_SIZE) {
> +			printk(KERN_ERR "vma is too small\n");
> +			goto cleanup;
> +		}
> +
> +		for (n = 0; n < n_pages; ++n, start += PAGE_SIZE) {
> +			ret = follow_pfn(vma, start, &pfn);
> +			if (ret) {
> +				printk(KERN_ERR "no page for address %lu\n",
> +					start);
> +				goto cleanup;
> +			}
> +			pages[n] = pfn_to_page(pfn);
> +			get_page(pages[n]);
> +		}
> +	} else {
> +		n = get_user_pages(current, current->mm, start & PAGE_MASK,
> +			n_pages, write, 1, pages, NULL);
> +		if (n != n_pages) {
> +			printk(KERN_ERR "got only %d of %d user pages\n",
> +				n, n_pages);
> +			goto cleanup;
> +		}
> +	}
> +
> +	*copy_vma = vb2_get_vma(vma);
> +	if (!*copy_vma) {
> +		printk(KERN_ERR "failed to copy vma\n");
> +		ret = -ENOMEM;
> +		goto cleanup;
> +	}
> +
> +	/* leaving critical section for mm access */
> +	up_read(&current->mm->mmap_sem);
> +
> +	return 0;
> +
> +cleanup:
> +	up_read(&current->mm->mmap_sem);
> +
> +	/* putting user pages if used, can be done wothout the lock */
> +	while (n)
> +		put_page(pages[--n]);
> +
> +	return ret;
> +}
> +
> +static void vb2_dc_put_userptr(void *buf_priv)
> +{
> +	struct vb2_dc_buf *buf = buf_priv;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	dma_unmap_sg(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
> +	vb2_dc_put_sgtable(sgt, !vma_is_io(buf->vma));
> +	vb2_put_vma(buf->vma);
> +	kfree(buf);
> +}
> +
> +static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
> +	unsigned long size, int write)
> +{
> +	struct vb2_dc_buf *buf;
> +	unsigned long start, end, offset, offset2;
> +	struct page **pages;
> +	int n_pages;
> +	int ret = 0;
> +	struct sg_table *sgt;
> +	unsigned long contig_size;
> +
> +	buf = kzalloc(sizeof *buf, GFP_KERNEL);
> +	if (!buf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	buf->dev = alloc_ctx;
> +	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +
> +	start = (unsigned long)vaddr & PAGE_MASK;
> +	offset = (unsigned long)vaddr & ~PAGE_MASK;
> +	end = PAGE_ALIGN((unsigned long)vaddr + size);
> +	offset2 = end - (unsigned long)vaddr - size;
> +	n_pages = (end - start) >> PAGE_SHIFT;
> +
> +	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
> +	if (!pages) {
> +		ret = -ENOMEM;
> +		printk(KERN_ERR "failed to allocate pages table\n");
> +		goto fail_buf;
> +	}
> +
> +	/* extract page list from userspace mapping */
> +	ret = vb2_dc_get_pages(start, pages, n_pages, &buf->vma, write);
> +	if (ret) {
> +		printk(KERN_ERR "failed to get user pages\n");
> +		goto fail_pages;
> +	}
> +
> +	sgt = vb2_dc_pages_to_sgt(pages, n_pages, offset, offset2);
> +	if (!sgt) {
> +		printk(KERN_ERR "failed to create scatterlist table\n");
> +		ret = -ENOMEM;
> +		goto fail_get_pages;
> +	}
> +
> +	/* pages are no longer needed */
> +	kfree(pages);
> +	pages = NULL;
> +
> +	sgt->nents = dma_map_sg(buf->dev, sgt->sgl, sgt->orig_nents,
> +		buf->dma_dir);
> +	if (sgt->nents <= 0) {
> +		printk(KERN_ERR "failed to map scatterlist\n");
> +		ret = -EIO;
> +		goto fail_sgt;
> +	}
> +
> +	contig_size = vb2_dc_get_contiguous_size(sgt);
> +	if (contig_size < size) {
> +		printk(KERN_ERR "contiguous mapping is too small %lu/%lu\n",
> +			contig_size, size);
> +		ret = -EFAULT;
> +		goto fail_map_sg;
> +	}
> +
> +	buf->dma_addr = sg_dma_address(sgt->sgl);
> +	buf->size = size;
> +	buf->dma_sgt = sgt;
> +
> +	atomic_inc(&buf->refcount);
> +
> +	return buf;
> +
> +fail_map_sg:
> +	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> +
> +fail_sgt:
> +	vb2_dc_put_sgtable(sgt, 0);
> +
> +fail_get_pages:
> +	while (pages && n_pages)
> +		put_page(pages[--n_pages]);
> +	vb2_put_vma(buf->vma);
> +
> +fail_pages:
> +	kfree(pages); /* kfree is NULL-proof */
> +
> +fail_buf:
> +	kfree(buf);
> +
> +	return ERR_PTR(ret);
> +}
> +
> +/*********************************************/
> +/*       callbacks for DMABUF buffers        */
> +/*********************************************/
> +
> +static int vb2_dc_map_dmabuf(void *mem_priv)
> +{
> +	struct vb2_dc_buf *buf = mem_priv;
> +	struct sg_table *sgt;
> +	unsigned long contig_size;
> +	int ret = 0;
> +
> +	if (WARN_ON(!buf->db_attach)) {
> +		printk(KERN_ERR "trying to pin a non attached buffer\n");
> +		return -EINVAL;
> +	}
> +
> +	if (WARN_ON(buf->dma_sgt)) {
> +		printk(KERN_ERR "dmabuf buffer is already pinned\n");
> +		return 0;
> +	}
> +
> +	/* get the associated scatterlist for this buffer */
> +	sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
> +	if (IS_ERR_OR_NULL(sgt)) {
> +		printk(KERN_ERR "Error getting dmabuf scatterlist\n");
> +		return -EINVAL;
> +	}
> +
> +	/* checking if dmabuf is big enough to store contiguous chunk */
> +	contig_size = vb2_dc_get_contiguous_size(sgt);
> +	if (contig_size < buf->size) {
> +		printk(KERN_ERR "contiguous chunk of dmabuf is too small\n");
> +		ret = -EFAULT;
> +		goto fail_map;
> +	}
> +
> +	buf->dma_addr = sg_dma_address(sgt->sgl);
> +	buf->dma_sgt = sgt;
> +
> +	return 0;
> +
> +fail_map:
> +	dma_buf_unmap_attachment(buf->db_attach, sgt);
> +
> +	return ret;
> +}
> +
> +static void vb2_dc_unmap_dmabuf(void *mem_priv)
> +{
> +	struct vb2_dc_buf *buf = mem_priv;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	if (WARN_ON(!buf->db_attach)) {
> +		printk(KERN_ERR "trying to unpin a not attached buffer\n");
> +		return;
> +	}
> +
> +	if (WARN_ON(!sgt)) {
> +		printk(KERN_ERR "dmabuf buffer is already unpinned\n");
> +		return;
> +	}
> +
> +	dma_buf_unmap_attachment(buf->db_attach, sgt);
> +
> +	buf->dma_addr = 0;
> +	buf->dma_sgt = NULL;
> +}
> +
> +static void vb2_dc_detach_dmabuf(void *mem_priv)
> +{
> +	struct vb2_dc_buf *buf = mem_priv;
> +
> +	if (buf->dma_addr)
> +		vb2_dc_unmap_dmabuf(buf);
> +
> +	/* detach this attachment */
> +	dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
> +	kfree(buf);
> +}
> +
> +static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
> +	unsigned long size, int write)
> +{
> +	struct vb2_dc_buf *buf;
> +	struct dma_buf_attachment *dba;
> +
> +	if (dbuf->size < size)
> +		return ERR_PTR(-EFAULT);
> +
> +	buf = kzalloc(sizeof *buf, GFP_KERNEL);
> +	if (!buf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	buf->dev = alloc_ctx;
> +	/* create attachment for the dmabuf with the user device */
> +	dba = dma_buf_attach(dbuf, buf->dev);
> +	if (IS_ERR(dba)) {
> +		printk(KERN_ERR "failed to attach dmabuf\n");
> +		kfree(buf);
> +		return dba;
> +	}
> +
> +	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +	buf->size = size;
> +	buf->db_attach = dba;
> +
> +	return buf;
> +}
> +
> +/*********************************************/
> +/*       DMA CONTIG exported functions       */
> +/*********************************************/
> +
> +const struct vb2_mem_ops vb2_dma_contig_memops = {
> +	.alloc		= vb2_dc_alloc,
> +	.put		= vb2_dc_put,
> +	.get_dmabuf	= vb2_dc_get_dmabuf,
> +	.cookie		= vb2_dc_cookie,
> +	.vaddr		= vb2_dc_vaddr,
> +	.mmap		= vb2_dc_mmap,
> +	.get_userptr	= vb2_dc_get_userptr,
> +	.put_userptr	= vb2_dc_put_userptr,
> +	.prepare	= vb2_dc_prepare,
> +	.finish		= vb2_dc_finish,
> +	.map_dmabuf	= vb2_dc_map_dmabuf,
> +	.unmap_dmabuf	= vb2_dc_unmap_dmabuf,
> +	.attach_dmabuf	= vb2_dc_attach_dmabuf,
> +	.detach_dmabuf	= vb2_dc_detach_dmabuf,
> +	.num_users	= vb2_dc_num_users,
> +};
> +EXPORT_SYMBOL_GPL(vb2_dma_contig_memops);
> +
> +void *vb2_dma_contig_init_ctx(struct device *dev)
> +{
> +	return dev;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_contig_init_ctx);
> +
> +void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
> +{
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
> +
> +MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2");
> +MODULE_AUTHOR("Pawel Osciak <pawel@osciak.com>");
> +MODULE_LICENSE("GPL");

Re: [PATCH 08/10] v4l: vb2-dma-contig: code refactoring, support for DMABUF exporting

[Tomasz Stanislawski]

Hi,

On 01/23/2012 03:26 PM, Mauro Carvalho Chehab wrote:
> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>> Signed-off-by: Pawel Osciak<pawel@osciak.com>
>> 	[author of the original file]
>> Signed-off-by: Marek Szyprowski<m.szyprowski@samsung.com>
>> 	[implemetation of mmap, finish/prepare]
>> Signed-off-by: Andrzej Pietrasiewicz<andrzej.p@samsung.com>
>> 	[implementation of userprt handling]
>> Signed-off-by: Sumit Semwal<sumit.semwal@ti.com>
>> Signed-off-by: Sumit Semwal<sumit.semwal@linaro.org>
>> 	[PoC for importing dmabuf buffer]
>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>>         [buffer exporting using dmabuf, code refactoring]
>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>
> Please provide a diff against the existing driver, in order to allow reviewing
> what has changed.

There is no difference patch due to the reasons described in the cover 
letter. This is a PoC patch and it is dedicated for easy reading.

Regards
Tomasz Stanislawski

Re: [PATCH 08/10] v4l: vb2-dma-contig: code refactoring, support for DMABUF exporting

[Mauro Carvalho Chehab]

Em 23-01-2012 12:35, Tomasz Stanislawski escreveu:
> Hi,
> 
> On 01/23/2012 03:26 PM, Mauro Carvalho Chehab wrote:
>> Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
>>> Signed-off-by: Pawel Osciak<pawel@osciak.com>
>>>     [author of the original file]
>>> Signed-off-by: Marek Szyprowski<m.szyprowski@samsung.com>
>>>     [implemetation of mmap, finish/prepare]
>>> Signed-off-by: Andrzej Pietrasiewicz<andrzej.p@samsung.com>
>>>     [implementation of userprt handling]
>>> Signed-off-by: Sumit Semwal<sumit.semwal@ti.com>
>>> Signed-off-by: Sumit Semwal<sumit.semwal@linaro.org>
>>>     [PoC for importing dmabuf buffer]
>>> Signed-off-by: Tomasz Stanislawski<t.stanislaws@samsung.com>
>>>         [buffer exporting using dmabuf, code refactoring]
>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>
>> Please provide a diff against the existing driver, in order to allow reviewing
>> what has changed.
> 
> There is no difference patch due to the reasons described in the cover letter. This is a PoC patch and it is dedicated for easy reading.

There's no way for a reviewer to check if some regressions were added by it,
as there's no diff. Those interested on doing an "easy reading" of the patch
changes could just apply the patch and see the results of it, but removing
and re-adding really sucks, as the ones interested on see what changed will
be a very bad time, especially if you moved functions inside the code.

> 
> Regards
> Tomasz Stanislawski
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: vb2-memops: Export vb2_get_vma symbol

[Laurent Pinchart]

The vb2_get_vma() function is called by videobuf2-dma-contig. Export it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/videobuf2-memops.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Hi Thomas,

The following patch is needed to compile videobuf2-dma-contig as a module.

diff --git a/drivers/media/video/videobuf2-memops.c b/drivers/media/video/videobuf2-memops.c
index 71a7a78..718f70e 100644
--- a/drivers/media/video/videobuf2-memops.c
+++ b/drivers/media/video/videobuf2-memops.c
@@ -55,6 +55,7 @@ struct vm_area_struct *vb2_get_vma(struct vm_area_struct *vma)
 
 	return vma_copy;
 }
+EXPORT_SYMBOL_GPL(vb2_get_vma);
 
 /**
  * vb2_put_userptr() - release a userspace virtual memory area
-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: vb2-memops: Export vb2_get_vma symbol

[Tomasz Stanislawski]

Hi Laurent,

Thank you for finding a bug in vb2-core.

Regards,
Tomasz Stanislawski

On 01/23/2012 03:35 PM, Laurent Pinchart wrote:
> The vb2_get_vma() function is called by videobuf2-dma-contig. Export it.
>
> Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
> ---
>   drivers/media/video/videobuf2-memops.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> Hi Thomas,
>
> The following patch is needed to compile videobuf2-dma-contig as a module.
>
> diff --git a/drivers/media/video/videobuf2-memops.c b/drivers/media/video/videobuf2-memops.c
> index 71a7a78..718f70e 100644
> --- a/drivers/media/video/videobuf2-memops.c
> +++ b/drivers/media/video/videobuf2-memops.c
> @@ -55,6 +55,7 @@ struct vm_area_struct *vb2_get_vma(struct vm_area_struct *vma)
>
>   	return vma_copy;
>   }
> +EXPORT_SYMBOL_GPL(vb2_get_vma);
>
>   /**
>    * vb2_put_userptr() - release a userspace virtual memory area

Re: [PATCH] media: vb2-memops: Export vb2_get_vma symbol

[Laurent Pinchart]

Hi Tomasz,

On Monday 23 January 2012 15:44:25 Tomasz Stanislawski wrote:
> Hi Laurent,
> 
> Thank you for finding a bug in vb2-core.

You're welcome.

Could you please take the patch in your tree ?

> On 01/23/2012 03:35 PM, Laurent Pinchart wrote:
> > The vb2_get_vma() function is called by videobuf2-dma-contig. Export it.
> > 
> > Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >   drivers/media/video/videobuf2-memops.c |    1 +
> >   1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > Hi Thomas,
> > 
> > The following patch is needed to compile videobuf2-dma-contig as a module.
> > 
> > diff --git a/drivers/media/video/videobuf2-memops.c
> > b/drivers/media/video/videobuf2-memops.c index 71a7a78..718f70e 100644
> > --- a/drivers/media/video/videobuf2-memops.c
> > +++ b/drivers/media/video/videobuf2-memops.c
> > @@ -55,6 +55,7 @@ struct vm_area_struct *vb2_get_vma(struct vm_area_struct
> > *vma)> 
> >   	return vma_copy;
> >   
> >   }
> > 
> > +EXPORT_SYMBOL_GPL(vb2_get_vma);
> > 
> >   /**
> >   
> >    * vb2_put_userptr() - release a userspace virtual memory area

-- 
Regards,

Laurent Pinchart

[PATCH 09/10] v4l: fimc: integrate capture i-face with dmabuf

[Tomasz Stanislawski]

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/s5p-fimc/fimc-capture.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c b/drivers/media/video/s5p-fimc/fimc-capture.c
index 2cc3b91..ba11c4a 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -1002,6 +1002,14 @@ static int fimc_cap_qbuf(struct file *file, void *priv,
 	return vb2_qbuf(&fimc->vid_cap.vbq, buf);
 }
 
+static int fimc_cap_expbuf(struct file *file, void *priv,
+			  unsigned int offset)
+{
+	struct fimc_dev *fimc = video_drvdata(file);
+
+	return vb2_expbuf(&fimc->vid_cap.vbq, offset);
+}
+
 static int fimc_cap_dqbuf(struct file *file, void *priv,
 			   struct v4l2_buffer *buf)
 {
@@ -1072,6 +1080,7 @@ static const struct v4l2_ioctl_ops fimc_capture_ioctl_ops = {
 
 	.vidioc_qbuf			= fimc_cap_qbuf,
 	.vidioc_dqbuf			= fimc_cap_dqbuf,
+	.vidioc_expbuf			= fimc_cap_expbuf,
 
 	.vidioc_streamon		= fimc_cap_streamon,
 	.vidioc_streamoff		= fimc_cap_streamoff,
@@ -1451,7 +1460,7 @@ int fimc_register_capture_device(struct fimc_dev *fimc,
 	q = &fimc->vid_cap.vbq;
 	memset(q, 0, sizeof(*q));
 	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
-	q->io_modes = VB2_MMAP | VB2_USERPTR;
+	q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
 	q->drv_priv = fimc->vid_cap.ctx;
 	q->ops = &fimc_capture_qops;
 	q->mem_ops = &vb2_dma_contig_memops;
-- 
1.7.5.4

[PATCH 10/10] v4l: s5p-tv: mixer: integrate with dmabuf

[Tomasz Stanislawski]

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/s5p-tv/mixer_video.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/s5p-tv/mixer_video.c b/drivers/media/video/s5p-tv/mixer_video.c
index b47d0c0..65e0722 100644
--- a/drivers/media/video/s5p-tv/mixer_video.c
+++ b/drivers/media/video/s5p-tv/mixer_video.c
@@ -592,6 +592,14 @@ static int mxr_dqbuf(struct file *file, void *priv, struct v4l2_buffer *p)
 	return vb2_dqbuf(&layer->vb_queue, p, file->f_flags & O_NONBLOCK);
 }
 
+static int mxr_expbuf(struct file *file, void *priv, unsigned int offset)
+{
+	struct mxr_layer *layer = video_drvdata(file);
+
+	mxr_dbg(layer->mdev, "%s:%d\n", __func__, __LINE__);
+	return vb2_expbuf(&layer->vb_queue, offset);
+}
+
 static int mxr_streamon(struct file *file, void *priv, enum v4l2_buf_type i)
 {
 	struct mxr_layer *layer = video_drvdata(file);
@@ -619,6 +627,7 @@ static const struct v4l2_ioctl_ops mxr_ioctl_ops = {
 	.vidioc_querybuf = mxr_querybuf,
 	.vidioc_qbuf = mxr_qbuf,
 	.vidioc_dqbuf = mxr_dqbuf,
+	.vidioc_expbuf = mxr_expbuf,
 	/* Streaming control */
 	.vidioc_streamon = mxr_streamon,
 	.vidioc_streamoff = mxr_streamoff,
@@ -973,7 +982,7 @@ struct mxr_layer *mxr_base_layer_create(struct mxr_device *mdev,
 
 	layer->vb_queue = (struct vb2_queue) {
 		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
-		.io_modes = VB2_MMAP | VB2_USERPTR,
+		.io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF,
 		.drv_priv = layer,
 		.buf_struct_size = sizeof(struct mxr_buffer),
 		.ops = &mxr_video_qops,
-- 
1.7.5.4

Re: [PATCH 00/10] Integration of videobuf2 with dmabuf

[Mauro Carvalho Chehab]

Hi Tomasz/Sumit,

Em 23-01-2012 11:51, Tomasz Stanislawski escreveu:
> Hello everyone,
> This patchset is an incremental patch to patchset created by Sumit
> Semwal [1].  The patches are dedicated to help find a better solution for
> support of buffer sharing by V4L2 API.  It is expected to start discussion on
> final installment for dma-buf in vb2-dma-contig allocator.  Current version of
> the patches contain little documentation. It is going to be fixed after
> achieving consensus about design for buffer exporting.  Moreover the API
> between vb2-core and the allocator should be revised.

I just raised a few points for the discussions for a few patches. Please don't
understand them as a full review. It isn't.

Btw, it would be nice to have vivi support for the dmabuf sharing, in order to
allow the patches to be tested by a wider audience, especially due to the new
userspace API proposal.

Regards,
Mauro
> 
> The amount of changes to vb2-dma-contig.c was significant making the difference
> patch very difficult to read.  Therefore the patch was split into two parts.
> One removes old file, the next patch creates the version of the file.
> 
> The patchset contains extension for DMA API and its implementation for ARM
> architecture. Therefore the patchset should be applied on the top of:
> 
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/3.2-dma-v5
> 
> After applying patches from [2] and [1].
> 
> v1: List of changes since [1].
> - support for DMA api extension dma_get_pages, the function is used to retrieve pages
>  used to create DMA mapping.
> - small fixes/code cleanup to videobuf2
> - added prepare and finish callbacks to vb2 allocators, it is used keep consistency between dma-cpu acess to the memory (by Marek Szyprowski)
> - support for exporting of DMABUF buffer in V4L2 and Videobuf2, originated from [3].
> - support for dma-buf exporting in vb2-dma-contig allocator
> - support for DMABUF for s5p-tv and s5p-fimc (capture interface) drivers, originated from [3]
> - changed handling for userptr buffers (by Marek Szyprowski, Andrzej Pietrasiewicz)
> - let mmap method to use dma_mmap_writecombine call (by Marek Szyprowski)
> 
> [1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966/focus=42968
> [2] https://lkml.org/lkml/2011/12/26/29
> [3] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/36354/focus=36355
> 
> 
> Marek Szyprowski (2):
>   [media] media: vb2: remove plane argument from call_memop and cleanup
>     mempriv usage
>   media: vb2: add prepare/finish callbacks to allocators
> 
> Tomasz Stanislawski (8):
>   arm: dma: support for dma_get_pages
>   v4l: vb2: fixes for DMABUF support
>   v4l: add buffer exporting via dmabuf
>   v4l: vb2: add buffer exporting via dmabuf
>   v4l: vb2: remove dma-contig allocator
>   v4l: vb2-dma-contig: code refactoring, support for DMABUF exporting
>   v4l: fimc: integrate capture i-face with dmabuf
>   v4l: s5p-tv: mixer: integrate with dmabuf
> 
>  arch/arm/include/asm/dma-mapping.h          |    8 +
>  arch/arm/mm/dma-mapping.c                   |   44 ++
>  drivers/media/video/s5p-fimc/fimc-capture.c |   11 +-
>  drivers/media/video/s5p-tv/mixer_video.c    |   11 +-
>  drivers/media/video/v4l2-compat-ioctl32.c   |    1 +
>  drivers/media/video/v4l2-ioctl.c            |   11 +
>  drivers/media/video/videobuf2-core.c        |  114 ++++-
>  drivers/media/video/videobuf2-dma-contig.c  |  754 +++++++++++++++++++++------
>  include/linux/dma-mapping.h                 |    2 +
>  include/linux/videodev2.h                   |    1 +
>  include/media/v4l2-ioctl.h                  |    1 +
>  include/media/videobuf2-core.h              |   10 +-
>  12 files changed, 789 insertions(+), 179 deletions(-)
>