| Message ID | 1391093491-23077-8-git-send-email-hverkuil@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/30/2014 03:51 PM, Hans Verkuil wrote: > From: Hans Verkuil <hans.verkuil@cisco.com> > > This new op is called after stop_streaming() or after a failed call to > start_streaming(). The driver needs to dequeue any pending active buffers > it got from the buf_queue() callback. > > The reason this op was added is that stop_streaming() traditionally dequeued > any pending active buffers after stopping the DMA engine. However, > stop_streaming() is never called if start_streaming() fails, even though any > prequeued buffers have been passed on to the driver. In that case those > pending active buffers may still be in the driver's active buffer list, > which can cause all sorts of problems if they are not removed. > > By splitting stop_streaming into stop_streaming (i.e. stop the DMA engine) > and reinit_streaming (i.e. reinitialize the buffer lists) this problem is > solved. After calling reinit_streaming() the vb2 core will also call > vb2_buffer_done() for any remaining active buffers. No need to review patches 7+8: this is going to change. I had a useful discussion with Pawel regarding this and we came up with a better solution. Regards, Hans > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/media/v4l2-core/videobuf2-core.c | 13 +++++++++++-- > include/media/videobuf2-core.h | 9 +++++++-- > 2 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c > index a3b4b4c..3030ef6 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -395,9 +395,9 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) > if (unbalanced || debug) { > pr_info("vb2: counters for queue %p:%s\n", q, > unbalanced ? " UNBALANCED!" : ""); > - pr_info("vb2: setup: %u start_streaming: %u stop_streaming: %u\n", > + pr_info("vb2: setup: %u start_streaming: %u stop_streaming: %u reinit_streaming: %u\n", > q->cnt_queue_setup, q->cnt_start_streaming, > - q->cnt_stop_streaming); > + q->cnt_stop_streaming, q->cnt_reinit_streaming); > pr_info("vb2: wait_prepare: %u wait_finish: %u\n", > q->cnt_wait_prepare, q->cnt_wait_finish); > } > @@ -406,6 +406,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) > q->cnt_wait_finish = 0; > q->cnt_start_streaming = 0; > q->cnt_stop_streaming = 0; > + q->cnt_reinit_streaming = 0; > } > for (buffer = 0; buffer < q->num_buffers; ++buffer) { > struct vb2_buffer *vb = q->bufs[buffer]; > @@ -1900,7 +1901,15 @@ static void __vb2_queue_cancel(struct vb2_queue *q) > */ > if (q->streaming) > call_qop(q, stop_streaming, q); > + > q->streaming = 0; > + if (atomic_read(&q->queued_count)) { > + call_qop(q, reinit_streaming, q); > + > + for (i = 0; i < q->num_buffers; ++i) > + if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) > + vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR); > + } > > /* > * Remove all buffers from videobuf's list... > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 82b7f0f..b40dfbc 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -294,8 +294,11 @@ struct vb2_buffer { > * buffer is queued. > * @stop_streaming: called when 'streaming' state must be disabled; driver > * should stop any DMA transactions or wait until they > - * finish and give back all buffers it got from buf_queue() > - * callback; may use vb2_wait_for_all_buffers() function > + * finish; may use vb2_wait_for_all_buffers() function. > + * @reinit_streaming: called after stop_streaming() or after a failed call to > + * start_streaming(). The driver needs to dequeue any > + * pending active buffers it got from the buf_queue() > + * callback. > * @buf_queue: passes buffer vb to the driver; driver may start > * hardware operation on this buffer; driver should give > * the buffer back by calling vb2_buffer_done() function; > @@ -318,6 +321,7 @@ struct vb2_ops { > > int (*start_streaming)(struct vb2_queue *q, unsigned int count); > int (*stop_streaming)(struct vb2_queue *q); > + void (*reinit_streaming)(struct vb2_queue *q); > > void (*buf_queue)(struct vb2_buffer *vb); > }; > @@ -408,6 +412,7 @@ struct vb2_queue { > u32 cnt_wait_finish; > u32 cnt_start_streaming; > u32 cnt_stop_streaming; > + u32 cnt_reinit_streaming; > #endif > }; > > -- 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
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index a3b4b4c..3030ef6 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -395,9 +395,9 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) if (unbalanced || debug) { pr_info("vb2: counters for queue %p:%s\n", q, unbalanced ? " UNBALANCED!" : ""); - pr_info("vb2: setup: %u start_streaming: %u stop_streaming: %u\n", + pr_info("vb2: setup: %u start_streaming: %u stop_streaming: %u reinit_streaming: %u\n", q->cnt_queue_setup, q->cnt_start_streaming, - q->cnt_stop_streaming); + q->cnt_stop_streaming, q->cnt_reinit_streaming); pr_info("vb2: wait_prepare: %u wait_finish: %u\n", q->cnt_wait_prepare, q->cnt_wait_finish); } @@ -406,6 +406,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) q->cnt_wait_finish = 0; q->cnt_start_streaming = 0; q->cnt_stop_streaming = 0; + q->cnt_reinit_streaming = 0; } for (buffer = 0; buffer < q->num_buffers; ++buffer) { struct vb2_buffer *vb = q->bufs[buffer]; @@ -1900,7 +1901,15 @@ static void __vb2_queue_cancel(struct vb2_queue *q) */ if (q->streaming) call_qop(q, stop_streaming, q); + q->streaming = 0; + if (atomic_read(&q->queued_count)) { + call_qop(q, reinit_streaming, q); + + for (i = 0; i < q->num_buffers; ++i) + if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) + vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR); + } /* * Remove all buffers from videobuf's list... diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 82b7f0f..b40dfbc 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -294,8 +294,11 @@ struct vb2_buffer { * buffer is queued. * @stop_streaming: called when 'streaming' state must be disabled; driver * should stop any DMA transactions or wait until they - * finish and give back all buffers it got from buf_queue() - * callback; may use vb2_wait_for_all_buffers() function + * finish; may use vb2_wait_for_all_buffers() function. + * @reinit_streaming: called after stop_streaming() or after a failed call to + * start_streaming(). The driver needs to dequeue any + * pending active buffers it got from the buf_queue() + * callback. * @buf_queue: passes buffer vb to the driver; driver may start * hardware operation on this buffer; driver should give * the buffer back by calling vb2_buffer_done() function; @@ -318,6 +321,7 @@ struct vb2_ops { int (*start_streaming)(struct vb2_queue *q, unsigned int count); int (*stop_streaming)(struct vb2_queue *q); + void (*reinit_streaming)(struct vb2_queue *q); void (*buf_queue)(struct vb2_buffer *vb); }; @@ -408,6 +412,7 @@ struct vb2_queue { u32 cnt_wait_finish; u32 cnt_start_streaming; u32 cnt_stop_streaming; + u32 cnt_reinit_streaming; #endif };