[PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops

[PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops

[Hans Verkuil]

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

This gives drivers a callback to validate the video pipeline and claim
or release streaming resources at the time that userspace indicates
that it wants to start streaming (VIDIOC_STREAMON) rather than when
the actual DMA engine kicks in (the start_streaming callback). This
is relevant for drivers that needs a minimum of X buffers before the
DMA can start. The actual pipeline validation needs to happen sooner
to avoid unnecessary errors at VIDIOC_QBUF time.

As a bonus this allows us to move the horrible call to
v4l_vb2q_enable_media_source() in vb2_core_streamon() to the au0828
driver (currently the only driver that uses this feature).

That call never belonged in vb2, but it had the same purpose as the
new prepare_streaming op: validate the pipeline.

This is a follow-up from my previous RFCv2:

https://patchwork.linuxtv.org/project/linux-media/patch/ba4bca14-488f-94ea-48d9-e7343103c5aa@xs4all.nl/

I would like to get consensus for this series.

Regards,

	Hans

Hans Verkuil (2):
  vb2: add (un)prepare_streaming queue ops
  vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver

 .../media/common/videobuf2/videobuf2-core.c   | 26 ++++++++++++++-----
 drivers/media/usb/au0828/au0828-vbi.c         |  2 ++
 drivers/media/usb/au0828/au0828-video.c       |  1 +
 include/media/videobuf2-core.h                | 14 ++++++++++
 4 files changed, 37 insertions(+), 6 deletions(-)

-- 
2.35.1

[PATCH 1/2] vb2: add (un)prepare_streaming queue ops

[Hans Verkuil]

When userspace called VIDIOC_STREAMON, then you want to claim any streaming
resources needed and validate the video pipeline. Waiting for
start_streaming to be called is too late, since that can be postponed
until the required minimum of buffers is queued.

So add a prepare_streaming op (optional) that can be used for that
purpose, and a matching unprepare_streaming op (optional) that can
release any claimed resources. The unprepare_streaming op is called
when VIDIOC_STREAMOFF is called and q->streaming is 1, or when the
filehandle is closed while q->streaming is 1.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 .../media/common/videobuf2/videobuf2-core.c   | 25 ++++++++++++++++---
 include/media/videobuf2-core.h                | 14 +++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index b203c1e26353..d4d4b433c0e5 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,12 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type)
 		return -EINVAL;
 	}
 
