[REVIEWv7 PATCH 00/12] vb2: improve dma-sg, expbuf

[REVIEWv7 PATCH 00/12] vb2: improve dma-sg, expbuf

[Hans Verkuil]

Changes since RFCv6:

- Dropped patches 12-16/16: introducing begin/end_cpu_access functions
  is not yet ready for upstreaming.
- Incorporated Pawel's review comments except for his comment regarding
  the use of 'if (!IS_ERR_OR_NULL(alloc_ctx))' in vb2_dma_sg_cleanup_ctx().
  I rather like it (it's more robust for one thing) and actually added a
  patch to do the same for vb2_dma_contig_cleanup_ctx(). If this really
  runs into resistance, then I can remove it again.

The patch series adds an allocation context to dma-sg and uses that to move
dma_(un)map_sg into the vb2 framework, which is where it belongs.

Some drivers needs to fixup the buffers before giving it back to userspace
(or before handing it over to the kernel). Document that this can be done
in buf_prepare and buf_finish.

DMABUF export support is added to dma-sg and vmalloc, so now all memory
models support DMABUF importing and exporting.

I'm planning to make a pull request on Friday if there are no more comments.

Regards,

	Hans

[REVIEWv7 PATCH 01/12] videobuf2-core.h: improve documentation

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Document that drivers can access/modify the buffer contents in buf_prepare
and buf_finish. That was not clearly stated before.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 include/media/videobuf2-core.h | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 6ef2d01..70ace7c 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -270,22 +270,24 @@ struct vb2_buffer {
  *			queue setup from completing successfully; optional.
  * @buf_prepare:	called every time the buffer is queued from userspace
  *			and from the VIDIOC_PREPARE_BUF ioctl; drivers may
- *			perform any initialization required before each hardware
- *			operation in this callback; drivers that support
- *			VIDIOC_CREATE_BUFS must also validate the buffer size;
- *			if an error is returned, the buffer will not be queued
- *			in driver; optional.
+ *			perform any initialization required before each
+ *			hardware operation in this callback; drivers can
+ *			access/modify the buffer here as it is still synced for
+ *			the CPU; drivers that support VIDIOC_CREATE_BUFS must
+ *			also validate the buffer size; if an error is returned,
+ *			the buffer will not be queued in driver; optional.
  * @buf_finish:		called before every dequeue of the buffer back to
- *			userspace; drivers may perform any operations required
- *			before userspace accesses the buffer; optional. The
- *			buffer state can be one of the following: DONE and
- *			ERROR occur while streaming is in progress, and the
- *			PREPARED state occurs when the queue has been canceled
- *			and all pending buffers are being returned to their
- *			default DEQUEUED state. Typically you only have to do
- *			something if the state is VB2_BUF_STATE_DONE, since in
- *			all other cases the buffer contents will be ignored
- *			anyway.
+ *			userspace; the buffer is synced for the CPU, so drivers
+ *			can access/modify the buffer contents; drivers may
+ *			perform any operations required before userspace
+ *			accesses the buffer; optional. The buffer state can be
+ *			one of the following: DONE and ERROR occur while
+ *			streaming is in progress, and the PREPARED state occurs
+ *			when the queue has been canceled and all pending
+ *			buffers are being returned to their default DEQUEUED
+ *			state. Typically you only have to do something if the
+ *			state is VB2_BUF_STATE_DONE, since in all other cases
+ *			the buffer contents will be ignored anyway.
  * @buf_cleanup:	called once before the buffer is freed; drivers may
  *			perform any additional cleanup; optional.
  * @start_streaming:	called once to enter 'streaming' state; the driver may
-- 
2.1.1

Re: [REVIEWv7 PATCH 01/12] videobuf2-core.h: improve documentation

[Pawel Osciak]

On Tue, Nov 18, 2014 at 9:50 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Document that drivers can access/modify the buffer contents in buf_prepare
> and buf_finish. That was not clearly stated before.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  include/media/videobuf2-core.h | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 6ef2d01..70ace7c 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -270,22 +270,24 @@ struct vb2_buffer {
>   *                     queue setup from completing successfully; optional.
>   * @buf_prepare:       called every time the buffer is queued from userspace
>   *                     and from the VIDIOC_PREPARE_BUF ioctl; drivers may
> - *                     perform any initialization required before each hardware
> - *                     operation in this callback; drivers that support
> - *                     VIDIOC_CREATE_BUFS must also validate the buffer size;
> - *                     if an error is returned, the buffer will not be queued
> - *                     in driver; optional.
> + *                     perform any initialization required before each
> + *                     hardware operation in this callback; drivers can
> + *                     access/modify the buffer here as it is still synced for
> + *                     the CPU; drivers that support VIDIOC_CREATE_BUFS must
> + *                     also validate the buffer size; if an error is returned,
> + *                     the buffer will not be queued in driver; optional.
>   * @buf_finish:                called before every dequeue of the buffer back to
> - *                     userspace; drivers may perform any operations required
> - *                     before userspace accesses the buffer; optional. The
> - *                     buffer state can be one of the following: DONE and
> - *                     ERROR occur while streaming is in progress, and the
> - *                     PREPARED state occurs when the queue has been canceled
> - *                     and all pending buffers are being returned to their
> - *                     default DEQUEUED state. Typically you only have to do
> - *                     something if the state is VB2_BUF_STATE_DONE, since in
> - *                     all other cases the buffer contents will be ignored
> - *                     anyway.
> + *                     userspace; the buffer is synced for the CPU, so drivers
> + *                     can access/modify the buffer contents; drivers may
> + *                     perform any operations required before userspace
> + *                     accesses the buffer; optional. The buffer state can be
> + *                     one of the following: DONE and ERROR occur while
> + *                     streaming is in progress, and the PREPARED state occurs
> + *                     when the queue has been canceled and all pending
> + *                     buffers are being returned to their default DEQUEUED
> + *                     state. Typically you only have to do something if the
> + *                     state is VB2_BUF_STATE_DONE, since in all other cases
> + *                     the buffer contents will be ignored anyway.
>   * @buf_cleanup:       called once before the buffer is freed; drivers may
>   *                     perform any additional cleanup; optional.
>   * @start_streaming:   called once to enter 'streaming' state; the driver may
> --
> 2.1.1
>



-- 
Best regards,
Pawel Osciak

Re: [REVIEWv7 PATCH 01/12] videobuf2-core.h: improve documentation

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Sunday 23 November 2014 20:01:22 Pawel Osciak wrote:
> On Tue, Nov 18, 2014 at 9:50 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > Document that drivers can access/modify the buffer contents in buf_prepare
> > and buf_finish. That was not clearly stated before.
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Acked-by: Pawel Osciak <pawel@osciak.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> 
> > ---
> > 
> >  include/media/videobuf2-core.h | 32 +++++++++++++++++---------------
> >  1 file changed, 17 insertions(+), 15 deletions(-)
> > 
> > diff --git a/include/media/videobuf2-core.h
> > b/include/media/videobuf2-core.h index 6ef2d01..70ace7c 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -270,22 +270,24 @@ struct vb2_buffer {
> > 
> >   *                     queue setup from completing successfully;
> >   optional.
> >   * @buf_prepare:       called every time the buffer is queued from
> >   userspace
> >   *                     and from the VIDIOC_PREPARE_BUF ioctl; drivers may
> > 
> > - *                     perform any initialization required before each
> > hardware - *                     operation in this callback; drivers that
> > support - *                     VIDIOC_CREATE_BUFS must also validate the
> > buffer size; - *                     if an error is returned, the buffer
> > will not be queued - *                     in driver; optional.
> > + *                     perform any initialization required before each
> > + *                     hardware operation in this callback; drivers can
> > + *                     access/modify the buffer here as it is still
> > synced for + *                     the CPU; drivers that support
> > VIDIOC_CREATE_BUFS must + *                     also validate the buffer
> > size; if an error is returned, + *                     the buffer will
> > not be queued in driver; optional.> 
> >   * @buf_finish:                called before every dequeue of the buffer
> >   back to> 
> > - *                     userspace; drivers may perform any operations
> > required - *                     before userspace accesses the buffer;
> > optional. The - *                     buffer state can be one of the
> > following: DONE and - *                     ERROR occur while streaming
> > is in progress, and the - *                     PREPARED state occurs
> > when the queue has been canceled - *                     and all pending
> > buffers are being returned to their - *                     default
> > DEQUEUED state. Typically you only have to do - *                    
> > something if the state is VB2_BUF_STATE_DONE, since in - *               
> >      all other cases the buffer contents will be ignored - *             
> >        anyway.
> > + *                     userspace; the buffer is synced for the CPU, so
> > drivers + *                     can access/modify the buffer contents;
> > drivers may + *                     perform any operations required
> > before userspace + *                     accesses the buffer; optional.
> > The buffer state can be + *                     one of the following:
> > DONE and ERROR occur while + *                     streaming is in
> > progress, and the PREPARED state occurs + *                     when the
> > queue has been canceled and all pending + *                     buffers
> > are being returned to their default DEQUEUED + *                    
> > state. Typically you only have to do something if the + *                
> >     state is VB2_BUF_STATE_DONE, since in all other cases + *            
> >         the buffer contents will be ignored anyway.> 
> >   * @buf_cleanup:       called once before the buffer is freed; drivers
> >   may
> >   *                     perform any additional cleanup; optional.
> >   * @start_streaming:   called once to enter 'streaming' state; the driver
> >   may> 
> > --
> > 2.1.1

-- 
Regards,

Laurent Pinchart

[REVIEWv7 PATCH 02/12] vb2: replace 'write' by 'dma_dir'

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The 'write' argument is very ambiguous. I first assumed that if it is 1,
then we're doing video output but instead it meant the reverse.

Since it is used to setup the dma_dir value anyway it is now replaced by
the correct dma_dir value which is unambiguous.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/v4l2-core/videobuf2-core.c       | 10 ++++---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 40 ++++++++++++++------------
 drivers/media/v4l2-core/videobuf2-dma-sg.c     | 13 +++++----
 drivers/media/v4l2-core/videobuf2-vmalloc.c    | 16 ++++++-----
 include/media/videobuf2-core.h                 |  6 ++--
 5 files changed, 47 insertions(+), 38 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index f2e43de..573f6fb 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1358,7 +1358,8 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	void *mem_priv;
 	unsigned int plane;
 	int ret;
-	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
+	enum dma_data_direction dma_dir =
+		V4L2_TYPE_IS_OUTPUT(q->type) ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	bool reacquired = vb->planes[0].mem_priv == NULL;
 
 	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1400,7 +1401,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		/* Acquire each plane's memory */
 		mem_priv = call_ptr_memop(vb, get_userptr, q->alloc_ctx[plane],
 				      planes[plane].m.userptr,
-				      planes[plane].length, write);
+				      planes[plane].length, dma_dir);
 		if (IS_ERR_OR_NULL(mem_priv)) {
 			dprintk(1, "failed acquiring userspace "
 						"memory for plane %d\n", plane);
@@ -1461,7 +1462,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	void *mem_priv;
 	unsigned int plane;
 	int ret;
-	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
+	enum dma_data_direction dma_dir =
+		V4L2_TYPE_IS_OUTPUT(q->type) ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	bool reacquired = vb->planes[0].mem_priv == NULL;
 
 	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1509,7 +1511,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 
 		/* Acquire each plane's memory */
 		mem_priv = call_ptr_memop(vb, attach_dmabuf, q->alloc_ctx[plane],
-			dbuf, planes[plane].length, write);
+			dbuf, planes[plane].length, dma_dir);
 		if (IS_ERR(mem_priv)) {
 			dprintk(1, "failed to attach dmabuf\n");
 			ret = PTR_ERR(mem_priv);
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 4a02ade..2bdffd3 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -229,7 +229,7 @@ static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma)
 
 struct vb2_dc_attachment {
 	struct sg_table sgt;
-	enum dma_data_direction dir;
+	enum dma_data_direction dma_dir;
 };
 
 static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
@@ -264,7 +264,7 @@ static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
 		wr = sg_next(wr);
 	}
 
-	attach->dir = DMA_NONE;
+	attach->dma_dir = DMA_NONE;
 	dbuf_attach->priv = attach;
 
 	return 0;
@@ -282,16 +282,16 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
 	sgt = &attach->sgt;
 
 	/* release the scatterlist cache */
-	if (attach->dir != DMA_NONE)
+	if (attach->dma_dir != DMA_NONE)
 		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
-			attach->dir);
+			attach->dma_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_attachment *db_attach, enum dma_data_direction dma_dir)
 {
 	struct vb2_dc_attachment *attach = db_attach->priv;
 	/* stealing dmabuf mutex to serialize map/unmap operations */
@@ -303,27 +303,27 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
 
 	sgt = &attach->sgt;
 	/* return previously mapped sg table */
-	if (attach->dir == dir) {
+	if (attach->dma_dir == dma_dir) {
 		mutex_unlock(lock);
 		return sgt;
 	}
 
 	/* release any previous cache */
-	if (attach->dir != DMA_NONE) {
+	if (attach->dma_dir != DMA_NONE) {
 		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
-			attach->dir);
-		attach->dir = DMA_NONE;
+			attach->dma_dir);
+		attach->dma_dir = DMA_NONE;
 	}
 
 	/* mapping to the client with new direction */
-	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dir);
+	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dma_dir);
 	if (ret <= 0) {
 		pr_err("failed to map scatterlist\n");
 		mutex_unlock(lock);
 		return ERR_PTR(-EIO);
 	}
 
-	attach->dir = dir;
+	attach->dma_dir = dma_dir;
 
 	mutex_unlock(lock);
 
@@ -331,7 +331,7 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
 }
 
 static void vb2_dc_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
-	struct sg_table *sgt, enum dma_data_direction dir)
+	struct sg_table *sgt, enum dma_data_direction dma_dir)
 {
 	/* nothing to be done here */
 }
@@ -460,7 +460,8 @@ static int vb2_dc_get_user_pfn(unsigned long start, int n_pages,
 }
 
 static int vb2_dc_get_user_pages(unsigned long start, struct page **pages,
-	int n_pages, struct vm_area_struct *vma, int write)
+	int n_pages, struct vm_area_struct *vma,
+	enum dma_data_direction dma_dir)
 {
 	if (vma_is_io(vma)) {
 		unsigned int i;
@@ -482,7 +483,7 @@ static int vb2_dc_get_user_pages(unsigned long start, struct page **pages,
 		int n;
 
 		n = get_user_pages(current, current->mm, start & PAGE_MASK,
-			n_pages, write, 1, pages, NULL);
+			n_pages, dma_dir == DMA_FROM_DEVICE, 1, pages, NULL);
 		/* negative error means that no page was pinned */
 		n = max(n, 0);
 		if (n != n_pages) {
@@ -551,7 +552,7 @@ static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long pfn
 #endif
 
 static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
-	unsigned long size, int write)
+	unsigned long size, enum dma_data_direction dma_dir)
 {
 	struct vb2_dc_conf *conf = alloc_ctx;
 	struct vb2_dc_buf *buf;
@@ -582,7 +583,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		return ERR_PTR(-ENOMEM);
 
 	buf->dev = conf->dev;
-	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_dir = dma_dir;
 
 	start = vaddr & PAGE_MASK;
 	offset = vaddr & ~PAGE_MASK;
@@ -618,7 +619,8 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	}
 
 	/* extract page list from userspace mapping */
-	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma, write);
+	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma,
+				    dma_dir == DMA_FROM_DEVICE);
 	if (ret) {
 		unsigned long pfn;
 		if (vb2_dc_get_user_pfn(start, n_pages, vma, &pfn) == 0) {
@@ -782,7 +784,7 @@ static void vb2_dc_detach_dmabuf(void *mem_priv)
 }
 
 static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
-	unsigned long size, int write)
+	unsigned long size, enum dma_data_direction dma_dir)
 {
 	struct vb2_dc_conf *conf = alloc_ctx;
 	struct vb2_dc_buf *buf;
@@ -804,7 +806,7 @@ static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
 		return dba;
 	}
 
-	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 	buf->db_attach = dba;
 
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 9b163a4..6b54a14 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -33,8 +33,8 @@ module_param(debug, int, 0644);
 struct vb2_dma_sg_buf {
 	void				*vaddr;
 	struct page			**pages;
-	int				write;
 	int				offset;
+	enum dma_data_direction		dma_dir;
 	struct sg_table			sg_table;
 	size_t				size;
 	unsigned int			num_pages;
@@ -97,7 +97,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_fla
 		return NULL;
 
 	buf->vaddr = NULL;
-	buf->write = 0;
+	buf->dma_dir = DMA_NONE;
 	buf->offset = 0;
 	buf->size = size;
 	/* size is already page aligned */
@@ -162,7 +162,8 @@ static inline int vma_is_io(struct vm_area_struct *vma)
 }
 
 static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
-				    unsigned long size, int write)
+				    unsigned long size,
+				    enum dma_data_direction dma_dir)
 {
 	struct vb2_dma_sg_buf *buf;
 	unsigned long first, last;
@@ -174,7 +175,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		return NULL;
 
 	buf->vaddr = NULL;
-	buf->write = write;
+	buf->dma_dir = dma_dir;
 	buf->offset = vaddr & ~PAGE_MASK;
 	buf->size = size;
 
@@ -221,7 +222,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		num_pages_from_user = get_user_pages(current, current->mm,
 					     vaddr & PAGE_MASK,
 					     buf->num_pages,
-					     write,
+					     buf->dma_dir == DMA_FROM_DEVICE,
 					     1, /* force */
 					     buf->pages,
 					     NULL);
@@ -265,7 +266,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
 		vm_unmap_ram(buf->vaddr, buf->num_pages);
 	sg_free_table(&buf->sg_table);
 	while (--i >= 0) {
-		if (buf->write)
+		if (buf->dma_dir == DMA_FROM_DEVICE)
 			set_page_dirty_lock(buf->pages[i]);
 		if (!vma_is_io(buf->vma))
 			put_page(buf->pages[i]);
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 313d977..fc1eb45 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -25,7 +25,7 @@ struct vb2_vmalloc_buf {
 	void				*vaddr;
 	struct page			**pages;
 	struct vm_area_struct		*vma;
-	int				write;
+	enum dma_data_direction		dma_dir;
 	unsigned long			size;
 	unsigned int			n_pages;
 	atomic_t			refcount;
@@ -70,7 +70,8 @@ static void vb2_vmalloc_put(void *buf_priv)
 }
 
 static void *vb2_vmalloc_get_userptr(void *alloc_ctx, unsigned long vaddr,
-				     unsigned long size, int write)
+				     unsigned long size,
+				     enum dma_data_direction dma_dir)
 {
 	struct vb2_vmalloc_buf *buf;
 	unsigned long first, last;
@@ -82,7 +83,7 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	if (!buf)
 		return NULL;
 
-	buf->write = write;
+	buf->dma_dir = dma_dir;
 	offset = vaddr & ~PAGE_MASK;
 	buf->size = size;
 
@@ -107,7 +108,8 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		/* current->mm->mmap_sem is taken by videobuf2 core */
 		n_pages = get_user_pages(current, current->mm,
 					 vaddr & PAGE_MASK, buf->n_pages,
-					 write, 1, /* force */
+					 dma_dir == DMA_FROM_DEVICE,
+					 1, /* force */
 					 buf->pages, NULL);
 		if (n_pages != buf->n_pages)
 			goto fail_get_user_pages;
@@ -144,7 +146,7 @@ static void vb2_vmalloc_put_userptr(void *buf_priv)
 		if (vaddr)
 			vm_unmap_ram((void *)vaddr, buf->n_pages);
 		for (i = 0; i < buf->n_pages; ++i) {
-			if (buf->write)
+			if (buf->dma_dir == DMA_FROM_DEVICE)
 				set_page_dirty_lock(buf->pages[i]);
 			put_page(buf->pages[i]);
 		}
@@ -240,7 +242,7 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv)
 }
 
 static void *vb2_vmalloc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
-	unsigned long size, int write)
+	unsigned long size, enum dma_data_direction dma_dir)
 {
 	struct vb2_vmalloc_buf *buf;
 
@@ -252,7 +254,7 @@ static void *vb2_vmalloc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
 		return ERR_PTR(-ENOMEM);
 
 	buf->dbuf = dbuf;
-	buf->write = write;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 
 	return buf;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 70ace7c..d607871 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -87,14 +87,16 @@ struct vb2_mem_ops {
 	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);
 
 	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
-					unsigned long size, int write);
+					unsigned long size,
+					enum dma_data_direction dma_dir);
 	void		(*put_userptr)(void *buf_priv);
 
 	void		(*prepare)(void *buf_priv);
 	void		(*finish)(void *buf_priv);
 
 	void		*(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf,
-				unsigned long size, int write);
+					  unsigned long size,
+					  enum dma_data_direction dma_dir);
 	void		(*detach_dmabuf)(void *buf_priv);
 	int		(*map_dmabuf)(void *buf_priv);
 	void		(*unmap_dmabuf)(void *buf_priv);
-- 
2.1.1

Re: [REVIEWv7 PATCH 02/12] vb2: replace 'write' by 'dma_dir'

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Tuesday 18 November 2014 13:50:58 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The 'write' argument is very ambiguous. I first assumed that if it is 1,
> then we're doing video output but instead it meant the reverse.
> 
> Since it is used to setup the dma_dir value anyway it is now replaced by
> the correct dma_dir value which is unambiguous.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Pawel Osciak <pawel@osciak.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c       | 10 ++++---
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 40
> ++++++++++++++------------ drivers/media/v4l2-core/videobuf2-dma-sg.c     |
> 13 +++++----
>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 16 ++++++-----
>  include/media/videobuf2-core.h                 |  6 ++--
>  5 files changed, 47 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index f2e43de..573f6fb 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1358,7 +1358,8 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const
> struct v4l2_buffer *b) void *mem_priv;
>  	unsigned int plane;
>  	int ret;
> -	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
> +	enum dma_data_direction dma_dir =
> +		V4L2_TYPE_IS_OUTPUT(q->type) ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>  	bool reacquired = vb->planes[0].mem_priv == NULL;
> 
>  	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
> @@ -1400,7 +1401,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const
> struct v4l2_buffer *b) /* Acquire each plane's memory */
>  		mem_priv = call_ptr_memop(vb, get_userptr, q->alloc_ctx[plane],
>  				      planes[plane].m.userptr,
> -				      planes[plane].length, write);
> +				      planes[plane].length, dma_dir);
>  		if (IS_ERR_OR_NULL(mem_priv)) {
>  			dprintk(1, "failed acquiring userspace "
>  						"memory for plane %d\n", plane);
> @@ -1461,7 +1462,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const
> struct v4l2_buffer *b) void *mem_priv;
>  	unsigned int plane;
>  	int ret;
> -	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
> +	enum dma_data_direction dma_dir =
> +		V4L2_TYPE_IS_OUTPUT(q->type) ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>  	bool reacquired = vb->planes[0].mem_priv == NULL;
> 
>  	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
> @@ -1509,7 +1511,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const
> struct v4l2_buffer *b)
> 
>  		/* Acquire each plane's memory */
>  		mem_priv = call_ptr_memop(vb, attach_dmabuf, q->alloc_ctx[plane],
> -			dbuf, planes[plane].length, write);
> +			dbuf, planes[plane].length, dma_dir);
>  		if (IS_ERR(mem_priv)) {
>  			dprintk(1, "failed to attach dmabuf\n");
>  			ret = PTR_ERR(mem_priv);
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 4a02ade..2bdffd3
> 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -229,7 +229,7 @@ static int vb2_dc_mmap(void *buf_priv, struct
> vm_area_struct *vma)
> 
>  struct vb2_dc_attachment {
>  	struct sg_table sgt;
> -	enum dma_data_direction dir;
> +	enum dma_data_direction dma_dir;
>  };
> 
>  static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device
> *dev, @@ -264,7 +264,7 @@ static int vb2_dc_dmabuf_ops_attach(struct
> dma_buf *dbuf, struct device *dev, wr = sg_next(wr);
>  	}
> 
> -	attach->dir = DMA_NONE;
> +	attach->dma_dir = DMA_NONE;
>  	dbuf_attach->priv = attach;
> 
>  	return 0;
> @@ -282,16 +282,16 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf
> *dbuf, sgt = &attach->sgt;
> 
>  	/* release the scatterlist cache */
> -	if (attach->dir != DMA_NONE)
> +	if (attach->dma_dir != DMA_NONE)
>  		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
> -			attach->dir);
> +			attach->dma_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_attachment *db_attach, enum dma_data_direction dma_dir)
>  {
>  	struct vb2_dc_attachment *attach = db_attach->priv;
>  	/* stealing dmabuf mutex to serialize map/unmap operations */
> @@ -303,27 +303,27 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
> 
>  	sgt = &attach->sgt;
>  	/* return previously mapped sg table */
> -	if (attach->dir == dir) {
> +	if (attach->dma_dir == dma_dir) {
>  		mutex_unlock(lock);
>  		return sgt;
>  	}
> 
>  	/* release any previous cache */
> -	if (attach->dir != DMA_NONE) {
> +	if (attach->dma_dir != DMA_NONE) {
>  		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
> -			attach->dir);
> -		attach->dir = DMA_NONE;
> +			attach->dma_dir);
> +		attach->dma_dir = DMA_NONE;
>  	}
> 
>  	/* mapping to the client with new direction */
> -	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dir);
> +	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dma_dir);
>  	if (ret <= 0) {
>  		pr_err("failed to map scatterlist\n");
>  		mutex_unlock(lock);
>  		return ERR_PTR(-EIO);
>  	}
> 
> -	attach->dir = dir;
> +	attach->dma_dir = dma_dir;
> 
>  	mutex_unlock(lock);
> 
> @@ -331,7 +331,7 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
>  }
> 
>  static void vb2_dc_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
> -	struct sg_table *sgt, enum dma_data_direction dir)
> +	struct sg_table *sgt, enum dma_data_direction dma_dir)
>  {
>  	/* nothing to be done here */
>  }
> @@ -460,7 +460,8 @@ static int vb2_dc_get_user_pfn(unsigned long start, int
> n_pages, }
> 
>  static int vb2_dc_get_user_pages(unsigned long start, struct page **pages,
> -	int n_pages, struct vm_area_struct *vma, int write)
> +	int n_pages, struct vm_area_struct *vma,
> +	enum dma_data_direction dma_dir)
>  {
>  	if (vma_is_io(vma)) {
>  		unsigned int i;
> @@ -482,7 +483,7 @@ static int vb2_dc_get_user_pages(unsigned long start,
> struct page **pages, int n;
> 
>  		n = get_user_pages(current, current->mm, start & PAGE_MASK,
> -			n_pages, write, 1, pages, NULL);
> +			n_pages, dma_dir == DMA_FROM_DEVICE, 1, pages, NULL);
>  		/* negative error means that no page was pinned */
>  		n = max(n, 0);
>  		if (n != n_pages) {
> @@ -551,7 +552,7 @@ static inline dma_addr_t vb2_dc_pfn_to_dma(struct device
> *dev, unsigned long pfn #endif
> 
>  static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
> -	unsigned long size, int write)
> +	unsigned long size, enum dma_data_direction dma_dir)
>  {
>  	struct vb2_dc_conf *conf = alloc_ctx;
>  	struct vb2_dc_buf *buf;
> @@ -582,7 +583,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx,
> unsigned long vaddr, return ERR_PTR(-ENOMEM);
> 
>  	buf->dev = conf->dev;
> -	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +	buf->dma_dir = dma_dir;
> 
>  	start = vaddr & PAGE_MASK;
>  	offset = vaddr & ~PAGE_MASK;
> @@ -618,7 +619,8 @@ static void *vb2_dc_get_userptr(void *alloc_ctx,
> unsigned long vaddr, }
> 
>  	/* extract page list from userspace mapping */
> -	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma, write);
> +	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma,
> +				    dma_dir == DMA_FROM_DEVICE);
>  	if (ret) {
>  		unsigned long pfn;
>  		if (vb2_dc_get_user_pfn(start, n_pages, vma, &pfn) == 0) {
> @@ -782,7 +784,7 @@ static void vb2_dc_detach_dmabuf(void *mem_priv)
>  }
> 
>  static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
> -	unsigned long size, int write)
> +	unsigned long size, enum dma_data_direction dma_dir)
>  {
>  	struct vb2_dc_conf *conf = alloc_ctx;
>  	struct vb2_dc_buf *buf;
> @@ -804,7 +806,7 @@ static void *vb2_dc_attach_dmabuf(void *alloc_ctx,
> struct dma_buf *dbuf, return dba;
>  	}
> 
> -	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +	buf->dma_dir = dma_dir;
>  	buf->size = size;
>  	buf->db_attach = dba;
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 9b163a4..6b54a14 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -33,8 +33,8 @@ module_param(debug, int, 0644);
>  struct vb2_dma_sg_buf {
>  	void				*vaddr;
>  	struct page			**pages;
> -	int				write;
>  	int				offset;
> +	enum dma_data_direction		dma_dir;
>  	struct sg_table			sg_table;
>  	size_t				size;
>  	unsigned int			num_pages;
> @@ -97,7 +97,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, gfp_t gfp_fla return NULL;
> 
>  	buf->vaddr = NULL;
> -	buf->write = 0;
> +	buf->dma_dir = DMA_NONE;
>  	buf->offset = 0;
>  	buf->size = size;
>  	/* size is already page aligned */
> @@ -162,7 +162,8 @@ static inline int vma_is_io(struct vm_area_struct *vma)
>  }
> 
>  static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
> -				    unsigned long size, int write)
> +				    unsigned long size,
> +				    enum dma_data_direction dma_dir)
>  {
>  	struct vb2_dma_sg_buf *buf;
>  	unsigned long first, last;
> @@ -174,7 +175,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, return NULL;
> 
>  	buf->vaddr = NULL;
> -	buf->write = write;
> +	buf->dma_dir = dma_dir;
>  	buf->offset = vaddr & ~PAGE_MASK;
>  	buf->size = size;
> 
> @@ -221,7 +222,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, num_pages_from_user = get_user_pages(current,
> current->mm,
>  					     vaddr & PAGE_MASK,
>  					     buf->num_pages,
> -					     write,
> +					     buf->dma_dir == DMA_FROM_DEVICE,
>  					     1, /* force */
>  					     buf->pages,
>  					     NULL);
> @@ -265,7 +266,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>  		vm_unmap_ram(buf->vaddr, buf->num_pages);
>  	sg_free_table(&buf->sg_table);
>  	while (--i >= 0) {
> -		if (buf->write)
> +		if (buf->dma_dir == DMA_FROM_DEVICE)
>  			set_page_dirty_lock(buf->pages[i]);
>  		if (!vma_is_io(buf->vma))
>  			put_page(buf->pages[i]);
> diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> b/drivers/media/v4l2-core/videobuf2-vmalloc.c index 313d977..fc1eb45 100644
> --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
> @@ -25,7 +25,7 @@ struct vb2_vmalloc_buf {
>  	void				*vaddr;
>  	struct page			**pages;
>  	struct vm_area_struct		*vma;
> -	int				write;
> +	enum dma_data_direction		dma_dir;
>  	unsigned long			size;
>  	unsigned int			n_pages;
>  	atomic_t			refcount;
> @@ -70,7 +70,8 @@ static void vb2_vmalloc_put(void *buf_priv)
>  }
> 
>  static void *vb2_vmalloc_get_userptr(void *alloc_ctx, unsigned long vaddr,
> -				     unsigned long size, int write)
> +				     unsigned long size,
> +				     enum dma_data_direction dma_dir)
>  {
>  	struct vb2_vmalloc_buf *buf;
>  	unsigned long first, last;
> @@ -82,7 +83,7 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx,
> unsigned long vaddr, if (!buf)
>  		return NULL;
> 
> -	buf->write = write;
> +	buf->dma_dir = dma_dir;
>  	offset = vaddr & ~PAGE_MASK;
>  	buf->size = size;
> 
> @@ -107,7 +108,8 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx,
> unsigned long vaddr, /* current->mm->mmap_sem is taken by videobuf2 core */
>  		n_pages = get_user_pages(current, current->mm,
>  					 vaddr & PAGE_MASK, buf->n_pages,
> -					 write, 1, /* force */
> +					 dma_dir == DMA_FROM_DEVICE,
> +					 1, /* force */
>  					 buf->pages, NULL);
>  		if (n_pages != buf->n_pages)
>  			goto fail_get_user_pages;
> @@ -144,7 +146,7 @@ static void vb2_vmalloc_put_userptr(void *buf_priv)
>  		if (vaddr)
>  			vm_unmap_ram((void *)vaddr, buf->n_pages);
>  		for (i = 0; i < buf->n_pages; ++i) {
> -			if (buf->write)
> +			if (buf->dma_dir == DMA_FROM_DEVICE)
>  				set_page_dirty_lock(buf->pages[i]);
>  			put_page(buf->pages[i]);
>  		}
> @@ -240,7 +242,7 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv)
>  }
> 
>  static void *vb2_vmalloc_attach_dmabuf(void *alloc_ctx, struct dma_buf
> *dbuf, -	unsigned long size, int write)
> +	unsigned long size, enum dma_data_direction dma_dir)
>  {
>  	struct vb2_vmalloc_buf *buf;
> 
> @@ -252,7 +254,7 @@ static void *vb2_vmalloc_attach_dmabuf(void *alloc_ctx,
> struct dma_buf *dbuf, return ERR_PTR(-ENOMEM);
> 
>  	buf->dbuf = dbuf;
> -	buf->write = write;
> +	buf->dma_dir = dma_dir;
>  	buf->size = size;
> 
>  	return buf;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 70ace7c..d607871 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -87,14 +87,16 @@ struct vb2_mem_ops {
>  	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);
> 
>  	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
> -					unsigned long size, int write);
> +					unsigned long size,
> +					enum dma_data_direction dma_dir);
>  	void		(*put_userptr)(void *buf_priv);
> 
>  	void		(*prepare)(void *buf_priv);
>  	void		(*finish)(void *buf_priv);
> 
>  	void		*(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf,
> -				unsigned long size, int write);
> +					  unsigned long size,
> +					  enum dma_data_direction dma_dir);
>  	void		(*detach_dmabuf)(void *buf_priv);
>  	int		(*map_dmabuf)(void *buf_priv);
>  	void		(*unmap_dmabuf)(void *buf_priv);

