linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCHv2] vb2: add support for (un)prepare_streaming queue ops
@ 2022-04-30  8:50 Hans Verkuil
  2022-05-05 13:16 ` Eugen.Hristev
  0 siblings, 1 reply; 2+ messages in thread
From: Hans Verkuil @ 2022-04-30  8:50 UTC (permalink / raw)
  To: linux-media; +Cc: Laurent Pinchart, Eugen Hristev

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@xs4all.nl>
---
This has only been compile tested. Eugen, can you try this with the
atmel-isc series?

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

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [RFC PATCHv2] vb2: add support for (un)prepare_streaming queue ops
  2022-04-30  8:50 [RFC PATCHv2] vb2: add support for (un)prepare_streaming queue ops Hans Verkuil
@ 2022-05-05 13:16 ` Eugen.Hristev
  0 siblings, 0 replies; 2+ messages in thread
From: Eugen.Hristev @ 2022-05-05 13:16 UTC (permalink / raw)
  To: hverkuil, linux-media, laurent.pinchart

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@xs4all.nl>
> ---
> 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@microchip.com>

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-05-05 13:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-30  8:50 [RFC PATCHv2] vb2: add support for (un)prepare_streaming queue ops Hans Verkuil
2022-05-05 13:16 ` Eugen.Hristev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).