+	ret = call_qop(q, prepare_streaming, q);
+	if (ret)
+		return ret;
+
+	q->streaming = 1;
+
 	/*
 	 * Tell driver to start streaming provided sufficient buffers
 	 * are available.
@@ -2109,16 +2123,19 @@ 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);
+	q->streaming = 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(vb2_core_streamon);
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5468b633b9d2..dbc965dec5ef 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -386,6 +386,12 @@ 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 and streaming resources can be claimed. It is
+ *			called when the VIDIOC_STREAMON ioctl is called. The
+ *			actual streaming starts when @start_streaming is called.
+ *			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 +411,10 @@ 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; any claimed
+ *			streaming resources can be released here. It is
+ *			called when the VIDIOC_STREAMOFF ioctls is called or
+ *			when the streaming filehandle is closed. 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 +442,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 +653,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
 };
 
-- 
2.35.1

[PATCH 2/2] vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver

[Hans Verkuil]

With the new prepare_streaming op it is possible to move the ugly
v4l_vb2q_enable_media_source() call in vb2_core_streamon to the
driver. It was called incorrectly in vb2 as well: it was only
called if sufficient buffers were queued at STREAMON time, but not
if more buffers were queued later. This was not an issue with the
au0828 driver since it never set min_buffers_needed.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 3 ---
 drivers/media/usb/au0828/au0828-vbi.c           | 2 ++
 drivers/media/usb/au0828/au0828-video.c         | 1 +
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index d4d4b433c0e5..6b4f8cc50a1c 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2121,9 +2121,6 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type)
 	 * are available.
 	 */
 	if (q->queued_count >= q->min_buffers_needed) {
-		ret = v4l_vb2q_enable_media_source(q);
-		if (ret)
-			goto unprepare;
 		ret = vb2_start_streaming(q);
 		if (ret)
 			goto unprepare;
diff --git a/drivers/media/usb/au0828/au0828-vbi.c b/drivers/media/usb/au0828/au0828-vbi.c
index 97f5e8733c2a..b0333637b747 100644
--- a/drivers/media/usb/au0828/au0828-vbi.c
+++ b/drivers/media/usb/au0828/au0828-vbi.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <media/v4l2-mc.h>
 
 /* ------------------------------------------------------------------ */
 
@@ -70,6 +71,7 @@ const struct vb2_ops au0828_vbi_qops = {
 	.queue_setup     = vbi_queue_setup,
 	.buf_prepare     = vbi_buffer_prepare,
 	.buf_queue       = vbi_buffer_queue,
+	.prepare_streaming = v4l_vb2q_enable_media_source,
 	.start_streaming = au0828_start_analog_streaming,
 	.stop_streaming  = au0828_stop_vbi_streaming,
 	.wait_prepare    = vb2_ops_wait_prepare,
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index c0f118563c7d..49678ddf247a 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -915,6 +915,7 @@ static const struct vb2_ops au0828_video_qops = {
 	.queue_setup     = queue_setup,
 	.buf_prepare     = buffer_prepare,
 	.buf_queue       = buffer_queue,
+	.prepare_streaming = v4l_vb2q_enable_media_source,
 	.start_streaming = au0828_start_analog_streaming,
 	.stop_streaming  = au0828_stop_streaming,
 	.wait_prepare    = vb2_ops_wait_prepare,
-- 
2.35.1

Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops

[Laurent Pinchart]

Hi Hans,

On Wed, Jun 22, 2022 at 11:31:43AM +0200, 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).
> 
> This gives drivers a callback to validate the video pipeline and claim
> or release streaming resources at the time that userspace indicates
> that it wants to start streaming (VIDIOC_STREAMON) rather than when
> the actual DMA engine kicks in (the start_streaming callback). This
> is relevant for drivers that needs a minimum of X buffers before the
> DMA can start. The actual pipeline validation needs to happen sooner
> to avoid unnecessary errors at VIDIOC_QBUF time.
> 
> As a bonus this allows us to move the horrible call to
> v4l_vb2q_enable_media_source() in vb2_core_streamon() to the au0828
> driver (currently the only driver that uses this feature).

Can we drop the horrible .enable_source() from media_device too ? :-)

> That call never belonged in vb2, but it had the same purpose as the
> new prepare_streaming op: validate the pipeline.
> 
> This is a follow-up from my previous RFCv2:
> 
> https://patchwork.linuxtv.org/project/linux-media/patch/ba4bca14-488f-94ea-48d9-e7343103c5aa@xs4all.nl/
> 
> I would like to get consensus for this series.

I don't like it much. vb2 is already doing too much in my opinion,
growing it isn't the right way to fix it.

I'm still working on a new version of the V4L2 streams series that may
conflict with this, I'd propose discussing the two together.

> Hans Verkuil (2):
>   vb2: add (un)prepare_streaming queue ops
>   vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver
> 
>  .../media/common/videobuf2/videobuf2-core.c   | 26 ++++++++++++++-----
>  drivers/media/usb/au0828/au0828-vbi.c         |  2 ++
>  drivers/media/usb/au0828/au0828-video.c       |  1 +
>  include/media/videobuf2-core.h                | 14 ++++++++++
>  4 files changed, 37 insertions(+), 6 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops

[Hans Verkuil]

Hi Laurent,

On 22/06/2022 11:44, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Wed, Jun 22, 2022 at 11:31:43AM +0200, 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).
>>
>> This gives drivers a callback to validate the video pipeline and claim
>> or release streaming resources at the time that userspace indicates
>> that it wants to start streaming (VIDIOC_STREAMON) rather than when
>> the actual DMA engine kicks in (the start_streaming callback). This
>> is relevant for drivers that needs a minimum of X buffers before the
>> DMA can start. The actual pipeline validation needs to happen sooner
>> to avoid unnecessary errors at VIDIOC_QBUF time.
>>
>> As a bonus this allows us to move the horrible call to
>> v4l_vb2q_enable_media_source() in vb2_core_streamon() to the au0828
>> driver (currently the only driver that uses this feature).
> 
> Can we drop the horrible .enable_source() from media_device too ? :-)

The second patch helps a bit with that, at least it's out of vb2.

> 
>> That call never belonged in vb2, but it had the same purpose as the
>> new prepare_streaming op: validate the pipeline.
>>
>> This is a follow-up from my previous RFCv2:
>>
>> https://patchwork.linuxtv.org/project/linux-media/patch/ba4bca14-488f-94ea-48d9-e7343103c5aa@xs4all.nl/
>>
>> I would like to get consensus for this series.
> 
> I don't like it much. vb2 is already doing too much in my opinion,
> growing it isn't the right way to fix it.

We disagree on that :-)

