* [PATCH] videobuf2: call buf_finish() on unprocessed buffers
@ 2011-07-14 21:09 Jonathan Corbet
2011-07-15 6:17 ` Marek Szyprowski
2011-07-24 22:10 ` Pawel Osciak
0 siblings, 2 replies; 4+ messages in thread
From: Jonathan Corbet @ 2011-07-14 21:09 UTC (permalink / raw)
To: linux-media; +Cc: Marek Szyprowski, Mauro Carvalho Chehab
When user space stops streaming, there may be buffers which have been given
to buf_prepare() and which may or may not have been passed to buf_queue().
The videobuf2 core simply takes those buffers back; if buf_prepare() does
work that needs cleaning up (like setting up a DMA mapping), that cleanup
will not happen.
This patch establishes a simple contract with drivers: buffers given to
buf_prepare() will eventually see a buf_finish() call.
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
drivers/media/video/videobuf2-core.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 6ba1461..2ba08ab 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -1177,6 +1177,7 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
*/
static void __vb2_queue_cancel(struct vb2_queue *q)
{
+ struct vb2_buffer *vb;
unsigned int i;
/*
@@ -1188,13 +1189,18 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
q->streaming = 0;
/*
- * Remove all buffers from videobuf's list...
+ * Remove all buffers from videobuf's list... Give the driver
+ * a chance to clean them up first, though.
*/
+ list_for_each_entry(vb, &q->queued_list, queued_entry)
+ call_qop(q, buf_finish, vb);
INIT_LIST_HEAD(&q->queued_list);
/*
* ...and done list; userspace will not receive any buffers it
* has not already dequeued before initiating cancel.
*/
+ list_for_each_entry(vb, &q->done_list, done_entry)
+ call_qop(q, buf_finish, vb);
INIT_LIST_HEAD(&q->done_list);
wake_up_all(&q->done_wq);
--
1.7.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH] videobuf2: call buf_finish() on unprocessed buffers
2011-07-14 21:09 [PATCH] videobuf2: call buf_finish() on unprocessed buffers Jonathan Corbet
@ 2011-07-15 6:17 ` Marek Szyprowski
2011-07-24 22:10 ` Pawel Osciak
1 sibling, 0 replies; 4+ messages in thread
From: Marek Szyprowski @ 2011-07-15 6:17 UTC (permalink / raw)
To: 'Jonathan Corbet', linux-media; +Cc: 'Mauro Carvalho Chehab'
Hello,
On Thursday, July 14, 2011 11:10 PM Jonathan Corbet wrote:
> When user space stops streaming, there may be buffers which have been given
> to buf_prepare() and which may or may not have been passed to buf_queue().
> The videobuf2 core simply takes those buffers back; if buf_prepare() does
> work that needs cleaning up (like setting up a DMA mapping), that cleanup
> will not happen.
>
> This patch establishes a simple contract with drivers: buffers given to
> buf_prepare() will eventually see a buf_finish() call.
>
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/media/video/videobuf2-core.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> index 6ba1461..2ba08ab 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -1177,6 +1177,7 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
> */
> static void __vb2_queue_cancel(struct vb2_queue *q)
> {
> + struct vb2_buffer *vb;
> unsigned int i;
>
> /*
> @@ -1188,13 +1189,18 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
> q->streaming = 0;
>
> /*
> - * Remove all buffers from videobuf's list...
> + * Remove all buffers from videobuf's list... Give the driver
> + * a chance to clean them up first, though.
> */
> + list_for_each_entry(vb, &q->queued_list, queued_entry)
> + call_qop(q, buf_finish, vb);
> INIT_LIST_HEAD(&q->queued_list);
> /*
> * ...and done list; userspace will not receive any buffers it
> * has not already dequeued before initiating cancel.
> */
> + list_for_each_entry(vb, &q->done_list, done_entry)
> + call_qop(q, buf_finish, vb);
> INIT_LIST_HEAD(&q->done_list);
> wake_up_all(&q->done_wq);
>
> --
> 1.7.6
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] videobuf2: call buf_finish() on unprocessed buffers
2011-07-14 21:09 [PATCH] videobuf2: call buf_finish() on unprocessed buffers Jonathan Corbet
2011-07-15 6:17 ` Marek Szyprowski
@ 2011-07-24 22:10 ` Pawel Osciak
2011-09-03 13:55 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 4+ messages in thread
From: Pawel Osciak @ 2011-07-24 22:10 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: linux-media, Marek Szyprowski, Mauro Carvalho Chehab
Hi Jon,
There is a misunderstanding here. This patch will call buf_finish for
some buffers twice. A buffer does not get removed from queued_list
even if it is on the done_list. A buffer gets into the queued_list on
qbuf, gets into the done_list after an operation is finished and is
removed from both lists on dqbuf. This means that you shouldn't call
buf_finish for buffers on both lists, only for all buffers in the
queued_list, as it is a superset of both.
--
Best regards,
Pawel Osciak
On Thu, Jul 14, 2011 at 14:09, Jonathan Corbet <corbet@lwn.net> wrote:
> When user space stops streaming, there may be buffers which have been given
> to buf_prepare() and which may or may not have been passed to buf_queue().
> The videobuf2 core simply takes those buffers back; if buf_prepare() does
> work that needs cleaning up (like setting up a DMA mapping), that cleanup
> will not happen.
>
> This patch establishes a simple contract with drivers: buffers given to
> buf_prepare() will eventually see a buf_finish() call.
>
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
> drivers/media/video/videobuf2-core.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
> index 6ba1461..2ba08ab 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -1177,6 +1177,7 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
> */
> static void __vb2_queue_cancel(struct vb2_queue *q)
> {
> + struct vb2_buffer *vb;
> unsigned int i;
>
> /*
> @@ -1188,13 +1189,18 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
> q->streaming = 0;
>
> /*
> - * Remove all buffers from videobuf's list...
> + * Remove all buffers from videobuf's list... Give the driver
> + * a chance to clean them up first, though.
> */
> + list_for_each_entry(vb, &q->queued_list, queued_entry)
> + call_qop(q, buf_finish, vb);
> INIT_LIST_HEAD(&q->queued_list);
> /*
> * ...and done list; userspace will not receive any buffers it
> * has not already dequeued before initiating cancel.
> */
> + list_for_each_entry(vb, &q->done_list, done_entry)
> + call_qop(q, buf_finish, vb);
> INIT_LIST_HEAD(&q->done_list);
> wake_up_all(&q->done_wq);
>
> --
> 1.7.6
>
> --
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] videobuf2: call buf_finish() on unprocessed buffers
2011-07-24 22:10 ` Pawel Osciak
@ 2011-09-03 13:55 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2011-09-03 13:55 UTC (permalink / raw)
To: Pawel Osciak; +Cc: Jonathan Corbet, linux-media, Marek Szyprowski
Em 24-07-2011 19:10, Pawel Osciak escreveu:
> Hi Jon,
> There is a misunderstanding here. This patch will call buf_finish for
> some buffers twice. A buffer does not get removed from queued_list
> even if it is on the done_list. A buffer gets into the queued_list on
> qbuf, gets into the done_list after an operation is finished and is
> removed from both lists on dqbuf. This means that you shouldn't call
> buf_finish for buffers on both lists, only for all buffers in the
> queued_list, as it is a superset of both.
Is there any update on this?
Thanks,
Mauro
> -- Best regards, Pawel Osciak On Thu, Jul 14, 2011 at 14:09, Jonathan Corbet <corbet@lwn.net> wrote:
>> > When user space stops streaming, there may be buffers which have been given
>> > to buf_prepare() and which may or may not have been passed to buf_queue().
>> > The videobuf2 core simply takes those buffers back; if buf_prepare() does
>> > work that needs cleaning up (like setting up a DMA mapping), that cleanup
>> > will not happen.
>> >
>> > This patch establishes a simple contract with drivers: buffers given to
>> > buf_prepare() will eventually see a buf_finish() call.
>> >
>> > Signed-off-by: Jonathan Corbet <corbet@lwn.net>
>> > ---
>> > drivers/media/video/videobuf2-core.c | 8 +++++++-
>> > 1 files changed, 7 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
>> > index 6ba1461..2ba08ab 100644
>> > --- a/drivers/media/video/videobuf2-core.c
>> > +++ b/drivers/media/video/videobuf2-core.c
>> > @@ -1177,6 +1177,7 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
>> > */
>> > static void __vb2_queue_cancel(struct vb2_queue *q)
>> > {
>> > + struct vb2_buffer *vb;
>> > unsigned int i;
>> >
>> > /*
>> > @@ -1188,13 +1189,18 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
>> > q->streaming = 0;
>> >
>> > /*
>> > - * Remove all buffers from videobuf's list...
>> > + * Remove all buffers from videobuf's list... Give the driver
>> > + * a chance to clean them up first, though.
>> > */
>> > + list_for_each_entry(vb, &q->queued_list, queued_entry)
>> > + call_qop(q, buf_finish, vb);
>> > INIT_LIST_HEAD(&q->queued_list);
>> > /*
>> > * ...and done list; userspace will not receive any buffers it
>> > * has not already dequeued before initiating cancel.
>> > */
>> > + list_for_each_entry(vb, &q->done_list, done_entry)
>> > + call_qop(q, buf_finish, vb);
>> > INIT_LIST_HEAD(&q->done_list);
>> > wake_up_all(&q->done_wq);
>> >
>> > --
>> > 1.7.6
>> >
>> > --
>> > 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
>> >
> --
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-09-03 13:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-14 21:09 [PATCH] videobuf2: call buf_finish() on unprocessed buffers Jonathan Corbet
2011-07-15 6:17 ` Marek Szyprowski
2011-07-24 22:10 ` Pawel Osciak
2011-09-03 13:55 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).