On 4/30/22 11:50 AM, Hans Verkuil wrote:
> Add support for two new (un)prepare_streaming queue ops that are called
> when the user calls VIDIOC_STREAMON and STREAMOFF (or closes the fh).
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
> This has only been compile tested. Eugen, can you try this with the
> atmel-isc series?
Hello Hans, Laurent,
I tested this on top of my series (v10 which I sent recently) and then
with a patch on top to move pipeline validation to prepare/unprepare
streaming (which I will be sending on the ML right away), and I have not
seen any problems. It works fine with my driver in capturing, messing
the pipeline and then checking it again, returns -EPIPE as expected.
Let me know if you have in mind any specific scenario you would like to
test ?
Tested-by: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>
Eugen
>
> The kerneldoc documentation in core.h can be better, but that's why this
> is an RFC :-)
>
> Changes since v1: fixed 'unbalanced' assignment in __vb2_queue_free,
> replacing a ';' with '||'.
> ---
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index b203c1e26353..7fd38fc4b9e2 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -544,6 +544,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
> */
> if (q->num_buffers) {
> bool unbalanced = q->cnt_start_streaming != q->cnt_stop_streaming ||
> + q->cnt_prepare_streaming != q->cnt_unprepare_streaming ||
> q->cnt_wait_prepare != q->cnt_wait_finish;
>
> if (unbalanced || debug) {
> @@ -552,14 +553,18 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
> pr_info(" setup: %u start_streaming: %u stop_streaming: %u\n",
> q->cnt_queue_setup, q->cnt_start_streaming,
> q->cnt_stop_streaming);
> + pr_info(" prepare_streaming: %u unprepare_streaming: %u\n",
> + q->cnt_prepare_streaming, q->cnt_unprepare_streaming);
> pr_info(" wait_prepare: %u wait_finish: %u\n",
> q->cnt_wait_prepare, q->cnt_wait_finish);
> }
> q->cnt_queue_setup = 0;
> q->cnt_wait_prepare = 0;
> q->cnt_wait_finish = 0;
> + q->cnt_prepare_streaming = 0;
> q->cnt_start_streaming = 0;
> q->cnt_stop_streaming = 0;
> + q->cnt_unprepare_streaming = 0;
> }
> for (buffer = 0; buffer < q->num_buffers; ++buffer) {
> struct vb2_buffer *vb = q->bufs[buffer];
> @@ -1991,6 +1996,9 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
> if (q->start_streaming_called)
> call_void_qop(q, stop_streaming, q);
>
> + if (q->streaming)
> + call_void_qop(q, unprepare_streaming, q);
> +
> /*
> * If you see this warning, then the driver isn't cleaning up properly
> * in stop_streaming(). See the stop_streaming() documentation in
> @@ -2102,6 +2110,10 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type)
> return -EINVAL;
> }
>
> + ret = call_qop(q, prepare_streaming, q);
> + if (ret)
> + return ret;
> +
> /*
> * Tell driver to start streaming provided sufficient buffers
> * are available.
> @@ -2109,16 +2121,20 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type)
> if (q->queued_count >= q->min_buffers_needed) {
> ret = v4l_vb2q_enable_media_source(q);
> if (ret)
> - return ret;
> + goto unprepare;
> ret = vb2_start_streaming(q);
> if (ret)
> - return ret;
> + goto unprepare;
> }
>
> q->streaming = 1;
>
> dprintk(q, 3, "successful\n");
> return 0;
> +
> +unprepare:
> + call_void_qop(q, unprepare_streaming, q);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(vb2_core_streamon);
>
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 5468b633b9d2..623146476157 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -386,6 +386,9 @@ struct vb2_buffer {
> * the buffer contents will be ignored anyway.
> * @buf_cleanup: called once before the buffer is freed; drivers may
> * perform any additional cleanup; optional.
> + * @prepare_streaming: called once to prepare for 'streaming' state; this is
> + * where validation can be done to verify everything is
> + * okay to start streaming later. Optional.
> * @start_streaming: called once to enter 'streaming' state; the driver may
> * receive buffers with @buf_queue callback
> * before @start_streaming is called; the driver gets the
> @@ -405,6 +408,7 @@ struct vb2_buffer {
> * callback by calling vb2_buffer_done() with either
> * %VB2_BUF_STATE_DONE or %VB2_BUF_STATE_ERROR; may use
> * vb2_wait_for_all_buffers() function
> + * @unprepare_streaming:called as counterpart to @prepare_streaming. Optional.
> * @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;
> @@ -432,8 +436,10 @@ struct vb2_ops {
> void (*buf_finish)(struct vb2_buffer *vb);
> void (*buf_cleanup)(struct vb2_buffer *vb);
>
> + int (*prepare_streaming)(struct vb2_queue *q);
> int (*start_streaming)(struct vb2_queue *q, unsigned int count);
> void (*stop_streaming)(struct vb2_queue *q);
> + void (*unprepare_streaming)(struct vb2_queue *q);
>
> void (*buf_queue)(struct vb2_buffer *vb);
>
> @@ -641,8 +647,10 @@ struct vb2_queue {
> u32 cnt_queue_setup;
> u32 cnt_wait_prepare;
> u32 cnt_wait_finish;
> + u32 cnt_prepare_streaming;
> u32 cnt_start_streaming;
> u32 cnt_stop_streaming;
> + u32 cnt_unprepare_streaming;
> #endif
> };
>