> 
> I'm still working on a new version of the V4L2 streams series that may
> conflict with this, I'd propose discussing the two together.

What is the ETA for that? I don't mind waiting a few months, but if it
takes a lot longer, then I'd rather merge this first so Eugen can use it
in his atmel MC support. It's a kernel API, so it can always be changed
or removed later.

Regards,

	Hans

> 
>> Hans Verkuil (2):
>>   vb2: add (un)prepare_streaming queue ops
>>   vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver
>>
>>  .../media/common/videobuf2/videobuf2-core.c   | 26 ++++++++++++++-----
>>  drivers/media/usb/au0828/au0828-vbi.c         |  2 ++
>>  drivers/media/usb/au0828/au0828-video.c       |  1 +
>>  include/media/videobuf2-core.h                | 14 ++++++++++
>>  4 files changed, 37 insertions(+), 6 deletions(-)
> 

Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops

[Laurent Pinchart]

Hi Hans,

On Wed, Jun 22, 2022 at 12:00:58PM +0200, Hans Verkuil wrote:
> On 22/06/2022 11:44, Laurent Pinchart wrote:
> > On Wed, Jun 22, 2022 at 11:31:43AM +0200, 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).
> >>
> >> This gives drivers a callback to validate the video pipeline and claim
> >> or release streaming resources at the time that userspace indicates
> >> that it wants to start streaming (VIDIOC_STREAMON) rather than when
> >> the actual DMA engine kicks in (the start_streaming callback). This
> >> is relevant for drivers that needs a minimum of X buffers before the
> >> DMA can start. The actual pipeline validation needs to happen sooner
> >> to avoid unnecessary errors at VIDIOC_QBUF time.
> >>
> >> As a bonus this allows us to move the horrible call to
> >> v4l_vb2q_enable_media_source() in vb2_core_streamon() to the au0828
> >> driver (currently the only driver that uses this feature).
> > 
> > Can we drop the horrible .enable_source() from media_device too ? :-)
> 
> The second patch helps a bit with that, at least it's out of vb2.
> 
> > 
> >> That call never belonged in vb2, but it had the same purpose as the
> >> new prepare_streaming op: validate the pipeline.
> >>
> >> This is a follow-up from my previous RFCv2:
> >>
> >> https://patchwork.linuxtv.org/project/linux-media/patch/ba4bca14-488f-94ea-48d9-e7343103c5aa@xs4all.nl/
> >>
> >> I would like to get consensus for this series.
> > 
> > I don't like it much. vb2 is already doing too much in my opinion,
> > growing it isn't the right way to fix it.
> 
> We disagree on that :-)

I like polite and constructive disagreements, they help moving forward
:-)

> > I'm still working on a new version of the V4L2 streams series that may
> > conflict with this, I'd propose discussing the two together.
> 
> What is the ETA for that? I don't mind waiting a few months, but if it
> takes a lot longer, then I'd rather merge this first so Eugen can use it
> in his atmel MC support. It's a kernel API, so it can always be changed
> or removed later.

I have a few other things to complete first, an dI plan to resume the
work in the first half of July, to post a v12 before the end of the
month.

> >> Hans Verkuil (2):
> >>   vb2: add (un)prepare_streaming queue ops
> >>   vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver
> >>
> >>  .../media/common/videobuf2/videobuf2-core.c   | 26 ++++++++++++++-----
> >>  drivers/media/usb/au0828/au0828-vbi.c         |  2 ++
> >>  drivers/media/usb/au0828/au0828-video.c       |  1 +
> >>  include/media/videobuf2-core.h                | 14 ++++++++++
> >>  4 files changed, 37 insertions(+), 6 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops

[Eugen.Hristev]

On 6/22/22 1:00 PM, Hans Verkuil wrote:

> Hi Laurent,
> 
> On 22/06/2022 11:44, Laurent Pinchart wrote:
>> Hi Hans,
>>
>> On Wed, Jun 22, 2022 at 11:31:43AM +0200, 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).
>>>
>>> This gives drivers a callback to validate the video pipeline and claim
>>> or release streaming resources at the time that userspace indicates
>>> that it wants to start streaming (VIDIOC_STREAMON) rather than when
>>> the actual DMA engine kicks in (the start_streaming callback). This
>>> is relevant for drivers that needs a minimum of X buffers before the
>>> DMA can start. The actual pipeline validation needs to happen sooner
>>> to avoid unnecessary errors at VIDIOC_QBUF time.
>>>
>>> As a bonus this allows us to move the horrible call to
>>> v4l_vb2q_enable_media_source() in vb2_core_streamon() to the au0828
>>> driver (currently the only driver that uses this feature).
>>
>> Can we drop the horrible .enable_source() from media_device too ? :-)
> 
> The second patch helps a bit with that, at least it's out of vb2.
> 
>>
>>> That call never belonged in vb2, but it had the same purpose as the
>>> new prepare_streaming op: validate the pipeline.
>>>
>>> This is a follow-up from my previous RFCv2:
>>>
>>> https://patchwork.linuxtv.org/project/linux-media/patch/ba4bca14-488f-94ea-48d9-e7343103c5aa@xs4all.nl/
>>>
>>> I would like to get consensus for this series.
>>
>> I don't like it much. vb2 is already doing too much in my opinion,
>> growing it isn't the right way to fix it.
> 
> We disagree on that :-)
> 
>>
>> I'm still working on a new version of the V4L2 streams series that may
>> conflict with this, I'd propose discussing the two together.
> 
> What is the ETA for that? I don't mind waiting a few months, but if it
> takes a lot longer, then I'd rather merge this first so Eugen can use it
> in his atmel MC support. It's a kernel API, so it can always be changed
> or removed later.

Hi,

The atmel MC support is done on top of v4l2 without this patch, and I 
have a subsequent patch that changes the atmel MC to use the new RFC API.

One option is to have the atmel MC like I initially sent it, and add my 
patch that switches to the prepare_streaming to this series .

> 
> Regards,
> 
>          Hans
> 
>>
>>> Hans Verkuil (2):
>>>    vb2: add (un)prepare_streaming queue ops
>>>    vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver
>>>
>>>   .../media/common/videobuf2/videobuf2-core.c   | 26 ++++++++++++++-----
>>>   drivers/media/usb/au0828/au0828-vbi.c         |  2 ++
>>>   drivers/media/usb/au0828/au0828-video.c       |  1 +
>>>   include/media/videobuf2-core.h                | 14 ++++++++++
>>>   4 files changed, 37 insertions(+), 6 deletions(-)
>>
> 

Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops

[Hans Verkuil]

Hi Laurent,

On 22/06/2022 12:08, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Wed, Jun 22, 2022 at 12:00:58PM +0200, Hans Verkuil wrote:
>> On 22/06/2022 11:44, Laurent Pinchart wrote:
>>> On Wed, Jun 22, 2022 at 11:31:43AM +0200, 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).
>>>>
>>>> This gives drivers a callback to validate the video pipeline and claim
>>>> or release streaming resources at the time that userspace indicates
>>>> that it wants to start streaming (VIDIOC_STREAMON) rather than when
>>>> the actual DMA engine kicks in (the start_streaming callback). This
>>>> is relevant for drivers that needs a minimum of X buffers before the
>>>> DMA can start. The actual pipeline validation needs to happen sooner
>>>> to avoid unnecessary errors at VIDIOC_QBUF time.
>>>>
>>>> As a bonus this allows us to move the horrible call to
>>>> v4l_vb2q_enable_media_source() in vb2_core_streamon() to the au0828
>>>> driver (currently the only driver that uses this feature).
>>>
>>> Can we drop the horrible .enable_source() from media_device too ? :-)
>>
>> The second patch helps a bit with that, at least it's out of vb2.
>>
>>>
>>>> That call never belonged in vb2, but it had the same purpose as the
>>>> new prepare_streaming op: validate the pipeline.
>>>>
>>>> This is a follow-up from my previous RFCv2:
>>>>
>>>> https://patchwork.linuxtv.org/project/linux-media/patch/ba4bca14-488f-94ea-48d9-e7343103c5aa@xs4all.nl/
>>>>
>>>> I would like to get consensus for this series.
>>>
>>> I don't like it much. vb2 is already doing too much in my opinion,
>>> growing it isn't the right way to fix it.
>>
>> We disagree on that :-)
> 
> I like polite and constructive disagreements, they help moving forward
> :-)
> 
>>> I'm still working on a new version of the V4L2 streams series that may
>>> conflict with this, I'd propose discussing the two together.
>>
>> What is the ETA for that? I don't mind waiting a few months, but if it
>> takes a lot longer, then I'd rather merge this first so Eugen can use it
>> in his atmel MC support. It's a kernel API, so it can always be changed
>> or removed later.
> 
> I have a few other things to complete first, an dI plan to resume the
> work in the first half of July, to post a v12 before the end of the
> month.

Looking at the latest v15 series there are no conflicts with this patch.

Eugen posted a v11 of his "atmel-isc: driver redesign" series, and wants
to use this functionality.