-- 
Regards,

Laurent Pinchart

[REVIEWv7 PATCH 03/12] vb2: add dma_dir to the alloc memop.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This is needed for the next patch where the dma-sg alloc memop needs
to know the dma_dir.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/v4l2-core/videobuf2-core.c       | 4 +++-
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 +++-
 drivers/media/v4l2-core/videobuf2-dma-sg.c     | 5 +++--
 drivers/media/v4l2-core/videobuf2-vmalloc.c    | 4 +++-
 include/media/videobuf2-core.h                 | 4 +++-
 5 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 573f6fb..7aed8f2 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -189,6 +189,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q);
 static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
+	enum dma_data_direction dma_dir =
+		V4L2_TYPE_IS_OUTPUT(q->type) ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	void *mem_priv;
 	int plane;
 
@@ -200,7 +202,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 		unsigned long size = PAGE_ALIGN(q->plane_sizes[plane]);
 
 		mem_priv = call_ptr_memop(vb, alloc, q->alloc_ctx[plane],
-				      size, q->gfp_flags);
+				      size, dma_dir, q->gfp_flags);
 		if (IS_ERR_OR_NULL(mem_priv))
 			goto free;
 
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 2bdffd3..c4305bf 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -155,7 +155,8 @@ static void vb2_dc_put(void *buf_priv)
 	kfree(buf);
 }
 
-static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags)
+static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size,
+			  enum dma_data_direction dma_dir, gfp_t gfp_flags)
 {
 	struct vb2_dc_conf *conf = alloc_ctx;
 	struct device *dev = conf->dev;
@@ -176,6 +177,7 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags)
 	/* Prevent the device from being released while the buffer is used */
 	buf->dev = get_device(dev);
 	buf->size = size;
+	buf->dma_dir = dma_dir;
 
 	buf->handler.refcount = &buf->refcount;
 	buf->handler.put = vb2_dc_put;
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 6b54a14..2529b83 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -86,7 +86,8 @@ static int vb2_dma_sg_alloc_compacted(struct vb2_dma_sg_buf *buf,
 	return 0;
 }
 
-static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags)
+static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
+			      enum dma_data_direction dma_dir, gfp_t gfp_flags)
 {
 	struct vb2_dma_sg_buf *buf;
 	int ret;
@@ -97,7 +98,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_fla
 		return NULL;
 
 	buf->vaddr = NULL;
-	buf->dma_dir = DMA_NONE;
+	buf->dma_dir = dma_dir;
 	buf->offset = 0;
 	buf->size = size;
 	/* size is already page aligned */
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index fc1eb45..bba2460 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -35,7 +35,8 @@ struct vb2_vmalloc_buf {
 
 static void vb2_vmalloc_put(void *buf_priv);
 
-static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags)
+static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size,
+			       enum dma_data_direction dma_dir, gfp_t gfp_flags)
 {
 	struct vb2_vmalloc_buf *buf;
 
@@ -45,6 +46,7 @@ static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_fl
 
 	buf->size = size;
 	buf->vaddr = vmalloc_user(buf->size);
+	buf->dma_dir = dma_dir;
 	buf->handler.refcount = &buf->refcount;
 	buf->handler.put = vb2_vmalloc_put;
 	buf->handler.arg = buf;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index d607871..bd2cec2 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -82,7 +82,9 @@ struct vb2_threadio_data;
  *				  unmap_dmabuf.
  */
 struct vb2_mem_ops {
-	void		*(*alloc)(void *alloc_ctx, unsigned long size, gfp_t gfp_flags);
+	void		*(*alloc)(void *alloc_ctx, unsigned long size,
+				  enum dma_data_direction dma_dir,
+				  gfp_t gfp_flags);
 	void		(*put)(void *buf_priv);
 	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);
 
-- 
2.1.1

Re: [REVIEWv7 PATCH 03/12] vb2: add dma_dir to the alloc memop.

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Tuesday 18 November 2014 13:50:59 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This is needed for the next patch where the dma-sg alloc memop needs
> to know the dma_dir.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Pawel Osciak <pawel@osciak.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c       | 4 +++-
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 +++-
>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 5 +++--
>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 4 +++-
>  include/media/videobuf2-core.h                 | 4 +++-
>  5 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index 573f6fb..7aed8f2 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -189,6 +189,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q);
>  static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>  {
>  	struct vb2_queue *q = vb->vb2_queue;
> +	enum dma_data_direction dma_dir =
> +		V4L2_TYPE_IS_OUTPUT(q->type) ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>  	void *mem_priv;
>  	int plane;
> 
> @@ -200,7 +202,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>  		unsigned long size = PAGE_ALIGN(q->plane_sizes[plane]);
> 
>  		mem_priv = call_ptr_memop(vb, alloc, q->alloc_ctx[plane],
> -				      size, q->gfp_flags);
> +				      size, dma_dir, q->gfp_flags);
>  		if (IS_ERR_OR_NULL(mem_priv))
>  			goto free;
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 2bdffd3..c4305bf
> 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -155,7 +155,8 @@ static void vb2_dc_put(void *buf_priv)
>  	kfree(buf);
>  }
> 
> -static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, gfp_t
> gfp_flags) +static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size,
> +			  enum dma_data_direction dma_dir, gfp_t gfp_flags)
>  {
>  	struct vb2_dc_conf *conf = alloc_ctx;
>  	struct device *dev = conf->dev;
> @@ -176,6 +177,7 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long
> size, gfp_t gfp_flags) /* Prevent the device from being released while the
> buffer is used */ buf->dev = get_device(dev);
>  	buf->size = size;
> +	buf->dma_dir = dma_dir;
> 
>  	buf->handler.refcount = &buf->refcount;
>  	buf->handler.put = vb2_dc_put;
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 6b54a14..2529b83 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -86,7 +86,8 @@ static int vb2_dma_sg_alloc_compacted(struct
> vb2_dma_sg_buf *buf, return 0;
>  }
> 
> -static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, gfp_t
> gfp_flags) +static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long
> size, +			      enum dma_data_direction dma_dir, gfp_t gfp_flags)
>  {
>  	struct vb2_dma_sg_buf *buf;
>  	int ret;
> @@ -97,7 +98,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, gfp_t gfp_fla return NULL;
> 
>  	buf->vaddr = NULL;
> -	buf->dma_dir = DMA_NONE;
> +	buf->dma_dir = dma_dir;
>  	buf->offset = 0;
>  	buf->size = size;
>  	/* size is already page aligned */
> diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> b/drivers/media/v4l2-core/videobuf2-vmalloc.c index fc1eb45..bba2460 100644
> --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
> @@ -35,7 +35,8 @@ struct vb2_vmalloc_buf {
> 
>  static void vb2_vmalloc_put(void *buf_priv);
> 
> -static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, gfp_t
> gfp_flags) +static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long
> size, +			       enum dma_data_direction dma_dir, gfp_t gfp_flags)
>  {
>  	struct vb2_vmalloc_buf *buf;
> 
> @@ -45,6 +46,7 @@ static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned
> long size, gfp_t gfp_fl
> 
>  	buf->size = size;
>  	buf->vaddr = vmalloc_user(buf->size);
> +	buf->dma_dir = dma_dir;
>  	buf->handler.refcount = &buf->refcount;
>  	buf->handler.put = vb2_vmalloc_put;
>  	buf->handler.arg = buf;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index d607871..bd2cec2 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -82,7 +82,9 @@ struct vb2_threadio_data;
>   *				  unmap_dmabuf.
>   */
>  struct vb2_mem_ops {
> -	void		*(*alloc)(void *alloc_ctx, unsigned long size, gfp_t gfp_flags);
> +	void		*(*alloc)(void *alloc_ctx, unsigned long size,
> +				  enum dma_data_direction dma_dir,
> +				  gfp_t gfp_flags);
>  	void		(*put)(void *buf_priv);
>  	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);

-- 
Regards,

Laurent Pinchart

[REVIEWv7 PATCH 04/12] vb2: don't free alloc context if it is ERR_PTR

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Don't try to free a pointer containing an ERR_PTR().

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index c4305bf..0bfc488 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -854,7 +854,8 @@ EXPORT_SYMBOL_GPL(vb2_dma_contig_init_ctx);
 
 void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
 {
-	kfree(alloc_ctx);
+	if (!IS_ERR_OR_NULL(alloc_ctx))
+		kfree(alloc_ctx);
 }
 EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
 
-- 
2.1.1

Re: [REVIEWv7 PATCH 04/12] vb2: don't free alloc context if it is ERR_PTR

[Pawel Osciak]

On Tue, Nov 18, 2014 at 9:51 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Don't try to free a pointer containing an ERR_PTR().
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

-- 
Best regards,
Pawel Osciak

Re: [REVIEWv7 PATCH 04/12] vb2: don't free alloc context if it is ERR_PTR

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Tuesday 18 November 2014 13:51:00 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Don't try to free a pointer containing an ERR_PTR().

Wouldn't it be easier to return NULL from vb2_dma_contig_alloc_ctx() instead 
of an ERR_PTR ?

> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index c4305bf..0bfc488
> 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -854,7 +854,8 @@ EXPORT_SYMBOL_GPL(vb2_dma_contig_init_ctx);
> 
>  void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
>  {
> -	kfree(alloc_ctx);
> +	if (!IS_ERR_OR_NULL(alloc_ctx))
> +		kfree(alloc_ctx);
>  }
>  EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);

-- 
Regards,

Laurent Pinchart

[REVIEWv7 PATCH 05/12] vb2-dma-sg: add allocation context to dma-sg

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Require that dma-sg also uses an allocation context. This is in preparation
for adding prepare/finish memops to sync the memory between DMA and CPU.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/cx23885/cx23885-417.c         |  1 +
 drivers/media/pci/cx23885/cx23885-core.c        | 10 +++++++-
 drivers/media/pci/cx23885/cx23885-dvb.c         |  1 +
 drivers/media/pci/cx23885/cx23885-vbi.c         |  1 +
 drivers/media/pci/cx23885/cx23885-video.c       |  1 +
 drivers/media/pci/cx23885/cx23885.h             |  1 +
 drivers/media/pci/saa7134/saa7134-core.c        | 18 ++++++++++----
 drivers/media/pci/saa7134/saa7134-ts.c          |  1 +
 drivers/media/pci/saa7134/saa7134-vbi.c         |  1 +
 drivers/media/pci/saa7134/saa7134-video.c       |  1 +
 drivers/media/pci/saa7134/saa7134.h             |  1 +
 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 10 ++++++++
 drivers/media/pci/solo6x10/solo6x10.h           |  1 +
 drivers/media/pci/tw68/tw68-core.c              | 15 +++++++++---
 drivers/media/pci/tw68/tw68-video.c             |  1 +
 drivers/media/pci/tw68/tw68.h                   |  1 +
 drivers/media/platform/marvell-ccic/mcam-core.c | 13 +++++++++-
 drivers/media/platform/marvell-ccic/mcam-core.h |  1 +
 drivers/media/v4l2-core/videobuf2-dma-sg.c      | 32 +++++++++++++++++++++++++
 include/media/videobuf2-dma-sg.h                |  3 +++
 20 files changed, 104 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c
index 3948db3..d72a3ec 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1148,6 +1148,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	dev->ts1.ts_packet_count = mpeglines;
 	*num_planes = 1;
 	sizes[0] = mpeglinesize * mpeglines;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	*num_buffers = mpegbufs;
 	return 0;
 }
diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
index 331edda..d452b5c 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -1997,9 +1997,14 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
 	if (!pci_dma_supported(pci_dev, 0xffffffff)) {
 		printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
 		err = -EIO;
-		goto fail_irq;
+		goto fail_context;
 	}
 
+	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
+	if (IS_ERR(dev->alloc_ctx)) {
+		err = PTR_ERR(dev->alloc_ctx);
+		goto fail_context;
+	}
 	err = request_irq(pci_dev->irq, cx23885_irq,
 			  IRQF_SHARED, dev->name, dev);
 	if (err < 0) {
@@ -2028,6 +2033,8 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
 	return 0;
 
 fail_irq:
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
+fail_context:
 	cx23885_dev_unregister(dev);
 fail_ctrl:
 	v4l2_ctrl_handler_free(hdl);
@@ -2053,6 +2060,7 @@ static void cx23885_finidev(struct pci_dev *pci_dev)
 	free_irq(pci_dev->irq, dev);
 
 	cx23885_dev_unregister(dev);
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
 	v4l2_ctrl_handler_free(&dev->ctrl_handler);
 	v4l2_device_unregister(v4l2_dev);
 	kfree(dev);
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 1ed92ee..44fafba 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -102,6 +102,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	port->ts_packet_count = 32;
 	*num_planes = 1;
 	sizes[0] = port->ts_packet_size * port->ts_packet_count;
+	alloc_ctxs[0] = port->dev->alloc_ctx;
 	*num_buffers = 32;
 	return 0;
 }
diff --git a/drivers/media/pci/cx23885/cx23885-vbi.c b/drivers/media/pci/cx23885/cx23885-vbi.c
index a7c6ef8..1d339a6 100644
--- a/drivers/media/pci/cx23885/cx23885-vbi.c
+++ b/drivers/media/pci/cx23885/cx23885-vbi.c
@@ -132,6 +132,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 		lines = VBI_NTSC_LINE_COUNT;
 	*num_planes = 1;
 	sizes[0] = lines * VBI_LINE_LENGTH * 2;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index 091f5db..371eecf 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -323,6 +323,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 
 	*num_planes = 1;
 	sizes[0] = (dev->fmt->depth * dev->width * dev->height) >> 3;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 
diff --git a/drivers/media/pci/cx23885/cx23885.h b/drivers/media/pci/cx23885/cx23885.h
index cf4efa4..f55cd12 100644
--- a/drivers/media/pci/cx23885/cx23885.h
+++ b/drivers/media/pci/cx23885/cx23885.h
@@ -425,6 +425,7 @@ struct cx23885_dev {
 	struct vb2_queue           vb2_vidq;
 	struct cx23885_dmaqueue    vbiq;
 	struct vb2_queue           vb2_vbiq;
+	void			   *alloc_ctx;
 
 	spinlock_t                 slock;
 
diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c
index 236ed72..a349e96 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -1001,13 +1001,18 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	saa7134_board_init1(dev);
 	saa7134_hwinit1(dev);
 
+	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
+	if (IS_ERR(dev->alloc_ctx)) {
+		err = PTR_ERR(dev->alloc_ctx);
+		goto fail3;
+	}
 	/* get irq */
 	err = request_irq(pci_dev->irq, saa7134_irq,
 			  IRQF_SHARED, dev->name, dev);
 	if (err < 0) {
 		printk(KERN_ERR "%s: can't get IRQ %d\n",
 		       dev->name,pci_dev->irq);
-		goto fail3;
+		goto fail4;
 	}
 
 	/* wait a bit, register i2c bus */
@@ -1065,7 +1070,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	if (err < 0) {
 		printk(KERN_INFO "%s: can't register video device\n",
 		       dev->name);
-		goto fail4;
+		goto fail5;
 	}
 	printk(KERN_INFO "%s: registered device %s [v4l2]\n",
 	       dev->name, video_device_node_name(dev->video_dev));
@@ -1078,7 +1083,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	err = video_register_device(dev->vbi_dev,VFL_TYPE_VBI,
 				    vbi_nr[dev->nr]);
 	if (err < 0)
-		goto fail4;
+		goto fail5;
 	printk(KERN_INFO "%s: registered device %s\n",
 	       dev->name, video_device_node_name(dev->vbi_dev));
 
@@ -1089,7 +1094,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 		err = video_register_device(dev->radio_dev,VFL_TYPE_RADIO,
 					    radio_nr[dev->nr]);
 		if (err < 0)
-			goto fail4;
+			goto fail5;
 		printk(KERN_INFO "%s: registered device %s\n",
 		       dev->name, video_device_node_name(dev->radio_dev));
 	}
@@ -1103,10 +1108,12 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	request_submodules(dev);
 	return 0;
 
- fail4:
+ fail5:
 	saa7134_unregister_video(dev);
 	saa7134_i2c_unregister(dev);
 	free_irq(pci_dev->irq, dev);
+ fail4:
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
  fail3:
 	saa7134_hwfini(dev);
 	iounmap(dev->lmmio);
@@ -1173,6 +1180,7 @@ static void saa7134_finidev(struct pci_dev *pci_dev)
 
 	/* release resources */
 	free_irq(pci_dev->irq, dev);
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
 	iounmap(dev->lmmio);
 	release_mem_region(pci_resource_start(pci_dev,0),
 			   pci_resource_len(pci_dev,0));
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
index bd25323..8eff4a7 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -142,6 +142,7 @@ int saa7134_ts_queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 		*nbuffers = 3;
 	*nplanes = 1;
 	sizes[0] = size;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(saa7134_ts_queue_setup);
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
index 4f0b101..e2cc684 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -156,6 +156,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	*nbuffers = saa7134_buffer_count(size, *nbuffers);
 	*nplanes = 1;
 	sizes[0] = size;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index fc4a427..ba02995 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -932,6 +932,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	*nbuffers = saa7134_buffer_count(size, *nbuffers);
 	*nplanes = 1;
 	sizes[0] = size;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 
diff --git a/drivers/media/pci/saa7134/saa7134.h b/drivers/media/pci/saa7134/saa7134.h
index 1a82dd0..c644c7d 100644
--- a/drivers/media/pci/saa7134/saa7134.h
+++ b/drivers/media/pci/saa7134/saa7134.h
@@ -588,6 +588,7 @@ struct saa7134_dev {
 
 
 	/* video+ts+vbi capture */
+	void			   *alloc_ctx;
 	struct saa7134_dmaqueue    video_q;
 	struct vb2_queue           video_vbq;
 	struct saa7134_dmaqueue    vbi_q;
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
index 30e09d9..4f6bfba 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
@@ -718,7 +718,10 @@ static int solo_enc_queue_setup(struct vb2_queue *q,
 				unsigned int *num_planes, unsigned int sizes[],
 				void *alloc_ctxs[])
 {
+	struct solo_enc_dev *solo_enc = vb2_get_drv_priv(q);
+
 	sizes[0] = FRAME_BUF_SIZE;
+	alloc_ctxs[0] = solo_enc->alloc_ctx;
 	*num_planes = 1;
 
 	if (*num_buffers < MIN_VID_BUFFERS)
@@ -1266,6 +1269,11 @@ static struct solo_enc_dev *solo_enc_alloc(struct solo_dev *solo_dev,
 		return ERR_PTR(-ENOMEM);
 
 	hdl = &solo_enc->hdl;
+	solo_enc->alloc_ctx = vb2_dma_sg_init_ctx(&solo_dev->pdev->dev);
+	if (IS_ERR(solo_enc->alloc_ctx)) {
+		ret = PTR_ERR(solo_enc->alloc_ctx);
+		goto hdl_free;
+	}
 	v4l2_ctrl_handler_init(hdl, 10);
 	v4l2_ctrl_new_std(hdl, &solo_ctrl_ops,
 			V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
@@ -1369,6 +1377,7 @@ pci_free:
 			solo_enc->desc_items, solo_enc->desc_dma);
 hdl_free:
 	v4l2_ctrl_handler_free(hdl);
+	vb2_dma_sg_cleanup_ctx(solo_enc->alloc_ctx);
 	kfree(solo_enc);
 	return ERR_PTR(ret);
 }
@@ -1383,6 +1392,7 @@ static void solo_enc_free(struct solo_enc_dev *solo_enc)
 			solo_enc->desc_items, solo_enc->desc_dma);
 	video_unregister_device(solo_enc->vfd);
 	v4l2_ctrl_handler_free(&solo_enc->hdl);
+	vb2_dma_sg_cleanup_ctx(solo_enc->alloc_ctx);
 	kfree(solo_enc);
 }
 
diff --git a/drivers/media/pci/solo6x10/solo6x10.h b/drivers/media/pci/solo6x10/solo6x10.h
index 72017b7..bd8edfa 100644
--- a/drivers/media/pci/solo6x10/solo6x10.h
+++ b/drivers/media/pci/solo6x10/solo6x10.h
@@ -180,6 +180,7 @@ struct solo_enc_dev {
 	u32			sequence;
 	struct vb2_queue	vidq;
 	struct list_head	vidq_active;
+	void			*alloc_ctx;
 	int			desc_count;
 	int			desc_nelts;
 	struct solo_p2m_desc	*desc_items;
diff --git a/drivers/media/pci/tw68/tw68-core.c b/drivers/media/pci/tw68/tw68-core.c
index 63f0b64..c135165 100644
--- a/drivers/media/pci/tw68/tw68-core.c
+++ b/drivers/media/pci/tw68/tw68-core.c
@@ -304,13 +304,19 @@ static int tw68_initdev(struct pci_dev *pci_dev,
 	/* Then do any initialisation wanted before interrupts are on */
 	tw68_hw_init1(dev);
 
+	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
+	if (IS_ERR(dev->alloc_ctx)) {
+		err = PTR_ERR(dev->alloc_ctx);
+		goto fail3;
+	}
+
 	/* get irq */
 	err = devm_request_irq(&pci_dev->dev, pci_dev->irq, tw68_irq,
 			  IRQF_SHARED, dev->name, dev);
 	if (err < 0) {
 		pr_err("%s: can't get IRQ %d\n",
 		       dev->name, pci_dev->irq);
-		goto fail3;
+		goto fail4;
 	}
 
 	/*
@@ -324,7 +330,7 @@ static int tw68_initdev(struct pci_dev *pci_dev,
 	if (err < 0) {
 		pr_err("%s: can't register video device\n",
 		       dev->name);
-		goto fail4;
+		goto fail5;
 	}
 	tw_setl(TW68_INTMASK, dev->pci_irqmask);
 
@@ -333,8 +339,10 @@ static int tw68_initdev(struct pci_dev *pci_dev,
 
 	return 0;
 
-fail4:
+fail5:
 	video_unregister_device(&dev->vdev);
+fail4:
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
 fail3:
 	iounmap(dev->lmmio);
 fail2:
@@ -358,6 +366,7 @@ static void tw68_finidev(struct pci_dev *pci_dev)
 	/* unregister */
 	video_unregister_device(&dev->vdev);
 	v4l2_ctrl_handler_free(&dev->hdl);
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
 
 	/* release resources */
 	iounmap(dev->lmmio);
diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c
index 5c94ac7..50dcce6 100644
--- a/drivers/media/pci/tw68/tw68-video.c
+++ b/drivers/media/pci/tw68/tw68-video.c
@@ -384,6 +384,7 @@ static int tw68_queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	unsigned tot_bufs = q->num_buffers + *num_buffers;
 
 	sizes[0] = (dev->fmt->depth * dev->width * dev->height) >> 3;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	/*
 	 * We allow create_bufs, but only if the sizeimage is the same as the
 	 * current sizeimage. The tw68_buffer_count calculation becomes quite
diff --git a/drivers/media/pci/tw68/tw68.h b/drivers/media/pci/tw68/tw68.h
index 2c8abe2..7a7501b 100644
--- a/drivers/media/pci/tw68/tw68.h
+++ b/drivers/media/pci/tw68/tw68.h
@@ -181,6 +181,7 @@ struct tw68_dev {
 	unsigned		field;
 	struct vb2_queue	vidq;
 	struct list_head	active;
+	void			*alloc_ctx;
 
 	/* various v4l controls */
 	const struct tw68_tvnorm *tvnorm;	/* video */
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index f0eeb6c..c3ff538 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1079,6 +1079,8 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
 		*nbufs = minbufs;
 	if (cam->buffer_mode == B_DMA_contig)
 		alloc_ctxs[0] = cam->vb_alloc_ctx;
+	else if (cam->buffer_mode == B_DMA_sg)
+		alloc_ctxs[0] = cam->vb_alloc_ctx_sg;
 	return 0;
 }
 
@@ -1286,10 +1288,12 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
 		vq->ops = &mcam_vb2_ops;
 		vq->mem_ops = &vb2_dma_contig_memops;
 		vq->buf_struct_size = sizeof(struct mcam_vb_buffer);
-		cam->vb_alloc_ctx = vb2_dma_contig_init_ctx(cam->dev);
 		vq->io_modes = VB2_MMAP | VB2_USERPTR;
 		cam->dma_setup = mcam_ctlr_dma_contig;
 		cam->frame_complete = mcam_dma_contig_done;
+		cam->vb_alloc_ctx = vb2_dma_contig_init_ctx(cam->dev);
+		if (IS_ERR(cam->vb_alloc_ctx))
+			return PTR_ERR(cam->vb_alloc_ctx);
 #endif
 		break;
 	case B_DMA_sg:
@@ -1300,6 +1304,9 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
 		vq->io_modes = VB2_MMAP | VB2_USERPTR;
 		cam->dma_setup = mcam_ctlr_dma_sg;
 		cam->frame_complete = mcam_dma_sg_done;
+		cam->vb_alloc_ctx_sg = vb2_dma_sg_init_ctx(cam->dev);
+		if (IS_ERR(cam->vb_alloc_ctx_sg))
+			return PTR_ERR(cam->vb_alloc_ctx_sg);
 #endif
 		break;
 	case B_vmalloc:
@@ -1325,6 +1332,10 @@ static void mcam_cleanup_vb2(struct mcam_camera *cam)
 	if (cam->buffer_mode == B_DMA_contig)
 		vb2_dma_contig_cleanup_ctx(cam->vb_alloc_ctx);
 #endif
+#ifdef MCAM_MODE_DMA_SG
+	if (cam->buffer_mode == B_DMA_sg)
+		vb2_dma_sg_cleanup_ctx(cam->vb_alloc_ctx_sg);
+#endif
 }
 
 
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h
index 60a8e1c..aa0c6ea 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -176,6 +176,7 @@ struct mcam_camera {
 	/* DMA buffers - DMA modes */
 	struct mcam_vb_buffer *vb_bufs[MAX_DMA_BUFS];
 	struct vb2_alloc_ctx *vb_alloc_ctx;
+	struct vb2_alloc_ctx *vb_alloc_ctx_sg;
 
 	/* Mode-specific ops, set at open time */
 	void (*dma_setup)(struct mcam_camera *cam);
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 2529b83..2bf13dc 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -30,7 +30,12 @@ module_param(debug, int, 0644);
 			printk(KERN_DEBUG "vb2-dma-sg: " fmt, ## arg);	\
 	} while (0)
 
+struct vb2_dma_sg_conf {
+	struct device		*dev;
+};
+
 struct vb2_dma_sg_buf {
+	struct device			*dev;
 	void				*vaddr;
 	struct page			**pages;
 	int				offset;
@@ -89,10 +94,13 @@ static int vb2_dma_sg_alloc_compacted(struct vb2_dma_sg_buf *buf,
 static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 			      enum dma_data_direction dma_dir, gfp_t gfp_flags)
 {
+	struct vb2_dma_sg_conf *conf = alloc_ctx;
 	struct vb2_dma_sg_buf *buf;
 	int ret;
 	int num_pages;
 
+	if (WARN_ON(alloc_ctx == NULL))
+		return NULL;
 	buf = kzalloc(sizeof *buf, GFP_KERNEL);
 	if (!buf)
 		return NULL;
@@ -118,6 +126,8 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 	if (ret)
 		goto fail_table_alloc;
 
+	/* Prevent the device from being released while the buffer is used */
+	buf->dev = get_device(conf->dev);
 	buf->handler.refcount = &buf->refcount;
 	buf->handler.put = vb2_dma_sg_put;
 	buf->handler.arg = buf;
@@ -153,6 +163,7 @@ static void vb2_dma_sg_put(void *buf_priv)
 		while (--i >= 0)
 			__free_page(buf->pages[i]);
 		kfree(buf->pages);
+		put_device(buf->dev);
 		kfree(buf);
 	}
 }
@@ -356,6 +367,27 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
 };
 EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
 
+void *vb2_dma_sg_init_ctx(struct device *dev)
+{
+	struct vb2_dma_sg_conf *conf;
+
+	conf = kzalloc(sizeof(*conf), GFP_KERNEL);
+	if (!conf)
+		return ERR_PTR(-ENOMEM);
+
+	conf->dev = dev;
+
+	return conf;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_sg_init_ctx);
+
+void vb2_dma_sg_cleanup_ctx(void *alloc_ctx)
+{
+	if (!IS_ERR_OR_NULL(alloc_ctx))
+		kfree(alloc_ctx);
+}
+EXPORT_SYMBOL_GPL(vb2_dma_sg_cleanup_ctx);
+
 MODULE_DESCRIPTION("dma scatter/gather memory handling routines for videobuf2");
 MODULE_AUTHOR("Andrzej Pietrasiewicz");
 MODULE_LICENSE("GPL");
diff --git a/include/media/videobuf2-dma-sg.h b/include/media/videobuf2-dma-sg.h
index 7b89852..14ce306 100644
--- a/include/media/videobuf2-dma-sg.h
+++ b/include/media/videobuf2-dma-sg.h
@@ -21,6 +21,9 @@ static inline struct sg_table *vb2_dma_sg_plane_desc(
 	return (struct sg_table *)vb2_plane_cookie(vb, plane_no);
 }
 
+void *vb2_dma_sg_init_ctx(struct device *dev);
+void vb2_dma_sg_cleanup_ctx(void *alloc_ctx);
+
 extern const struct vb2_mem_ops vb2_dma_sg_memops;
 
 #endif
-- 
2.1.1

Re: [REVIEWv7 PATCH 05/12] vb2-dma-sg: add allocation context to dma-sg

[Pawel Osciak]

On Tue, Nov 18, 2014 at 9:51 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Require that dma-sg also uses an allocation context. This is in preparation
> for adding prepare/finish memops to sync the memory between DMA and CPU.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

-- 
Best regards,
Pawel Osciak

Re: [REVIEWv7 PATCH 05/12] vb2-dma-sg: add allocation context to dma-sg

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Tuesday 18 November 2014 13:51:01 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Require that dma-sg also uses an allocation context. This is in preparation
> for adding prepare/finish memops to sync the memory between DMA and CPU.

I think this has been raised before, but given that our allocation contexts 
just hold a struct device pointer, wouldn't it be simpler to just pass it 
explicitly when creating the queue ? Do we have use cases for using different 
struct device instances per plane ?

> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/pci/cx23885/cx23885-417.c         |  1 +
>  drivers/media/pci/cx23885/cx23885-core.c        | 10 +++++++-
>  drivers/media/pci/cx23885/cx23885-dvb.c         |  1 +
>  drivers/media/pci/cx23885/cx23885-vbi.c         |  1 +
>  drivers/media/pci/cx23885/cx23885-video.c       |  1 +
>  drivers/media/pci/cx23885/cx23885.h             |  1 +
>  drivers/media/pci/saa7134/saa7134-core.c        | 18 ++++++++++----
>  drivers/media/pci/saa7134/saa7134-ts.c          |  1 +
>  drivers/media/pci/saa7134/saa7134-vbi.c         |  1 +
>  drivers/media/pci/saa7134/saa7134-video.c       |  1 +
>  drivers/media/pci/saa7134/saa7134.h             |  1 +
>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 10 ++++++++
>  drivers/media/pci/solo6x10/solo6x10.h           |  1 +
>  drivers/media/pci/tw68/tw68-core.c              | 15 +++++++++---
>  drivers/media/pci/tw68/tw68-video.c             |  1 +
>  drivers/media/pci/tw68/tw68.h                   |  1 +
>  drivers/media/platform/marvell-ccic/mcam-core.c | 13 +++++++++-
>  drivers/media/platform/marvell-ccic/mcam-core.h |  1 +
>  drivers/media/v4l2-core/videobuf2-dma-sg.c      | 32 ++++++++++++++++++++++
>  include/media/videobuf2-dma-sg.h                |  3 +++
>  20 files changed, 104 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/pci/cx23885/cx23885-417.c
> b/drivers/media/pci/cx23885/cx23885-417.c index 3948db3..d72a3ec 100644
> --- a/drivers/media/pci/cx23885/cx23885-417.c
> +++ b/drivers/media/pci/cx23885/cx23885-417.c
> @@ -1148,6 +1148,7 @@ static int queue_setup(struct vb2_queue *q, const
> struct v4l2_format *fmt, dev->ts1.ts_packet_count = mpeglines;
>  	*num_planes = 1;
>  	sizes[0] = mpeglinesize * mpeglines;
> +	alloc_ctxs[0] = dev->alloc_ctx;
>  	*num_buffers = mpegbufs;
>  	return 0;
>  }
> diff --git a/drivers/media/pci/cx23885/cx23885-core.c
> b/drivers/media/pci/cx23885/cx23885-core.c index 331edda..d452b5c 100644
> --- a/drivers/media/pci/cx23885/cx23885-core.c
> +++ b/drivers/media/pci/cx23885/cx23885-core.c
> @@ -1997,9 +1997,14 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
>  	if (!pci_dma_supported(pci_dev, 0xffffffff)) {
>  		printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
>  		err = -EIO;
> -		goto fail_irq;
> +		goto fail_context;
>  	}
> 
> +	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
> +	if (IS_ERR(dev->alloc_ctx)) {
> +		err = PTR_ERR(dev->alloc_ctx);
> +		goto fail_context;
> +	}
>  	err = request_irq(pci_dev->irq, cx23885_irq,
>  			  IRQF_SHARED, dev->name, dev);
>  	if (err < 0) {
> @@ -2028,6 +2033,8 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
>  	return 0;
> 
>  fail_irq:
> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
> +fail_context:
>  	cx23885_dev_unregister(dev);
>  fail_ctrl:
>  	v4l2_ctrl_handler_free(hdl);
> @@ -2053,6 +2060,7 @@ static void cx23885_finidev(struct pci_dev *pci_dev)
>  	free_irq(pci_dev->irq, dev);
> 
>  	cx23885_dev_unregister(dev);
> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>  	v4l2_ctrl_handler_free(&dev->ctrl_handler);
>  	v4l2_device_unregister(v4l2_dev);
>  	kfree(dev);
> diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c
> b/drivers/media/pci/cx23885/cx23885-dvb.c index 1ed92ee..44fafba 100644
> --- a/drivers/media/pci/cx23885/cx23885-dvb.c
> +++ b/drivers/media/pci/cx23885/cx23885-dvb.c
> @@ -102,6 +102,7 @@ static int queue_setup(struct vb2_queue *q, const struct
> v4l2_format *fmt, port->ts_packet_count = 32;
>  	*num_planes = 1;
>  	sizes[0] = port->ts_packet_size * port->ts_packet_count;
> +	alloc_ctxs[0] = port->dev->alloc_ctx;
>  	*num_buffers = 32;
>  	return 0;
>  }
> diff --git a/drivers/media/pci/cx23885/cx23885-vbi.c
> b/drivers/media/pci/cx23885/cx23885-vbi.c index a7c6ef8..1d339a6 100644
> --- a/drivers/media/pci/cx23885/cx23885-vbi.c
> +++ b/drivers/media/pci/cx23885/cx23885-vbi.c
> @@ -132,6 +132,7 @@ static int queue_setup(struct vb2_queue *q, const struct
> v4l2_format *fmt, lines = VBI_NTSC_LINE_COUNT;
>  	*num_planes = 1;
>  	sizes[0] = lines * VBI_LINE_LENGTH * 2;
> +	alloc_ctxs[0] = dev->alloc_ctx;
>  	return 0;
>  }
> 
> diff --git a/drivers/media/pci/cx23885/cx23885-video.c
> b/drivers/media/pci/cx23885/cx23885-video.c index 091f5db..371eecf 100644
> --- a/drivers/media/pci/cx23885/cx23885-video.c
> +++ b/drivers/media/pci/cx23885/cx23885-video.c
> @@ -323,6 +323,7 @@ static int queue_setup(struct vb2_queue *q, const struct
> v4l2_format *fmt,
> 
>  	*num_planes = 1;
>  	sizes[0] = (dev->fmt->depth * dev->width * dev->height) >> 3;
> +	alloc_ctxs[0] = dev->alloc_ctx;
>  	return 0;
>  }
> 
> diff --git a/drivers/media/pci/cx23885/cx23885.h
> b/drivers/media/pci/cx23885/cx23885.h index cf4efa4..f55cd12 100644
> --- a/drivers/media/pci/cx23885/cx23885.h
> +++ b/drivers/media/pci/cx23885/cx23885.h
> @@ -425,6 +425,7 @@ struct cx23885_dev {
>  	struct vb2_queue           vb2_vidq;
>  	struct cx23885_dmaqueue    vbiq;
>  	struct vb2_queue           vb2_vbiq;
> +	void			   *alloc_ctx;
> 
>  	spinlock_t                 slock;
> 
> diff --git a/drivers/media/pci/saa7134/saa7134-core.c
> b/drivers/media/pci/saa7134/saa7134-core.c index 236ed72..a349e96 100644
> --- a/drivers/media/pci/saa7134/saa7134-core.c
> +++ b/drivers/media/pci/saa7134/saa7134-core.c
> @@ -1001,13 +1001,18 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>  	saa7134_board_init1(dev);
>  	saa7134_hwinit1(dev);
> 
> +	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
> +	if (IS_ERR(dev->alloc_ctx)) {
> +		err = PTR_ERR(dev->alloc_ctx);
> +		goto fail3;
> +	}
>  	/* get irq */
>  	err = request_irq(pci_dev->irq, saa7134_irq,
>  			  IRQF_SHARED, dev->name, dev);
>  	if (err < 0) {
>  		printk(KERN_ERR "%s: can't get IRQ %d\n",
>  		       dev->name,pci_dev->irq);
> -		goto fail3;
> +		goto fail4;
>  	}
> 
>  	/* wait a bit, register i2c bus */
> @@ -1065,7 +1070,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>  	if (err < 0) {
>  		printk(KERN_INFO "%s: can't register video device\n",
>  		       dev->name);
> -		goto fail4;
> +		goto fail5;
>  	}
>  	printk(KERN_INFO "%s: registered device %s [v4l2]\n",
>  	       dev->name, video_device_node_name(dev->video_dev));
> @@ -1078,7 +1083,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>  	err = video_register_device(dev->vbi_dev,VFL_TYPE_VBI,
>  				    vbi_nr[dev->nr]);
>  	if (err < 0)
> -		goto fail4;
> +		goto fail5;
>  	printk(KERN_INFO "%s: registered device %s\n",
>  	       dev->name, video_device_node_name(dev->vbi_dev));
> 
> @@ -1089,7 +1094,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>  		err = video_register_device(dev->radio_dev,VFL_TYPE_RADIO,
>  					    radio_nr[dev->nr]);
>  		if (err < 0)
> -			goto fail4;
> +			goto fail5;
>  		printk(KERN_INFO "%s: registered device %s\n",
>  		       dev->name, video_device_node_name(dev->radio_dev));
>  	}
> @@ -1103,10 +1108,12 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>  	request_submodules(dev);
>  	return 0;
> 
> - fail4:
> + fail5:
>  	saa7134_unregister_video(dev);
>  	saa7134_i2c_unregister(dev);
>  	free_irq(pci_dev->irq, dev);
> + fail4:
> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>   fail3:
>  	saa7134_hwfini(dev);
>  	iounmap(dev->lmmio);
> @@ -1173,6 +1180,7 @@ static void saa7134_finidev(struct pci_dev *pci_dev)
> 
>  	/* release resources */
>  	free_irq(pci_dev->irq, dev);
> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>  	iounmap(dev->lmmio);
>  	release_mem_region(pci_resource_start(pci_dev,0),
>  			   pci_resource_len(pci_dev,0));
> diff --git a/drivers/media/pci/saa7134/saa7134-ts.c
> b/drivers/media/pci/saa7134/saa7134-ts.c index bd25323..8eff4a7 100644
> --- a/drivers/media/pci/saa7134/saa7134-ts.c
> +++ b/drivers/media/pci/saa7134/saa7134-ts.c
> @@ -142,6 +142,7 @@ int saa7134_ts_queue_setup(struct vb2_queue *q, const
> struct v4l2_format *fmt, *nbuffers = 3;
>  	*nplanes = 1;
>  	sizes[0] = size;
> +	alloc_ctxs[0] = dev->alloc_ctx;
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(saa7134_ts_queue_setup);
> diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c
> b/drivers/media/pci/saa7134/saa7134-vbi.c index 4f0b101..e2cc684 100644
> --- a/drivers/media/pci/saa7134/saa7134-vbi.c
> +++ b/drivers/media/pci/saa7134/saa7134-vbi.c
> @@ -156,6 +156,7 @@ static int queue_setup(struct vb2_queue *q, const struct
> v4l2_format *fmt, *nbuffers = saa7134_buffer_count(size, *nbuffers);
>  	*nplanes = 1;
>  	sizes[0] = size;
> +	alloc_ctxs[0] = dev->alloc_ctx;
>  	return 0;
>  }
> 
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c
> b/drivers/media/pci/saa7134/saa7134-video.c index fc4a427..ba02995 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -932,6 +932,7 @@ static int queue_setup(struct vb2_queue *q, const struct
> v4l2_format *fmt, *nbuffers = saa7134_buffer_count(size, *nbuffers);
>  	*nplanes = 1;
>  	sizes[0] = size;
> +	alloc_ctxs[0] = dev->alloc_ctx;
>  	return 0;
>  }
> 
> diff --git a/drivers/media/pci/saa7134/saa7134.h
> b/drivers/media/pci/saa7134/saa7134.h index 1a82dd0..c644c7d 100644
> --- a/drivers/media/pci/saa7134/saa7134.h
> +++ b/drivers/media/pci/saa7134/saa7134.h
> @@ -588,6 +588,7 @@ struct saa7134_dev {
> 
> 
>  	/* video+ts+vbi capture */
> +	void			   *alloc_ctx;
>  	struct saa7134_dmaqueue    video_q;
>  	struct vb2_queue           video_vbq;
>  	struct saa7134_dmaqueue    vbi_q;
> diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c index 30e09d9..4f6bfba
> 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
> @@ -718,7 +718,10 @@ static int solo_enc_queue_setup(struct vb2_queue *q,
>  				unsigned int *num_planes, unsigned int sizes[],
>  				void *alloc_ctxs[])
>  {
> +	struct solo_enc_dev *solo_enc = vb2_get_drv_priv(q);
> +
>  	sizes[0] = FRAME_BUF_SIZE;
> +	alloc_ctxs[0] = solo_enc->alloc_ctx;
>  	*num_planes = 1;
> 
>  	if (*num_buffers < MIN_VID_BUFFERS)
> @@ -1266,6 +1269,11 @@ static struct solo_enc_dev *solo_enc_alloc(struct
> solo_dev *solo_dev, return ERR_PTR(-ENOMEM);
> 
>  	hdl = &solo_enc->hdl;
> +	solo_enc->alloc_ctx = vb2_dma_sg_init_ctx(&solo_dev->pdev->dev);
> +	if (IS_ERR(solo_enc->alloc_ctx)) {
> +		ret = PTR_ERR(solo_enc->alloc_ctx);
> +		goto hdl_free;
> +	}
>  	v4l2_ctrl_handler_init(hdl, 10);
>  	v4l2_ctrl_new_std(hdl, &solo_ctrl_ops,
>  			V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
> @@ -1369,6 +1377,7 @@ pci_free:
>  			solo_enc->desc_items, solo_enc->desc_dma);
>  hdl_free:
>  	v4l2_ctrl_handler_free(hdl);
> +	vb2_dma_sg_cleanup_ctx(solo_enc->alloc_ctx);
>  	kfree(solo_enc);
>  	return ERR_PTR(ret);
>  }
> @@ -1383,6 +1392,7 @@ static void solo_enc_free(struct solo_enc_dev
> *solo_enc) solo_enc->desc_items, solo_enc->desc_dma);
>  	video_unregister_device(solo_enc->vfd);
>  	v4l2_ctrl_handler_free(&solo_enc->hdl);
> +	vb2_dma_sg_cleanup_ctx(solo_enc->alloc_ctx);
>  	kfree(solo_enc);
>  }
> 
> diff --git a/drivers/media/pci/solo6x10/solo6x10.h
> b/drivers/media/pci/solo6x10/solo6x10.h index 72017b7..bd8edfa 100644
> --- a/drivers/media/pci/solo6x10/solo6x10.h
> +++ b/drivers/media/pci/solo6x10/solo6x10.h
> @@ -180,6 +180,7 @@ struct solo_enc_dev {
>  	u32			sequence;
>  	struct vb2_queue	vidq;
>  	struct list_head	vidq_active;
> +	void			*alloc_ctx;
>  	int			desc_count;
>  	int			desc_nelts;
>  	struct solo_p2m_desc	*desc_items;
> diff --git a/drivers/media/pci/tw68/tw68-core.c
> b/drivers/media/pci/tw68/tw68-core.c index 63f0b64..c135165 100644
> --- a/drivers/media/pci/tw68/tw68-core.c
> +++ b/drivers/media/pci/tw68/tw68-core.c
> @@ -304,13 +304,19 @@ static int tw68_initdev(struct pci_dev *pci_dev,
>  	/* Then do any initialisation wanted before interrupts are on */
>  	tw68_hw_init1(dev);
> 
> +	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
> +	if (IS_ERR(dev->alloc_ctx)) {
> +		err = PTR_ERR(dev->alloc_ctx);
> +		goto fail3;
> +	}
> +
>  	/* get irq */
>  	err = devm_request_irq(&pci_dev->dev, pci_dev->irq, tw68_irq,
>  			  IRQF_SHARED, dev->name, dev);
>  	if (err < 0) {
>  		pr_err("%s: can't get IRQ %d\n",
>  		       dev->name, pci_dev->irq);
> -		goto fail3;
> +		goto fail4;
>  	}
> 
>  	/*
> @@ -324,7 +330,7 @@ static int tw68_initdev(struct pci_dev *pci_dev,
>  	if (err < 0) {
>  		pr_err("%s: can't register video device\n",
>  		       dev->name);
> -		goto fail4;
> +		goto fail5;
>  	}
>  	tw_setl(TW68_INTMASK, dev->pci_irqmask);
> 
> @@ -333,8 +339,10 @@ static int tw68_initdev(struct pci_dev *pci_dev,
> 
>  	return 0;
> 
> -fail4:
> +fail5:
>  	video_unregister_device(&dev->vdev);
> +fail4:
> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>  fail3:
>  	iounmap(dev->lmmio);
>  fail2:
> @@ -358,6 +366,7 @@ static void tw68_finidev(struct pci_dev *pci_dev)
>  	/* unregister */
>  	video_unregister_device(&dev->vdev);
>  	v4l2_ctrl_handler_free(&dev->hdl);
> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
> 
>  	/* release resources */
>  	iounmap(dev->lmmio);
> diff --git a/drivers/media/pci/tw68/tw68-video.c
> b/drivers/media/pci/tw68/tw68-video.c index 5c94ac7..50dcce6 100644
> --- a/drivers/media/pci/tw68/tw68-video.c
> +++ b/drivers/media/pci/tw68/tw68-video.c
> @@ -384,6 +384,7 @@ static int tw68_queue_setup(struct vb2_queue *q, const
> struct v4l2_format *fmt, unsigned tot_bufs = q->num_buffers + *num_buffers;
> 
>  	sizes[0] = (dev->fmt->depth * dev->width * dev->height) >> 3;
> +	alloc_ctxs[0] = dev->alloc_ctx;
>  	/*
>  	 * We allow create_bufs, but only if the sizeimage is the same as the
>  	 * current sizeimage. The tw68_buffer_count calculation becomes quite
> diff --git a/drivers/media/pci/tw68/tw68.h b/drivers/media/pci/tw68/tw68.h
> index 2c8abe2..7a7501b 100644
> --- a/drivers/media/pci/tw68/tw68.h
> +++ b/drivers/media/pci/tw68/tw68.h
> @@ -181,6 +181,7 @@ struct tw68_dev {
>  	unsigned		field;
>  	struct vb2_queue	vidq;
>  	struct list_head	active;
> +	void			*alloc_ctx;
> 
>  	/* various v4l controls */
>  	const struct tw68_tvnorm *tvnorm;	/* video */
> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
> b/drivers/media/platform/marvell-ccic/mcam-core.c index f0eeb6c..c3ff538
> 100644
> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
> @@ -1079,6 +1079,8 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
>  		*nbufs = minbufs;
>  	if (cam->buffer_mode == B_DMA_contig)
>  		alloc_ctxs[0] = cam->vb_alloc_ctx;
> +	else if (cam->buffer_mode == B_DMA_sg)
> +		alloc_ctxs[0] = cam->vb_alloc_ctx_sg;
>  	return 0;
>  }
> 
> @@ -1286,10 +1288,12 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
>  		vq->ops = &mcam_vb2_ops;
>  		vq->mem_ops = &vb2_dma_contig_memops;
>  		vq->buf_struct_size = sizeof(struct mcam_vb_buffer);
> -		cam->vb_alloc_ctx = vb2_dma_contig_init_ctx(cam->dev);
>  		vq->io_modes = VB2_MMAP | VB2_USERPTR;
>  		cam->dma_setup = mcam_ctlr_dma_contig;
>  		cam->frame_complete = mcam_dma_contig_done;
> +		cam->vb_alloc_ctx = vb2_dma_contig_init_ctx(cam->dev);
> +		if (IS_ERR(cam->vb_alloc_ctx))
> +			return PTR_ERR(cam->vb_alloc_ctx);
>  #endif
>  		break;
>  	case B_DMA_sg:
> @@ -1300,6 +1304,9 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
>  		vq->io_modes = VB2_MMAP | VB2_USERPTR;
>  		cam->dma_setup = mcam_ctlr_dma_sg;
>  		cam->frame_complete = mcam_dma_sg_done;
> +		cam->vb_alloc_ctx_sg = vb2_dma_sg_init_ctx(cam->dev);
> +		if (IS_ERR(cam->vb_alloc_ctx_sg))
> +			return PTR_ERR(cam->vb_alloc_ctx_sg);
>  #endif
>  		break;
>  	case B_vmalloc:
> @@ -1325,6 +1332,10 @@ static void mcam_cleanup_vb2(struct mcam_camera *cam)
> if (cam->buffer_mode == B_DMA_contig)
>  		vb2_dma_contig_cleanup_ctx(cam->vb_alloc_ctx);
>  #endif
> +#ifdef MCAM_MODE_DMA_SG
> +	if (cam->buffer_mode == B_DMA_sg)
> +		vb2_dma_sg_cleanup_ctx(cam->vb_alloc_ctx_sg);
> +#endif
>  }
> 
> 
> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
> b/drivers/media/platform/marvell-ccic/mcam-core.h index 60a8e1c..aa0c6ea
> 100644
> --- a/drivers/media/platform/marvell-ccic/mcam-core.h
> +++ b/drivers/media/platform/marvell-ccic/mcam-core.h
> @@ -176,6 +176,7 @@ struct mcam_camera {
>  	/* DMA buffers - DMA modes */
>  	struct mcam_vb_buffer *vb_bufs[MAX_DMA_BUFS];
>  	struct vb2_alloc_ctx *vb_alloc_ctx;
> +	struct vb2_alloc_ctx *vb_alloc_ctx_sg;
> 
>  	/* Mode-specific ops, set at open time */
>  	void (*dma_setup)(struct mcam_camera *cam);
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 2529b83..2bf13dc 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -30,7 +30,12 @@ module_param(debug, int, 0644);
>  			printk(KERN_DEBUG "vb2-dma-sg: " fmt, ## arg);	\
>  	} while (0)
> 
> +struct vb2_dma_sg_conf {
> +	struct device		*dev;
> +};
> +
>  struct vb2_dma_sg_buf {
> +	struct device			*dev;
>  	void				*vaddr;
>  	struct page			**pages;
>  	int				offset;
> @@ -89,10 +94,13 @@ static int vb2_dma_sg_alloc_compacted(struct
> vb2_dma_sg_buf *buf, static void *vb2_dma_sg_alloc(void *alloc_ctx,
> unsigned long size, enum dma_data_direction dma_dir, gfp_t gfp_flags)
>  {
> +	struct vb2_dma_sg_conf *conf = alloc_ctx;
>  	struct vb2_dma_sg_buf *buf;
>  	int ret;
>  	int num_pages;
> 
> +	if (WARN_ON(alloc_ctx == NULL))
> +		return NULL;
>  	buf = kzalloc(sizeof *buf, GFP_KERNEL);
>  	if (!buf)
>  		return NULL;
> @@ -118,6 +126,8 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, if (ret)
>  		goto fail_table_alloc;
> 
> +	/* Prevent the device from being released while the buffer is used */
> +	buf->dev = get_device(conf->dev);
>  	buf->handler.refcount = &buf->refcount;
>  	buf->handler.put = vb2_dma_sg_put;
>  	buf->handler.arg = buf;
> @@ -153,6 +163,7 @@ static void vb2_dma_sg_put(void *buf_priv)
>  		while (--i >= 0)
>  			__free_page(buf->pages[i]);
>  		kfree(buf->pages);
> +		put_device(buf->dev);
>  		kfree(buf);
>  	}
>  }
> @@ -356,6 +367,27 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>  };
>  EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
> 
> +void *vb2_dma_sg_init_ctx(struct device *dev)
> +{
> +	struct vb2_dma_sg_conf *conf;
> +
> +	conf = kzalloc(sizeof(*conf), GFP_KERNEL);
> +	if (!conf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	conf->dev = dev;
> +
> +	return conf;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_sg_init_ctx);
> +
> +void vb2_dma_sg_cleanup_ctx(void *alloc_ctx)
> +{
> +	if (!IS_ERR_OR_NULL(alloc_ctx))
> +		kfree(alloc_ctx);
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_sg_cleanup_ctx);
> +
>  MODULE_DESCRIPTION("dma scatter/gather memory handling routines for
> videobuf2"); MODULE_AUTHOR("Andrzej Pietrasiewicz");
>  MODULE_LICENSE("GPL");
> diff --git a/include/media/videobuf2-dma-sg.h
> b/include/media/videobuf2-dma-sg.h index 7b89852..14ce306 100644
> --- a/include/media/videobuf2-dma-sg.h
> +++ b/include/media/videobuf2-dma-sg.h
> @@ -21,6 +21,9 @@ static inline struct sg_table *vb2_dma_sg_plane_desc(
>  	return (struct sg_table *)vb2_plane_cookie(vb, plane_no);
>  }
> 
> +void *vb2_dma_sg_init_ctx(struct device *dev);
> +void vb2_dma_sg_cleanup_ctx(void *alloc_ctx);
> +
>  extern const struct vb2_mem_ops vb2_dma_sg_memops;
> 
>  #endif

-- 
Regards,

Laurent Pinchart

Re: [REVIEWv7 PATCH 05/12] vb2-dma-sg: add allocation context to dma-sg

[Hans Verkuil]

Hi Laurent,

On 11/26/2014 09:01 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Tuesday 18 November 2014 13:51:01 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Require that dma-sg also uses an allocation context. This is in preparation
>> for adding prepare/finish memops to sync the memory between DMA and CPU.
> 
> I think this has been raised before, but given that our allocation contexts 
> just hold a struct device pointer, wouldn't it be simpler to just pass it 
> explicitly when creating the queue ? Do we have use cases for using different 
> struct device instances per plane ?

No, we don't, and yes, I plan on replacing the allocation context with a simple
device struct pointer. But I didn't want to mix that in with the rest of this
series. You can expect a patch changing this in the near future (and that will
also take care of your comment regarding the previous patch).

Regards,

	Hans

> 
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  drivers/media/pci/cx23885/cx23885-417.c         |  1 +
>>  drivers/media/pci/cx23885/cx23885-core.c        | 10 +++++++-
>>  drivers/media/pci/cx23885/cx23885-dvb.c         |  1 +
>>  drivers/media/pci/cx23885/cx23885-vbi.c         |  1 +
>>  drivers/media/pci/cx23885/cx23885-video.c       |  1 +
>>  drivers/media/pci/cx23885/cx23885.h             |  1 +
>>  drivers/media/pci/saa7134/saa7134-core.c        | 18 ++++++++++----
>>  drivers/media/pci/saa7134/saa7134-ts.c          |  1 +
>>  drivers/media/pci/saa7134/saa7134-vbi.c         |  1 +
>>  drivers/media/pci/saa7134/saa7134-video.c       |  1 +
>>  drivers/media/pci/saa7134/saa7134.h             |  1 +
>>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 10 ++++++++
>>  drivers/media/pci/solo6x10/solo6x10.h           |  1 +
>>  drivers/media/pci/tw68/tw68-core.c              | 15 +++++++++---
>>  drivers/media/pci/tw68/tw68-video.c             |  1 +
>>  drivers/media/pci/tw68/tw68.h                   |  1 +
>>  drivers/media/platform/marvell-ccic/mcam-core.c | 13 +++++++++-
>>  drivers/media/platform/marvell-ccic/mcam-core.h |  1 +
>>  drivers/media/v4l2-core/videobuf2-dma-sg.c      | 32 ++++++++++++++++++++++
>>  include/media/videobuf2-dma-sg.h                |  3 +++
>>  20 files changed, 104 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/pci/cx23885/cx23885-417.c
>> b/drivers/media/pci/cx23885/cx23885-417.c index 3948db3..d72a3ec 100644
>> --- a/drivers/media/pci/cx23885/cx23885-417.c
>> +++ b/drivers/media/pci/cx23885/cx23885-417.c
>> @@ -1148,6 +1148,7 @@ static int queue_setup(struct vb2_queue *q, const
>> struct v4l2_format *fmt, dev->ts1.ts_packet_count = mpeglines;
>>  	*num_planes = 1;
>>  	sizes[0] = mpeglinesize * mpeglines;
>> +	alloc_ctxs[0] = dev->alloc_ctx;
>>  	*num_buffers = mpegbufs;
>>  	return 0;
>>  }
>> diff --git a/drivers/media/pci/cx23885/cx23885-core.c
>> b/drivers/media/pci/cx23885/cx23885-core.c index 331edda..d452b5c 100644
>> --- a/drivers/media/pci/cx23885/cx23885-core.c
>> +++ b/drivers/media/pci/cx23885/cx23885-core.c
>> @@ -1997,9 +1997,14 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
>>  	if (!pci_dma_supported(pci_dev, 0xffffffff)) {
>>  		printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
>>  		err = -EIO;
>> -		goto fail_irq;
>> +		goto fail_context;
>>  	}
>>
>> +	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
>> +	if (IS_ERR(dev->alloc_ctx)) {
>> +		err = PTR_ERR(dev->alloc_ctx);
>> +		goto fail_context;
>> +	}
>>  	err = request_irq(pci_dev->irq, cx23885_irq,
>>  			  IRQF_SHARED, dev->name, dev);
>>  	if (err < 0) {
>> @@ -2028,6 +2033,8 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
>>  	return 0;
>>
>>  fail_irq:
>> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>> +fail_context:
>>  	cx23885_dev_unregister(dev);
>>  fail_ctrl:
>>  	v4l2_ctrl_handler_free(hdl);
>> @@ -2053,6 +2060,7 @@ static void cx23885_finidev(struct pci_dev *pci_dev)
>>  	free_irq(pci_dev->irq, dev);
>>
>>  	cx23885_dev_unregister(dev);
>> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>>  	v4l2_ctrl_handler_free(&dev->ctrl_handler);
>>  	v4l2_device_unregister(v4l2_dev);
>>  	kfree(dev);
>> diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c
>> b/drivers/media/pci/cx23885/cx23885-dvb.c index 1ed92ee..44fafba 100644
>> --- a/drivers/media/pci/cx23885/cx23885-dvb.c
>> +++ b/drivers/media/pci/cx23885/cx23885-dvb.c
>> @@ -102,6 +102,7 @@ static int queue_setup(struct vb2_queue *q, const struct
>> v4l2_format *fmt, port->ts_packet_count = 32;
>>  	*num_planes = 1;
>>  	sizes[0] = port->ts_packet_size * port->ts_packet_count;
>> +	alloc_ctxs[0] = port->dev->alloc_ctx;
>>  	*num_buffers = 32;
>>  	return 0;
>>  }
>> diff --git a/drivers/media/pci/cx23885/cx23885-vbi.c
>> b/drivers/media/pci/cx23885/cx23885-vbi.c index a7c6ef8..1d339a6 100644
>> --- a/drivers/media/pci/cx23885/cx23885-vbi.c
>> +++ b/drivers/media/pci/cx23885/cx23885-vbi.c
>> @@ -132,6 +132,7 @@ static int queue_setup(struct vb2_queue *q, const struct
>> v4l2_format *fmt, lines = VBI_NTSC_LINE_COUNT;
>>  	*num_planes = 1;
>>  	sizes[0] = lines * VBI_LINE_LENGTH * 2;
>> +	alloc_ctxs[0] = dev->alloc_ctx;
>>  	return 0;
>>  }
>>
>> diff --git a/drivers/media/pci/cx23885/cx23885-video.c
>> b/drivers/media/pci/cx23885/cx23885-video.c index 091f5db..371eecf 100644
>> --- a/drivers/media/pci/cx23885/cx23885-video.c
>> +++ b/drivers/media/pci/cx23885/cx23885-video.c
>> @@ -323,6 +323,7 @@ static int queue_setup(struct vb2_queue *q, const struct
>> v4l2_format *fmt,
>>
>>  	*num_planes = 1;
>>  	sizes[0] = (dev->fmt->depth * dev->width * dev->height) >> 3;
>> +	alloc_ctxs[0] = dev->alloc_ctx;
>>  	return 0;
>>  }
>>
>> diff --git a/drivers/media/pci/cx23885/cx23885.h
>> b/drivers/media/pci/cx23885/cx23885.h index cf4efa4..f55cd12 100644
>> --- a/drivers/media/pci/cx23885/cx23885.h
>> +++ b/drivers/media/pci/cx23885/cx23885.h
>> @@ -425,6 +425,7 @@ struct cx23885_dev {
>>  	struct vb2_queue           vb2_vidq;
>>  	struct cx23885_dmaqueue    vbiq;
>>  	struct vb2_queue           vb2_vbiq;
>> +	void			   *alloc_ctx;
>>
>>  	spinlock_t                 slock;
>>
>> diff --git a/drivers/media/pci/saa7134/saa7134-core.c
>> b/drivers/media/pci/saa7134/saa7134-core.c index 236ed72..a349e96 100644
>> --- a/drivers/media/pci/saa7134/saa7134-core.c
>> +++ b/drivers/media/pci/saa7134/saa7134-core.c
>> @@ -1001,13 +1001,18 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>>  	saa7134_board_init1(dev);
>>  	saa7134_hwinit1(dev);
>>
>> +	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
>> +	if (IS_ERR(dev->alloc_ctx)) {
>> +		err = PTR_ERR(dev->alloc_ctx);
>> +		goto fail3;
>> +	}
>>  	/* get irq */
>>  	err = request_irq(pci_dev->irq, saa7134_irq,
>>  			  IRQF_SHARED, dev->name, dev);
>>  	if (err < 0) {
>>  		printk(KERN_ERR "%s: can't get IRQ %d\n",
>>  		       dev->name,pci_dev->irq);
>> -		goto fail3;
>> +		goto fail4;
>>  	}
>>
>>  	/* wait a bit, register i2c bus */
>> @@ -1065,7 +1070,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>>  	if (err < 0) {
>>  		printk(KERN_INFO "%s: can't register video device\n",
>>  		       dev->name);
>> -		goto fail4;
>> +		goto fail5;
>>  	}
>>  	printk(KERN_INFO "%s: registered device %s [v4l2]\n",
>>  	       dev->name, video_device_node_name(dev->video_dev));
>> @@ -1078,7 +1083,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>>  	err = video_register_device(dev->vbi_dev,VFL_TYPE_VBI,
>>  				    vbi_nr[dev->nr]);
>>  	if (err < 0)
>> -		goto fail4;
>> +		goto fail5;
>>  	printk(KERN_INFO "%s: registered device %s\n",
>>  	       dev->name, video_device_node_name(dev->vbi_dev));
>>
>> @@ -1089,7 +1094,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>>  		err = video_register_device(dev->radio_dev,VFL_TYPE_RADIO,
>>  					    radio_nr[dev->nr]);
>>  		if (err < 0)
>> -			goto fail4;
>> +			goto fail5;
>>  		printk(KERN_INFO "%s: registered device %s\n",
>>  		       dev->name, video_device_node_name(dev->radio_dev));
>>  	}
>> @@ -1103,10 +1108,12 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
>>  	request_submodules(dev);
>>  	return 0;
>>
>> - fail4:
>> + fail5:
>>  	saa7134_unregister_video(dev);
>>  	saa7134_i2c_unregister(dev);
>>  	free_irq(pci_dev->irq, dev);
>> + fail4:
>> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>>   fail3:
>>  	saa7134_hwfini(dev);
>>  	iounmap(dev->lmmio);
>> @@ -1173,6 +1180,7 @@ static void saa7134_finidev(struct pci_dev *pci_dev)
>>
>>  	/* release resources */
>>  	free_irq(pci_dev->irq, dev);
>> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>>  	iounmap(dev->lmmio);
>>  	release_mem_region(pci_resource_start(pci_dev,0),
>>  			   pci_resource_len(pci_dev,0));
>> diff --git a/drivers/media/pci/saa7134/saa7134-ts.c
>> b/drivers/media/pci/saa7134/saa7134-ts.c index bd25323..8eff4a7 100644
>> --- a/drivers/media/pci/saa7134/saa7134-ts.c
>> +++ b/drivers/media/pci/saa7134/saa7134-ts.c
>> @@ -142,6 +142,7 @@ int saa7134_ts_queue_setup(struct vb2_queue *q, const
>> struct v4l2_format *fmt, *nbuffers = 3;
>>  	*nplanes = 1;
>>  	sizes[0] = size;
>> +	alloc_ctxs[0] = dev->alloc_ctx;
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL_GPL(saa7134_ts_queue_setup);
>> diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c
>> b/drivers/media/pci/saa7134/saa7134-vbi.c index 4f0b101..e2cc684 100644
>> --- a/drivers/media/pci/saa7134/saa7134-vbi.c
>> +++ b/drivers/media/pci/saa7134/saa7134-vbi.c
>> @@ -156,6 +156,7 @@ static int queue_setup(struct vb2_queue *q, const struct
>> v4l2_format *fmt, *nbuffers = saa7134_buffer_count(size, *nbuffers);
>>  	*nplanes = 1;
>>  	sizes[0] = size;
>> +	alloc_ctxs[0] = dev->alloc_ctx;
>>  	return 0;
>>  }
>>
>> diff --git a/drivers/media/pci/saa7134/saa7134-video.c
>> b/drivers/media/pci/saa7134/saa7134-video.c index fc4a427..ba02995 100644
>> --- a/drivers/media/pci/saa7134/saa7134-video.c
>> +++ b/drivers/media/pci/saa7134/saa7134-video.c
>> @@ -932,6 +932,7 @@ static int queue_setup(struct vb2_queue *q, const struct
>> v4l2_format *fmt, *nbuffers = saa7134_buffer_count(size, *nbuffers);
>>  	*nplanes = 1;
>>  	sizes[0] = size;
>> +	alloc_ctxs[0] = dev->alloc_ctx;
>>  	return 0;
>>  }
>>
>> diff --git a/drivers/media/pci/saa7134/saa7134.h
>> b/drivers/media/pci/saa7134/saa7134.h index 1a82dd0..c644c7d 100644
>> --- a/drivers/media/pci/saa7134/saa7134.h
>> +++ b/drivers/media/pci/saa7134/saa7134.h
>> @@ -588,6 +588,7 @@ struct saa7134_dev {
>>
>>
>>  	/* video+ts+vbi capture */
>> +	void			   *alloc_ctx;
>>  	struct saa7134_dmaqueue    video_q;
>>  	struct vb2_queue           video_vbq;
>>  	struct saa7134_dmaqueue    vbi_q;
>> diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
>> b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c index 30e09d9..4f6bfba
>> 100644
>> --- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
>> +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
>> @@ -718,7 +718,10 @@ static int solo_enc_queue_setup(struct vb2_queue *q,
>>  				unsigned int *num_planes, unsigned int sizes[],
>>  				void *alloc_ctxs[])
>>  {
>> +	struct solo_enc_dev *solo_enc = vb2_get_drv_priv(q);
>> +
>>  	sizes[0] = FRAME_BUF_SIZE;
>> +	alloc_ctxs[0] = solo_enc->alloc_ctx;
>>  	*num_planes = 1;
>>
>>  	if (*num_buffers < MIN_VID_BUFFERS)
>> @@ -1266,6 +1269,11 @@ static struct solo_enc_dev *solo_enc_alloc(struct
>> solo_dev *solo_dev, return ERR_PTR(-ENOMEM);
>>
>>  	hdl = &solo_enc->hdl;
>> +	solo_enc->alloc_ctx = vb2_dma_sg_init_ctx(&solo_dev->pdev->dev);
>> +	if (IS_ERR(solo_enc->alloc_ctx)) {
>> +		ret = PTR_ERR(solo_enc->alloc_ctx);
>> +		goto hdl_free;
>> +	}
>>  	v4l2_ctrl_handler_init(hdl, 10);
>>  	v4l2_ctrl_new_std(hdl, &solo_ctrl_ops,
>>  			V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
>> @@ -1369,6 +1377,7 @@ pci_free:
>>  			solo_enc->desc_items, solo_enc->desc_dma);
>>  hdl_free:
>>  	v4l2_ctrl_handler_free(hdl);
>> +	vb2_dma_sg_cleanup_ctx(solo_enc->alloc_ctx);
>>  	kfree(solo_enc);
>>  	return ERR_PTR(ret);
>>  }
>> @@ -1383,6 +1392,7 @@ static void solo_enc_free(struct solo_enc_dev
>> *solo_enc) solo_enc->desc_items, solo_enc->desc_dma);
>>  	video_unregister_device(solo_enc->vfd);
>>  	v4l2_ctrl_handler_free(&solo_enc->hdl);
>> +	vb2_dma_sg_cleanup_ctx(solo_enc->alloc_ctx);
>>  	kfree(solo_enc);
>>  }
>>
>> diff --git a/drivers/media/pci/solo6x10/solo6x10.h
>> b/drivers/media/pci/solo6x10/solo6x10.h index 72017b7..bd8edfa 100644
>> --- a/drivers/media/pci/solo6x10/solo6x10.h
>> +++ b/drivers/media/pci/solo6x10/solo6x10.h
>> @@ -180,6 +180,7 @@ struct solo_enc_dev {
>>  	u32			sequence;
>>  	struct vb2_queue	vidq;
>>  	struct list_head	vidq_active;
>> +	void			*alloc_ctx;
>>  	int			desc_count;
>>  	int			desc_nelts;
>>  	struct solo_p2m_desc	*desc_items;
>> diff --git a/drivers/media/pci/tw68/tw68-core.c
>> b/drivers/media/pci/tw68/tw68-core.c index 63f0b64..c135165 100644
>> --- a/drivers/media/pci/tw68/tw68-core.c
>> +++ b/drivers/media/pci/tw68/tw68-core.c
>> @@ -304,13 +304,19 @@ static int tw68_initdev(struct pci_dev *pci_dev,
>>  	/* Then do any initialisation wanted before interrupts are on */
>>  	tw68_hw_init1(dev);
>>
>> +	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
>> +	if (IS_ERR(dev->alloc_ctx)) {
>> +		err = PTR_ERR(dev->alloc_ctx);
>> +		goto fail3;
>> +	}
>> +
>>  	/* get irq */
>>  	err = devm_request_irq(&pci_dev->dev, pci_dev->irq, tw68_irq,
>>  			  IRQF_SHARED, dev->name, dev);
>>  	if (err < 0) {
>>  		pr_err("%s: can't get IRQ %d\n",
>>  		       dev->name, pci_dev->irq);
>> -		goto fail3;
>> +		goto fail4;
>>  	}
>>
>>  	/*
>> @@ -324,7 +330,7 @@ static int tw68_initdev(struct pci_dev *pci_dev,
>>  	if (err < 0) {
>>  		pr_err("%s: can't register video device\n",
>>  		       dev->name);
>> -		goto fail4;
>> +		goto fail5;
>>  	}
>>  	tw_setl(TW68_INTMASK, dev->pci_irqmask);
>>
>> @@ -333,8 +339,10 @@ static int tw68_initdev(struct pci_dev *pci_dev,
>>
>>  	return 0;
>>
>> -fail4:
>> +fail5:
>>  	video_unregister_device(&dev->vdev);
>> +fail4:
>> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>>  fail3:
>>  	iounmap(dev->lmmio);
>>  fail2:
>> @@ -358,6 +366,7 @@ static void tw68_finidev(struct pci_dev *pci_dev)
>>  	/* unregister */
>>  	video_unregister_device(&dev->vdev);
>>  	v4l2_ctrl_handler_free(&dev->hdl);
>> +	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>>
>>  	/* release resources */
>>  	iounmap(dev->lmmio);
>> diff --git a/drivers/media/pci/tw68/tw68-video.c
>> b/drivers/media/pci/tw68/tw68-video.c index 5c94ac7..50dcce6 100644
>> --- a/drivers/media/pci/tw68/tw68-video.c
>> +++ b/drivers/media/pci/tw68/tw68-video.c
>> @@ -384,6 +384,7 @@ static int tw68_queue_setup(struct vb2_queue *q, const
>> struct v4l2_format *fmt, unsigned tot_bufs = q->num_buffers + *num_buffers;
>>
>>  	sizes[0] = (dev->fmt->depth * dev->width * dev->height) >> 3;
>> +	alloc_ctxs[0] = dev->alloc_ctx;
>>  	/*
>>  	 * We allow create_bufs, but only if the sizeimage is the same as the
>>  	 * current sizeimage. The tw68_buffer_count calculation becomes quite
>> diff --git a/drivers/media/pci/tw68/tw68.h b/drivers/media/pci/tw68/tw68.h
>> index 2c8abe2..7a7501b 100644
>> --- a/drivers/media/pci/tw68/tw68.h
>> +++ b/drivers/media/pci/tw68/tw68.h
>> @@ -181,6 +181,7 @@ struct tw68_dev {
>>  	unsigned		field;
>>  	struct vb2_queue	vidq;
>>  	struct list_head	active;
>> +	void			*alloc_ctx;
>>
>>  	/* various v4l controls */
>>  	const struct tw68_tvnorm *tvnorm;	/* video */
>> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c
>> b/drivers/media/platform/marvell-ccic/mcam-core.c index f0eeb6c..c3ff538
>> 100644
>> --- a/drivers/media/platform/marvell-ccic/mcam-core.c
>> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c
>> @@ -1079,6 +1079,8 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
>>  		*nbufs = minbufs;
>>  	if (cam->buffer_mode == B_DMA_contig)
>>  		alloc_ctxs[0] = cam->vb_alloc_ctx;
>> +	else if (cam->buffer_mode == B_DMA_sg)
>> +		alloc_ctxs[0] = cam->vb_alloc_ctx_sg;
>>  	return 0;
>>  }
>>
>> @@ -1286,10 +1288,12 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
>>  		vq->ops = &mcam_vb2_ops;
>>  		vq->mem_ops = &vb2_dma_contig_memops;
>>  		vq->buf_struct_size = sizeof(struct mcam_vb_buffer);
>> -		cam->vb_alloc_ctx = vb2_dma_contig_init_ctx(cam->dev);
>>  		vq->io_modes = VB2_MMAP | VB2_USERPTR;
>>  		cam->dma_setup = mcam_ctlr_dma_contig;
>>  		cam->frame_complete = mcam_dma_contig_done;
>> +		cam->vb_alloc_ctx = vb2_dma_contig_init_ctx(cam->dev);
>> +		if (IS_ERR(cam->vb_alloc_ctx))
>> +			return PTR_ERR(cam->vb_alloc_ctx);
>>  #endif
>>  		break;
>>  	case B_DMA_sg:
>> @@ -1300,6 +1304,9 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
>>  		vq->io_modes = VB2_MMAP | VB2_USERPTR;
>>  		cam->dma_setup = mcam_ctlr_dma_sg;
>>  		cam->frame_complete = mcam_dma_sg_done;
>> +		cam->vb_alloc_ctx_sg = vb2_dma_sg_init_ctx(cam->dev);
>> +		if (IS_ERR(cam->vb_alloc_ctx_sg))
>> +			return PTR_ERR(cam->vb_alloc_ctx_sg);
>>  #endif
>>  		break;
>>  	case B_vmalloc:
>> @@ -1325,6 +1332,10 @@ static void mcam_cleanup_vb2(struct mcam_camera *cam)
>> if (cam->buffer_mode == B_DMA_contig)
>>  		vb2_dma_contig_cleanup_ctx(cam->vb_alloc_ctx);
>>  #endif
>> +#ifdef MCAM_MODE_DMA_SG
>> +	if (cam->buffer_mode == B_DMA_sg)
>> +		vb2_dma_sg_cleanup_ctx(cam->vb_alloc_ctx_sg);
>> +#endif
>>  }
>>
>>
>> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
>> b/drivers/media/platform/marvell-ccic/mcam-core.h index 60a8e1c..aa0c6ea
>> 100644
>> --- a/drivers/media/platform/marvell-ccic/mcam-core.h
>> +++ b/drivers/media/platform/marvell-ccic/mcam-core.h
>> @@ -176,6 +176,7 @@ struct mcam_camera {
>>  	/* DMA buffers - DMA modes */
>>  	struct mcam_vb_buffer *vb_bufs[MAX_DMA_BUFS];
>>  	struct vb2_alloc_ctx *vb_alloc_ctx;
>> +	struct vb2_alloc_ctx *vb_alloc_ctx_sg;
>>
>>  	/* Mode-specific ops, set at open time */
>>  	void (*dma_setup)(struct mcam_camera *cam);
>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 2529b83..2bf13dc 100644
>> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
>> @@ -30,7 +30,12 @@ module_param(debug, int, 0644);
>>  			printk(KERN_DEBUG "vb2-dma-sg: " fmt, ## arg);	\
>>  	} while (0)
>>
>> +struct vb2_dma_sg_conf {
>> +	struct device		*dev;
>> +};
>> +
>>  struct vb2_dma_sg_buf {
>> +	struct device			*dev;
>>  	void				*vaddr;
>>  	struct page			**pages;
>>  	int				offset;
>> @@ -89,10 +94,13 @@ static int vb2_dma_sg_alloc_compacted(struct
>> vb2_dma_sg_buf *buf, static void *vb2_dma_sg_alloc(void *alloc_ctx,
>> unsigned long size, enum dma_data_direction dma_dir, gfp_t gfp_flags)
>>  {
>> +	struct vb2_dma_sg_conf *conf = alloc_ctx;
>>  	struct vb2_dma_sg_buf *buf;
>>  	int ret;
>>  	int num_pages;
>>
>> +	if (WARN_ON(alloc_ctx == NULL))
>> +		return NULL;
>>  	buf = kzalloc(sizeof *buf, GFP_KERNEL);
>>  	if (!buf)
>>  		return NULL;
>> @@ -118,6 +126,8 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
>> long size, if (ret)
>>  		goto fail_table_alloc;
>>
>> +	/* Prevent the device from being released while the buffer is used */
>> +	buf->dev = get_device(conf->dev);
>>  	buf->handler.refcount = &buf->refcount;
>>  	buf->handler.put = vb2_dma_sg_put;
>>  	buf->handler.arg = buf;
>> @@ -153,6 +163,7 @@ static void vb2_dma_sg_put(void *buf_priv)
>>  		while (--i >= 0)
>>  			__free_page(buf->pages[i]);
>>  		kfree(buf->pages);
>> +		put_device(buf->dev);
>>  		kfree(buf);
>>  	}
>>  }
>> @@ -356,6 +367,27 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>>  };
>>  EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
>>
>> +void *vb2_dma_sg_init_ctx(struct device *dev)
>> +{
>> +	struct vb2_dma_sg_conf *conf;
>> +
>> +	conf = kzalloc(sizeof(*conf), GFP_KERNEL);
>> +	if (!conf)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	conf->dev = dev;
>> +
>> +	return conf;
>> +}
>> +EXPORT_SYMBOL_GPL(vb2_dma_sg_init_ctx);
>> +
>> +void vb2_dma_sg_cleanup_ctx(void *alloc_ctx)
>> +{
>> +	if (!IS_ERR_OR_NULL(alloc_ctx))
>> +		kfree(alloc_ctx);
>> +}
>> +EXPORT_SYMBOL_GPL(vb2_dma_sg_cleanup_ctx);
>> +
>>  MODULE_DESCRIPTION("dma scatter/gather memory handling routines for
>> videobuf2"); MODULE_AUTHOR("Andrzej Pietrasiewicz");
>>  MODULE_LICENSE("GPL");
>> diff --git a/include/media/videobuf2-dma-sg.h
>> b/include/media/videobuf2-dma-sg.h index 7b89852..14ce306 100644
>> --- a/include/media/videobuf2-dma-sg.h
>> +++ b/include/media/videobuf2-dma-sg.h
>> @@ -21,6 +21,9 @@ static inline struct sg_table *vb2_dma_sg_plane_desc(
>>  	return (struct sg_table *)vb2_plane_cookie(vb, plane_no);
>>  }
>>
>> +void *vb2_dma_sg_init_ctx(struct device *dev);
>> +void vb2_dma_sg_cleanup_ctx(void *alloc_ctx);
>> +
>>  extern const struct vb2_mem_ops vb2_dma_sg_memops;
>>
>>  #endif
> 

[REVIEWv7 PATCH 06/12] vb2-dma-sg: move dma_(un)map_sg here

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This moves dma_(un)map_sg to the get_userptr/put_userptr and alloc/put
memops of videobuf2-dma-sg.c and adds dma_sync_sg_for_device/cpu to the
prepare/finish memops.

Now that vb2-dma-sg will sync the buffers for you in the prepare/finish
memops we can drop that from the drivers that use dma-sg.

For the solo6x10 driver that was a bit more involved because it needs to
copy JPEG or MPEG headers to the buffer before returning it to userspace,
and that cannot be done in the old place since the buffer there is still
setup for DMA access, not for CPU access. However, the buf_finish
op is the ideal place to do this. By the time buf_finish is called
the buffer is available for CPU access, so copying to the buffer is fine.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/pci/cx23885/cx23885-417.c         |  3 --
 drivers/media/pci/cx23885/cx23885-core.c        |  5 ---
 drivers/media/pci/cx23885/cx23885-dvb.c         |  3 --
 drivers/media/pci/cx23885/cx23885-vbi.c         |  9 -----
 drivers/media/pci/cx23885/cx23885-video.c       |  9 -----
 drivers/media/pci/saa7134/saa7134-empress.c     |  1 -
 drivers/media/pci/saa7134/saa7134-ts.c          | 16 --------
 drivers/media/pci/saa7134/saa7134-vbi.c         | 15 --------
 drivers/media/pci/saa7134/saa7134-video.c       | 15 --------
 drivers/media/pci/saa7134/saa7134.h             |  1 -
 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 50 +++++++++++--------------
 drivers/media/pci/tw68/tw68-video.c             |  8 ----
 drivers/media/platform/marvell-ccic/mcam-core.c | 18 +--------
 drivers/media/v4l2-core/videobuf2-dma-sg.c      | 39 +++++++++++++++++++
 14 files changed, 62 insertions(+), 130 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c
index d72a3ec..e4901a5 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1167,11 +1167,8 @@ static void buffer_finish(struct vb2_buffer *vb)
 	struct cx23885_dev *dev = vb->vb2_queue->drv_priv;
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
-	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 
 	cx23885_free_buffer(dev, buf);
-
-	dma_unmap_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
 }
 
 static void buffer_queue(struct vb2_buffer *vb)
diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
index d452b5c..d07b04a 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -1453,17 +1453,12 @@ int cx23885_buf_prepare(struct cx23885_buffer *buf, struct cx23885_tsport *port)
 	struct cx23885_dev *dev = port->dev;
 	int size = port->ts_packet_size * port->ts_packet_count;
 	struct sg_table *sgt = vb2_dma_sg_plane_desc(&buf->vb, 0);
-	int rc;
 
 	dprintk(1, "%s: %p\n", __func__, buf);
 	if (vb2_plane_size(&buf->vb, 0) < size)
 		return -EINVAL;
 	vb2_set_plane_payload(&buf->vb, 0, size);
 
-	rc = dma_map_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
-	if (!rc)
-		return -EIO;
-
 	cx23885_risc_databuffer(dev->pci, &buf->risc,
 				sgt->sgl,
 				port->ts_packet_size, port->ts_packet_count, 0);
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 44fafba..c47d182 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -123,11 +123,8 @@ static void buffer_finish(struct vb2_buffer *vb)
 	struct cx23885_dev *dev = port->dev;
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
-	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 
 	cx23885_free_buffer(dev, buf);
-
-	dma_unmap_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
 }
 
 static void buffer_queue(struct vb2_buffer *vb)
diff --git a/drivers/media/pci/cx23885/cx23885-vbi.c b/drivers/media/pci/cx23885/cx23885-vbi.c
index 1d339a6..d362d38 100644
--- a/drivers/media/pci/cx23885/cx23885-vbi.c
+++ b/drivers/media/pci/cx23885/cx23885-vbi.c
@@ -143,7 +143,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		struct cx23885_buffer, vb);
 	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 	unsigned lines = VBI_PAL_LINE_COUNT;
-	int ret;
 
 	if (dev->tvnorm & V4L2_STD_525_60)
 		lines = VBI_NTSC_LINE_COUNT;
@@ -152,10 +151,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		return -EINVAL;
 	vb2_set_plane_payload(vb, 0, lines * VBI_LINE_LENGTH * 2);
 
-	ret = dma_map_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
-
 	cx23885_risc_vbibuffer(dev->pci, &buf->risc,
 			 sgt->sgl,
 			 0, VBI_LINE_LENGTH * lines,
@@ -166,14 +161,10 @@ static int buffer_prepare(struct vb2_buffer *vb)
 
 static void buffer_finish(struct vb2_buffer *vb)
 {
-	struct cx23885_dev *dev = vb->vb2_queue->drv_priv;
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
-	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 
 	cx23885_free_buffer(vb->vb2_queue->drv_priv, buf);
-
-	dma_unmap_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
 }
 
 /*
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index 371eecf..5e93c68 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -335,7 +335,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	u32 line0_offset, line1_offset;
 	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 	int field_tff;
-	int ret;
 
 	buf->bpl = (dev->width * dev->fmt->depth) >> 3;
 
@@ -343,10 +342,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		return -EINVAL;
 	vb2_set_plane_payload(vb, 0, dev->height * buf->bpl);
 
-	ret = dma_map_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
-
 	switch (dev->field) {
 	case V4L2_FIELD_TOP:
 		cx23885_risc_buffer(dev->pci, &buf->risc,
@@ -414,14 +409,10 @@ static int buffer_prepare(struct vb2_buffer *vb)
 
 static void buffer_finish(struct vb2_buffer *vb)
 {
-	struct cx23885_dev *dev = vb->vb2_queue->drv_priv;
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
-	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 
 	cx23885_free_buffer(vb->vb2_queue->drv_priv, buf);
-
-	dma_unmap_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
 }
 
 /*
diff --git a/drivers/media/pci/saa7134/saa7134-empress.c b/drivers/media/pci/saa7134/saa7134-empress.c
index 8b3bb78..594dc3a 100644
--- a/drivers/media/pci/saa7134/saa7134-empress.c
+++ b/drivers/media/pci/saa7134/saa7134-empress.c
@@ -96,7 +96,6 @@ static struct vb2_ops saa7134_empress_qops = {
 	.queue_setup	= saa7134_ts_queue_setup,
 	.buf_init	= saa7134_ts_buffer_init,
 	.buf_prepare	= saa7134_ts_buffer_prepare,
-	.buf_finish	= saa7134_ts_buffer_finish,
 	.buf_queue	= saa7134_vb2_buffer_queue,
 	.wait_prepare	= vb2_ops_wait_prepare,
 	.wait_finish	= vb2_ops_wait_finish,
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
index 8eff4a7..2709b83 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -94,7 +94,6 @@ int saa7134_ts_buffer_prepare(struct vb2_buffer *vb2)
 	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
 	struct sg_table *dma = vb2_dma_sg_plane_desc(vb2, 0);
 	unsigned int lines, llength, size;
-	int ret;
 
 	dprintk("buffer_prepare [%p]\n", buf);
 
@@ -108,25 +107,11 @@ int saa7134_ts_buffer_prepare(struct vb2_buffer *vb2)
 	vb2_set_plane_payload(vb2, 0, size);
 	vb2->v4l2_buf.field = dev->field;
 
-	ret = dma_map_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
 	return saa7134_pgtable_build(dev->pci, &dmaq->pt, dma->sgl, dma->nents,
 				    saa7134_buffer_startpage(buf));
 }
 EXPORT_SYMBOL_GPL(saa7134_ts_buffer_prepare);
 
-void saa7134_ts_buffer_finish(struct vb2_buffer *vb2)
-{
-	struct saa7134_dmaqueue *dmaq = vb2->vb2_queue->drv_priv;
-	struct saa7134_dev *dev = dmaq->dev;
-	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
-	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
-
-	dma_unmap_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-}
-EXPORT_SYMBOL_GPL(saa7134_ts_buffer_finish);
-
 int saa7134_ts_queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 			   unsigned int *nbuffers, unsigned int *nplanes,
 			   unsigned int sizes[], void *alloc_ctxs[])
@@ -188,7 +173,6 @@ struct vb2_ops saa7134_ts_qops = {
 	.queue_setup	= saa7134_ts_queue_setup,
 	.buf_init	= saa7134_ts_buffer_init,
 	.buf_prepare	= saa7134_ts_buffer_prepare,
-	.buf_finish	= saa7134_ts_buffer_finish,
 	.buf_queue	= saa7134_vb2_buffer_queue,
 	.wait_prepare	= vb2_ops_wait_prepare,
 	.wait_finish	= vb2_ops_wait_finish,
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
index e2cc684..5306e54 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -120,7 +120,6 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
 	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
 	unsigned int size;
-	int ret;
 
 	if (dma->sgl->offset) {
 		pr_err("The buffer is not page-aligned\n");
@@ -132,9 +131,6 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 
 	vb2_set_plane_payload(vb2, 0, size);
 
-	ret = dma_map_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
 	return saa7134_pgtable_build(dev->pci, &dmaq->pt, dma->sgl, dma->nents,
 				    saa7134_buffer_startpage(buf));
 }
@@ -170,21 +166,10 @@ static int buffer_init(struct vb2_buffer *vb2)
 	return 0;
 }
 
-static void buffer_finish(struct vb2_buffer *vb2)
-{
-	struct saa7134_dmaqueue *dmaq = vb2->vb2_queue->drv_priv;
-	struct saa7134_dev *dev = dmaq->dev;
-	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
-	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
-
-	dma_unmap_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-}
-
 struct vb2_ops saa7134_vbi_qops = {
 	.queue_setup	= queue_setup,
 	.buf_init	= buffer_init,
 	.buf_prepare	= buffer_prepare,
-	.buf_finish	= buffer_finish,
 	.buf_queue	= saa7134_vb2_buffer_queue,
 	.wait_prepare	= vb2_ops_wait_prepare,
 	.wait_finish	= vb2_ops_wait_finish,
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index ba02995..701b52f 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -883,7 +883,6 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
 	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
 	unsigned int size;
-	int ret;
 
 	if (dma->sgl->offset) {
 		pr_err("The buffer is not page-aligned\n");
@@ -896,23 +895,10 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 	vb2_set_plane_payload(vb2, 0, size);
 	vb2->v4l2_buf.field = dev->field;
 
-	ret = dma_map_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
 	return saa7134_pgtable_build(dev->pci, &dmaq->pt, dma->sgl, dma->nents,
 				    saa7134_buffer_startpage(buf));
 }
 
-static void buffer_finish(struct vb2_buffer *vb2)
-{
-	struct saa7134_dmaqueue *dmaq = vb2->vb2_queue->drv_priv;
-	struct saa7134_dev *dev = dmaq->dev;
-	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
-	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
-
-	dma_unmap_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-}
-
 static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 			   unsigned int *nbuffers, unsigned int *nplanes,
 			   unsigned int sizes[], void *alloc_ctxs[])
@@ -1005,7 +991,6 @@ static struct vb2_ops vb2_qops = {
 	.queue_setup	= queue_setup,
 	.buf_init	= buffer_init,
 	.buf_prepare	= buffer_prepare,
-	.buf_finish	= buffer_finish,
 	.buf_queue	= saa7134_vb2_buffer_queue,
 	.wait_prepare	= vb2_ops_wait_prepare,
 	.wait_finish	= vb2_ops_wait_finish,
diff --git a/drivers/media/pci/saa7134/saa7134.h b/drivers/media/pci/saa7134/saa7134.h
index c644c7d..8bf0553 100644
--- a/drivers/media/pci/saa7134/saa7134.h
+++ b/drivers/media/pci/saa7134/saa7134.h
@@ -815,7 +815,6 @@ void saa7134_video_fini(struct saa7134_dev *dev);
 
 int saa7134_ts_buffer_init(struct vb2_buffer *vb2);
 int saa7134_ts_buffer_prepare(struct vb2_buffer *vb2);
-void saa7134_ts_buffer_finish(struct vb2_buffer *vb2);
 int saa7134_ts_queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 			   unsigned int *nbuffers, unsigned int *nplanes,
 			   unsigned int sizes[], void *alloc_ctxs[]);
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
index 4f6bfba..6e933d3 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
@@ -463,7 +463,6 @@ static int solo_fill_jpeg(struct solo_enc_dev *solo_enc,
 	struct solo_dev *solo_dev = solo_enc->solo_dev;
 	struct sg_table *vbuf = vb2_dma_sg_plane_desc(vb, 0);
 	int frame_size;
-	int ret;
 
 	vb->v4l2_buf.flags |= V4L2_BUF_FLAG_KEYFRAME;
 
@@ -473,22 +472,10 @@ static int solo_fill_jpeg(struct solo_enc_dev *solo_enc,
 	frame_size = ALIGN(vop_jpeg_size(vh) + solo_enc->jpeg_len, DMA_ALIGN);
 	vb2_set_plane_payload(vb, 0, vop_jpeg_size(vh) + solo_enc->jpeg_len);
 
-	/* may discard all previous data in vbuf->sgl */
-	if (!dma_map_sg(&solo_dev->pdev->dev, vbuf->sgl, vbuf->nents,
-			DMA_FROM_DEVICE))
-		return -ENOMEM;
-	ret = solo_send_desc(solo_enc, solo_enc->jpeg_len, vbuf,
+	return solo_send_desc(solo_enc, solo_enc->jpeg_len, vbuf,
 			     vop_jpeg_offset(vh) - SOLO_JPEG_EXT_ADDR(solo_dev),
 			     frame_size, SOLO_JPEG_EXT_ADDR(solo_dev),
 			     SOLO_JPEG_EXT_SIZE(solo_dev));
-	dma_unmap_sg(&solo_dev->pdev->dev, vbuf->sgl, vbuf->nents,
-			DMA_FROM_DEVICE);
-
-	/* add the header only after dma_unmap_sg() */
-	sg_copy_from_buffer(vbuf->sgl, vbuf->nents,
-			    solo_enc->jpeg_header, solo_enc->jpeg_len);
-
-	return ret;
 }
 
 static int solo_fill_mpeg(struct solo_enc_dev *solo_enc,
@@ -498,7 +485,6 @@ static int solo_fill_mpeg(struct solo_enc_dev *solo_enc,
 	struct sg_table *vbuf = vb2_dma_sg_plane_desc(vb, 0);
 	int frame_off, frame_size;
 	int skip = 0;
-	int ret;
 
 	if (vb2_plane_size(vb, 0) < vop_mpeg_size(vh))
 		return -EIO;
@@ -521,21 +507,9 @@ static int solo_fill_mpeg(struct solo_enc_dev *solo_enc,
 		sizeof(*vh)) % SOLO_MP4E_EXT_SIZE(solo_dev);
 	frame_size = ALIGN(vop_mpeg_size(vh) + skip, DMA_ALIGN);
 
-	/* may discard all previous data in vbuf->sgl */
-	if (!dma_map_sg(&solo_dev->pdev->dev, vbuf->sgl, vbuf->nents,
-			DMA_FROM_DEVICE))
-		return -ENOMEM;
-	ret = solo_send_desc(solo_enc, skip, vbuf, frame_off, frame_size,
+	return solo_send_desc(solo_enc, skip, vbuf, frame_off, frame_size,
 			SOLO_MP4E_EXT_ADDR(solo_dev),
 			SOLO_MP4E_EXT_SIZE(solo_dev));
-	dma_unmap_sg(&solo_dev->pdev->dev, vbuf->sgl, vbuf->nents,
-			DMA_FROM_DEVICE);
-
-	/* add the header only after dma_unmap_sg() */
-	if (!vop_type(vh))
-		sg_copy_from_buffer(vbuf->sgl, vbuf->nents,
-				    solo_enc->vop, solo_enc->vop_len);
-	return ret;
 }
 
 static int solo_enc_fillbuf(struct solo_enc_dev *solo_enc,
@@ -793,9 +767,29 @@ static void solo_enc_stop_streaming(struct vb2_queue *q)
 	spin_unlock_irqrestore(&solo_enc->av_lock, flags);
 }
 
+static void solo_enc_buf_finish(struct vb2_buffer *vb)
+{
+	struct solo_enc_dev *solo_enc = vb2_get_drv_priv(vb->vb2_queue);
+	struct sg_table *vbuf = vb2_dma_sg_plane_desc(vb, 0);
+
+	switch (solo_enc->fmt) {
+	case V4L2_PIX_FMT_MPEG4:
+	case V4L2_PIX_FMT_H264:
+		if (vb->v4l2_buf.flags & V4L2_BUF_FLAG_KEYFRAME)
+			sg_copy_from_buffer(vbuf->sgl, vbuf->nents,
+					solo_enc->vop, solo_enc->vop_len);
+		break;
+	default: /* V4L2_PIX_FMT_MJPEG */
+		sg_copy_from_buffer(vbuf->sgl, vbuf->nents,
+				solo_enc->jpeg_header, solo_enc->jpeg_len);
+		break;
+	}
+}
+
 static struct vb2_ops solo_enc_video_qops = {
 	.queue_setup	= solo_enc_queue_setup,
 	.buf_queue	= solo_enc_buf_queue,
+	.buf_finish	= solo_enc_buf_finish,
 	.start_streaming = solo_enc_start_streaming,
 	.stop_streaming = solo_enc_stop_streaming,
 	.wait_prepare	= vb2_ops_wait_prepare,
diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c
index 50dcce6..8355e55 100644
--- a/drivers/media/pci/tw68/tw68-video.c
+++ b/drivers/media/pci/tw68/tw68-video.c
@@ -462,17 +462,12 @@ static int tw68_buf_prepare(struct vb2_buffer *vb)
 	struct tw68_buf *buf = container_of(vb, struct tw68_buf, vb);
 	struct sg_table *dma = vb2_dma_sg_plane_desc(vb, 0);
 	unsigned size, bpl;
-	int rc;
 
 	size = (dev->width * dev->height * dev->fmt->depth) >> 3;
 	if (vb2_plane_size(vb, 0) < size)
 		return -EINVAL;
 	vb2_set_plane_payload(vb, 0, size);
 
-	rc = dma_map_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-	if (!rc)
-		return -EIO;
-
 	bpl = (dev->width * dev->fmt->depth) >> 3;
 	switch (dev->field) {
 	case V4L2_FIELD_TOP:
@@ -506,11 +501,8 @@ static void tw68_buf_finish(struct vb2_buffer *vb)
 {
 	struct vb2_queue *vq = vb->vb2_queue;
 	struct tw68_dev *dev = vb2_get_drv_priv(vq);
-	struct sg_table *dma = vb2_dma_sg_plane_desc(vb, 0);
 	struct tw68_buf *buf = container_of(vb, struct tw68_buf, vb);
 
-	dma_unmap_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-
 	pci_free_consistent(dev->pci, buf->size, buf->cpu, buf->dma);
 }
 
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index c3ff538..ce00cba 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1220,17 +1220,12 @@ static int mcam_vb_sg_buf_init(struct vb2_buffer *vb)
 static int mcam_vb_sg_buf_prepare(struct vb2_buffer *vb)
 {
 	struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
-	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
 	struct sg_table *sg_table = vb2_dma_sg_plane_desc(vb, 0);
 	struct mcam_dma_desc *desc = mvb->dma_desc;
 	struct scatterlist *sg;
 	int i;
 
-	mvb->dma_desc_nent = dma_map_sg(cam->dev, sg_table->sgl,
-			sg_table->nents, DMA_FROM_DEVICE);
-	if (mvb->dma_desc_nent <= 0)
-		return -EIO;  /* Not sure what's right here */
-	for_each_sg(sg_table->sgl, sg, mvb->dma_desc_nent, i) {
+	for_each_sg(sg_table->sgl, sg, sg_table->nents, i) {
 		desc->dma_addr = sg_dma_address(sg);
 		desc->segment_len = sg_dma_len(sg);
 		desc++;
@@ -1238,16 +1233,6 @@ static int mcam_vb_sg_buf_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void mcam_vb_sg_buf_finish(struct vb2_buffer *vb)
-{
-	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
-	struct sg_table *sg_table = vb2_dma_sg_plane_desc(vb, 0);
-
-	if (sg_table)
-		dma_unmap_sg(cam->dev, sg_table->sgl,
-				sg_table->nents, DMA_FROM_DEVICE);
-}
-
 static void mcam_vb_sg_buf_cleanup(struct vb2_buffer *vb)
 {
 	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
@@ -1264,7 +1249,6 @@ static const struct vb2_ops mcam_vb2_sg_ops = {
 	.buf_init		= mcam_vb_sg_buf_init,
 	.buf_prepare		= mcam_vb_sg_buf_prepare,
 	.buf_queue		= mcam_vb_buf_queue,
-	.buf_finish		= mcam_vb_sg_buf_finish,
 	.buf_cleanup		= mcam_vb_sg_buf_cleanup,
 	.start_streaming	= mcam_vb_start_streaming,
 	.stop_streaming		= mcam_vb_stop_streaming,
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 2bf13dc..f671fab 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -96,6 +96,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 {
 	struct vb2_dma_sg_conf *conf = alloc_ctx;
 	struct vb2_dma_sg_buf *buf;
+	struct sg_table *sgt;
 	int ret;
 	int num_pages;
 
@@ -128,6 +129,12 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 
 	/* Prevent the device from being released while the buffer is used */
 	buf->dev = get_device(conf->dev);
+
+	sgt = &buf->sg_table;
+	if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
+		goto fail_map;
+	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+
 	buf->handler.refcount = &buf->refcount;
 	buf->handler.put = vb2_dma_sg_put;
 	buf->handler.arg = buf;
@@ -138,6 +145,9 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 		__func__, buf->num_pages);
 	return buf;
 
+fail_map:
+	put_device(buf->dev);
+	sg_free_table(buf->dma_sgt);
 fail_table_alloc:
 	num_pages = buf->num_pages;
 	while (num_pages--)
@@ -152,11 +162,13 @@ fail_pages_array_alloc:
 static void vb2_dma_sg_put(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct sg_table *sgt = &buf->sg_table;
 	int i = buf->num_pages;
 
 	if (atomic_dec_and_test(&buf->refcount)) {
 		dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
 			buf->num_pages);
+		dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 		if (buf->vaddr)
 			vm_unmap_ram(buf->vaddr, buf->num_pages);
 		sg_free_table(&buf->sg_table);
@@ -168,6 +180,22 @@ static void vb2_dma_sg_put(void *buf_priv)
 	}
 }
 
+static void vb2_dma_sg_prepare(void *buf_priv)
+{
+	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct sg_table *sgt = &buf->sg_table;
+
+	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+}
+
+static void vb2_dma_sg_finish(void *buf_priv)
+{
+	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct sg_table *sgt = &buf->sg_table;
+
+	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+}
+
 static inline int vma_is_io(struct vm_area_struct *vma)
 {
 	return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
@@ -181,6 +209,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	unsigned long first, last;
 	int num_pages_from_user;
 	struct vm_area_struct *vma;
+	struct sg_table *sgt;
 
 	buf = kzalloc(sizeof *buf, GFP_KERNEL);
 	if (!buf)
@@ -246,8 +275,14 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 			buf->num_pages, buf->offset, size, 0))
 		goto userptr_fail_alloc_table_from_pages;
 
+	sgt = &buf->sg_table;
+	if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
+		goto userptr_fail_map;
+	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 	return buf;
 
+userptr_fail_map:
+	sg_free_table(&buf->sg_table);
 userptr_fail_alloc_table_from_pages:
 userptr_fail_get_user_pages:
 	dprintk(1, "get_user_pages requested/got: %d/%d]\n",
@@ -270,10 +305,12 @@ userptr_fail_alloc_pages:
 static void vb2_dma_sg_put_userptr(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct sg_table *sgt = &buf->sg_table;
 	int i = buf->num_pages;
 
 	dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
 	       __func__, buf->num_pages);
+	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 	if (buf->vaddr)
 		vm_unmap_ram(buf->vaddr, buf->num_pages);
 	sg_free_table(&buf->sg_table);
@@ -360,6 +397,8 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
 	.put		= vb2_dma_sg_put,
 	.get_userptr	= vb2_dma_sg_get_userptr,
 	.put_userptr	= vb2_dma_sg_put_userptr,
+	.prepare	= vb2_dma_sg_prepare,
+	.finish		= vb2_dma_sg_finish,
 	.vaddr		= vb2_dma_sg_vaddr,
 	.mmap		= vb2_dma_sg_mmap,
 	.num_users	= vb2_dma_sg_num_users,
-- 
2.1.1

Re: [REVIEWv7 PATCH 06/12] vb2-dma-sg: move dma_(un)map_sg here

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Tuesday 18 November 2014 13:51:02 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This moves dma_(un)map_sg to the get_userptr/put_userptr and alloc/put
> memops of videobuf2-dma-sg.c and adds dma_sync_sg_for_device/cpu to the
> prepare/finish memops.
> 
> Now that vb2-dma-sg will sync the buffers for you in the prepare/finish
> memops we can drop that from the drivers that use dma-sg.
> 
> For the solo6x10 driver that was a bit more involved because it needs to
> copy JPEG or MPEG headers to the buffer before returning it to userspace,
> and that cannot be done in the old place since the buffer there is still
> setup for DMA access, not for CPU access. However, the buf_finish
> op is the ideal place to do this. By the time buf_finish is called
> the buffer is available for CPU access, so copying to the buffer is fine.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Pawel Osciak <pawel@osciak.com>
> ---
>  drivers/media/pci/cx23885/cx23885-417.c         |  3 --
>  drivers/media/pci/cx23885/cx23885-core.c        |  5 ---
>  drivers/media/pci/cx23885/cx23885-dvb.c         |  3 --
>  drivers/media/pci/cx23885/cx23885-vbi.c         |  9 -----
>  drivers/media/pci/cx23885/cx23885-video.c       |  9 -----
>  drivers/media/pci/saa7134/saa7134-empress.c     |  1 -
>  drivers/media/pci/saa7134/saa7134-ts.c          | 16 --------
>  drivers/media/pci/saa7134/saa7134-vbi.c         | 15 --------
>  drivers/media/pci/saa7134/saa7134-video.c       | 15 --------
>  drivers/media/pci/saa7134/saa7134.h             |  1 -
>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 50 ++++++++++------------
>  drivers/media/pci/tw68/tw68-video.c             |  8 ----
>  drivers/media/platform/marvell-ccic/mcam-core.c | 18 +--------
>  drivers/media/v4l2-core/videobuf2-dma-sg.c      | 39 +++++++++++++++++++
>  14 files changed, 62 insertions(+), 130 deletions(-)

[snip]


> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 2bf13dc..f671fab 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -96,6 +96,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, {
>  	struct vb2_dma_sg_conf *conf = alloc_ctx;
>  	struct vb2_dma_sg_buf *buf;
> +	struct sg_table *sgt;
>  	int ret;
>  	int num_pages;
> 
> @@ -128,6 +129,12 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size,
> 
>  	/* Prevent the device from being released while the buffer is used */
>  	buf->dev = get_device(conf->dev);
> +
> +	sgt = &buf->sg_table;
> +	if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
> +		goto fail_map;
> +	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);

I can't help feeling there's a problem here if we need to sync the buffer for 
the CPU right before mapping it. I wonder whether we could just remove the 
dma_sync_sg_for_cpu() call. It depends on whether the cpu to dev sync 
implicitly performed by dma_map_sg is defined as only making the memory 
consistent for the device without making it inconsistent for the CPU, or as 
passing the memory ownership from the CPU to the device completely.

Some comment for the similar implementation in put_userptr.

>  	buf->handler.refcount = &buf->refcount;
>  	buf->handler.put = vb2_dma_sg_put;
>  	buf->handler.arg = buf;
> @@ -138,6 +145,9 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, __func__, buf->num_pages);
>  	return buf;
> 
> +fail_map:
> +	put_device(buf->dev);
> +	sg_free_table(buf->dma_sgt);

That's an unrelated bug fix, it should be split to a separate patch.

>  fail_table_alloc:
>  	num_pages = buf->num_pages;
>  	while (num_pages--)
> @@ -152,11 +162,13 @@ fail_pages_array_alloc:
>  static void vb2_dma_sg_put(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> +	struct sg_table *sgt = &buf->sg_table;
>  	int i = buf->num_pages;
> 
>  	if (atomic_dec_and_test(&buf->refcount)) {
>  		dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
>  			buf->num_pages);
> +		dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>  		if (buf->vaddr)
>  			vm_unmap_ram(buf->vaddr, buf->num_pages);
>  		sg_free_table(&buf->sg_table);
> @@ -168,6 +180,22 @@ static void vb2_dma_sg_put(void *buf_priv)
>  	}
>  }
> 
> +static void vb2_dma_sg_prepare(void *buf_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = buf_priv;
> +	struct sg_table *sgt = &buf->sg_table;
> +
> +	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> +}
> +
> +static void vb2_dma_sg_finish(void *buf_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = buf_priv;
> +	struct sg_table *sgt = &buf->sg_table;
> +
> +	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> +}
> +
>  static inline int vma_is_io(struct vm_area_struct *vma)
>  {
>  	return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
> @@ -181,6 +209,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, unsigned long first, last;
>  	int num_pages_from_user;
>  	struct vm_area_struct *vma;
> +	struct sg_table *sgt;
> 
>  	buf = kzalloc(sizeof *buf, GFP_KERNEL);
>  	if (!buf)
> @@ -246,8 +275,14 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, buf->num_pages, buf->offset, size, 0))
>  		goto userptr_fail_alloc_table_from_pages;
> 
> +	sgt = &buf->sg_table;
> +	if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
> +		goto userptr_fail_map;
> +	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> +
>  	return buf;
> 
> +userptr_fail_map:
> +	sg_free_table(&buf->sg_table);
>  userptr_fail_alloc_table_from_pages:
>  userptr_fail_get_user_pages:
>  	dprintk(1, "get_user_pages requested/got: %d/%d]\n",
> @@ -270,10 +305,12 @@ userptr_fail_alloc_pages:
>  static void vb2_dma_sg_put_userptr(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> +	struct sg_table *sgt = &buf->sg_table;
>  	int i = buf->num_pages;
> 
>  	dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
>  	       __func__, buf->num_pages);
> +	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>  	if (buf->vaddr)
>  		vm_unmap_ram(buf->vaddr, buf->num_pages);
>  	sg_free_table(&buf->sg_table);
> @@ -360,6 +397,8 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>  	.put		= vb2_dma_sg_put,
>  	.get_userptr	= vb2_dma_sg_get_userptr,
>  	.put_userptr	= vb2_dma_sg_put_userptr,
> +	.prepare	= vb2_dma_sg_prepare,
> +	.finish		= vb2_dma_sg_finish,
>  	.vaddr		= vb2_dma_sg_vaddr,
>  	.mmap		= vb2_dma_sg_mmap,
>  	.num_users	= vb2_dma_sg_num_users,

-- 
Regards,

Laurent Pinchart

Re: [REVIEWv7 PATCH 06/12] vb2-dma-sg: move dma_(un)map_sg here

[Hans Verkuil]

On 11/26/2014 09:43 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Tuesday 18 November 2014 13:51:02 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> This moves dma_(un)map_sg to the get_userptr/put_userptr and alloc/put
>> memops of videobuf2-dma-sg.c and adds dma_sync_sg_for_device/cpu to the
>> prepare/finish memops.
>>
>> Now that vb2-dma-sg will sync the buffers for you in the prepare/finish
>> memops we can drop that from the drivers that use dma-sg.
>>
>> For the solo6x10 driver that was a bit more involved because it needs to
>> copy JPEG or MPEG headers to the buffer before returning it to userspace,
>> and that cannot be done in the old place since the buffer there is still
>> setup for DMA access, not for CPU access. However, the buf_finish
>> op is the ideal place to do this. By the time buf_finish is called
>> the buffer is available for CPU access, so copying to the buffer is fine.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Acked-by: Pawel Osciak <pawel@osciak.com>
>> ---
>>  drivers/media/pci/cx23885/cx23885-417.c         |  3 --
>>  drivers/media/pci/cx23885/cx23885-core.c        |  5 ---
>>  drivers/media/pci/cx23885/cx23885-dvb.c         |  3 --
>>  drivers/media/pci/cx23885/cx23885-vbi.c         |  9 -----
>>  drivers/media/pci/cx23885/cx23885-video.c       |  9 -----
>>  drivers/media/pci/saa7134/saa7134-empress.c     |  1 -
>>  drivers/media/pci/saa7134/saa7134-ts.c          | 16 --------
>>  drivers/media/pci/saa7134/saa7134-vbi.c         | 15 --------
>>  drivers/media/pci/saa7134/saa7134-video.c       | 15 --------
>>  drivers/media/pci/saa7134/saa7134.h             |  1 -
>>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 50 ++++++++++------------
>>  drivers/media/pci/tw68/tw68-video.c             |  8 ----
>>  drivers/media/platform/marvell-ccic/mcam-core.c | 18 +--------
>>  drivers/media/v4l2-core/videobuf2-dma-sg.c      | 39 +++++++++++++++++++
>>  14 files changed, 62 insertions(+), 130 deletions(-)
> 
> [snip]
> 
> 
>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 2bf13dc..f671fab 100644
>> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
>> @@ -96,6 +96,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
>> long size, {
>>  	struct vb2_dma_sg_conf *conf = alloc_ctx;
>>  	struct vb2_dma_sg_buf *buf;
>> +	struct sg_table *sgt;
>>  	int ret;
>>  	int num_pages;
>>
>> @@ -128,6 +129,12 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
>> long size,
>>
>>  	/* Prevent the device from being released while the buffer is used */
>>  	buf->dev = get_device(conf->dev);
>> +
>> +	sgt = &buf->sg_table;
>> +	if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
>> +		goto fail_map;
>> +	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> 
> I can't help feeling there's a problem here if we need to sync the buffer for 
> the CPU right before mapping it. I wonder whether we could just remove the 
> dma_sync_sg_for_cpu() call. It depends on whether the cpu to dev sync 
> implicitly performed by dma_map_sg is defined as only making the memory 
> consistent for the device without making it inconsistent for the CPU, or as 
> passing the memory ownership from the CPU to the device completely.

dma_map_sg does effectively do a sync_sg_for_device. Note that patch 12/12
switched to dma_map_sg_attrs to avoid the sync_sg_for_device+sync_sg_for_cpu
combo.

Also note that this might very well disappear in the near future. The way the
CPU syncing is handled today is not really compatible with dmabuf. I have
fixes for this which make it explicit in the driver whenever the driver needs
to have CPU buffer access. See patches 12-16 of my RFCv6 patch series:

http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/84399

I did not include those yet since those patches need more work (two drivers
have not yet been converted for this). Once that is in place there shouldn't
be any need anymore to map buffers for cpu access unless requested explicitly.

In the meantime buffers need to be mapped initially for CPU access since that
is what some drivers expect today.

Regards,

	Hans

> 
> Some comment for the similar implementation in put_userptr.
> 
>>  	buf->handler.refcount = &buf->refcount;
>>  	buf->handler.put = vb2_dma_sg_put;
>>  	buf->handler.arg = buf;
>> @@ -138,6 +145,9 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
>> long size, __func__, buf->num_pages);
>>  	return buf;
>>
>> +fail_map:
>> +	put_device(buf->dev);
>> +	sg_free_table(buf->dma_sgt);
> 
> That's an unrelated bug fix, it should be split to a separate patch.
> 
>>  fail_table_alloc:
>>  	num_pages = buf->num_pages;
>>  	while (num_pages--)
>> @@ -152,11 +162,13 @@ fail_pages_array_alloc:
>>  static void vb2_dma_sg_put(void *buf_priv)
>>  {
>>  	struct vb2_dma_sg_buf *buf = buf_priv;
>> +	struct sg_table *sgt = &buf->sg_table;
>>  	int i = buf->num_pages;
>>
>>  	if (atomic_dec_and_test(&buf->refcount)) {
>>  		dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
>>  			buf->num_pages);
>> +		dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>>  		if (buf->vaddr)
>>  			vm_unmap_ram(buf->vaddr, buf->num_pages);
>>  		sg_free_table(&buf->sg_table);
>> @@ -168,6 +180,22 @@ static void vb2_dma_sg_put(void *buf_priv)
>>  	}
>>  }
>>
>> +static void vb2_dma_sg_prepare(void *buf_priv)
>> +{
>> +	struct vb2_dma_sg_buf *buf = buf_priv;
>> +	struct sg_table *sgt = &buf->sg_table;
>> +
>> +	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>> +}
>> +
>> +static void vb2_dma_sg_finish(void *buf_priv)
>> +{
>> +	struct vb2_dma_sg_buf *buf = buf_priv;
>> +	struct sg_table *sgt = &buf->sg_table;
>> +
>> +	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>> +}
>> +
>>  static inline int vma_is_io(struct vm_area_struct *vma)
>>  {
>>  	return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
>> @@ -181,6 +209,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
>> unsigned long vaddr, unsigned long first, last;
>>  	int num_pages_from_user;
>>  	struct vm_area_struct *vma;
>> +	struct sg_table *sgt;
>>
>>  	buf = kzalloc(sizeof *buf, GFP_KERNEL);
>>  	if (!buf)
>> @@ -246,8 +275,14 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
>> unsigned long vaddr, buf->num_pages, buf->offset, size, 0))
>>  		goto userptr_fail_alloc_table_from_pages;
>>
>> +	sgt = &buf->sg_table;
>> +	if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
>> +		goto userptr_fail_map;
>> +	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>> +
>>  	return buf;
>>
>> +userptr_fail_map:
>> +	sg_free_table(&buf->sg_table);
>>  userptr_fail_alloc_table_from_pages:
>>  userptr_fail_get_user_pages:
>>  	dprintk(1, "get_user_pages requested/got: %d/%d]\n",
>> @@ -270,10 +305,12 @@ userptr_fail_alloc_pages:
>>  static void vb2_dma_sg_put_userptr(void *buf_priv)
>>  {
>>  	struct vb2_dma_sg_buf *buf = buf_priv;
>> +	struct sg_table *sgt = &buf->sg_table;
>>  	int i = buf->num_pages;
>>
>>  	dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
>>  	       __func__, buf->num_pages);
>> +	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>>  	if (buf->vaddr)
>>  		vm_unmap_ram(buf->vaddr, buf->num_pages);
>>  	sg_free_table(&buf->sg_table);
>> @@ -360,6 +397,8 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>>  	.put		= vb2_dma_sg_put,
>>  	.get_userptr	= vb2_dma_sg_get_userptr,
>>  	.put_userptr	= vb2_dma_sg_put_userptr,
>> +	.prepare	= vb2_dma_sg_prepare,
>> +	.finish		= vb2_dma_sg_finish,
>>  	.vaddr		= vb2_dma_sg_vaddr,
>>  	.mmap		= vb2_dma_sg_mmap,
>>  	.num_users	= vb2_dma_sg_num_users,
> 

[REVIEWv7 PATCH 07/12] vb2-dma-sg: add dmabuf import support

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Add support for importing dmabuf to videobuf2-dma-sg.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 149 ++++++++++++++++++++++++++---
 1 file changed, 136 insertions(+), 13 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index f671fab..ad6d5c7 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -41,11 +41,19 @@ struct vb2_dma_sg_buf {
 	int				offset;
 	enum dma_data_direction		dma_dir;
 	struct sg_table			sg_table;
+	/*
+	 * This will point to sg_table when used with the MMAP or USERPTR
+	 * memory model, and to the dma_buf sglist when used with the
+	 * DMABUF memory model.
+	 */
+	struct sg_table			*dma_sgt;
 	size_t				size;
 	unsigned int			num_pages;
 	atomic_t			refcount;
 	struct vb2_vmarea_handler	handler;
 	struct vm_area_struct		*vma;
+
+	struct dma_buf_attachment	*db_attach;
 };
 
 static void vb2_dma_sg_put(void *buf_priv);
@@ -112,6 +120,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 	buf->size = size;
 	/* size is already page aligned */
 	buf->num_pages = size >> PAGE_SHIFT;
+	buf->dma_sgt = &buf->sg_table;
 
 	buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
 			     GFP_KERNEL);
@@ -122,7 +131,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 	if (ret)
 		goto fail_pages_alloc;
 
-	ret = sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
+	ret = sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
 			buf->num_pages, 0, size, GFP_KERNEL);
 	if (ret)
 		goto fail_table_alloc;
@@ -171,7 +180,7 @@ static void vb2_dma_sg_put(void *buf_priv)
 		dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 		if (buf->vaddr)
 			vm_unmap_ram(buf->vaddr, buf->num_pages);
-		sg_free_table(&buf->sg_table);
+		sg_free_table(buf->dma_sgt);
 		while (--i >= 0)
 			__free_page(buf->pages[i]);
 		kfree(buf->pages);
@@ -183,7 +192,11 @@ static void vb2_dma_sg_put(void *buf_priv)
 static void vb2_dma_sg_prepare(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
-	struct sg_table *sgt = &buf->sg_table;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	/* DMABUF exporter will flush the cache for us */
+	if (buf->db_attach)
+		return;
 
 	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 }
@@ -191,7 +204,11 @@ static void vb2_dma_sg_prepare(void *buf_priv)
 static void vb2_dma_sg_finish(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
-	struct sg_table *sgt = &buf->sg_table;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	/* DMABUF exporter will flush the cache for us */
+	if (buf->db_attach)
+		return;
 
 	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 }
@@ -219,6 +236,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	buf->dma_dir = dma_dir;
 	buf->offset = vaddr & ~PAGE_MASK;
 	buf->size = size;
+	buf->dma_sgt = &buf->sg_table;
 
 	first = (vaddr           & PAGE_MASK) >> PAGE_SHIFT;
 	last  = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
@@ -271,7 +289,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	if (num_pages_from_user != buf->num_pages)
 		goto userptr_fail_get_user_pages;
 
-	if (sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
+	if (sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
 			buf->num_pages, buf->offset, size, 0))
 		goto userptr_fail_alloc_table_from_pages;
 
@@ -313,7 +331,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
 	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 	if (buf->vaddr)
 		vm_unmap_ram(buf->vaddr, buf->num_pages);
-	sg_free_table(&buf->sg_table);
+	sg_free_table(buf->dma_sgt);
 	while (--i >= 0) {
 		if (buf->dma_dir == DMA_FROM_DEVICE)
 			set_page_dirty_lock(buf->pages[i]);
@@ -331,14 +349,16 @@ static void *vb2_dma_sg_vaddr(void *buf_priv)
 
 	BUG_ON(!buf);
 
-	if (!buf->vaddr)
-		buf->vaddr = vm_map_ram(buf->pages,
-					buf->num_pages,
-					-1,
-					PAGE_KERNEL);
+	if (!buf->vaddr) {
+		if (buf->db_attach)
+			buf->vaddr = dma_buf_vmap(buf->db_attach->dmabuf);
+		else
+			buf->vaddr = vm_map_ram(buf->pages,
+					buf->num_pages, -1, PAGE_KERNEL);
+	}
 
 	/* add offset in case userptr is not page-aligned */
-	return buf->vaddr + buf->offset;
+	return buf->vaddr ? buf->vaddr + buf->offset : NULL;
 }
 
 static unsigned int vb2_dma_sg_num_users(void *buf_priv)
@@ -385,11 +405,110 @@ static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma)
 	return 0;
 }
 
+/*********************************************/
+/*       callbacks for DMABUF buffers        */
+/*********************************************/
+
+static int vb2_dma_sg_map_dmabuf(void *mem_priv)
+{
+	struct vb2_dma_sg_buf *buf = mem_priv;
+	struct sg_table *sgt;
+
+	if (WARN_ON(!buf->db_attach)) {
+		pr_err("trying to pin a non attached buffer\n");
+		return -EINVAL;
+	}
+
+	if (WARN_ON(buf->dma_sgt)) {
+		pr_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(sgt)) {
+		pr_err("Error getting dmabuf scatterlist\n");
+		return -EINVAL;
+	}
+
+	buf->dma_sgt = sgt;
+	buf->vaddr = NULL;
+
+	return 0;
+}
+
+static void vb2_dma_sg_unmap_dmabuf(void *mem_priv)
+{
+	struct vb2_dma_sg_buf *buf = mem_priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	if (WARN_ON(!buf->db_attach)) {
+		pr_err("trying to unpin a not attached buffer\n");
+		return;
+	}
+
+	if (WARN_ON(!sgt)) {
+		pr_err("dmabuf buffer is already unpinned\n");
+		return;
+	}
+
+	if (buf->vaddr) {
+		dma_buf_vunmap(buf->db_attach->dmabuf, buf->vaddr);
+		buf->vaddr = NULL;
+	}
+	dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
+
+	buf->dma_sgt = NULL;
+}
+
+static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
+{
+	struct vb2_dma_sg_buf *buf = mem_priv;
+
+	/* if vb2 works correctly you should never detach mapped buffer */
+	if (WARN_ON(buf->dma_sgt))
+		vb2_dma_sg_unmap_dmabuf(buf);
+
+	/* detach this attachment */
+	dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
+	kfree(buf);
+}
+
+static void *vb2_dma_sg_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
+	unsigned long size, enum dma_data_direction dma_dir)
+{
+	struct vb2_dma_sg_conf *conf = alloc_ctx;
+	struct vb2_dma_sg_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 = conf->dev;
+	/* create attachment for the dmabuf with the user device */
+	dba = dma_buf_attach(dbuf, buf->dev);
+	if (IS_ERR(dba)) {
+		pr_err("failed to attach dmabuf\n");
+		kfree(buf);
+		return dba;
+	}
+
+	buf->dma_dir = dma_dir;
+	buf->size = size;
+	buf->db_attach = dba;
+
+	return buf;
+}
+
 static void *vb2_dma_sg_cookie(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
 
-	return &buf->sg_table;
+	return buf->dma_sgt;
 }
 
 const struct vb2_mem_ops vb2_dma_sg_memops = {
@@ -402,6 +521,10 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
 	.vaddr		= vb2_dma_sg_vaddr,
 	.mmap		= vb2_dma_sg_mmap,
 	.num_users	= vb2_dma_sg_num_users,
+	.map_dmabuf	= vb2_dma_sg_map_dmabuf,
+	.unmap_dmabuf	= vb2_dma_sg_unmap_dmabuf,
+	.attach_dmabuf	= vb2_dma_sg_attach_dmabuf,
+	.detach_dmabuf	= vb2_dma_sg_detach_dmabuf,
 	.cookie		= vb2_dma_sg_cookie,
 };
 EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
-- 
2.1.1

Re: [REVIEWv7 PATCH 07/12] vb2-dma-sg: add dmabuf import support

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Tuesday 18 November 2014 13:51:03 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Add support for importing dmabuf to videobuf2-dma-sg.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Pawel Osciak <pawel@osciak.com>
> ---
>  drivers/media/v4l2-core/videobuf2-dma-sg.c | 149 +++++++++++++++++++++++---
>  1 file changed, 136 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index f671fab..ad6d5c7 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -41,11 +41,19 @@ struct vb2_dma_sg_buf {
>  	int				offset;
>  	enum dma_data_direction		dma_dir;
>  	struct sg_table			sg_table;
> +	/*
> +	 * This will point to sg_table when used with the MMAP or USERPTR
> +	 * memory model, and to the dma_buf sglist when used with the
> +	 * DMABUF memory model.
> +	 */
> +	struct sg_table			*dma_sgt;
>  	size_t				size;
>  	unsigned int			num_pages;
>  	atomic_t			refcount;
>  	struct vb2_vmarea_handler	handler;
>  	struct vm_area_struct		*vma;
> +
> +	struct dma_buf_attachment	*db_attach;
>  };
> 
>  static void vb2_dma_sg_put(void *buf_priv);
> @@ -112,6 +120,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, buf->size = size;
>  	/* size is already page aligned */
>  	buf->num_pages = size >> PAGE_SHIFT;
> +	buf->dma_sgt = &buf->sg_table;
> 
>  	buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
>  			     GFP_KERNEL);
> @@ -122,7 +131,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, if (ret)
>  		goto fail_pages_alloc;
> 
> -	ret = sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
> +	ret = sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
>  			buf->num_pages, 0, size, GFP_KERNEL);
>  	if (ret)
>  		goto fail_table_alloc;
> @@ -171,7 +180,7 @@ static void vb2_dma_sg_put(void *buf_priv)
>  		dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>  		if (buf->vaddr)
>  			vm_unmap_ram(buf->vaddr, buf->num_pages);
> -		sg_free_table(&buf->sg_table);
> +		sg_free_table(buf->dma_sgt);
>  		while (--i >= 0)
>  			__free_page(buf->pages[i]);
>  		kfree(buf->pages);
> @@ -183,7 +192,11 @@ static void vb2_dma_sg_put(void *buf_priv)
>  static void vb2_dma_sg_prepare(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> -	struct sg_table *sgt = &buf->sg_table;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	/* DMABUF exporter will flush the cache for us */
> +	if (buf->db_attach)
> +		return;

Is this actually true ? If you look at the export code in patch 08/12, I don't 
see where the exporter would sync the buffer for the importer device.

> 
>  	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>  }
> @@ -191,7 +204,11 @@ static void vb2_dma_sg_prepare(void *buf_priv)
>  static void vb2_dma_sg_finish(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> -	struct sg_table *sgt = &buf->sg_table;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	/* DMABUF exporter will flush the cache for us */
> +	if (buf->db_attach)
> +		return;
> 
>  	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>  }
> @@ -219,6 +236,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, buf->dma_dir = dma_dir;
>  	buf->offset = vaddr & ~PAGE_MASK;
>  	buf->size = size;
> +	buf->dma_sgt = &buf->sg_table;
> 
>  	first = (vaddr           & PAGE_MASK) >> PAGE_SHIFT;
>  	last  = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
> @@ -271,7 +289,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, if (num_pages_from_user != buf->num_pages)
>  		goto userptr_fail_get_user_pages;
> 
> -	if (sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
> +	if (sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
>  			buf->num_pages, buf->offset, size, 0))
>  		goto userptr_fail_alloc_table_from_pages;
> 
> @@ -313,7 +331,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>  	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>  	if (buf->vaddr)
>  		vm_unmap_ram(buf->vaddr, buf->num_pages);
> -	sg_free_table(&buf->sg_table);
> +	sg_free_table(buf->dma_sgt);
>  	while (--i >= 0) {
>  		if (buf->dma_dir == DMA_FROM_DEVICE)
>  			set_page_dirty_lock(buf->pages[i]);
> @@ -331,14 +349,16 @@ static void *vb2_dma_sg_vaddr(void *buf_priv)
> 
>  	BUG_ON(!buf);
> 
> -	if (!buf->vaddr)
> -		buf->vaddr = vm_map_ram(buf->pages,
> -					buf->num_pages,
> -					-1,
> -					PAGE_KERNEL);
> +	if (!buf->vaddr) {
> +		if (buf->db_attach)
> +			buf->vaddr = dma_buf_vmap(buf->db_attach->dmabuf);
> +		else
> +			buf->vaddr = vm_map_ram(buf->pages,
> +					buf->num_pages, -1, PAGE_KERNEL);
> +	}
> 
>  	/* add offset in case userptr is not page-aligned */
> -	return buf->vaddr + buf->offset;
> +	return buf->vaddr ? buf->vaddr + buf->offset : NULL;

Just nitpicking here, can vm_map_ram fail and return NULL ? If so this change 
is a separate bug fix and should be split to a separate patch.

>  }
> 
>  static unsigned int vb2_dma_sg_num_users(void *buf_priv)
> @@ -385,11 +405,110 @@ static int vb2_dma_sg_mmap(void *buf_priv, struct
> vm_area_struct *vma) return 0;
>  }
> 
> +/*********************************************/
> +/*       callbacks for DMABUF buffers        */
> +/*********************************************/
> +
> +static int vb2_dma_sg_map_dmabuf(void *mem_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = mem_priv;
> +	struct sg_table *sgt;
> +
> +	if (WARN_ON(!buf->db_attach)) {
> +		pr_err("trying to pin a non attached buffer\n");
> +		return -EINVAL;
> +	}
> +
> +	if (WARN_ON(buf->dma_sgt)) {
> +		pr_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(sgt)) {
> +		pr_err("Error getting dmabuf scatterlist\n");
> +		return -EINVAL;
> +	}
> +
> +	buf->dma_sgt = sgt;
> +	buf->vaddr = NULL;
> +
> +	return 0;
> +}
> +
> +static void vb2_dma_sg_unmap_dmabuf(void *mem_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = mem_priv;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	if (WARN_ON(!buf->db_attach)) {
> +		pr_err("trying to unpin a not attached buffer\n");
> +		return;
> +	}
> +
> +	if (WARN_ON(!sgt)) {
> +		pr_err("dmabuf buffer is already unpinned\n");
> +		return;
> +	}
> +
> +	if (buf->vaddr) {
> +		dma_buf_vunmap(buf->db_attach->dmabuf, buf->vaddr);
> +		buf->vaddr = NULL;
> +	}
> +	dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
> +
> +	buf->dma_sgt = NULL;
> +}
> +
> +static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = mem_priv;
> +
> +	/* if vb2 works correctly you should never detach mapped buffer */
> +	if (WARN_ON(buf->dma_sgt))
> +		vb2_dma_sg_unmap_dmabuf(buf);
> +
> +	/* detach this attachment */
> +	dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
> +	kfree(buf);
> +}
> +
> +static void *vb2_dma_sg_attach_dmabuf(void *alloc_ctx, struct dma_buf
> *dbuf, +	unsigned long size, enum dma_data_direction dma_dir)
> +{
> +	struct vb2_dma_sg_conf *conf = alloc_ctx;
> +	struct vb2_dma_sg_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 = conf->dev;
> +	/* create attachment for the dmabuf with the user device */
> +	dba = dma_buf_attach(dbuf, buf->dev);
> +	if (IS_ERR(dba)) {
> +		pr_err("failed to attach dmabuf\n");
> +		kfree(buf);
> +		return dba;
> +	}
> +
> +	buf->dma_dir = dma_dir;
> +	buf->size = size;
> +	buf->db_attach = dba;
> +
> +	return buf;
> +}
> +
>  static void *vb2_dma_sg_cookie(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> 
> -	return &buf->sg_table;
> +	return buf->dma_sgt;
>  }
> 
>  const struct vb2_mem_ops vb2_dma_sg_memops = {
> @@ -402,6 +521,10 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>  	.vaddr		= vb2_dma_sg_vaddr,
>  	.mmap		= vb2_dma_sg_mmap,
>  	.num_users	= vb2_dma_sg_num_users,
> +	.map_dmabuf	= vb2_dma_sg_map_dmabuf,
> +	.unmap_dmabuf	= vb2_dma_sg_unmap_dmabuf,
> +	.attach_dmabuf	= vb2_dma_sg_attach_dmabuf,
> +	.detach_dmabuf	= vb2_dma_sg_detach_dmabuf,
>  	.cookie		= vb2_dma_sg_cookie,
>  };
>  EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);

-- 
Regards,

Laurent Pinchart

Re: [REVIEWv7 PATCH 07/12] vb2-dma-sg: add dmabuf import support

[Hans Verkuil]

Hi Laurent,

On 11/26/2014 10:00 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Tuesday 18 November 2014 13:51:03 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Add support for importing dmabuf to videobuf2-dma-sg.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Acked-by: Pawel Osciak <pawel@osciak.com>
>> ---
>>  drivers/media/v4l2-core/videobuf2-dma-sg.c | 149 +++++++++++++++++++++++---
>>  1 file changed, 136 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index f671fab..ad6d5c7 100644
>> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
>> @@ -41,11 +41,19 @@ struct vb2_dma_sg_buf {
>>  	int				offset;
>>  	enum dma_data_direction		dma_dir;
>>  	struct sg_table			sg_table;
>> +	/*
>> +	 * This will point to sg_table when used with the MMAP or USERPTR
>> +	 * memory model, and to the dma_buf sglist when used with the
>> +	 * DMABUF memory model.
>> +	 */
>> +	struct sg_table			*dma_sgt;
>>  	size_t				size;
>>  	unsigned int			num_pages;
>>  	atomic_t			refcount;
>>  	struct vb2_vmarea_handler	handler;
>>  	struct vm_area_struct		*vma;
>> +
>> +	struct dma_buf_attachment	*db_attach;
>>  };
>>
>>  static void vb2_dma_sg_put(void *buf_priv);
>> @@ -112,6 +120,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
>> long size, buf->size = size;
>>  	/* size is already page aligned */
>>  	buf->num_pages = size >> PAGE_SHIFT;
>> +	buf->dma_sgt = &buf->sg_table;
>>
>>  	buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
>>  			     GFP_KERNEL);
>> @@ -122,7 +131,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
>> long size, if (ret)
>>  		goto fail_pages_alloc;
>>
>> -	ret = sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
>> +	ret = sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
>>  			buf->num_pages, 0, size, GFP_KERNEL);
>>  	if (ret)
>>  		goto fail_table_alloc;
>> @@ -171,7 +180,7 @@ static void vb2_dma_sg_put(void *buf_priv)
>>  		dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>>  		if (buf->vaddr)
>>  			vm_unmap_ram(buf->vaddr, buf->num_pages);
>> -		sg_free_table(&buf->sg_table);
>> +		sg_free_table(buf->dma_sgt);
>>  		while (--i >= 0)
>>  			__free_page(buf->pages[i]);
>>  		kfree(buf->pages);
>> @@ -183,7 +192,11 @@ static void vb2_dma_sg_put(void *buf_priv)
>>  static void vb2_dma_sg_prepare(void *buf_priv)
>>  {
>>  	struct vb2_dma_sg_buf *buf = buf_priv;
>> -	struct sg_table *sgt = &buf->sg_table;
>> +	struct sg_table *sgt = buf->dma_sgt;
>> +
>> +	/* DMABUF exporter will flush the cache for us */
>> +	if (buf->db_attach)
>> +		return;
> 
> Is this actually true ? If you look at the export code in patch 08/12, I don't 
> see where the exporter would sync the buffer for the importer device.

I think this was true at some point in the past. It ties in with my comment for
patch 06/12: cpu/device syncing for dma-buf is (and was) broken, although nobody
has noticed since none of the DMABUF-aware drivers that are used as such today
need CPU access to the buffer, or are only used on Intel architectures where
this is all moot. Patches 12-16 of my RFCv6 series really fix this. This particular
comment was copied from the dma-contig version. The basic idea was that when the
driver needs CPU access it will call the vaddr memop, which will map the buffer
for CPU access.

However, I am not sure whether dmabuf actually did the right thing there originally.
Later dmabuf was extended with begin/end_for_cpu_access ops to make this explicit,
but that was never implemented in vb2. That's what the second part of RFCv6 does.

Right now dma-sg is bug-compatible with dma-contig.

I spend 1-2 hours with Pawel in Düsseldorf figuring this out, it is not exactly
trivial to understand.

Regards,

	Hans

> 
>>
>>  	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>>  }
>> @@ -191,7 +204,11 @@ static void vb2_dma_sg_prepare(void *buf_priv)
>>  static void vb2_dma_sg_finish(void *buf_priv)
>>  {
>>  	struct vb2_dma_sg_buf *buf = buf_priv;
>> -	struct sg_table *sgt = &buf->sg_table;
>> +	struct sg_table *sgt = buf->dma_sgt;
>> +
>> +	/* DMABUF exporter will flush the cache for us */
>> +	if (buf->db_attach)
>> +		return;
>>
>>  	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>>  }
>> @@ -219,6 +236,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
>> unsigned long vaddr, buf->dma_dir = dma_dir;
>>  	buf->offset = vaddr & ~PAGE_MASK;
>>  	buf->size = size;
>> +	buf->dma_sgt = &buf->sg_table;
>>
>>  	first = (vaddr           & PAGE_MASK) >> PAGE_SHIFT;
>>  	last  = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
>> @@ -271,7 +289,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
>> unsigned long vaddr, if (num_pages_from_user != buf->num_pages)
>>  		goto userptr_fail_get_user_pages;
>>
>> -	if (sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
>> +	if (sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
>>  			buf->num_pages, buf->offset, size, 0))
>>  		goto userptr_fail_alloc_table_from_pages;
>>
>> @@ -313,7 +331,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>>  	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>>  	if (buf->vaddr)
>>  		vm_unmap_ram(buf->vaddr, buf->num_pages);
>> -	sg_free_table(&buf->sg_table);
>> +	sg_free_table(buf->dma_sgt);
>>  	while (--i >= 0) {
>>  		if (buf->dma_dir == DMA_FROM_DEVICE)
>>  			set_page_dirty_lock(buf->pages[i]);
>> @@ -331,14 +349,16 @@ static void *vb2_dma_sg_vaddr(void *buf_priv)
>>
>>  	BUG_ON(!buf);
>>
>> -	if (!buf->vaddr)
>> -		buf->vaddr = vm_map_ram(buf->pages,
>> -					buf->num_pages,
>> -					-1,
>> -					PAGE_KERNEL);
>> +	if (!buf->vaddr) {
>> +		if (buf->db_attach)
>> +			buf->vaddr = dma_buf_vmap(buf->db_attach->dmabuf);
>> +		else
>> +			buf->vaddr = vm_map_ram(buf->pages,
>> +					buf->num_pages, -1, PAGE_KERNEL);
>> +	}
>>
>>  	/* add offset in case userptr is not page-aligned */
>> -	return buf->vaddr + buf->offset;
>> +	return buf->vaddr ? buf->vaddr + buf->offset : NULL;
> 
> Just nitpicking here, can vm_map_ram fail and return NULL ? If so this change 
> is a separate bug fix and should be split to a separate patch.
> 
>>  }
>>
>>  static unsigned int vb2_dma_sg_num_users(void *buf_priv)
>> @@ -385,11 +405,110 @@ static int vb2_dma_sg_mmap(void *buf_priv, struct
>> vm_area_struct *vma) return 0;
>>  }
>>
>> +/*********************************************/
>> +/*       callbacks for DMABUF buffers        */
>> +/*********************************************/
>> +
>> +static int vb2_dma_sg_map_dmabuf(void *mem_priv)
>> +{
>> +	struct vb2_dma_sg_buf *buf = mem_priv;
>> +	struct sg_table *sgt;
>> +
>> +	if (WARN_ON(!buf->db_attach)) {
>> +		pr_err("trying to pin a non attached buffer\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (WARN_ON(buf->dma_sgt)) {
>> +		pr_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(sgt)) {
>> +		pr_err("Error getting dmabuf scatterlist\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	buf->dma_sgt = sgt;
>> +	buf->vaddr = NULL;
>> +
>> +	return 0;
>> +}
>> +
>> +static void vb2_dma_sg_unmap_dmabuf(void *mem_priv)
>> +{
>> +	struct vb2_dma_sg_buf *buf = mem_priv;
>> +	struct sg_table *sgt = buf->dma_sgt;
>> +
>> +	if (WARN_ON(!buf->db_attach)) {
>> +		pr_err("trying to unpin a not attached buffer\n");
>> +		return;
>> +	}
>> +
>> +	if (WARN_ON(!sgt)) {
>> +		pr_err("dmabuf buffer is already unpinned\n");
>> +		return;
>> +	}
>> +
>> +	if (buf->vaddr) {
>> +		dma_buf_vunmap(buf->db_attach->dmabuf, buf->vaddr);
>> +		buf->vaddr = NULL;
>> +	}
>> +	dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
>> +
>> +	buf->dma_sgt = NULL;
>> +}
>> +
>> +static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
>> +{
>> +	struct vb2_dma_sg_buf *buf = mem_priv;
>> +
>> +	/* if vb2 works correctly you should never detach mapped buffer */
>> +	if (WARN_ON(buf->dma_sgt))
>> +		vb2_dma_sg_unmap_dmabuf(buf);
>> +
>> +	/* detach this attachment */
>> +	dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
>> +	kfree(buf);
>> +}
>> +
>> +static void *vb2_dma_sg_attach_dmabuf(void *alloc_ctx, struct dma_buf
>> *dbuf, +	unsigned long size, enum dma_data_direction dma_dir)
>> +{
>> +	struct vb2_dma_sg_conf *conf = alloc_ctx;
>> +	struct vb2_dma_sg_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 = conf->dev;
>> +	/* create attachment for the dmabuf with the user device */
>> +	dba = dma_buf_attach(dbuf, buf->dev);
>> +	if (IS_ERR(dba)) {
>> +		pr_err("failed to attach dmabuf\n");
>> +		kfree(buf);
>> +		return dba;
>> +	}
>> +
>> +	buf->dma_dir = dma_dir;
>> +	buf->size = size;
>> +	buf->db_attach = dba;
>> +
>> +	return buf;
>> +}
>> +
>>  static void *vb2_dma_sg_cookie(void *buf_priv)
>>  {
>>  	struct vb2_dma_sg_buf *buf = buf_priv;
>>
>> -	return &buf->sg_table;
>> +	return buf->dma_sgt;
>>  }
>>
>>  const struct vb2_mem_ops vb2_dma_sg_memops = {
>> @@ -402,6 +521,10 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>>  	.vaddr		= vb2_dma_sg_vaddr,
>>  	.mmap		= vb2_dma_sg_mmap,
>>  	.num_users	= vb2_dma_sg_num_users,
>> +	.map_dmabuf	= vb2_dma_sg_map_dmabuf,
>> +	.unmap_dmabuf	= vb2_dma_sg_unmap_dmabuf,
>> +	.attach_dmabuf	= vb2_dma_sg_attach_dmabuf,
>> +	.detach_dmabuf	= vb2_dma_sg_detach_dmabuf,
>>  	.cookie		= vb2_dma_sg_cookie,
>>  };
>>  EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
> 

Re: [REVIEWv7 PATCH 07/12] vb2-dma-sg: add dmabuf import support

[Laurent Pinchart]

Hi Hans,

On Thursday 27 November 2014 10:02:14 Hans Verkuil wrote:
> On 11/26/2014 10:00 PM, Laurent Pinchart wrote:
> > On Tuesday 18 November 2014 13:51:03 Hans Verkuil wrote:
> >> From: Hans Verkuil <hans.verkuil@cisco.com>
> >> 
> >> Add support for importing dmabuf to videobuf2-dma-sg.
> >> 
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> Acked-by: Pawel Osciak <pawel@osciak.com>
> >> ---
> >> 
> >>  drivers/media/v4l2-core/videobuf2-dma-sg.c | 149 ++++++++++++++++++++---
> >>  1 file changed, 136 insertions(+), 13 deletions(-)
> >> 
> >> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> >> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index f671fab..ad6d5c7
> >> 100644
> >> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> >> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c

[snip]

> >> @@ -183,7 +192,11 @@ static void vb2_dma_sg_put(void *buf_priv)
> >>  static void vb2_dma_sg_prepare(void *buf_priv)
> >>  {
> >>  	struct vb2_dma_sg_buf *buf = buf_priv;
> >> -	struct sg_table *sgt = &buf->sg_table;
> >> +	struct sg_table *sgt = buf->dma_sgt;
> >> +
> >> +	/* DMABUF exporter will flush the cache for us */
> >> +	if (buf->db_attach)
> >> +		return;
> > 
> > Is this actually true ? If you look at the export code in patch 08/12, I
> > don't see where the exporter would sync the buffer for the importer
> > device.
>
> I think this was true at some point in the past. It ties in with my comment
> for patch 06/12: cpu/device syncing for dma-buf is (and was) broken,
> although nobody has noticed since none of the DMABUF-aware drivers that are
> used as such today need CPU access to the buffer, or are only used on Intel
> architectures where this is all moot. Patches 12-16 of my RFCv6 series
> really fix this. This particular comment was copied from the dma-contig
> version. The basic idea was that when the driver needs CPU access it will
> call the vaddr memop, which will map the buffer for CPU access.
> 
> However, I am not sure whether dmabuf actually did the right thing there
> originally. Later dmabuf was extended with begin/end_for_cpu_access ops to
> make this explicit, but that was never implemented in vb2. That's what the
> second part of RFCv6 does.
> 
> Right now dma-sg is bug-compatible with dma-contig.
> 
> I spend 1-2 hours with Pawel in Düsseldorf figuring this out, it is not
> exactly trivial to understand.

I agree that the situation is a mess, but we'll need to fix it one way or 
another :-/ The more we wait the more painful it will be. Please note that the 
problem isn't specific to CPU access, we need to sync for the device even in 
direct device to device transfers (although in practice that shouldn't be 
strictly required with the platforms we target today).

> >>  	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> >>  }

-- 
Regards,

Laurent Pinchart

Re: [REVIEWv7 PATCH 07/12] vb2-dma-sg: add dmabuf import support

[Hans Verkuil]

On 12/01/2014 11:46 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Thursday 27 November 2014 10:02:14 Hans Verkuil wrote:
>> On 11/26/2014 10:00 PM, Laurent Pinchart wrote:
>>> On Tuesday 18 November 2014 13:51:03 Hans Verkuil wrote:
>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>
>>>> Add support for importing dmabuf to videobuf2-dma-sg.
>>>>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> Acked-by: Pawel Osciak <pawel@osciak.com>
>>>> ---
>>>>
>>>>  drivers/media/v4l2-core/videobuf2-dma-sg.c | 149 ++++++++++++++++++++---
>>>>  1 file changed, 136 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>>>> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index f671fab..ad6d5c7
>>>> 100644
>>>> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
>>>> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> 
> [snip]
> 
>>>> @@ -183,7 +192,11 @@ static void vb2_dma_sg_put(void *buf_priv)
>>>>  static void vb2_dma_sg_prepare(void *buf_priv)
>>>>  {
>>>>  	struct vb2_dma_sg_buf *buf = buf_priv;
>>>> -	struct sg_table *sgt = &buf->sg_table;
>>>> +	struct sg_table *sgt = buf->dma_sgt;
>>>> +
>>>> +	/* DMABUF exporter will flush the cache for us */
>>>> +	if (buf->db_attach)
>>>> +		return;
>>>
>>> Is this actually true ? If you look at the export code in patch 08/12, I
>>> don't see where the exporter would sync the buffer for the importer
>>> device.
>>
>> I think this was true at some point in the past. It ties in with my comment
>> for patch 06/12: cpu/device syncing for dma-buf is (and was) broken,
>> although nobody has noticed since none of the DMABUF-aware drivers that are
>> used as such today need CPU access to the buffer, or are only used on Intel
>> architectures where this is all moot. Patches 12-16 of my RFCv6 series
>> really fix this. This particular comment was copied from the dma-contig
>> version. The basic idea was that when the driver needs CPU access it will
>> call the vaddr memop, which will map the buffer for CPU access.
>>
>> However, I am not sure whether dmabuf actually did the right thing there
>> originally. Later dmabuf was extended with begin/end_for_cpu_access ops to
>> make this explicit, but that was never implemented in vb2. That's what the
>> second part of RFCv6 does.
>>
>> Right now dma-sg is bug-compatible with dma-contig.
>>
>> I spend 1-2 hours with Pawel in Düsseldorf figuring this out, it is not
>> exactly trivial to understand.
> 
> I agree that the situation is a mess, but we'll need to fix it one way or 
> another :-/ The more we wait the more painful it will be. Please note that the 
> problem isn't specific to CPU access, we need to sync for the device even in 
> direct device to device transfers (although in practice that shouldn't be 
> strictly required with the platforms we target today).

As I mentioned, I do have fixes for this. But I still need to implement that
in two more drivers where it wasn't obvious how it should be done. I hope to
be able to tackle that soon.

Regards,

	Hans

[REVIEWv7 PATCH 08/12] vb2-dma-sg: add support for dmabuf exports

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

Add DMABUF export support to vb2-dma-sg.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
Acked-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 170 +++++++++++++++++++++++++++++
 1 file changed, 170 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index ad6d5c7..c2ec2c4 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -406,6 +406,175 @@ static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma)
 }
 
 /*********************************************/
+/*         DMABUF ops for exporters          */
+/*********************************************/
+
+struct vb2_dma_sg_attachment {
+	struct sg_table sgt;
+	enum dma_data_direction dma_dir;
+};
+
+static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
+	struct dma_buf_attachment *dbuf_attach)
+{
+	struct vb2_dma_sg_attachment *attach;
+	unsigned int i;
+	struct scatterlist *rd, *wr;
+	struct sg_table *sgt;
+	struct vb2_dma_sg_buf *buf = dbuf->priv;
+	int ret;
+
+	attach = kzalloc(sizeof(*attach), GFP_KERNEL);
+	if (!attach)
+		return -ENOMEM;
+
+	sgt = &attach->sgt;
+	/* Copy the buf->base_sgt scatter list to the attachment, as we can't
+	 * map the same scatter list to multiple attachments at the same time.
+	 */
+	ret = sg_alloc_table(sgt, buf->dma_sgt->orig_nents, GFP_KERNEL);
+	if (ret) {
+		kfree(attach);
+		return -ENOMEM;
+	}
+
+	rd = buf->dma_sgt->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);
+	}
+
+	attach->dma_dir = DMA_NONE;
+	dbuf_attach->priv = attach;
+
+	return 0;
+}
+
+static void vb2_dma_sg_dmabuf_ops_detach(struct dma_buf *dbuf,
+	struct dma_buf_attachment *db_attach)
+{
+	struct vb2_dma_sg_attachment *attach = db_attach->priv;
+	struct sg_table *sgt;
+
+	if (!attach)
+		return;
+
+	sgt = &attach->sgt;
+
+	/* release the scatterlist cache */
+	if (attach->dma_dir != DMA_NONE)
+		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
+			attach->dma_dir);
+	sg_free_table(sgt);
+	kfree(attach);
+	db_attach->priv = NULL;
+}
+
+static struct sg_table *vb2_dma_sg_dmabuf_ops_map(
+	struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir)
+{
+	struct vb2_dma_sg_attachment *attach = db_attach->priv;
+	/* stealing dmabuf mutex to serialize map/unmap operations */
+	struct mutex *lock = &db_attach->dmabuf->lock;
+	struct sg_table *sgt;
+	int ret;
+
+	mutex_lock(lock);
+
+	sgt = &attach->sgt;
+	/* return previously mapped sg table */
+	if (attach->dma_dir == dma_dir) {
+		mutex_unlock(lock);
+		return sgt;
+	}
+
+	/* release any previous cache */
+	if (attach->dma_dir != DMA_NONE) {
+		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
+			attach->dma_dir);
+		attach->dma_dir = DMA_NONE;
+	}
+
+	/* mapping to the client with new direction */
+	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dma_dir);
+	if (ret <= 0) {
+		pr_err("failed to map scatterlist\n");
+		mutex_unlock(lock);
+		return ERR_PTR(-EIO);
+	}
+
+	attach->dma_dir = dma_dir;
+
+	mutex_unlock(lock);
+
+	return sgt;
+}
+
+static void vb2_dma_sg_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
+	struct sg_table *sgt, enum dma_data_direction dma_dir)
+{
+	/* nothing to be done here */
+}
+
+static void vb2_dma_sg_dmabuf_ops_release(struct dma_buf *dbuf)
+{
+	/* drop reference obtained in vb2_dma_sg_get_dmabuf */
+	vb2_dma_sg_put(dbuf->priv);
+}
+
+static void *vb2_dma_sg_dmabuf_ops_kmap(struct dma_buf *dbuf, unsigned long pgnum)
+{
+	struct vb2_dma_sg_buf *buf = dbuf->priv;
+
+	return buf->vaddr ? buf->vaddr + pgnum * PAGE_SIZE : NULL;
+}
+
+static void *vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf)
+{
+	struct vb2_dma_sg_buf *buf = dbuf->priv;
+
+	return vb2_dma_sg_vaddr(buf);
+}
+
+static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
+	struct vm_area_struct *vma)
+{
+	return vb2_dma_sg_mmap(dbuf->priv, vma);
+}
+
+static struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
+	.attach = vb2_dma_sg_dmabuf_ops_attach,
+	.detach = vb2_dma_sg_dmabuf_ops_detach,
+	.map_dma_buf = vb2_dma_sg_dmabuf_ops_map,
+	.unmap_dma_buf = vb2_dma_sg_dmabuf_ops_unmap,
+	.kmap = vb2_dma_sg_dmabuf_ops_kmap,
+	.kmap_atomic = vb2_dma_sg_dmabuf_ops_kmap,
+	.vmap = vb2_dma_sg_dmabuf_ops_vmap,
+	.mmap = vb2_dma_sg_dmabuf_ops_mmap,
+	.release = vb2_dma_sg_dmabuf_ops_release,
+};
+
+static struct dma_buf *vb2_dma_sg_get_dmabuf(void *buf_priv, unsigned long flags)
+{
+	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct dma_buf *dbuf;
+
+	if (WARN_ON(!buf->dma_sgt))
+		return NULL;
+
+	dbuf = dma_buf_export(buf, &vb2_dma_sg_dmabuf_ops, buf->size, flags, NULL);
+	if (IS_ERR(dbuf))
+		return NULL;
+
+	/* dmabuf keeps reference to vb2 buffer */
+	atomic_inc(&buf->refcount);
+
+	return dbuf;
+}
+
+/*********************************************/
 /*       callbacks for DMABUF buffers        */
 /*********************************************/
 
