diff mbox

[4/4] v4l2-mem2mem: allow reqbufs(0) with "in use" MMAP buffers

Message ID 1394578325-11298-5-git-send-email-sheu@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

John Sheu March 11, 2014, 10:52 p.m. UTC
v4l2-mem2mem presently does not allow VIDIOC_REQBUFS to destroy
outstanding buffers if the queue is of type V4L2_MEMORY_MMAP, and if the
buffers are considered "in use".  This is different behavior than for
other memory types, and prevents us for deallocating buffers in a few
cases:

* In the case that there are outstanding mmap()ed views on the buffer,
  refcounting on the videobuf2 buffer backing the vm_area will track
  lifetime appropriately,
* In the case that the buffer has been exported as a DMABUF, refcounting
  on the videobuf2 bufer backing the DMABUF will track lifetime
  appropriately.

Remove the specific check for type V4L2_MEMOMRY_MMAP when freeing all
buffers through VIDIOC_REQBUFS.

Signed-off-by: John Sheu <sheu@google.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 26 +-------------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

Comments

Kamil Debski April 7, 2014, 2:41 p.m. UTC | #1
Pawel, Marek,

Before taking this to my tree I wanted to get an ACK from one of the
videobuf2 maintainers. Could you spare a moment to look through this
patch?

Best wishes,
Marek Szyprowski April 8, 2014, 10:51 a.m. UTC | #2
Hello,

On 2014-04-07 16:41, Kamil Debski wrote:
> Pawel, Marek,
>
> Before taking this to my tree I wanted to get an ACK from one of the
> videobuf2 maintainers. Could you spare a moment to look through this
> patch?

It's not a bug, it is a feature. This was one of the fundamental design 
requirements to allow applications to track if the memory is used or not.

Best regards
Nicolas Dufresne Nov. 2, 2015, 6:31 p.m. UTC | #3
Le mardi 08 avril 2014 à 12:51 +0200, Marek Szyprowski a écrit :
> Hello,
> 
> On 2014-04-07 16:41, Kamil Debski wrote:
> > Pawel, Marek,
> > 
> > Before taking this to my tree I wanted to get an ACK from one of
> > the
> > videobuf2 maintainers. Could you spare a moment to look through
> > this
> > patch?
> 
> It's not a bug, it is a feature. This was one of the fundamental
> design 
> requirements to allow applications to track if the memory is used or
> not.

I still have the impression it is not fully correct for the case
buffers are exported using DMABUF. Like if the dmabuf was owning the
vb2 buffer instead of the opposite ...

Nicolas
Jean-Michel Hautbois Nov. 11, 2015, 9:29 p.m. UTC | #4
Hi,

2015-11-02 19:31 GMT+01:00 Nicolas Dufresne <nicolas.dufresne@collabora.com>:
> Le mardi 08 avril 2014 à 12:51 +0200, Marek Szyprowski a écrit :
>> Hello,
>>
>> On 2014-04-07 16:41, Kamil Debski wrote:
>> > Pawel, Marek,
>> >
>> > Before taking this to my tree I wanted to get an ACK from one of
>> > the
>> > videobuf2 maintainers. Could you spare a moment to look through
>> > this
>> > patch?
>>
>> It's not a bug, it is a feature. This was one of the fundamental
>> design
>> requirements to allow applications to track if the memory is used or
>> not.
>
> I still have the impression it is not fully correct for the case
> buffers are exported using DMABUF. Like if the dmabuf was owning the
> vb2 buffer instead of the opposite ...

I am observing this behaviour too... Tried it, but seems to not do the
job on dmabuf buffers... with gstreamer at least ;-).

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

Patch

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 8e6695c9..5b6f9da6 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -414,8 +414,7 @@  static int __verify_length(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 }
 
 /**
- * __buffer_in_use() - return true if the buffer is in use and
- * the queue cannot be freed (by the means of REQBUFS(0)) call
+ * __buffer_in_use() - return true if the buffer is in use.
  */
 static bool __buffer_in_use(struct vb2_queue *q, struct vb2_buffer *vb)
 {
@@ -435,20 +434,6 @@  static bool __buffer_in_use(struct vb2_queue *q, struct vb2_buffer *vb)
 }
 
 /**
- * __buffers_in_use() - return true if any buffers on the queue are in use and
- * the queue cannot be freed (by the means of REQBUFS(0)) call
- */
-static bool __buffers_in_use(struct vb2_queue *q)
-{
-	unsigned int buffer;
-	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
-		if (__buffer_in_use(q, q->bufs[buffer]))
-			return true;
-	}
-	return false;
-}
-
-/**
  * __fill_v4l2_buffer() - fill in a struct v4l2_buffer with information to be
  * returned to userspace
  */
@@ -681,15 +666,6 @@  static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	}
 
 	if (req->count == 0 || q->num_buffers != 0 || q->memory != req->memory) {
-		/*
-		 * We already have buffers allocated, so first check if they
-		 * are not in use and can be freed.
-		 */
-		if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
-			dprintk(1, "reqbufs: memory in use, cannot free\n");
-			return -EBUSY;
-		}
-
 		ret = __vb2_queue_free(q, q->num_buffers);
 		if (ret)
 			return ret;