On 07/03/2015 03:28 PM, Sakari Ailus wrote: > Hi Hans, > > Hans Verkuil wrote: >> On 07/03/2015 02:47 PM, Sakari Ailus wrote: >>> Buffers can be returned back to videobuf2 in driver's streamon handler. In >>> this case vb2_buffer_done() with buffer state VB2_BUF_STATE_QUEUED will >>> cause the driver's buf_queue vb2 operation to be called, queueing the same >>> buffer again only to be returned to videobuf2 using vb2_buffer_done() and so >>> on. >>> >>> Add a new buffer state VB2_BUF_STATE_REQUEUEING which, when used as the >> >> It's spelled as requeuing (no e). The verb is 'to queue', but the -ing form is >> queuing. Check the dictionary: http://dictionary.reference.com/browse/queuing > > My dictionary disagrees with yours. :-) > > http://dictionary.cambridge.org/dictionary/british/queue?q=queueing $ git grep -i queueing|wc 655 5660 54709 $ git grep -i queuing|wc 650 5623 55249 That's not helpful either... On the other hand: $ git grep -i queuing drivers/media/|wc 19 200 1846 $ git grep -i queueing drivers/media/|wc 2 25 203 Within drivers/media there seems to be a clear preference for queuing :-) But yes, both spellings are OK, but for me queueing has way too many vowels! And interestingly, my spellchecker thinks queueing is wrong. > >> >>> state argument to vb2_buffer_done(), will result in buffers queued to the >>> driver. Using VB2_BUF_STATE_QUEUED will leave the buffer to videobuf2, as it >>> was before "[media] vb2: allow requeuing buffers while streaming". >>> >>> Fixes: ce0eff016f72 ("[media] vb2: allow requeuing buffers while streaming") >>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> >>> Cc: stable@xxxxxxxxxxxxxxx # for v4.1 >>> --- >>> since v1: >>> >>> - Instead of relying on q->start_streaming_called and q->streaming, add a >>> new buffer state VB2_BUF_STATE_REQUEUEING, as suggested by Hans. The >>> cobalt driver will need the new flag as it returns the buffer back to the >>> driver's queue, the rest will continue using VB2_BUF_STATE_QUEUED. >>> >>> drivers/media/pci/cobalt/cobalt-irq.c | 2 +- >>> drivers/media/v4l2-core/videobuf2-core.c | 15 +++++++++++---- >>> include/media/videobuf2-core.h | 2 ++ >>> 3 files changed, 14 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/media/pci/cobalt/cobalt-irq.c b/drivers/media/pci/cobalt/cobalt-irq.c >>> index e18f49e..2687cb0 100644 >>> --- a/drivers/media/pci/cobalt/cobalt-irq.c >>> +++ b/drivers/media/pci/cobalt/cobalt-irq.c >>> @@ -134,7 +134,7 @@ done: >>> also know about dropped frames. */ >>> cb->vb.v4l2_buf.sequence = s->sequence++; >>> vb2_buffer_done(&cb->vb, (skip || s->unstable_frame) ? >>> - VB2_BUF_STATE_QUEUED : VB2_BUF_STATE_DONE); >>> + VB2_BUF_STATE_REQUEUEING : VB2_BUF_STATE_DONE); >>> } >>> >>> irqreturn_t cobalt_irq_handler(int irq, void *dev_id) >>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c >>> index 1a096a6..ca8c041 100644 >>> --- a/drivers/media/v4l2-core/videobuf2-core.c >>> +++ b/drivers/media/v4l2-core/videobuf2-core.c >>> @@ -1182,7 +1182,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) >>> >>> if (WARN_ON(state != VB2_BUF_STATE_DONE && >>> state != VB2_BUF_STATE_ERROR && >>> - state != VB2_BUF_STATE_QUEUED)) >>> + state != VB2_BUF_STATE_QUEUED && >>> + state != VB2_BUF_STATE_REQUEUEING)) >>> state = VB2_BUF_STATE_ERROR; >>> >>> #ifdef CONFIG_VIDEO_ADV_DEBUG >>> @@ -1199,15 +1200,21 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) >>> for (plane = 0; plane < vb->num_planes; ++plane) >>> call_void_memop(vb, finish, vb->planes[plane].mem_priv); >>> >>> - /* Add the buffer to the done buffers list */ >>> spin_lock_irqsave(&q->done_lock, flags); >>> - vb->state = state; >>> - if (state != VB2_BUF_STATE_QUEUED) >>> + if (state == VB2_BUF_STATE_QUEUED || >>> + state == VB2_BUF_STATE_REQUEUEING) { >>> + vb->state = VB2_BUF_STATE_QUEUED; >>> + } else { >>> + /* Add the buffer to the done buffers list */ >>> list_add_tail(&vb->done_entry, &q->done_list); >>> + vb->state = state; >>> + } >>> atomic_dec(&q->owned_by_drv_count); >>> spin_unlock_irqrestore(&q->done_lock, flags); >>> >>> if (state == VB2_BUF_STATE_QUEUED) { >>> + return; >>> + } else if (state == VB2_BUF_STATE_REQUEUEING) { >> >> No 'else' is needed here since the 'if' just returns. > > Will fix. > Regards, Hans -- 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