@@ -521,6 +690,7 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
 	.vaddr		= vb2_dma_sg_vaddr,
 	.mmap		= vb2_dma_sg_mmap,
 	.num_users	= vb2_dma_sg_num_users,
+	.get_dmabuf	= vb2_dma_sg_get_dmabuf,
 	.map_dmabuf	= vb2_dma_sg_map_dmabuf,
 	.unmap_dmabuf	= vb2_dma_sg_unmap_dmabuf,
 	.attach_dmabuf	= vb2_dma_sg_attach_dmabuf,
-- 
2.1.1

[REVIEWv7 PATCH 09/12] vb2-vmalloc: add support for dmabuf exports

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

Add support for DMABUF exporting to the vb2-vmalloc implementation.

All memory models now have support for both importing and exporting of DMABUFs.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-vmalloc.c | 171 ++++++++++++++++++++++++++++
 1 file changed, 171 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index bba2460..3966b12 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -213,6 +213,176 @@ static int vb2_vmalloc_mmap(void *buf_priv, struct vm_area_struct *vma)
 }
 
 /*********************************************/
+/*         DMABUF ops for exporters          */
+/*********************************************/
+
+struct vb2_vmalloc_attachment {
+	struct sg_table sgt;
+	enum dma_data_direction dma_dir;
+};
+
+static int vb2_vmalloc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
+	struct dma_buf_attachment *dbuf_attach)
+{
+	struct vb2_vmalloc_attachment *attach;
+	struct vb2_vmalloc_buf *buf = dbuf->priv;
+	int num_pages = PAGE_ALIGN(buf->size) / PAGE_SIZE;
+	struct sg_table *sgt;
+	struct scatterlist *sg;
+	void *vaddr = buf->vaddr;
+	int ret;
+	int i;
+
+	attach = kzalloc(sizeof(*attach), GFP_KERNEL);
+	if (!attach)
+		return -ENOMEM;
+
+	sgt = &attach->sgt;
+	ret = sg_alloc_table(sgt, num_pages, GFP_KERNEL);
+	if (ret) {
+		kfree(attach);
+		return ret;
+	}
+	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+		struct page *page = vmalloc_to_page(vaddr);
+
+		if (!page) {
+			sg_free_table(sgt);
+			kfree(attach);
+			return -ENOMEM;
+		}
+		sg_set_page(sg, page, PAGE_SIZE, 0);
+		vaddr += PAGE_SIZE;
+	}
+
+	attach->dma_dir = DMA_NONE;
+	dbuf_attach->priv = attach;
+	return 0;
+}
+
+static void vb2_vmalloc_dmabuf_ops_detach(struct dma_buf *dbuf,
+	struct dma_buf_attachment *db_attach)
+{
+	struct vb2_vmalloc_attachment *attach = db_attach->priv;
+	struct sg_table *sgt;
+
+	if (!attach)
+		return;
+
+	sgt = &attach->sgt;
+
+	/* release the scatterlist cache */
+	if (attach->dma_dir != DMA_NONE)
+		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
+			attach->dma_dir);
+	sg_free_table(sgt);
+	kfree(attach);
+	db_attach->priv = NULL;
+}
+
+static struct sg_table *vb2_vmalloc_dmabuf_ops_map(
+	struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir)
+{
+	struct vb2_vmalloc_attachment *attach = db_attach->priv;
+	/* stealing dmabuf mutex to serialize map/unmap operations */
+	struct mutex *lock = &db_attach->dmabuf->lock;
+	struct sg_table *sgt;
+	int ret;
+
+	mutex_lock(lock);
+
+	sgt = &attach->sgt;
+	/* return previously mapped sg table */
+	if (attach->dma_dir == dma_dir) {
+		mutex_unlock(lock);
+		return sgt;
+	}
+
+	/* release any previous cache */
+	if (attach->dma_dir != DMA_NONE) {
+		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
+			attach->dma_dir);
+		attach->dma_dir = DMA_NONE;
+	}
+
+	/* mapping to the client with new direction */
+	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dma_dir);
+	if (ret <= 0) {
+		pr_err("failed to map scatterlist\n");
+		mutex_unlock(lock);
+		return ERR_PTR(-EIO);
+	}
+
+	attach->dma_dir = dma_dir;
+
+	mutex_unlock(lock);
+
+	return sgt;
+}
+
+static void vb2_vmalloc_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
+	struct sg_table *sgt, enum dma_data_direction dma_dir)
+{
+	/* nothing to be done here */
+}
+
+static void vb2_vmalloc_dmabuf_ops_release(struct dma_buf *dbuf)
+{
+	/* drop reference obtained in vb2_vmalloc_get_dmabuf */
+	vb2_vmalloc_put(dbuf->priv);
+}
+
+static void *vb2_vmalloc_dmabuf_ops_kmap(struct dma_buf *dbuf, unsigned long pgnum)
+{
+	struct vb2_vmalloc_buf *buf = dbuf->priv;
+
+	return buf->vaddr + pgnum * PAGE_SIZE;
+}
+
+static void *vb2_vmalloc_dmabuf_ops_vmap(struct dma_buf *dbuf)
+{
+	struct vb2_vmalloc_buf *buf = dbuf->priv;
+
+	return buf->vaddr;
+}
+
+static int vb2_vmalloc_dmabuf_ops_mmap(struct dma_buf *dbuf,
+	struct vm_area_struct *vma)
+{
+	return vb2_vmalloc_mmap(dbuf->priv, vma);
+}
+
+static struct dma_buf_ops vb2_vmalloc_dmabuf_ops = {
+	.attach = vb2_vmalloc_dmabuf_ops_attach,
+	.detach = vb2_vmalloc_dmabuf_ops_detach,
+	.map_dma_buf = vb2_vmalloc_dmabuf_ops_map,
+	.unmap_dma_buf = vb2_vmalloc_dmabuf_ops_unmap,
+	.kmap = vb2_vmalloc_dmabuf_ops_kmap,
+	.kmap_atomic = vb2_vmalloc_dmabuf_ops_kmap,
+	.vmap = vb2_vmalloc_dmabuf_ops_vmap,
+	.mmap = vb2_vmalloc_dmabuf_ops_mmap,
+	.release = vb2_vmalloc_dmabuf_ops_release,
+};
+
+static struct dma_buf *vb2_vmalloc_get_dmabuf(void *buf_priv, unsigned long flags)
+{
+	struct vb2_vmalloc_buf *buf = buf_priv;
+	struct dma_buf *dbuf;
+
+	if (WARN_ON(!buf->vaddr))
+		return NULL;
+
+	dbuf = dma_buf_export(buf, &vb2_vmalloc_dmabuf_ops, buf->size, flags, NULL);
+	if (IS_ERR(dbuf))
+		return NULL;
+
+	/* dmabuf keeps reference to vb2 buffer */
+	atomic_inc(&buf->refcount);
+
+	return dbuf;
+}
+
+/*********************************************/
 /*       callbacks for DMABUF buffers        */
 /*********************************************/
 
