RE: [PATCH 1/5] [media] vb2: redesign the stop_streaming() callback and make it obligatory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello,

On Wednesday, April 06, 2011 5:03 AM Pawel Osciak wrote:

> 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.

Yes, I must have been blind or really tired that I've missed it. I'm sorry 
for the confusion.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center


--
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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux