* [PATCH v2 0/3] media: add vb2_queue_change_type() helper @ 2021-04-12 11:02 Tomi Valkeinen 2021-04-12 11:02 ` [PATCH v2 1/3] media: videobuf2-v4l2.c: " Tomi Valkeinen ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Tomi Valkeinen @ 2021-04-12 11:02 UTC (permalink / raw) To: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media, Laurent Pinchart Cc: Tomi Valkeinen Hi, Here's v2, with a small change in the vb2_queue_change_type help text, and with vivid changed to use vb2_queue_change_type. I'm not familiar with vivid, or even the v4l2 features it's using, so please review with care. v4l2-compliance passes, but I'm not sure if I tested all the functionality vivid offers. Tomi Tomi Valkeinen (3): media: videobuf2-v4l2.c: add vb2_queue_change_type() helper media: vivid: remove stream_sliced_vbi_cap field media: vivid: use vb2_queue_change_type .../media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++ drivers/media/test-drivers/vivid/vivid-core.c | 44 ++++++++++++++++++- drivers/media/test-drivers/vivid/vivid-core.h | 1 - .../test-drivers/vivid/vivid-kthread-cap.c | 2 +- .../media/test-drivers/vivid/vivid-vbi-cap.c | 8 +--- include/media/videobuf2-v4l2.h | 15 +++++++ 6 files changed, 74 insertions(+), 10 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/3] media: videobuf2-v4l2.c: add vb2_queue_change_type() helper 2021-04-12 11:02 [PATCH v2 0/3] media: add vb2_queue_change_type() helper Tomi Valkeinen @ 2021-04-12 11:02 ` Tomi Valkeinen 2021-04-23 8:40 ` Tomi Valkeinen ` (2 more replies) 2021-04-12 11:02 ` [PATCH v2 2/3] media: vivid: remove stream_sliced_vbi_cap field Tomi Valkeinen 2021-04-12 11:02 ` [PATCH v2 3/3] media: vivid: use vb2_queue_change_type Tomi Valkeinen 2 siblings, 3 replies; 11+ messages in thread From: Tomi Valkeinen @ 2021-04-12 11:02 UTC (permalink / raw) To: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media, Laurent Pinchart Cc: Tomi Valkeinen On some platforms a video device can capture either video data or metadata. The driver can implement vidioc functions for both video and metadata, and use a single vb2_queue for the buffers. However, vb2_queue requires choosing a single buffer type, which conflicts with the idea of capturing either video or metadata. The buffer type of vb2_queue can be changed, but it's not obvious how this should be done in the drivers. To help this, add a new helper function vb2_queue_change_type() which ensures the correct checks and documents how it can be used. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++++ include/media/videobuf2-v4l2.h | 15 +++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 7e96f67c60ba..2988bb38ceb1 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -939,6 +939,20 @@ void vb2_queue_release(struct vb2_queue *q) } EXPORT_SYMBOL_GPL(vb2_queue_release); +int vb2_queue_change_type(struct vb2_queue *q, unsigned int type) +{ + if (type == q->type) + return 0; + + if (vb2_is_busy(q)) + return -EBUSY; + + q->type = type; + + return 0; +} +EXPORT_SYMBOL_GPL(vb2_queue_change_type); + __poll_t vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) { struct video_device *vfd = video_devdata(file); diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h index c203047eb834..12fa72a664cf 100644 --- a/include/media/videobuf2-v4l2.h +++ b/include/media/videobuf2-v4l2.h @@ -261,6 +261,21 @@ int __must_check vb2_queue_init_name(struct vb2_queue *q, const char *name); */ void vb2_queue_release(struct vb2_queue *q); +/** + * vb2_queue_change_type() - change the type of an inactive vb2_queue + * @q: pointer to &struct vb2_queue with videobuf2 queue. + * + * This function changes the type of the vb2_queue. This is only possible + * if the queue is not busy (i.e. no buffers have been allocated). + * + * vb2_queue_change_type() can be used to support multiple buffer types using + * the same queue. The driver can implement v4l2_ioctl_ops.vidioc_reqbufs and + * v4l2_ioctl_ops.vidioc_create_bufs functions and call vb2_queue_change_type() + * before calling vb2_ioctl_reqbufs() or vb2_ioctl_create_bufs(), and thus + * "lock" the buffer type until the buffers have been released. + */ +int vb2_queue_change_type(struct vb2_queue *q, unsigned int type); + /** * vb2_poll() - implements poll userspace operation * @q: pointer to &struct vb2_queue with videobuf2 queue. -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] media: videobuf2-v4l2.c: add vb2_queue_change_type() helper 2021-04-12 11:02 ` [PATCH v2 1/3] media: videobuf2-v4l2.c: " Tomi Valkeinen @ 2021-04-23 8:40 ` Tomi Valkeinen 2021-06-04 11:13 ` Tomasz Figa 2021-06-04 14:38 ` Laurent Pinchart 2 siblings, 0 replies; 11+ messages in thread From: Tomi Valkeinen @ 2021-04-23 8:40 UTC (permalink / raw) To: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media, Laurent Pinchart On 12/04/2021 14:02, Tomi Valkeinen wrote: > On some platforms a video device can capture either video data or > metadata. The driver can implement vidioc functions for both video and > metadata, and use a single vb2_queue for the buffers. However, vb2_queue > requires choosing a single buffer type, which conflicts with the idea of > capturing either video or metadata. > > The buffer type of vb2_queue can be changed, but it's not obvious how > this should be done in the drivers. To help this, add a new helper > function vb2_queue_change_type() which ensures the correct checks and > documents how it can be used. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++++ > include/media/videobuf2-v4l2.h | 15 +++++++++++++++ > 2 files changed, 29 insertions(+) > > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 7e96f67c60ba..2988bb38ceb1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -939,6 +939,20 @@ void vb2_queue_release(struct vb2_queue *q) > } > EXPORT_SYMBOL_GPL(vb2_queue_release); > > +int vb2_queue_change_type(struct vb2_queue *q, unsigned int type) > +{ > + if (type == q->type) > + return 0; > + > + if (vb2_is_busy(q)) > + return -EBUSY; > + > + q->type = type; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(vb2_queue_change_type); > + > __poll_t vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) > { > struct video_device *vfd = video_devdata(file); > diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h > index c203047eb834..12fa72a664cf 100644 > --- a/include/media/videobuf2-v4l2.h > +++ b/include/media/videobuf2-v4l2.h > @@ -261,6 +261,21 @@ int __must_check vb2_queue_init_name(struct vb2_queue *q, const char *name); > */ > void vb2_queue_release(struct vb2_queue *q); > > +/** > + * vb2_queue_change_type() - change the type of an inactive vb2_queue > + * @q: pointer to &struct vb2_queue with videobuf2 queue. > + * > + * This function changes the type of the vb2_queue. This is only possible > + * if the queue is not busy (i.e. no buffers have been allocated). > + * > + * vb2_queue_change_type() can be used to support multiple buffer types using > + * the same queue. The driver can implement v4l2_ioctl_ops.vidioc_reqbufs and > + * v4l2_ioctl_ops.vidioc_create_bufs functions and call vb2_queue_change_type() > + * before calling vb2_ioctl_reqbufs() or vb2_ioctl_create_bufs(), and thus > + * "lock" the buffer type until the buffers have been released. > + */ > +int vb2_queue_change_type(struct vb2_queue *q, unsigned int type); > + > /** > * vb2_poll() - implements poll userspace operation > * @q: pointer to &struct vb2_queue with videobuf2 queue. > This is missing doc for the type parameter. I have added this to my branch: @type: The type to change to (V4L2_BUF_TYPE_VIDEO_*) I'll send a v3 after I get feedback on the vivid patches. Tomi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] media: videobuf2-v4l2.c: add vb2_queue_change_type() helper 2021-04-12 11:02 ` [PATCH v2 1/3] media: videobuf2-v4l2.c: " Tomi Valkeinen 2021-04-23 8:40 ` Tomi Valkeinen @ 2021-06-04 11:13 ` Tomasz Figa 2021-06-04 14:09 ` Tomi Valkeinen 2021-06-04 14:38 ` Laurent Pinchart 2 siblings, 1 reply; 11+ messages in thread From: Tomasz Figa @ 2021-06-04 11:13 UTC (permalink / raw) To: Tomi Valkeinen Cc: Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media, Laurent Pinchart Hi Tomi, On Mon, Apr 12, 2021 at 02:02:09PM +0300, Tomi Valkeinen wrote: > On some platforms a video device can capture either video data or > metadata. The driver can implement vidioc functions for both video and > metadata, and use a single vb2_queue for the buffers. However, vb2_queue > requires choosing a single buffer type, which conflicts with the idea of > capturing either video or metadata. > > The buffer type of vb2_queue can be changed, but it's not obvious how > this should be done in the drivers. To help this, add a new helper > function vb2_queue_change_type() which ensures the correct checks and > documents how it can be used. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++++ > include/media/videobuf2-v4l2.h | 15 +++++++++++++++ > 2 files changed, 29 insertions(+) > Good to see you contributing to the media subsystem. Not sure if you still remember me from the Common Display Framework discussions. ;) Anyway, thanks for the patch. I think the code itself is okay, but I'm wondering why the driver couldn't just have two queues, one for each type? Best regards, Tomasz > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 7e96f67c60ba..2988bb38ceb1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -939,6 +939,20 @@ void vb2_queue_release(struct vb2_queue *q) > } > EXPORT_SYMBOL_GPL(vb2_queue_release); > > +int vb2_queue_change_type(struct vb2_queue *q, unsigned int type) > +{ > + if (type == q->type) > + return 0; > + > + if (vb2_is_busy(q)) > + return -EBUSY; > + > + q->type = type; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(vb2_queue_change_type); > + > __poll_t vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) > { > struct video_device *vfd = video_devdata(file); > diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h > index c203047eb834..12fa72a664cf 100644 > --- a/include/media/videobuf2-v4l2.h > +++ b/include/media/videobuf2-v4l2.h > @@ -261,6 +261,21 @@ int __must_check vb2_queue_init_name(struct vb2_queue *q, const char *name); > */ > void vb2_queue_release(struct vb2_queue *q); > > +/** > + * vb2_queue_change_type() - change the type of an inactive vb2_queue > + * @q: pointer to &struct vb2_queue with videobuf2 queue. > + * > + * This function changes the type of the vb2_queue. This is only possible > + * if the queue is not busy (i.e. no buffers have been allocated). > + * > + * vb2_queue_change_type() can be used to support multiple buffer types using > + * the same queue. The driver can implement v4l2_ioctl_ops.vidioc_reqbufs and > + * v4l2_ioctl_ops.vidioc_create_bufs functions and call vb2_queue_change_type() > + * before calling vb2_ioctl_reqbufs() or vb2_ioctl_create_bufs(), and thus > + * "lock" the buffer type until the buffers have been released. > + */ > +int vb2_queue_change_type(struct vb2_queue *q, unsigned int type); > + > /** > * vb2_poll() - implements poll userspace operation > * @q: pointer to &struct vb2_queue with videobuf2 queue. > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] media: videobuf2-v4l2.c: add vb2_queue_change_type() helper 2021-06-04 11:13 ` Tomasz Figa @ 2021-06-04 14:09 ` Tomi Valkeinen 2021-06-08 5:11 ` Tomasz Figa 0 siblings, 1 reply; 11+ messages in thread From: Tomi Valkeinen @ 2021-06-04 14:09 UTC (permalink / raw) To: Tomasz Figa Cc: Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media, Laurent Pinchart Hi Tomasz, On 04/06/2021 14:13, Tomasz Figa wrote: > Hi Tomi, > > On Mon, Apr 12, 2021 at 02:02:09PM +0300, Tomi Valkeinen wrote: >> On some platforms a video device can capture either video data or >> metadata. The driver can implement vidioc functions for both video and >> metadata, and use a single vb2_queue for the buffers. However, vb2_queue >> requires choosing a single buffer type, which conflicts with the idea of >> capturing either video or metadata. >> >> The buffer type of vb2_queue can be changed, but it's not obvious how >> this should be done in the drivers. To help this, add a new helper >> function vb2_queue_change_type() which ensures the correct checks and >> documents how it can be used. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> >> --- >> drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++++ >> include/media/videobuf2-v4l2.h | 15 +++++++++++++++ >> 2 files changed, 29 insertions(+) >> > > Good to see you contributing to the media subsystem. Not sure if you > still remember me from the Common Display Framework discussions. ;) I barely remember CDF... ;) > Anyway, thanks for the patch. I think the code itself is okay, but I'm > wondering why the driver couldn't just have two queues, one for each > type? There was an email thread about this: https://www.spinics.net/lists/linux-media/msg189144.html struct video_device has 'queue' field, so if you have two queues, you'd need to change the vd->queue based on the format. Possibly that could be a solution too (and, if I recall right, that's what I initially tried as a quick hack). Changing the whole queue sounds riskier than changing just the type. Tomi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] media: videobuf2-v4l2.c: add vb2_queue_change_type() helper 2021-06-04 14:09 ` Tomi Valkeinen @ 2021-06-08 5:11 ` Tomasz Figa 0 siblings, 0 replies; 11+ messages in thread From: Tomasz Figa @ 2021-06-08 5:11 UTC (permalink / raw) To: Tomi Valkeinen Cc: Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, Linux Media Mailing List, Laurent Pinchart On Fri, Jun 4, 2021 at 11:09 PM Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > Hi Tomasz, > > On 04/06/2021 14:13, Tomasz Figa wrote: > > Hi Tomi, > > > > On Mon, Apr 12, 2021 at 02:02:09PM +0300, Tomi Valkeinen wrote: > >> On some platforms a video device can capture either video data or > >> metadata. The driver can implement vidioc functions for both video and > >> metadata, and use a single vb2_queue for the buffers. However, vb2_queue > >> requires choosing a single buffer type, which conflicts with the idea of > >> capturing either video or metadata. > >> > >> The buffer type of vb2_queue can be changed, but it's not obvious how > >> this should be done in the drivers. To help this, add a new helper > >> function vb2_queue_change_type() which ensures the correct checks and > >> documents how it can be used. > >> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > >> --- > >> drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++++ > >> include/media/videobuf2-v4l2.h | 15 +++++++++++++++ > >> 2 files changed, 29 insertions(+) > >> > > > > Good to see you contributing to the media subsystem. Not sure if you > > still remember me from the Common Display Framework discussions. ;) > > I barely remember CDF... ;) > > > Anyway, thanks for the patch. I think the code itself is okay, but I'm > > wondering why the driver couldn't just have two queues, one for each > > type? > > There was an email thread about this: > > https://www.spinics.net/lists/linux-media/msg189144.html > > struct video_device has 'queue' field, so if you have two queues, you'd > need to change the vd->queue based on the format. Possibly that could be > a solution too (and, if I recall right, that's what I initially tried as > a quick hack). Changing the whole queue sounds riskier than changing > just the type. Okay, I see. Thanks for the pointer. Generally I'm fine with this, although it's worth noting that it's not true that one video_device can only have 1 queue. See the numerous mem-to-mem devices, which use two queues simultaneously. Anyway, Acked-by: Tomasz Figa <tfiga@chromium.org> Best regards, Tomasz ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] media: videobuf2-v4l2.c: add vb2_queue_change_type() helper 2021-04-12 11:02 ` [PATCH v2 1/3] media: videobuf2-v4l2.c: " Tomi Valkeinen 2021-04-23 8:40 ` Tomi Valkeinen 2021-06-04 11:13 ` Tomasz Figa @ 2021-06-04 14:38 ` Laurent Pinchart 2 siblings, 0 replies; 11+ messages in thread From: Laurent Pinchart @ 2021-06-04 14:38 UTC (permalink / raw) To: Tomi Valkeinen Cc: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media Hi Tomi, Thank you for the patch. On Mon, Apr 12, 2021 at 02:02:09PM +0300, Tomi Valkeinen wrote: > On some platforms a video device can capture either video data or > metadata. The driver can implement vidioc functions for both video and > metadata, and use a single vb2_queue for the buffers. However, vb2_queue > requires choosing a single buffer type, which conflicts with the idea of > capturing either video or metadata. > > The buffer type of vb2_queue can be changed, but it's not obvious how > this should be done in the drivers. To help this, add a new helper > function vb2_queue_change_type() which ensures the correct checks and > documents how it can be used. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++++ > include/media/videobuf2-v4l2.h | 15 +++++++++++++++ > 2 files changed, 29 insertions(+) > > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 7e96f67c60ba..2988bb38ceb1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -939,6 +939,20 @@ void vb2_queue_release(struct vb2_queue *q) > } > EXPORT_SYMBOL_GPL(vb2_queue_release); > > +int vb2_queue_change_type(struct vb2_queue *q, unsigned int type) > +{ > + if (type == q->type) > + return 0; > + > + if (vb2_is_busy(q)) > + return -EBUSY; > + > + q->type = type; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(vb2_queue_change_type); > + > __poll_t vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) > { > struct video_device *vfd = video_devdata(file); > diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h > index c203047eb834..12fa72a664cf 100644 > --- a/include/media/videobuf2-v4l2.h > +++ b/include/media/videobuf2-v4l2.h > @@ -261,6 +261,21 @@ int __must_check vb2_queue_init_name(struct vb2_queue *q, const char *name); > */ > void vb2_queue_release(struct vb2_queue *q); > > +/** > + * vb2_queue_change_type() - change the type of an inactive vb2_queue > + * @q: pointer to &struct vb2_queue with videobuf2 queue. > + * > + * This function changes the type of the vb2_queue. This is only possible > + * if the queue is not busy (i.e. no buffers have been allocated). > + * > + * vb2_queue_change_type() can be used to support multiple buffer types using > + * the same queue. The driver can implement v4l2_ioctl_ops.vidioc_reqbufs and I think this should be &v4l2_ioctl_ops.vidioc_reqbufs() (same below) to generate links properly. > + * v4l2_ioctl_ops.vidioc_create_bufs functions and call vb2_queue_change_type() > + * before calling vb2_ioctl_reqbufs() or vb2_ioctl_create_bufs(), and thus > + * "lock" the buffer type until the buffers have been released. It would be nice if selection of the type could be moved to the .queue_setup() operation, but that's more difficult and would require rework of the reqbufs and create_bufs implementations that may be tricky. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > +int vb2_queue_change_type(struct vb2_queue *q, unsigned int type); > + > /** > * vb2_poll() - implements poll userspace operation > * @q: pointer to &struct vb2_queue with videobuf2 queue. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] media: vivid: remove stream_sliced_vbi_cap field 2021-04-12 11:02 [PATCH v2 0/3] media: add vb2_queue_change_type() helper Tomi Valkeinen 2021-04-12 11:02 ` [PATCH v2 1/3] media: videobuf2-v4l2.c: " Tomi Valkeinen @ 2021-04-12 11:02 ` Tomi Valkeinen 2021-06-04 14:41 ` Laurent Pinchart 2021-04-12 11:02 ` [PATCH v2 3/3] media: vivid: use vb2_queue_change_type Tomi Valkeinen 2 siblings, 1 reply; 11+ messages in thread From: Tomi Valkeinen @ 2021-04-12 11:02 UTC (permalink / raw) To: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media, Laurent Pinchart Cc: Tomi Valkeinen Vivid tracks the VBI capture mode in vivid_dev->stream_sliced_vbi_cap field. We can just look at the buffer type instead, and drop the field. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- drivers/media/test-drivers/vivid/vivid-core.h | 1 - drivers/media/test-drivers/vivid/vivid-kthread-cap.c | 2 +- drivers/media/test-drivers/vivid/vivid-vbi-cap.c | 6 ++---- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/media/test-drivers/vivid/vivid-core.h b/drivers/media/test-drivers/vivid/vivid-core.h index 9c2d1470b597..9af7e843c2cf 100644 --- a/drivers/media/test-drivers/vivid/vivid-core.h +++ b/drivers/media/test-drivers/vivid/vivid-core.h @@ -428,7 +428,6 @@ struct vivid_dev { u32 vbi_cap_seq_start; u32 vbi_cap_seq_count; bool vbi_cap_streaming; - bool stream_sliced_vbi_cap; u32 meta_cap_seq_start; u32 meta_cap_seq_count; bool meta_cap_streaming; diff --git a/drivers/media/test-drivers/vivid/vivid-kthread-cap.c b/drivers/media/test-drivers/vivid/vivid-kthread-cap.c index 67fb3c00f9ad..781763c193eb 100644 --- a/drivers/media/test-drivers/vivid/vivid-kthread-cap.c +++ b/drivers/media/test-drivers/vivid/vivid-kthread-cap.c @@ -750,7 +750,7 @@ static noinline_for_stack void vivid_thread_vid_cap_tick(struct vivid_dev *dev, v4l2_ctrl_request_setup(vbi_cap_buf->vb.vb2_buf.req_obj.req, &dev->ctrl_hdl_vbi_cap); - if (dev->stream_sliced_vbi_cap) + if (vbi_cap_buf->vb.vb2_buf.type == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE) vivid_sliced_vbi_cap_process(dev, vbi_cap_buf); else vivid_raw_vbi_cap_process(dev, vbi_cap_buf); diff --git a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c index 1a9348eea781..387df4ff01b0 100644 --- a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c +++ b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c @@ -255,9 +255,8 @@ int vidioc_s_fmt_vbi_cap(struct file *file, void *priv, if (ret) return ret; - if (dev->stream_sliced_vbi_cap && vb2_is_busy(&dev->vb_vbi_cap_q)) + if (f->type != V4L2_BUF_TYPE_VBI_CAPTURE && vb2_is_busy(&dev->vb_vbi_cap_q)) return -EBUSY; - dev->stream_sliced_vbi_cap = false; dev->vbi_cap_dev.queue->type = V4L2_BUF_TYPE_VBI_CAPTURE; return 0; } @@ -322,10 +321,9 @@ int vidioc_s_fmt_sliced_vbi_cap(struct file *file, void *fh, struct v4l2_format if (ret) return ret; - if (!dev->stream_sliced_vbi_cap && vb2_is_busy(&dev->vb_vbi_cap_q)) + if (fmt->type != V4L2_BUF_TYPE_SLICED_VBI_CAPTURE && vb2_is_busy(&dev->vb_vbi_cap_q)) return -EBUSY; dev->service_set_cap = vbi->service_set; - dev->stream_sliced_vbi_cap = true; dev->vbi_cap_dev.queue->type = V4L2_BUF_TYPE_SLICED_VBI_CAPTURE; return 0; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] media: vivid: remove stream_sliced_vbi_cap field 2021-04-12 11:02 ` [PATCH v2 2/3] media: vivid: remove stream_sliced_vbi_cap field Tomi Valkeinen @ 2021-06-04 14:41 ` Laurent Pinchart 0 siblings, 0 replies; 11+ messages in thread From: Laurent Pinchart @ 2021-06-04 14:41 UTC (permalink / raw) To: Tomi Valkeinen Cc: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media Hi Tomi, Thank you for the patch. On Mon, Apr 12, 2021 at 02:02:10PM +0300, Tomi Valkeinen wrote: > Vivid tracks the VBI capture mode in vivid_dev->stream_sliced_vbi_cap > field. We can just look at the buffer type instead, and drop the field. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/test-drivers/vivid/vivid-core.h | 1 - > drivers/media/test-drivers/vivid/vivid-kthread-cap.c | 2 +- > drivers/media/test-drivers/vivid/vivid-vbi-cap.c | 6 ++---- > 3 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/test-drivers/vivid/vivid-core.h b/drivers/media/test-drivers/vivid/vivid-core.h > index 9c2d1470b597..9af7e843c2cf 100644 > --- a/drivers/media/test-drivers/vivid/vivid-core.h > +++ b/drivers/media/test-drivers/vivid/vivid-core.h > @@ -428,7 +428,6 @@ struct vivid_dev { > u32 vbi_cap_seq_start; > u32 vbi_cap_seq_count; > bool vbi_cap_streaming; > - bool stream_sliced_vbi_cap; > u32 meta_cap_seq_start; > u32 meta_cap_seq_count; > bool meta_cap_streaming; > diff --git a/drivers/media/test-drivers/vivid/vivid-kthread-cap.c b/drivers/media/test-drivers/vivid/vivid-kthread-cap.c > index 67fb3c00f9ad..781763c193eb 100644 > --- a/drivers/media/test-drivers/vivid/vivid-kthread-cap.c > +++ b/drivers/media/test-drivers/vivid/vivid-kthread-cap.c > @@ -750,7 +750,7 @@ static noinline_for_stack void vivid_thread_vid_cap_tick(struct vivid_dev *dev, > > v4l2_ctrl_request_setup(vbi_cap_buf->vb.vb2_buf.req_obj.req, > &dev->ctrl_hdl_vbi_cap); > - if (dev->stream_sliced_vbi_cap) > + if (vbi_cap_buf->vb.vb2_buf.type == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE) > vivid_sliced_vbi_cap_process(dev, vbi_cap_buf); > else > vivid_raw_vbi_cap_process(dev, vbi_cap_buf); > diff --git a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c > index 1a9348eea781..387df4ff01b0 100644 > --- a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c > +++ b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c > @@ -255,9 +255,8 @@ int vidioc_s_fmt_vbi_cap(struct file *file, void *priv, > > if (ret) > return ret; > - if (dev->stream_sliced_vbi_cap && vb2_is_busy(&dev->vb_vbi_cap_q)) > + if (f->type != V4L2_BUF_TYPE_VBI_CAPTURE && vb2_is_busy(&dev->vb_vbi_cap_q)) I think think this is correct. The code checks if the currently configured format is the sliced variant, and you replace this with a check for the new format. > return -EBUSY; > - dev->stream_sliced_vbi_cap = false; > dev->vbi_cap_dev.queue->type = V4L2_BUF_TYPE_VBI_CAPTURE; > return 0; > } > @@ -322,10 +321,9 @@ int vidioc_s_fmt_sliced_vbi_cap(struct file *file, void *fh, struct v4l2_format > > if (ret) > return ret; > - if (!dev->stream_sliced_vbi_cap && vb2_is_busy(&dev->vb_vbi_cap_q)) > + if (fmt->type != V4L2_BUF_TYPE_SLICED_VBI_CAPTURE && vb2_is_busy(&dev->vb_vbi_cap_q)) > return -EBUSY; > dev->service_set_cap = vbi->service_set; > - dev->stream_sliced_vbi_cap = true; > dev->vbi_cap_dev.queue->type = V4L2_BUF_TYPE_SLICED_VBI_CAPTURE; > return 0; > } -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] media: vivid: use vb2_queue_change_type 2021-04-12 11:02 [PATCH v2 0/3] media: add vb2_queue_change_type() helper Tomi Valkeinen 2021-04-12 11:02 ` [PATCH v2 1/3] media: videobuf2-v4l2.c: " Tomi Valkeinen 2021-04-12 11:02 ` [PATCH v2 2/3] media: vivid: remove stream_sliced_vbi_cap field Tomi Valkeinen @ 2021-04-12 11:02 ` Tomi Valkeinen 2021-06-04 14:44 ` Laurent Pinchart 2 siblings, 1 reply; 11+ messages in thread From: Tomi Valkeinen @ 2021-04-12 11:02 UTC (permalink / raw) To: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media, Laurent Pinchart Cc: Tomi Valkeinen Use the new vb2_queue_change_type() function in .vidioc_reqbufs and .vidioc_create_bufs instead of changing the queue type manually in vidioc_s_fmt_vbi_cap() and vidioc_s_fmt_sliced_vbi_cap(). This allows for a more consistent behavior, as .vidioc_reqbufs and .vidioc_create_bufs are when the queue will become "busy". Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- drivers/media/test-drivers/vivid/vivid-core.c | 44 ++++++++++++++++++- .../media/test-drivers/vivid/vivid-vbi-cap.c | 2 - 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c index 0dc65ef3aa14..70676f1ac268 100644 --- a/drivers/media/test-drivers/vivid/vivid-core.c +++ b/drivers/media/test-drivers/vivid/vivid-core.c @@ -656,6 +656,46 @@ static const struct v4l2_file_operations vivid_radio_fops = { .unlocked_ioctl = video_ioctl2, }; +static int vidioc_reqbufs(struct file *file, void *priv, + struct v4l2_requestbuffers *p) +{ + struct video_device *vdev = video_devdata(file); + int r; + + /* + * Sliced and raw VBI capture share the same queue so we must + * change the type. + */ + if (p->type == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE || + p->type == V4L2_BUF_TYPE_VBI_CAPTURE) { + r = vb2_queue_change_type(vdev->queue, p->type); + if (r) + return r; + } + + return vb2_ioctl_reqbufs(file, priv, p); +} + +static int vidioc_create_bufs(struct file *file, void *priv, + struct v4l2_create_buffers *p) +{ + struct video_device *vdev = video_devdata(file); + int r; + + /* + * Sliced and raw VBI capture share the same queue so we must + * change the type. + */ + if (p->format.type == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE || + p->format.type == V4L2_BUF_TYPE_VBI_CAPTURE) { + r = vb2_queue_change_type(vdev->queue, p->format.type); + if (r) + return r; + } + + return vb2_ioctl_create_bufs(file, priv, p); +} + static const struct v4l2_ioctl_ops vivid_ioctl_ops = { .vidioc_querycap = vidioc_querycap, @@ -717,8 +757,8 @@ static const struct v4l2_ioctl_ops vivid_ioctl_ops = { .vidioc_g_fbuf = vidioc_g_fbuf, .vidioc_s_fbuf = vidioc_s_fbuf, - .vidioc_reqbufs = vb2_ioctl_reqbufs, - .vidioc_create_bufs = vb2_ioctl_create_bufs, + .vidioc_reqbufs = vidioc_reqbufs, + .vidioc_create_bufs = vidioc_create_bufs, .vidioc_prepare_buf = vb2_ioctl_prepare_buf, .vidioc_querybuf = vb2_ioctl_querybuf, .vidioc_qbuf = vb2_ioctl_qbuf, diff --git a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c index 387df4ff01b0..b65b02eeeb97 100644 --- a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c +++ b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c @@ -257,7 +257,6 @@ int vidioc_s_fmt_vbi_cap(struct file *file, void *priv, return ret; if (f->type != V4L2_BUF_TYPE_VBI_CAPTURE && vb2_is_busy(&dev->vb_vbi_cap_q)) return -EBUSY; - dev->vbi_cap_dev.queue->type = V4L2_BUF_TYPE_VBI_CAPTURE; return 0; } @@ -324,7 +323,6 @@ int vidioc_s_fmt_sliced_vbi_cap(struct file *file, void *fh, struct v4l2_format if (fmt->type != V4L2_BUF_TYPE_SLICED_VBI_CAPTURE && vb2_is_busy(&dev->vb_vbi_cap_q)) return -EBUSY; dev->service_set_cap = vbi->service_set; - dev->vbi_cap_dev.queue->type = V4L2_BUF_TYPE_SLICED_VBI_CAPTURE; return 0; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] media: vivid: use vb2_queue_change_type 2021-04-12 11:02 ` [PATCH v2 3/3] media: vivid: use vb2_queue_change_type Tomi Valkeinen @ 2021-06-04 14:44 ` Laurent Pinchart 0 siblings, 0 replies; 11+ messages in thread From: Laurent Pinchart @ 2021-06-04 14:44 UTC (permalink / raw) To: Tomi Valkeinen Cc: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab, Hans Verkuil, linux-media Hi Tomi, Thank you for the patch. On Mon, Apr 12, 2021 at 02:02:11PM +0300, Tomi Valkeinen wrote: > Use the new vb2_queue_change_type() function in .vidioc_reqbufs and > .vidioc_create_bufs instead of changing the queue type manually in > vidioc_s_fmt_vbi_cap() and vidioc_s_fmt_sliced_vbi_cap(). > > This allows for a more consistent behavior, as .vidioc_reqbufs and > .vidioc_create_bufs are when the queue will become "busy". > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/test-drivers/vivid/vivid-core.c | 44 ++++++++++++++++++- > .../media/test-drivers/vivid/vivid-vbi-cap.c | 2 - > 2 files changed, 42 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c > index 0dc65ef3aa14..70676f1ac268 100644 > --- a/drivers/media/test-drivers/vivid/vivid-core.c > +++ b/drivers/media/test-drivers/vivid/vivid-core.c > @@ -656,6 +656,46 @@ static const struct v4l2_file_operations vivid_radio_fops = { > .unlocked_ioctl = video_ioctl2, > }; > > +static int vidioc_reqbufs(struct file *file, void *priv, > + struct v4l2_requestbuffers *p) > +{ > + struct video_device *vdev = video_devdata(file); > + int r; s/r/ret/ to match the driver's coding style. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > + /* > + * Sliced and raw VBI capture share the same queue so we must > + * change the type. > + */ > + if (p->type == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE || > + p->type == V4L2_BUF_TYPE_VBI_CAPTURE) { > + r = vb2_queue_change_type(vdev->queue, p->type); > + if (r) > + return r; > + } > + > + return vb2_ioctl_reqbufs(file, priv, p); > +} > + > +static int vidioc_create_bufs(struct file *file, void *priv, > + struct v4l2_create_buffers *p) > +{ > + struct video_device *vdev = video_devdata(file); > + int r; > + > + /* > + * Sliced and raw VBI capture share the same queue so we must > + * change the type. > + */ > + if (p->format.type == V4L2_BUF_TYPE_SLICED_VBI_CAPTURE || > + p->format.type == V4L2_BUF_TYPE_VBI_CAPTURE) { > + r = vb2_queue_change_type(vdev->queue, p->format.type); > + if (r) > + return r; > + } > + > + return vb2_ioctl_create_bufs(file, priv, p); > +} > + > static const struct v4l2_ioctl_ops vivid_ioctl_ops = { > .vidioc_querycap = vidioc_querycap, > > @@ -717,8 +757,8 @@ static const struct v4l2_ioctl_ops vivid_ioctl_ops = { > .vidioc_g_fbuf = vidioc_g_fbuf, > .vidioc_s_fbuf = vidioc_s_fbuf, > > - .vidioc_reqbufs = vb2_ioctl_reqbufs, > - .vidioc_create_bufs = vb2_ioctl_create_bufs, > + .vidioc_reqbufs = vidioc_reqbufs, > + .vidioc_create_bufs = vidioc_create_bufs, > .vidioc_prepare_buf = vb2_ioctl_prepare_buf, > .vidioc_querybuf = vb2_ioctl_querybuf, > .vidioc_qbuf = vb2_ioctl_qbuf, > diff --git a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c > index 387df4ff01b0..b65b02eeeb97 100644 > --- a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c > +++ b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c > @@ -257,7 +257,6 @@ int vidioc_s_fmt_vbi_cap(struct file *file, void *priv, > return ret; > if (f->type != V4L2_BUF_TYPE_VBI_CAPTURE && vb2_is_busy(&dev->vb_vbi_cap_q)) > return -EBUSY; > - dev->vbi_cap_dev.queue->type = V4L2_BUF_TYPE_VBI_CAPTURE; > return 0; > } > > @@ -324,7 +323,6 @@ int vidioc_s_fmt_sliced_vbi_cap(struct file *file, void *fh, struct v4l2_format > if (fmt->type != V4L2_BUF_TYPE_SLICED_VBI_CAPTURE && vb2_is_busy(&dev->vb_vbi_cap_q)) > return -EBUSY; > dev->service_set_cap = vbi->service_set; > - dev->vbi_cap_dev.queue->type = V4L2_BUF_TYPE_SLICED_VBI_CAPTURE; > return 0; > } > -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-06-08 5:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-12 11:02 [PATCH v2 0/3] media: add vb2_queue_change_type() helper Tomi Valkeinen 2021-04-12 11:02 ` [PATCH v2 1/3] media: videobuf2-v4l2.c: " Tomi Valkeinen 2021-04-23 8:40 ` Tomi Valkeinen 2021-06-04 11:13 ` Tomasz Figa 2021-06-04 14:09 ` Tomi Valkeinen 2021-06-08 5:11 ` Tomasz Figa 2021-06-04 14:38 ` Laurent Pinchart 2021-04-12 11:02 ` [PATCH v2 2/3] media: vivid: remove stream_sliced_vbi_cap field Tomi Valkeinen 2021-06-04 14:41 ` Laurent Pinchart 2021-04-12 11:02 ` [PATCH v2 3/3] media: vivid: use vb2_queue_change_type Tomi Valkeinen 2021-06-04 14:44 ` Laurent Pinchart
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.