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]

 



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.

True, setting queued_count to 0 is not enough. Hm, I'm wondering why
tests on vivi and qv4l2 didn't catch this, it uses poll as well...

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


[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