* [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops
@ 2022-06-22 9:31 Hans Verkuil
2022-06-22 9:31 ` [PATCH 1/2] vb2: add (un)prepare_streaming queue ops Hans Verkuil
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Hans Verkuil @ 2022-06-22 9:31 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).
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] vb2: add (un)prepare_streaming queue ops 2022-06-22 9:31 [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops Hans Verkuil @ 2022-06-22 9:31 ` Hans Verkuil 2022-06-22 9:31 ` [PATCH 2/2] vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver Hans Verkuil 2022-06-22 9:44 ` [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops Laurent Pinchart 2 siblings, 0 replies; 9+ messages in thread From: Hans Verkuil @ 2022-06-22 9:31 UTC (permalink / raw) To: linux-media; +Cc: Laurent Pinchart, Eugen Hristev, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver 2022-06-22 9:31 [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops Hans Verkuil 2022-06-22 9:31 ` [PATCH 1/2] vb2: add (un)prepare_streaming queue ops Hans Verkuil @ 2022-06-22 9:31 ` Hans Verkuil 2022-06-22 9:44 ` [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops Laurent Pinchart 2 siblings, 0 replies; 9+ messages in thread From: Hans Verkuil @ 2022-06-22 9:31 UTC (permalink / raw) To: linux-media; +Cc: Laurent Pinchart, Eugen Hristev, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops 2022-06-22 9:31 [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops Hans Verkuil 2022-06-22 9:31 ` [PATCH 1/2] vb2: add (un)prepare_streaming queue ops Hans Verkuil 2022-06-22 9:31 ` [PATCH 2/2] vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver Hans Verkuil @ 2022-06-22 9:44 ` Laurent Pinchart 2022-06-22 10:00 ` Hans Verkuil 2 siblings, 1 reply; 9+ messages in thread From: Laurent Pinchart @ 2022-06-22 9:44 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media, Eugen Hristev 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops 2022-06-22 9:44 ` [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops Laurent Pinchart @ 2022-06-22 10:00 ` Hans Verkuil 2022-06-22 10:08 ` Laurent Pinchart 2022-06-22 10:23 ` Eugen.Hristev 0 siblings, 2 replies; 9+ messages in thread From: Hans Verkuil @ 2022-06-22 10:00 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, Eugen Hristev 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(-) > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops 2022-06-22 10:00 ` Hans Verkuil @ 2022-06-22 10:08 ` Laurent Pinchart 2022-11-04 9:46 ` Hans Verkuil 2022-06-22 10:23 ` Eugen.Hristev 1 sibling, 1 reply; 9+ messages in thread From: Laurent Pinchart @ 2022-06-22 10:08 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media, Eugen Hristev 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops 2022-06-22 10:08 ` Laurent Pinchart @ 2022-11-04 9:46 ` Hans Verkuil 2022-11-04 10:57 ` Eugen.Hristev 0 siblings, 1 reply; 9+ messages in thread From: Hans Verkuil @ 2022-11-04 9:46 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, Eugen Hristev 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(-) > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops 2022-11-04 9:46 ` Hans Verkuil @ 2022-11-04 10:57 ` Eugen.Hristev 0 siblings, 0 replies; 9+ messages in thread From: Eugen.Hristev @ 2022-11-04 10:57 UTC (permalink / raw) To: hverkuil-cisco, laurent.pinchart; +Cc: linux-media 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(-) >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops 2022-06-22 10:00 ` Hans Verkuil 2022-06-22 10:08 ` Laurent Pinchart @ 2022-06-22 10:23 ` Eugen.Hristev 1 sibling, 0 replies; 9+ messages in thread From: Eugen.Hristev @ 2022-06-22 10:23 UTC (permalink / raw) To: hverkuil-cisco, laurent.pinchart; +Cc: linux-media 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(-) >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-11-04 10:57 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-22 9:31 [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops Hans Verkuil 2022-06-22 9:31 ` [PATCH 1/2] vb2: add (un)prepare_streaming queue ops Hans Verkuil 2022-06-22 9:31 ` [PATCH 2/2] vb2/au0828: move the v4l_vb2q_enable_media_source to the au0828 driver Hans Verkuil 2022-06-22 9:44 ` [PATCH 0/2] vb2: add (un)prepare_streaming vb2_queue ops Laurent Pinchart 2022-06-22 10:00 ` Hans Verkuil 2022-06-22 10:08 ` Laurent Pinchart 2022-11-04 9:46 ` Hans Verkuil 2022-11-04 10:57 ` Eugen.Hristev 2022-06-22 10:23 ` Eugen.Hristev
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.