vb2: fix buf_init/buf_cleanup call sequences
authorHans Verkuil <hans.verkuil@cisco.com>
Wed, 29 Jan 2014 16:36:53 +0000 (13:36 -0300)
committerChanho Park <chanho61.park@samsung.com>
Tue, 18 Nov 2014 02:59:49 +0000 (11:59 +0900)
Ensure that these ops are properly balanced.

There are two scenarios:

1) for MMAP buf_init is called when the buffers are created and buf_cleanup
   must be called when the queue is finally freed. This scenario was always
   working.

2) for USERPTR and DMABUF it is more complicated. When a buffer is queued
   the code checks if all planes of this buffer have been acquired before.
   If that's the case, then only buf_prepare has to be called. Otherwise
   buf_cleanup needs to be called if the buffer was acquired before, then,
   once all changed planes have been (re)acquired, buf_init has to be
   called followed by buf_prepare. Should buf_prepare fail, then buf_cleanup
   must be called on the newly acquired planes to release them in.

Finally, in __vb2_queue_free we have to check if the buffer was actually
acquired before calling buf_cleanup. While that it always true for MMAP
mode, it is not necessarily true for the other modes. E.g. if you just
call REQBUFS and close the file handle, then buffers were never queued and
so no buf_init was ever called.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
[Backport from upstream to support dmabuf memory]
Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
Change-Id: I9a020bd5bca6376f70f3a9a85cce63ff0ff555ed

drivers/media/v4l2-core/videobuf2-core.c

index 2e85fb4..adce37c 100644 (file)
@@ -437,21 +437,14 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
 {
        unsigned int buffer;
 
-       /*
-        * Sanity check: when preparing a buffer the queue lock is released for
-        * a short while (see __buf_prepare for the details), which would allow
-        * a race with a reqbufs which can call this function. Removing the
-        * buffers from underneath __buf_prepare is obviously a bad idea, so we
-        * check if any of the buffers is in the state PREPARING, and if so we
-        * just return -EAGAIN.
-        */
-       for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
-            ++buffer) {
-               if (q->bufs[buffer] == NULL)
-                       continue;
-               if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
-                       dprintk(1, "preparing buffers, cannot free\n");
-                       return -EAGAIN;
+       /* Call driver-provided cleanup function for each buffer, if provided */
+       if (q->ops->buf_cleanup) {
+               for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
+                    ++buffer) {
+                       struct vb2_buffer *vb = q->bufs[buffer];
+
+                       if (vb && vb->planes[0].mem_priv)
+                               q->ops->buf_cleanup(vb);
                }
        }
 
@@ -1361,9 +1354,9 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
                if (vb->planes[plane].mem_priv) {
                        if (!reacquired) {
                                reacquired = true;
-                               call_void_vb_qop(vb, buf_cleanup, vb);
+                               q->ops->buf_cleanup(vb);
                        }
-                       call_void_memop(vb, put_userptr, vb->planes[plane].mem_priv);
+                       call_memop(q, put_userptr, vb->planes[plane].mem_priv);
                }
 
                vb->planes[plane].mem_priv = NULL;
@@ -1395,17 +1388,17 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
                 * the driver-specific initialization on the newly acquired
                 * buffer, if provided.
                 */
-               ret = call_vb_qop(vb, buf_init, vb);
+               ret = call_qop(q, buf_init, vb);
                if (ret) {
-                       dprintk(1, "buffer initialization failed\n");
+                       dprintk(1, "qbuf: buffer initialization failed\n");
                        goto err;
                }
        }
 
-       ret = call_vb_qop(vb, buf_prepare, vb);
+       ret = call_qop(q, buf_prepare, vb);
        if (ret) {
-               dprintk(1, "buffer preparation failed\n");
-               call_void_vb_qop(vb, buf_cleanup, vb);
+               dprintk(1, "qbuf: buffer preparation failed\n");
+               call_qop(q, buf_cleanup, vb);
                goto err;
        }
 
@@ -1424,6 +1417,19 @@ err:
 }
 
 /**
+ * __qbuf_mmap() - handle qbuf of an MMAP buffer
+ */
+static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
+{
+       int ret;
+       struct vb2_queue *q = vb->vb2_queue;
+
+       __fill_vb2_buffer(vb, b, vb->v4l2_planes);
+       ret = call_qop(q, buf_prepare, vb);
+       return ret;
+}
+
+/**
  * __qbuf_dmabuf() - handle qbuf of a DMABUF buffer
  */
 static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
@@ -1475,6 +1481,11 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
                        call_void_vb_qop(vb, buf_cleanup, vb);
                }
 
+               if (!reacquired) {
+                       reacquired = true;
+                       call_qop(q, buf_cleanup, vb);
+               }
+
                /* Release previously acquired memory if present */
                __vb2_plane_dmabuf_put(vb, &vb->planes[plane]);
                memset(&vb->v4l2_planes[plane], 0, sizeof(struct v4l2_plane));
@@ -1519,17 +1530,17 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
                 * Call driver-specific initialization on the newly acquired buffer,
                 * if provided.
                 */
-               ret = call_vb_qop(vb, buf_init, vb);
+               ret = call_qop(q, buf_init, vb);
                if (ret) {
-                       dprintk(1, "buffer initialization failed\n");
+                       dprintk(1, "qbuf: buffer initialization failed\n");
                        goto err;
                }
        }
 
-       ret = call_vb_qop(vb, buf_prepare, vb);
+       ret = call_qop(q, buf_prepare, vb);
        if (ret) {
-               dprintk(1, "buffer preparation failed\n");
-               call_void_vb_qop(vb, buf_cleanup, vb);
+               dprintk(1, "qbuf: buffer preparation failed\n");
+               call_qop(q, buf_cleanup, vb);
                goto err;
        }