[media] vb2: remove TODO comment for dma-buf in QBUF
authorJavier Martinez Canillas <javier@osg.samsung.com>
Wed, 20 Jul 2016 16:18:25 +0000 (13:18 -0300)
committerMauro Carvalho Chehab <mchehab@s-opensource.com>
Wed, 24 Aug 2016 13:09:33 +0000 (10:09 -0300)
There is a TODO comment about the dma-buf being mapped in VIDIOC_QBUF
instead of doing it closer to when the actual DMA is going to happen
when the buffers are queued in the driver (i.e: __enqueue_in_driver).

But there is a reason to do it earlier in QBUF, and is that userspace
has no way to know if a exported dma-buf can be imported successfully
and so relies on QBUF succeeding as indication that the dma-buf mapped.

If QBUF fails, the application can fallback to another streaming I/O
method. But moving the dma-buf mapping later when queueing the buffers
can be too late for userspace to recover, since it may had dropped the
buffer(s) already when it knows that the dma-buf mapping failed.

So remove the TODO instead and change the comment to explain this.

Suggested-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
drivers/media/v4l2-core/videobuf2-core.c

index 3d59518..f50ff6f 100644 (file)
@@ -1273,9 +1273,10 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const void *pb)
                vb->planes[plane].mem_priv = mem_priv;
        }
 
-       /* TODO: This pins the buffer(s) with  dma_buf_map_attachment()).. but
-        * really we want to do this just before the DMA, not while queueing
-        * the buffer(s)..
+       /*
+        * This pins the buffer(s) with dma_buf_map_attachment()). It's done
+        * here instead just before the DMA, while queueing the buffer(s) so
+        * userspace knows sooner rather than later if the dma-buf map fails.
         */
        for (plane = 0; plane < vb->num_planes; ++plane) {
                ret = call_memop(vb, map_dmabuf, vb->planes[plane].mem_priv);