On 04/06/2012 03:29 PM, Laurent Pinchart wrote:
Hi Tomasz,
On Thursday 05 April 2012 16:00:00 Tomasz Stanislawski wrote:
From: Sumit Semwal sumit.semwal@ti.com
This patch adds support for DMABUF memory type in videobuf2. It calls relevant APIs of dma_buf for v4l reqbuf / qbuf / dqbuf operations.
For this version, the support is for videobuf2 as a user of the shared buffer; so the allocation of the buffer is done outside of V4L2. [A sample allocator of dma-buf shared buffer is given at [1]]
[1]: Rob Clark's DRM: https://github.com/robclark/kernel-omap4/commits/drmplane-dmabuf
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com [original work in the PoC for buffer sharing] Signed-off-by: Sumit Semwal sumit.semwal@ti.com Signed-off-by: Sumit Semwal sumit.semwal@linaro.org
drivers/media/video/videobuf2-core.c | 184 ++++++++++++++++++++++++++++++- include/media/videobuf2-core.h | 31 ++++++ 2 files changed, 214 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index 2e8f1df..b37feea 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c
[snip]
@@ -451,6 +482,21 @@ static int __verify_mmap_ops(struct vb2_queue *q) }
/**
- __verify_dmabuf_ops() - verify that all memory operations required for
- DMABUF queue type have been provided
- */
+static int __verify_dmabuf_ops(struct vb2_queue *q) +{
- if (!(q->io_modes & VB2_DMABUF) || !q->mem_ops->attach_dmabuf
|| !q->mem_ops->detach_dmabuf|| !q->mem_ops->map_dmabuf|| !q->mem_ops->unmap_dmabuf)That's pretty strange indentation. What about
if (!(q->io_modes & VB2_DMABUF) || !q->mem_ops->attach_dmabuf || !q->mem_ops->detach_dmabuf || !q->mem_ops->map_dmabuf || !q->mem_ops->unmap_dmabuf)
ok
return -EINVAL;- return 0;
+}
+/**
- vb2_reqbufs() - Initiate streaming
- @q: videobuf2 queue
- @req: struct passed from userspace to vidioc_reqbufs handler in driver
@@ -484,6 +530,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) }
if (req->memory != V4L2_MEMORY_MMAP
&& req->memory != V4L2_MEMORY_DMABUF && req->memory != V4L2_MEMORY_USERPTR) {Ditto.
[snip]
@@ -620,7 +672,8 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) }
if (create->memory != V4L2_MEMORY_MMAP
&& create->memory != V4L2_MEMORY_USERPTR) {
&& create->memory != V4L2_MEMORY_USERPTR&& create->memory != V4L2_MEMORY_DMABUF) {And here too.
[snip]
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index a15d1f1..665e846 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h
[snip]
@@ -65,6 +82,17 @@ struct vb2_mem_ops { unsigned long size, int write); void (*put_userptr)(void *buf_priv);
- /*
* Comment from Rob Clark: XXX: I think the attach / detach could be* handled in the vb2 core, and vb2_mem_ops really just need to get/put* the sglist (and make sure that the sglist fits it's needs..)*/I think we should address this now. We've previously discussed the question, but haven't reached an agreement.
Quoting my reply to "[RFCv2 PATCH 3/9] v4l: vb2: Add dma-contig allocator as dma_buf user" on 28/03/2012:
Calling dma_buf_attach could be moved to vb2-core. But it gives little gain. First, the pointer of dma_buf_attachment would have to be added to struct vb2_plane. Second, the allocator would have to keep in the copy of this pointer in its context structure for use of vb2_dc_(un)map_dmabuf functions.
Right. Would it make sense to pass the vb2_plane pointer, or possibly the dma_buf_attachment pointer, to the mmap_dmabuf and unmap_dmabuf operations ?
Third, dma_buf_attach requires a pointer to 'struct device' which is not available in the vb2-core layer.
OK, that's a problem.
Because of the mentioned reasons I decided to keep attach_dmabuf in allocator-specific code.
Maybe it would make sense to create a vb2_mem_buf structure from which vb2_dc_buf (and other allocator-specific buffer descriptors) would inherit ? That structure would store the dma_buf_attach pointer, and common dma-buf code could be put in videobuf2-memops.c and shared between allocators. Just an idea.
If we find out that the best course of action is to leave the code as-is, we should remove the above comment.
Ok. Adding support for VIVI introduces new problem. This driver is not associated with any struct device. Therefore attach_dmabuf for vmalloc allocator must not call dma_buf_attach because this call fails if the device is NULL.
Hiding (non)calling dma_buf_attach inside allocator code really helps.
- void *(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf,
unsigned long size, int write);- void (*detach_dmabuf)(void *buf_priv);
- int (*map_dmabuf)(void *buf_priv);
- void (*unmap_dmabuf)(void *buf_priv);
- void *(*vaddr)(void *buf_priv); void *(*cookie)(void *buf_priv);
Regards, Tomasz Stanislawski