Hi again Marek On Sun, Apr 3, 2011 at 22:49, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: > Hello, > > On Monday, April 04, 2011 1:51 AM Pawel Osciak wrote: > >> Drivers are now required to implement the stop_streaming() callback >> to ensure that all ongoing hardware operations are finished and their >> ownership of buffers is ceded. >> Drivers do not have to call vb2_buffer_done() for each buffer they own >> anymore. >> Also remove the return value from the callback. >> >> Signed-off-by: Pawel Osciak <pawel@xxxxxxxxxx> >> --- >> drivers/media/video/videobuf2-core.c | 16 ++++++++++++++-- >> include/media/videobuf2-core.h | 16 +++++++--------- >> 2 files changed, 21 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c >> index 6e69584..59d5e8b 100644 >> --- a/drivers/media/video/videobuf2-core.c >> +++ b/drivers/media/video/videobuf2-core.c >> @@ -640,6 +640,9 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) >> struct vb2_queue *q = vb->vb2_queue; >> unsigned long flags; >> >> + if (atomic_read(&q->queued_count) == 0) >> + return; >> + >> if (vb->state != VB2_BUF_STATE_ACTIVE) >> return; >> >> @@ -1178,12 +1181,20 @@ static void __vb2_queue_cancel(struct vb2_queue *q) >> unsigned int i; >> >> /* >> - * Tell driver to stop all transactions and release all queued >> + * Tell the driver to stop all transactions and release all queued >> * buffers. >> */ >> if (q->streaming) >> call_qop(q, stop_streaming, q); >> + >> + /* >> + * All buffers should now not be in use by the driver anymore, but we >> + * have to manually set queued_count to 0, as the driver was not >> + * required to call vb2_buffer_done() from stop_streaming() for all >> + * buffers it had queued. >> + */ >> q->streaming = 0; >> + atomic_set(&q->queued_count, 0); > > If you removed the need to call vb2_buffer_done() then you must also call > wake_up_all(&q->done_wq) to wake any other threads/processes that might be > sleeping waiting for buffers. You made me doubt myself for a second there, but the patch is correct. There is a call to wake_up_all a few lines below this. -- Best regards, Pawel Osciak -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html