I think with this change it is also possible to remove the enable_source
callback from the mc. I can try to post a v2 that does this, if that's
what it takes to convince you :-)

Regards,

	Hans

> 
>>>> Hans Verkuil (2):
>>>>   vb2: add (un)prepare_streaming queue ops
>>>>   vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver
>>>>
>>>>  .../media/common/videobuf2/videobuf2-core.c   | 26 ++++++++++++++-----
>>>>  drivers/media/usb/au0828/au0828-vbi.c         |  2 ++
>>>>  drivers/media/usb/au0828/au0828-video.c       |  1 +
>>>>  include/media/videobuf2-core.h                | 14 ++++++++++
>>>>  4 files changed, 37 insertions(+), 6 deletions(-)
> 

Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops

[Eugen.Hristev]

On 11/4/22 11:46, Hans Verkuil wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Laurent,
> 
> On 22/06/2022 12:08, Laurent Pinchart wrote:
>> Hi Hans,
>>
>> On Wed, Jun 22, 2022 at 12:00:58PM +0200, Hans Verkuil wrote:
>>> On 22/06/2022 11:44, Laurent Pinchart wrote:
>>>> On Wed, Jun 22, 2022 at 11:31:43AM +0200, 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).
>>>>>
>>>>> This gives drivers a callback to validate the video pipeline and claim
>>>>> or release streaming resources at the time that userspace indicates
>>>>> that it wants to start streaming (VIDIOC_STREAMON) rather than when
>>>>> the actual DMA engine kicks in (the start_streaming callback). This
>>>>> is relevant for drivers that needs a minimum of X buffers before the
>>>>> DMA can start. The actual pipeline validation needs to happen sooner
>>>>> to avoid unnecessary errors at VIDIOC_QBUF time.
>>>>>
>>>>> As a bonus this allows us to move the horrible call to
>>>>> v4l_vb2q_enable_media_source() in vb2_core_streamon() to the au0828
>>>>> driver (currently the only driver that uses this feature).
>>>>
>>>> Can we drop the horrible .enable_source() from media_device too ? :-)
>>>
>>> The second patch helps a bit with that, at least it's out of vb2.
>>>
>>>>
>>>>> That call never belonged in vb2, but it had the same purpose as the
>>>>> new prepare_streaming op: validate the pipeline.
>>>>>
>>>>> This is a follow-up from my previous RFCv2:
>>>>>
>>>>> https://patchwork.linuxtv.org/project/linux-media/patch/ba4bca14-488f-94ea-48d9-e7343103c5aa@xs4all.nl/
>>>>>
>>>>> I would like to get consensus for this series.
>>>>
>>>> I don't like it much. vb2 is already doing too much in my opinion,
>>>> growing it isn't the right way to fix it.
>>>
>>> We disagree on that :-)
>>
>> I like polite and constructive disagreements, they help moving forward
>> :-)
>>
>>>> I'm still working on a new version of the V4L2 streams series that may
>>>> conflict with this, I'd propose discussing the two together.
>>>
>>> What is the ETA for that? I don't mind waiting a few months, but if it
>>> takes a lot longer, then I'd rather merge this first so Eugen can use it
>>> in his atmel MC support. It's a kernel API, so it can always be changed
>>> or removed later.
>>
>> I have a few other things to complete first, an dI plan to resume the
>> work in the first half of July, to post a v12 before the end of the
>> month.
> 
> Looking at the latest v15 series there are no conflicts with this patch.
> 
> Eugen posted a v11 of his "atmel-isc: driver redesign" series, and wants
> to use this functionality.
> 
> I think with this change it is also possible to remove the enable_source
> callback from the mc. I can try to post a v2 that does this, if that's
> what it takes to convince you :-)

Hello,

The ISC driver looks more clean with these ops.
I don't know how much it matters though.
It's not a big improvement, rather small, but it looks cleaner.

Thanks,
Eugen

> 
> Regards,
> 
>          Hans
> 
>>
>>>>> Hans Verkuil (2):
>>>>>    vb2: add (un)prepare_streaming queue ops
>>>>>    vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver
>>>>>
>>>>>   .../media/common/videobuf2/videobuf2-core.c   | 26 ++++++++++++++-----
>>>>>   drivers/media/usb/au0828/au0828-vbi.c         |  2 ++
>>>>>   drivers/media/usb/au0828/au0828-video.c       |  1 +
>>>>>   include/media/videobuf2-core.h                | 14 ++++++++++
>>>>>   4 files changed, 37 insertions(+), 6 deletions(-)
>>
>