@@ -268,6 +438,7 @@ const struct vb2_mem_ops vb2_vmalloc_memops = {
 	.put		= vb2_vmalloc_put,
 	.get_userptr	= vb2_vmalloc_get_userptr,
 	.put_userptr	= vb2_vmalloc_put_userptr,
+	.get_dmabuf	= vb2_vmalloc_get_dmabuf,
 	.map_dmabuf	= vb2_vmalloc_map_dmabuf,
 	.unmap_dmabuf	= vb2_vmalloc_unmap_dmabuf,
 	.attach_dmabuf	= vb2_vmalloc_attach_dmabuf,
-- 
2.1.1

Re: [REVIEWv7 PATCH 09/12] vb2-vmalloc: add support for dmabuf exports

[Pawel Osciak]

On Tue, Nov 18, 2014 at 9:51 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hansverk@cisco.com>
>
> Add support for DMABUF exporting to the vb2-vmalloc implementation.
>
> All memory models now have support for both importing and exporting of DMABUFs.
>
> Signed-off-by: Hans Verkuil <hansverk@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  drivers/media/v4l2-core/videobuf2-vmalloc.c | 171 ++++++++++++++++++++++++++++
>  1 file changed, 171 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
> index bba2460..3966b12 100644
> --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
> @@ -213,6 +213,176 @@ static int vb2_vmalloc_mmap(void *buf_priv, struct vm_area_struct *vma)
>  }
>
>  /*********************************************/
> +/*         DMABUF ops for exporters          */
> +/*********************************************/
> +
> +struct vb2_vmalloc_attachment {
> +       struct sg_table sgt;
> +       enum dma_data_direction dma_dir;
> +};
> +
> +static int vb2_vmalloc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
> +       struct dma_buf_attachment *dbuf_attach)
> +{
> +       struct vb2_vmalloc_attachment *attach;
> +       struct vb2_vmalloc_buf *buf = dbuf->priv;
> +       int num_pages = PAGE_ALIGN(buf->size) / PAGE_SIZE;
> +       struct sg_table *sgt;
> +       struct scatterlist *sg;
> +       void *vaddr = buf->vaddr;
> +       int ret;
> +       int i;
> +
> +       attach = kzalloc(sizeof(*attach), GFP_KERNEL);
> +       if (!attach)
> +               return -ENOMEM;
> +
> +       sgt = &attach->sgt;
> +       ret = sg_alloc_table(sgt, num_pages, GFP_KERNEL);
> +       if (ret) {
> +               kfree(attach);
> +               return ret;
> +       }
> +       for_each_sg(sgt->sgl, sg, sgt->nents, i) {
> +               struct page *page = vmalloc_to_page(vaddr);
> +
> +               if (!page) {
> +                       sg_free_table(sgt);
> +                       kfree(attach);
> +                       return -ENOMEM;
> +               }
> +               sg_set_page(sg, page, PAGE_SIZE, 0);
> +               vaddr += PAGE_SIZE;
> +       }
> +
> +       attach->dma_dir = DMA_NONE;
> +       dbuf_attach->priv = attach;
> +       return 0;
> +}
> +
> +static void vb2_vmalloc_dmabuf_ops_detach(struct dma_buf *dbuf,
> +       struct dma_buf_attachment *db_attach)
> +{
> +       struct vb2_vmalloc_attachment *attach = db_attach->priv;
> +       struct sg_table *sgt;
> +
> +       if (!attach)
> +               return;
> +
> +       sgt = &attach->sgt;
> +
> +       /* release the scatterlist cache */
> +       if (attach->dma_dir != DMA_NONE)
> +               dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
> +                       attach->dma_dir);
> +       sg_free_table(sgt);
> +       kfree(attach);
> +       db_attach->priv = NULL;
> +}
> +
> +static struct sg_table *vb2_vmalloc_dmabuf_ops_map(
> +       struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir)
> +{
> +       struct vb2_vmalloc_attachment *attach = db_attach->priv;
> +       /* stealing dmabuf mutex to serialize map/unmap operations */
> +       struct mutex *lock = &db_attach->dmabuf->lock;
> +       struct sg_table *sgt;
> +       int ret;
> +
> +       mutex_lock(lock);
> +
> +       sgt = &attach->sgt;
> +       /* return previously mapped sg table */
> +       if (attach->dma_dir == dma_dir) {
> +               mutex_unlock(lock);
> +               return sgt;
> +       }
> +
> +       /* release any previous cache */
> +       if (attach->dma_dir != DMA_NONE) {
> +               dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
> +                       attach->dma_dir);
> +               attach->dma_dir = DMA_NONE;
> +       }
> +
> +       /* mapping to the client with new direction */
> +       ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dma_dir);
> +       if (ret <= 0) {
> +               pr_err("failed to map scatterlist\n");
> +               mutex_unlock(lock);
> +               return ERR_PTR(-EIO);
> +       }
> +
> +       attach->dma_dir = dma_dir;
> +
> +       mutex_unlock(lock);
> +
> +       return sgt;
> +}
> +
> +static void vb2_vmalloc_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
> +       struct sg_table *sgt, enum dma_data_direction dma_dir)
> +{
> +       /* nothing to be done here */
> +}
> +
> +static void vb2_vmalloc_dmabuf_ops_release(struct dma_buf *dbuf)
> +{
> +       /* drop reference obtained in vb2_vmalloc_get_dmabuf */
> +       vb2_vmalloc_put(dbuf->priv);
> +}
> +
> +static void *vb2_vmalloc_dmabuf_ops_kmap(struct dma_buf *dbuf, unsigned long pgnum)
> +{
> +       struct vb2_vmalloc_buf *buf = dbuf->priv;
> +
> +       return buf->vaddr + pgnum * PAGE_SIZE;
> +}
> +
> +static void *vb2_vmalloc_dmabuf_ops_vmap(struct dma_buf *dbuf)
> +{
> +       struct vb2_vmalloc_buf *buf = dbuf->priv;
> +
> +       return buf->vaddr;
> +}
> +
> +static int vb2_vmalloc_dmabuf_ops_mmap(struct dma_buf *dbuf,
> +       struct vm_area_struct *vma)
> +{
> +       return vb2_vmalloc_mmap(dbuf->priv, vma);
> +}
> +
> +static struct dma_buf_ops vb2_vmalloc_dmabuf_ops = {
> +       .attach = vb2_vmalloc_dmabuf_ops_attach,
> +       .detach = vb2_vmalloc_dmabuf_ops_detach,
> +       .map_dma_buf = vb2_vmalloc_dmabuf_ops_map,
> +       .unmap_dma_buf = vb2_vmalloc_dmabuf_ops_unmap,
> +       .kmap = vb2_vmalloc_dmabuf_ops_kmap,
> +       .kmap_atomic = vb2_vmalloc_dmabuf_ops_kmap,
> +       .vmap = vb2_vmalloc_dmabuf_ops_vmap,
> +       .mmap = vb2_vmalloc_dmabuf_ops_mmap,
> +       .release = vb2_vmalloc_dmabuf_ops_release,
> +};
> +
> +static struct dma_buf *vb2_vmalloc_get_dmabuf(void *buf_priv, unsigned long flags)
> +{
> +       struct vb2_vmalloc_buf *buf = buf_priv;
> +       struct dma_buf *dbuf;
> +
> +       if (WARN_ON(!buf->vaddr))
> +               return NULL;
> +
> +       dbuf = dma_buf_export(buf, &vb2_vmalloc_dmabuf_ops, buf->size, flags, NULL);
> +       if (IS_ERR(dbuf))
> +               return NULL;
> +
> +       /* dmabuf keeps reference to vb2 buffer */
> +       atomic_inc(&buf->refcount);
> +
> +       return dbuf;
> +}
> +
> +/*********************************************/
>  /*       callbacks for DMABUF buffers        */
>  /*********************************************/
>
> @@ -268,6 +438,7 @@ const struct vb2_mem_ops vb2_vmalloc_memops = {
>         .put            = vb2_vmalloc_put,
>         .get_userptr    = vb2_vmalloc_get_userptr,
>         .put_userptr    = vb2_vmalloc_put_userptr,
> +       .get_dmabuf     = vb2_vmalloc_get_dmabuf,
>         .map_dmabuf     = vb2_vmalloc_map_dmabuf,
>         .unmap_dmabuf   = vb2_vmalloc_unmap_dmabuf,
>         .attach_dmabuf  = vb2_vmalloc_attach_dmabuf,
> --
> 2.1.1
>



-- 
Best regards,
Pawel Osciak

[REVIEWv7 PATCH 10/12] vivid: enable vb2_expbuf support.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Now that vb2 supports DMABUF export for dma-sg and vmalloc memory
modes, we can enable the vb2_expbuf support in vivid.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/platform/vivid/vivid-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
index 686c3c2..f80d1ca 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -586,7 +586,7 @@ static const struct v4l2_ioctl_ops vivid_ioctl_ops = {
 	.vidioc_querybuf		= vb2_ioctl_querybuf,
 	.vidioc_qbuf			= vb2_ioctl_qbuf,
 	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
-/* Not yet	.vidioc_expbuf		= vb2_ioctl_expbuf,*/
+	.vidioc_expbuf			= vb2_ioctl_expbuf,
 	.vidioc_streamon		= vb2_ioctl_streamon,
 	.vidioc_streamoff		= vb2_ioctl_streamoff,
 
-- 
2.1.1

Re: [REVIEWv7 PATCH 10/12] vivid: enable vb2_expbuf support.

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Tuesday 18 November 2014 13:51:06 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Now that vb2 supports DMABUF export for dma-sg and vmalloc memory
> modes, we can enable the vb2_expbuf support in vivid.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Reviewed-by: Pawel Osciak <pawel@osciak.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/vivid/vivid-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vivid/vivid-core.c
> b/drivers/media/platform/vivid/vivid-core.c index 686c3c2..f80d1ca 100644
> --- a/drivers/media/platform/vivid/vivid-core.c
> +++ b/drivers/media/platform/vivid/vivid-core.c
> @@ -586,7 +586,7 @@ static const struct v4l2_ioctl_ops vivid_ioctl_ops = {
>  	.vidioc_querybuf		= vb2_ioctl_querybuf,
>  	.vidioc_qbuf			= vb2_ioctl_qbuf,
>  	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
> -/* Not yet	.vidioc_expbuf		= vb2_ioctl_expbuf,*/
> +	.vidioc_expbuf			= vb2_ioctl_expbuf,
>  	.vidioc_streamon		= vb2_ioctl_streamon,
>  	.vidioc_streamoff		= vb2_ioctl_streamoff,

-- 
Regards,

Laurent Pinchart

[REVIEWv7 PATCH 11/12] vim2m: support expbuf

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/platform/vim2m.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 87af47a..1105c11 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -697,6 +697,7 @@ static const struct v4l2_ioctl_ops vim2m_ioctl_ops = {
 	.vidioc_querybuf	= v4l2_m2m_ioctl_querybuf,
 	.vidioc_qbuf		= v4l2_m2m_ioctl_qbuf,
 	.vidioc_dqbuf		= v4l2_m2m_ioctl_dqbuf,
+	.vidioc_expbuf		= v4l2_m2m_ioctl_expbuf,
 
 	.vidioc_streamon	= v4l2_m2m_ioctl_streamon,
 	.vidioc_streamoff	= v4l2_m2m_ioctl_streamoff,
-- 
2.1.1

Re: [REVIEWv7 PATCH 11/12] vim2m: support expbuf

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Tuesday 18 November 2014 13:51:07 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Reviewed-by: Pawel Osciak <pawel@osciak.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/vim2m.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
> index 87af47a..1105c11 100644
> --- a/drivers/media/platform/vim2m.c
> +++ b/drivers/media/platform/vim2m.c
> @@ -697,6 +697,7 @@ static const struct v4l2_ioctl_ops vim2m_ioctl_ops = {
>  	.vidioc_querybuf	= v4l2_m2m_ioctl_querybuf,
>  	.vidioc_qbuf		= v4l2_m2m_ioctl_qbuf,
>  	.vidioc_dqbuf		= v4l2_m2m_ioctl_dqbuf,
> +	.vidioc_expbuf		= v4l2_m2m_ioctl_expbuf,
> 
>  	.vidioc_streamon	= v4l2_m2m_ioctl_streamon,
>  	.vidioc_streamoff	= v4l2_m2m_ioctl_streamoff,

-- 
Regards,

Laurent Pinchart

[REVIEWv7 PATCH 12/12] vb2: use dma_map_sg_attrs to prevent unnecessary sync

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

By default dma_map_sg syncs the mapped buffer to the device. But
buf_prepare expects a buffer syncs for the cpu and the buffer
will be synced to the device in the prepare memop.

The reverse is true for dma_unmap_sg, buf_finish and the finish
memop.

To prevent unnecessary syncs we ask dma_(un)map_sg to skip the
sync.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 24 +++++++++++++++----
 drivers/media/v4l2-core/videobuf2-dma-sg.c     | 33 +++++++++++++++++++++-----
 2 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 0bfc488..b481d20 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -511,7 +511,15 @@ static void vb2_dc_put_userptr(void *buf_priv)
 	struct sg_table *sgt = buf->dma_sgt;
 
 	if (sgt) {
-		dma_unmap_sg(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
+		DEFINE_DMA_ATTRS(attrs);
+
+		dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
+		/*
+		 * No need to sync to CPU, it's already synced to the CPU
+		 * since the finish() memop will have been called before this.
+		 */
+		dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
+				   buf->dma_dir, &attrs);
 		if (!vma_is_io(buf->vma))
 			vb2_dc_sgt_foreach_page(sgt, vb2_dc_put_dirty_page);
 
@@ -568,6 +576,9 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	struct sg_table *sgt;
 	unsigned long contig_size;
 	unsigned long dma_align = dma_get_cache_alignment();
+	DEFINE_DMA_ATTRS(attrs);
+
+	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
 
 	/* Only cache aligned DMA transfers are reliable */
 	if (!IS_ALIGNED(vaddr | size, dma_align)) {
@@ -654,8 +665,12 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	kfree(pages);
 	pages = NULL;
 
-	sgt->nents = dma_map_sg(buf->dev, sgt->sgl, sgt->orig_nents,
-		buf->dma_dir);
+	/*
+	 * No need to sync to the device, this will happen later when the
+	 * prepare() memop is called.
+	 */
+	sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
+				      buf->dma_dir, &attrs);
 	if (sgt->nents <= 0) {
 		pr_err("failed to map scatterlist\n");
 		ret = -EIO;
@@ -677,7 +692,8 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	return buf;
 
 fail_map_sg:
-	dma_unmap_sg(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
+	dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
+			   buf->dma_dir, &attrs);
 
 fail_sgt_init:
 	if (!vma_is_io(buf->vma))
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index c2ec2c4..d75fcf1 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -107,6 +107,9 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 	struct sg_table *sgt;
 	int ret;
 	int num_pages;
+	DEFINE_DMA_ATTRS(attrs);
+
+	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
 
 	if (WARN_ON(alloc_ctx == NULL))
 		return NULL;
@@ -140,9 +143,13 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
 	buf->dev = get_device(conf->dev);
 
 	sgt = &buf->sg_table;
-	if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
+	/*
+	 * No need to sync to the device, this will happen later when the
+	 * prepare() memop is called.
+	 */
+	if (dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
+			     buf->dma_dir, &attrs) == 0)
 		goto fail_map;
-	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 
 	buf->handler.refcount = &buf->refcount;
 	buf->handler.put = vb2_dma_sg_put;
@@ -175,9 +182,13 @@ static void vb2_dma_sg_put(void *buf_priv)
 	int i = buf->num_pages;
 
 	if (atomic_dec_and_test(&buf->refcount)) {
+		DEFINE_DMA_ATTRS(attrs);
+
+		dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
 		dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
 			buf->num_pages);
-		dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+		dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
+				   buf->dma_dir, &attrs);
 		if (buf->vaddr)
 			vm_unmap_ram(buf->vaddr, buf->num_pages);
 		sg_free_table(buf->dma_sgt);
@@ -227,6 +238,9 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	int num_pages_from_user;
 	struct vm_area_struct *vma;
 	struct sg_table *sgt;
+	DEFINE_DMA_ATTRS(attrs);
+
+	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
 
 	buf = kzalloc(sizeof *buf, GFP_KERNEL);
 	if (!buf)
@@ -294,9 +308,13 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		goto userptr_fail_alloc_table_from_pages;
 
 	sgt = &buf->sg_table;
-	if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
+	/*
+	 * No need to sync to the device, this will happen later when the
+	 * prepare() memop is called.
+	 */
+	if (dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
+			     buf->dma_dir, &attrs) == 0)
 		goto userptr_fail_map;
-	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 	return buf;
 
 userptr_fail_map:
@@ -325,10 +343,13 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
 	struct vb2_dma_sg_buf *buf = buf_priv;
 	struct sg_table *sgt = &buf->sg_table;
 	int i = buf->num_pages;
+	DEFINE_DMA_ATTRS(attrs);
+
+	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
 
 	dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
 	       __func__, buf->num_pages);
-	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+	dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir, &attrs);
 	if (buf->vaddr)
 		vm_unmap_ram(buf->vaddr, buf->num_pages);
 	sg_free_table(buf->dma_sgt);
-- 
2.1.1

Re: [REVIEWv7 PATCH 12/12] vb2: use dma_map_sg_attrs to prevent unnecessary sync

[Pawel Osciak]

On Tue, Nov 18, 2014 at 9:51 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> By default dma_map_sg syncs the mapped buffer to the device. But
> buf_prepare expects a buffer syncs for the cpu and the buffer
> will be synced to the device in the prepare memop.
>
> The reverse is true for dma_unmap_sg, buf_finish and the finish
> memop.
>
> To prevent unnecessary syncs we ask dma_(un)map_sg to skip the
> sync.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Pawel Osciak <pawel@osciak.com>

> ---
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 24 +++++++++++++++----
>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 33 +++++++++++++++++++++-----
>  2 files changed, 47 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> index 0bfc488..b481d20 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -511,7 +511,15 @@ static void vb2_dc_put_userptr(void *buf_priv)
>         struct sg_table *sgt = buf->dma_sgt;
>
>         if (sgt) {
> -               dma_unmap_sg(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
> +               DEFINE_DMA_ATTRS(attrs);
> +
> +               dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
> +               /*
> +                * No need to sync to CPU, it's already synced to the CPU
> +                * since the finish() memop will have been called before this.
> +                */
> +               dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
> +                                  buf->dma_dir, &attrs);
>                 if (!vma_is_io(buf->vma))
>                         vb2_dc_sgt_foreach_page(sgt, vb2_dc_put_dirty_page);
>
> @@ -568,6 +576,9 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
>         struct sg_table *sgt;
>         unsigned long contig_size;
>         unsigned long dma_align = dma_get_cache_alignment();
> +       DEFINE_DMA_ATTRS(attrs);
> +
> +       dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
>
>         /* Only cache aligned DMA transfers are reliable */
>         if (!IS_ALIGNED(vaddr | size, dma_align)) {
> @@ -654,8 +665,12 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
>         kfree(pages);
>         pages = NULL;
>
> -       sgt->nents = dma_map_sg(buf->dev, sgt->sgl, sgt->orig_nents,
> -               buf->dma_dir);
> +       /*
> +        * No need to sync to the device, this will happen later when the
> +        * prepare() memop is called.
> +        */
> +       sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
> +                                     buf->dma_dir, &attrs);
>         if (sgt->nents <= 0) {
>                 pr_err("failed to map scatterlist\n");
>                 ret = -EIO;
> @@ -677,7 +692,8 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
>         return buf;
>
>  fail_map_sg:
> -       dma_unmap_sg(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
> +       dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
> +                          buf->dma_dir, &attrs);
>
>  fail_sgt_init:
>         if (!vma_is_io(buf->vma))
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> index c2ec2c4..d75fcf1 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -107,6 +107,9 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
>         struct sg_table *sgt;
>         int ret;
>         int num_pages;
> +       DEFINE_DMA_ATTRS(attrs);
> +
> +       dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
>
>         if (WARN_ON(alloc_ctx == NULL))
>                 return NULL;
> @@ -140,9 +143,13 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
>         buf->dev = get_device(conf->dev);
>
>         sgt = &buf->sg_table;
> -       if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
> +       /*
> +        * No need to sync to the device, this will happen later when the
> +        * prepare() memop is called.
> +        */
> +       if (dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
> +                            buf->dma_dir, &attrs) == 0)
>                 goto fail_map;
> -       dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>
>         buf->handler.refcount = &buf->refcount;
>         buf->handler.put = vb2_dma_sg_put;
> @@ -175,9 +182,13 @@ static void vb2_dma_sg_put(void *buf_priv)
>         int i = buf->num_pages;
>
>         if (atomic_dec_and_test(&buf->refcount)) {
> +               DEFINE_DMA_ATTRS(attrs);
> +
> +               dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
>                 dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
>                         buf->num_pages);
> -               dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> +               dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
> +                                  buf->dma_dir, &attrs);
>                 if (buf->vaddr)
>                         vm_unmap_ram(buf->vaddr, buf->num_pages);
>                 sg_free_table(buf->dma_sgt);
> @@ -227,6 +238,9 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
>         int num_pages_from_user;
>         struct vm_area_struct *vma;
>         struct sg_table *sgt;
> +       DEFINE_DMA_ATTRS(attrs);
> +
> +       dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
>
>         buf = kzalloc(sizeof *buf, GFP_KERNEL);
>         if (!buf)
> @@ -294,9 +308,13 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
>                 goto userptr_fail_alloc_table_from_pages;
>
>         sgt = &buf->sg_table;
> -       if (dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) == 0)
> +       /*
> +        * No need to sync to the device, this will happen later when the
> +        * prepare() memop is called.
> +        */
> +       if (dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->nents,
> +                            buf->dma_dir, &attrs) == 0)
>                 goto userptr_fail_map;
> -       dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>         return buf;
>
>  userptr_fail_map:
> @@ -325,10 +343,13 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>         struct vb2_dma_sg_buf *buf = buf_priv;
>         struct sg_table *sgt = &buf->sg_table;
>         int i = buf->num_pages;
> +       DEFINE_DMA_ATTRS(attrs);
> +
> +       dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
>
>         dprintk(1, "%s: Releasing userspace buffer of %d pages\n",
>                __func__, buf->num_pages);
> -       dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> +       dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir, &attrs);
>         if (buf->vaddr)
>                 vm_unmap_ram(buf->vaddr, buf->num_pages);
>         sg_free_table(buf->dma_sgt);
> --
> 2.1.1
>



-- 
Best regards,
Pawel Osciak