Re: [RFCv2 PATCH 7/9] vb2: add thread support
Hi Hans, On Wednesday 04 December 2013 08:47:25 Hans Verkuil wrote: On 12/04/2013 02:17 AM, Laurent Pinchart wrote: On Tuesday 03 December 2013 10:56:07 Hans Verkuil wrote: On 11/29/13 19:21, Laurent Pinchart wrote: On Friday 29 November 2013 10:58:42 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com In order to implement vb2 DVB or ALSA support you need to be able to start a kernel thread that queues and dequeues buffers, calling a callback function for every captured/displayed buffer. This patch adds support for that. It's based on drivers/media/v4l2-core/videobuf-dvb.c, but with all the DVB specific stuff stripped out, thus making it much more generic. Do you see any use for this outside of videobuf2-dvb ? If not I wonder whether the code shouldn't be moved there. The sync objects framework being developed for KMS will in my opinion cover the other use cases, and I'd like to discourage non-DVB drivers to use vb2 threads in the meantime. I'm using it for ALSA drivers which, at least in my case, require almost identical functionality as that needed by DVB. You're using videobuf2 for audio ? For this particular board the audio DMA is just another DMA channel. Handling audio DMA is identical to video DMA. Why reinvent the wheel? videobuf2 is more about buffer management than DMA management. As the code is based around a two-dimensional, possibly multiplanar, buffer it's quite hackish to reuse it for audio. Doesn't ALSA offer a buffer management library ? The board I developed this for has somewhat peculiar audio handling (sorry, it's an internal product and I can't go into details), but I'll do the same exercise for another board that I can open source and there audio handling is standard. I want to see if I can use that to develop a videobuf2-alsa.c module that takes care of most of the alsa complexity. I don't know yet how that will work out, I'll have to experiment a bit. But regardless of that, I really don't like the way it was done in the old videobuf framework, mixing low-level videobuf calls/data structure accesses with DVB code. That should be separate. The vb2 core framework should provide the low-level functionality that is needed by the videobuf2-dvb to build on. Right, but I want to make sure that drivers will not start using this directly. What sort of use-cases were you thinking of, other than DVB and ALSA? I don't off-hand see one. That's the thing, I don't see any valid use case, I just want to make sure we won't get crazy use cases implemented with vb2 threads in the future :-) It should be an internal videobuf2 API. I happily add comments to the source and header mentioning that it is for core use only and that for any other uses the mailinglist should be contacted, but I really don't want to mix core vb2 code with DVB code. That should remain separate. OK, that sounds good with me. What about moving thread support to videobuf2-thread.c ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 7/9] vb2: add thread support
On 12/04/2013 05:33 PM, Laurent Pinchart wrote: Hi Hans, On Wednesday 04 December 2013 08:47:25 Hans Verkuil wrote: On 12/04/2013 02:17 AM, Laurent Pinchart wrote: On Tuesday 03 December 2013 10:56:07 Hans Verkuil wrote: On 11/29/13 19:21, Laurent Pinchart wrote: On Friday 29 November 2013 10:58:42 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com In order to implement vb2 DVB or ALSA support you need to be able to start a kernel thread that queues and dequeues buffers, calling a callback function for every captured/displayed buffer. This patch adds support for that. It's based on drivers/media/v4l2-core/videobuf-dvb.c, but with all the DVB specific stuff stripped out, thus making it much more generic. Do you see any use for this outside of videobuf2-dvb ? If not I wonder whether the code shouldn't be moved there. The sync objects framework being developed for KMS will in my opinion cover the other use cases, and I'd like to discourage non-DVB drivers to use vb2 threads in the meantime. I'm using it for ALSA drivers which, at least in my case, require almost identical functionality as that needed by DVB. You're using videobuf2 for audio ? For this particular board the audio DMA is just another DMA channel. Handling audio DMA is identical to video DMA. Why reinvent the wheel? videobuf2 is more about buffer management than DMA management. As the code is based around a two-dimensional, possibly multiplanar, buffer it's quite hackish to reuse it for audio. I disagree with that. vb2 has all the right hooks to start/stop DMA and queue/dequeue buffers. It's used for VBI as well and can just as easily support meta data. For this particular board there is no difference between audio and video DMA. Doesn't ALSA offer a buffer management library? Yes it does. But due to the peculiarities of the particular board it wasn't sufficient. Specifically I must copy the audio data from the alsa buffers to vb2 buffers since the layout of the data differs. As mentioned I will also work on a different board where the audio DMA is much more standard (i.e. the same buffer layout can be used), and I want to investigate if using vb2 in that case makes sense or not. The board I developed this for has somewhat peculiar audio handling (sorry, it's an internal product and I can't go into details), but I'll do the same exercise for another board that I can open source and there audio handling is standard. I want to see if I can use that to develop a videobuf2-alsa.c module that takes care of most of the alsa complexity. I don't know yet how that will work out, I'll have to experiment a bit. But regardless of that, I really don't like the way it was done in the old videobuf framework, mixing low-level videobuf calls/data structure accesses with DVB code. That should be separate. The vb2 core framework should provide the low-level functionality that is needed by the videobuf2-dvb to build on. Right, but I want to make sure that drivers will not start using this directly. What sort of use-cases were you thinking of, other than DVB and ALSA? I don't off-hand see one. That's the thing, I don't see any valid use case, I just want to make sure we won't get crazy use cases implemented with vb2 threads in the future :-) It should be an internal videobuf2 API. I happily add comments to the source and header mentioning that it is for core use only and that for any other uses the mailinglist should be contacted, but I really don't want to mix core vb2 code with DVB code. That should remain separate. OK, that sounds good with me. What about moving thread support to videobuf2-thread.c ? I tried that originally, but in order to do that I had to make a number of low-level vb2 functions extern instead of static, and that was quite messy. So I decided against that. It's not that much code (106 lines), after all. That said, it might be interesting at some point to split off the fileio and thread handling into a separate file. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 7/9] vb2: add thread support
Hi Hans, On Wednesday 04 December 2013 18:03:47 Hans Verkuil wrote: On 12/04/2013 05:33 PM, Laurent Pinchart wrote: On Wednesday 04 December 2013 08:47:25 Hans Verkuil wrote: On 12/04/2013 02:17 AM, Laurent Pinchart wrote: On Tuesday 03 December 2013 10:56:07 Hans Verkuil wrote: On 11/29/13 19:21, Laurent Pinchart wrote: On Friday 29 November 2013 10:58:42 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com In order to implement vb2 DVB or ALSA support you need to be able to start a kernel thread that queues and dequeues buffers, calling a callback function for every captured/displayed buffer. This patch adds support for that. It's based on drivers/media/v4l2-core/videobuf-dvb.c, but with all the DVB specific stuff stripped out, thus making it much more generic. Do you see any use for this outside of videobuf2-dvb ? If not I wonder whether the code shouldn't be moved there. The sync objects framework being developed for KMS will in my opinion cover the other use cases, and I'd like to discourage non-DVB drivers to use vb2 threads in the meantime. I'm using it for ALSA drivers which, at least in my case, require almost identical functionality as that needed by DVB. You're using videobuf2 for audio ? For this particular board the audio DMA is just another DMA channel. Handling audio DMA is identical to video DMA. Why reinvent the wheel? videobuf2 is more about buffer management than DMA management. As the code is based around a two-dimensional, possibly multiplanar, buffer it's quite hackish to reuse it for audio. I disagree with that. vb2 has all the right hooks to start/stop DMA and queue/dequeue buffers. It's used for VBI as well and can just as easily support meta data. For this particular board there is no difference between audio and video DMA. Doesn't ALSA offer a buffer management library? Yes it does. But due to the peculiarities of the particular board it wasn't sufficient. Specifically I must copy the audio data from the alsa buffers to vb2 buffers since the layout of the data differs. As mentioned I will also work on a different board where the audio DMA is much more standard (i.e. the same buffer layout can be used), and I want to investigate if using vb2 in that case makes sense or not. Let's revisit the topic when you'll work on that board then. The board I developed this for has somewhat peculiar audio handling (sorry, it's an internal product and I can't go into details), but I'll do the same exercise for another board that I can open source and there audio handling is standard. I want to see if I can use that to develop a videobuf2-alsa.c module that takes care of most of the alsa complexity. I don't know yet how that will work out, I'll have to experiment a bit. But regardless of that, I really don't like the way it was done in the old videobuf framework, mixing low-level videobuf calls/data structure accesses with DVB code. That should be separate. The vb2 core framework should provide the low-level functionality that is needed by the videobuf2-dvb to build on. Right, but I want to make sure that drivers will not start using this directly. What sort of use-cases were you thinking of, other than DVB and ALSA? I don't off-hand see one. That's the thing, I don't see any valid use case, I just want to make sure we won't get crazy use cases implemented with vb2 threads in the future :-) It should be an internal videobuf2 API. I happily add comments to the source and header mentioning that it is for core use only and that for any other uses the mailinglist should be contacted, but I really don't want to mix core vb2 code with DVB code. That should remain separate. OK, that sounds good with me. What about moving thread support to videobuf2-thread.c ? I tried that originally, but in order to do that I had to make a number of low-level vb2 functions extern instead of static, and that was quite messy. So I decided against that. It's not that much code (106 lines), after all. OK, fair enough. Please just add a comment to the header saying that drivers must not use that API then. That said, it might be interesting at some point to split off the fileio and thread handling into a separate file. That would be nice indeed. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 7/9] vb2: add thread support
On 11/29/13 19:21, Laurent Pinchart wrote:
Hi Hans,
Thank you for the patch.
On Friday 29 November 2013 10:58:42 Hans Verkuil wrote:
From: Hans Verkuil hans.verk...@cisco.com
In order to implement vb2 DVB or ALSA support you need to be able to start
a kernel thread that queues and dequeues buffers, calling a callback
function for every captured/displayed buffer. This patch adds support for
that.
It's based on drivers/media/v4l2-core/videobuf-dvb.c, but with all the DVB
specific stuff stripped out, thus making it much more generic.
Do you see any use for this outside of videobuf2-dvb ? If not I wonder
whether
the code shouldn't be moved there. The sync objects framework being developed
for KMS will in my opinion cover the other use cases, and I'd like to
discourage non-DVB drivers to use vb2 threads in the meantime.
I'm using it for ALSA drivers which, at least in my case, require almost
identical functionality as that needed by DVB. But regardless of that, I really
don't like the way it was done in the old videobuf framework, mixing low-level
videobuf calls/data structure accesses with DVB code. That should be separate.
The vb2 core framework should provide the low-level functionality that is
needed by the videobuf2-dvb to build on.
Regards,
Hans
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/videobuf2-core.c | 134
include/media/videobuf2-core.h | 28 +++
2 files changed, 162 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c index 853d391..a86d033 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -6,6 +6,9 @@
* Author: Pawel Osciak pa...@osciak.com
* Marek Szyprowski m.szyprow...@samsung.com
*
+ * The vb2_thread implementation was based on code from videobuf-dvb.c:
+ * (c) 2004 Gerd Knorr kra...@bytesex.org [SUSE Labs]
+ *
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation.
@@ -18,6 +21,8 @@
#include linux/poll.h
#include linux/slab.h
#include linux/sched.h
+#include linux/freezer.h
+#include linux/kthread.h
#include media/v4l2-dev.h
#include media/v4l2-fh.h
@@ -2508,6 +2513,135 @@ size_t vb2_write(struct vb2_queue *q, const char
__user *data, size_t count, }
EXPORT_SYMBOL_GPL(vb2_write);
+struct vb2_threadio_data {
+struct task_struct *thread;
+vb2_thread_fnc fnc;
+void *priv;
+bool stop;
+};
+
+static int vb2_thread(void *data)
+{
+struct vb2_queue *q = data;
+struct vb2_threadio_data *threadio = q-threadio;
+struct vb2_fileio_data *fileio = q-fileio;
+int prequeue = 0;
+int index = 0;
+int ret = 0;
+
+if (V4L2_TYPE_IS_OUTPUT(q-type))
+prequeue = q-num_buffers;
+
+set_freezable();
+
+for (;;) {
+struct vb2_buffer *vb;
+
+/*
+ * Call vb2_dqbuf to get buffer back.
+ */
+memset(fileio-b, 0, sizeof(fileio-b));
+fileio-b.type = q-type;
+fileio-b.memory = q-memory;
+if (prequeue) {
+fileio-b.index = index++;
+prequeue--;
+} else {
+call_qop(q, wait_finish, q);
+ret = vb2_internal_dqbuf(q, fileio-b, 0);
+call_qop(q, wait_prepare, q);
+dprintk(5, file io: vb2_dqbuf result: %d\n, ret);
+}
+if (threadio-stop)
+break;
+if (ret)
+break;
+try_to_freeze();
+
+vb = q-bufs[fileio-b.index];
+if (!(fileio-b.flags V4L2_BUF_FLAG_ERROR))
+ret = threadio-fnc(vb, threadio-priv);
+if (ret)
+break;
+call_qop(q, wait_finish, q);
+ret = vb2_internal_qbuf(q, fileio-b);
+call_qop(q, wait_prepare, q);
+if (ret)
+break;
+}
+
+/* Hmm, linux becomes *very* unhappy without this ... */
+while (!kthread_should_stop()) {
+set_current_state(TASK_INTERRUPTIBLE);
+schedule();
+}
+return 0;
+}
+
+int vb2_thread_start(struct vb2_queue *q, vb2_thread_fnc fnc, void *priv,
+ const char *thread_name)
+{
+struct vb2_threadio_data *threadio;
+int ret = 0;
+
+if (q-threadio)
+return -EBUSY;
+if (vb2_is_busy(q))
+return -EBUSY;
+if (WARN_ON(q-fileio))
+return -EBUSY;
+
+threadio = kzalloc(sizeof(*threadio), GFP_KERNEL);
+if (threadio == NULL)
+return -ENOMEM;
+threadio-fnc = fnc;
+threadio-priv = priv;
+
+
Re: [RFCv2 PATCH 7/9] vb2: add thread support
Hi Hans, On Tuesday 03 December 2013 10:56:07 Hans Verkuil wrote: On 11/29/13 19:21, Laurent Pinchart wrote: On Friday 29 November 2013 10:58:42 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com In order to implement vb2 DVB or ALSA support you need to be able to start a kernel thread that queues and dequeues buffers, calling a callback function for every captured/displayed buffer. This patch adds support for that. It's based on drivers/media/v4l2-core/videobuf-dvb.c, but with all the DVB specific stuff stripped out, thus making it much more generic. Do you see any use for this outside of videobuf2-dvb ? If not I wonder whether the code shouldn't be moved there. The sync objects framework being developed for KMS will in my opinion cover the other use cases, and I'd like to discourage non-DVB drivers to use vb2 threads in the meantime. I'm using it for ALSA drivers which, at least in my case, require almost identical functionality as that needed by DVB. You're using videobuf2 for audio ? But regardless of that, I really don't like the way it was done in the old videobuf framework, mixing low-level videobuf calls/data structure accesses with DVB code. That should be separate. The vb2 core framework should provide the low-level functionality that is needed by the videobuf2-dvb to build on. Right, but I want to make sure that drivers will not start using this directly. It should be an internal videobuf2 API. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 7/9] vb2: add thread support
On 12/04/2013 02:17 AM, Laurent Pinchart wrote: Hi Hans, On Tuesday 03 December 2013 10:56:07 Hans Verkuil wrote: On 11/29/13 19:21, Laurent Pinchart wrote: On Friday 29 November 2013 10:58:42 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com In order to implement vb2 DVB or ALSA support you need to be able to start a kernel thread that queues and dequeues buffers, calling a callback function for every captured/displayed buffer. This patch adds support for that. It's based on drivers/media/v4l2-core/videobuf-dvb.c, but with all the DVB specific stuff stripped out, thus making it much more generic. Do you see any use for this outside of videobuf2-dvb ? If not I wonder whether the code shouldn't be moved there. The sync objects framework being developed for KMS will in my opinion cover the other use cases, and I'd like to discourage non-DVB drivers to use vb2 threads in the meantime. I'm using it for ALSA drivers which, at least in my case, require almost identical functionality as that needed by DVB. You're using videobuf2 for audio ? For this particular board the audio DMA is just another DMA channel. Handling audio DMA is identical to video DMA. Why reinvent the wheel? The board I developed this for has somewhat peculiar audio handling (sorry, it's an internal product and I can't go into details), but I'll do the same exercise for another board that I can open source and there audio handling is standard. I want to see if I can use that to develop a videobuf2-alsa.c module that takes care of most of the alsa complexity. I don't know yet how that will work out, I'll have to experiment a bit. But regardless of that, I really don't like the way it was done in the old videobuf framework, mixing low-level videobuf calls/data structure accesses with DVB code. That should be separate. The vb2 core framework should provide the low-level functionality that is needed by the videobuf2-dvb to build on. Right, but I want to make sure that drivers will not start using this directly. What sort of use-cases were you thinking of, other than DVB and ALSA? I don't off-hand see one. It should be an internal videobuf2 API. I happily add comments to the source and header mentioning that it is for core use only and that for any other uses the mailinglist should be contacted, but I really don't want to mix core vb2 code with DVB code. That should remain separate. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 7/9] vb2: add thread support
Hi Hans,
Thank you for the patch.
On Friday 29 November 2013 10:58:42 Hans Verkuil wrote:
From: Hans Verkuil hans.verk...@cisco.com
In order to implement vb2 DVB or ALSA support you need to be able to start
a kernel thread that queues and dequeues buffers, calling a callback
function for every captured/displayed buffer. This patch adds support for
that.
It's based on drivers/media/v4l2-core/videobuf-dvb.c, but with all the DVB
specific stuff stripped out, thus making it much more generic.
Do you see any use for this outside of videobuf2-dvb ? If not I wonder whether
the code shouldn't be moved there. The sync objects framework being developed
for KMS will in my opinion cover the other use cases, and I'd like to
discourage non-DVB drivers to use vb2 threads in the meantime.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/videobuf2-core.c | 134
include/media/videobuf2-core.h | 28 +++
2 files changed, 162 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c index 853d391..a86d033 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -6,6 +6,9 @@
* Author: Pawel Osciak pa...@osciak.com
* Marek Szyprowski m.szyprow...@samsung.com
*
+ * The vb2_thread implementation was based on code from videobuf-dvb.c:
+ * (c) 2004 Gerd Knorr kra...@bytesex.org [SUSE Labs]
+ *
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation.
@@ -18,6 +21,8 @@
#include linux/poll.h
#include linux/slab.h
#include linux/sched.h
+#include linux/freezer.h
+#include linux/kthread.h
#include media/v4l2-dev.h
#include media/v4l2-fh.h
@@ -2508,6 +2513,135 @@ size_t vb2_write(struct vb2_queue *q, const char
__user *data, size_t count, }
EXPORT_SYMBOL_GPL(vb2_write);
+struct vb2_threadio_data {
+ struct task_struct *thread;
+ vb2_thread_fnc fnc;
+ void *priv;
+ bool stop;
+};
+
+static int vb2_thread(void *data)
+{
+ struct vb2_queue *q = data;
+ struct vb2_threadio_data *threadio = q-threadio;
+ struct vb2_fileio_data *fileio = q-fileio;
+ int prequeue = 0;
+ int index = 0;
+ int ret = 0;
+
+ if (V4L2_TYPE_IS_OUTPUT(q-type))
+ prequeue = q-num_buffers;
+
+ set_freezable();
+
+ for (;;) {
+ struct vb2_buffer *vb;
+
+ /*
+ * Call vb2_dqbuf to get buffer back.
+ */
+ memset(fileio-b, 0, sizeof(fileio-b));
+ fileio-b.type = q-type;
+ fileio-b.memory = q-memory;
+ if (prequeue) {
+ fileio-b.index = index++;
+ prequeue--;
+ } else {
+ call_qop(q, wait_finish, q);
+ ret = vb2_internal_dqbuf(q, fileio-b, 0);
+ call_qop(q, wait_prepare, q);
+ dprintk(5, file io: vb2_dqbuf result: %d\n, ret);
+ }
+ if (threadio-stop)
+ break;
+ if (ret)
+ break;
+ try_to_freeze();
+
+ vb = q-bufs[fileio-b.index];
+ if (!(fileio-b.flags V4L2_BUF_FLAG_ERROR))
+ ret = threadio-fnc(vb, threadio-priv);
+ if (ret)
+ break;
+ call_qop(q, wait_finish, q);
+ ret = vb2_internal_qbuf(q, fileio-b);
+ call_qop(q, wait_prepare, q);
+ if (ret)
+ break;
+ }
+
+ /* Hmm, linux becomes *very* unhappy without this ... */
+ while (!kthread_should_stop()) {
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
+ }
+ return 0;
+}
+
+int vb2_thread_start(struct vb2_queue *q, vb2_thread_fnc fnc, void *priv,
+ const char *thread_name)
+{
+ struct vb2_threadio_data *threadio;
+ int ret = 0;
+
+ if (q-threadio)
+ return -EBUSY;
+ if (vb2_is_busy(q))
+ return -EBUSY;
+ if (WARN_ON(q-fileio))
+ return -EBUSY;
+
+ threadio = kzalloc(sizeof(*threadio), GFP_KERNEL);
+ if (threadio == NULL)
+ return -ENOMEM;
+ threadio-fnc = fnc;
+ threadio-priv = priv;
+
+ ret = __vb2_init_fileio(q, !V4L2_TYPE_IS_OUTPUT(q-type));
+ dprintk(3, file io: vb2_init_fileio result: %d\n, ret);
+ if (ret)
+ goto nomem;
+ q-threadio = threadio;
+ threadio-thread = kthread_run(vb2_thread, q, vb2-%s, thread_name);
+ if (IS_ERR(threadio-thread)) {
+ ret = PTR_ERR(threadio-thread);
+ threadio-thread = NULL;
+ goto nothread;
+ }
+
