* [PATCH 00/11] dvb: add support for memory mapped I/O
@ 2017-12-21 16:17 Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 01/11] media: vb2-core: Fix a bug about unnecessary calls to queue cancel and free Mauro Carvalho Chehab
` (10 more replies)
0 siblings, 11 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:17 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List, Sakari Ailus, Geunyoung Kim,
Philippe Ombredanne, Laurent Pinchart, Seung-Woo Kim,
Junghak Sung, Kyungmin Park, Satendra Singh Thakur, linux-fsdevel,
devendra sharma, Alexander Viro, Pawel Osciak, Marek Szyprowski,
Greg Kroah-Hartman, Thomas Gleixner, Inki Dae, Kate Stewart
This patch series is based on a work made by Samsung in 2015 meant
to add memory-mapped I/O to the Linux media, in order to improve
performance. The preparation patches were merged on that time, but
we didn't have time to test and finish the final patch.
Fortunately, Satendra helped us doing such port. On my tests, even
on USB drivers, where we need to do DMA at URB buffers, the
performance gains seem considerable.
On the tests I did today, with perf stat, the gains were expressive:
- the number of task clocks reduced by 3,5 times;
- the number of context switches reduced by about 4,5 times;
- the number of CPU cycles reduced by almost 3,5 times;
- the number of executed instructions reduced almost 2 times;
- the number of cache references reduced by almost 8 times;
- the number of cache misses reduced more than 4,5 times.
The patches are at:
https://git.linuxtv.org/mchehab/experimental.git/log/?h=dvb_mmap
An userspace test patchset is at:
https://git.linuxtv.org/mchehab/experimental-v4l-utils.git/log/?h=dvb_mmap
More details about my tests can be found on my comments to the
original Satendra's patch:
https://patchwork.linuxtv.org/patch/46101/
Mauro Carvalho Chehab (9):
media: vb2-core: add pr_fmt() macro
media: vb2-core: add a new warning about pending buffers
media: dvb_vb2: fix a warning about streamoff logic
media: move videobuf2 to drivers/media/common
media: dvb uAPI docs: document demux mmap/munmap syscalls
media: dvb uAPI docs: document mmap-related ioctls
media: dvb-core: get rid of mmap reserved field
fs: compat_ioctl: add new DVB demux ioctls
media: dvb_vb2: add SPDX headers
Satendra Singh Thakur (2):
media: vb2-core: Fix a bug about unnecessary calls to queue cancel and
free
media: videobuf2: Add new uAPI for DVB streaming I/O
Documentation/media/uapi/dvb/dmx-expbuf.rst | 88 +++++
Documentation/media/uapi/dvb/dmx-mmap.rst | 116 ++++++
Documentation/media/uapi/dvb/dmx-munmap.rst | 54 +++
Documentation/media/uapi/dvb/dmx-qbuf.rst | 83 ++++
Documentation/media/uapi/dvb/dmx-querybuf.rst | 63 +++
Documentation/media/uapi/dvb/dmx-reqbufs.rst | 74 ++++
Documentation/media/uapi/dvb/dmx_fcalls.rst | 6 +
drivers/media/common/Kconfig | 1 +
drivers/media/common/Makefile | 2 +-
drivers/media/common/videobuf/Kconfig | 31 ++
drivers/media/common/videobuf/Makefile | 7 +
.../videobuf}/videobuf2-core.c | 38 +-
.../videobuf}/videobuf2-dma-contig.c | 0
.../videobuf}/videobuf2-dma-sg.c | 0
.../{v4l2-core => common/videobuf}/videobuf2-dvb.c | 0
.../videobuf}/videobuf2-memops.c | 0
.../videobuf}/videobuf2-v4l2.c | 0
.../videobuf}/videobuf2-vmalloc.c | 0
drivers/media/dvb-core/Makefile | 2 +-
drivers/media/dvb-core/dmxdev.c | 196 ++++++++--
drivers/media/dvb-core/dmxdev.h | 4 +
drivers/media/dvb-core/dvb_vb2.c | 430 +++++++++++++++++++++
drivers/media/dvb-core/dvb_vb2.h | 74 ++++
drivers/media/v4l2-core/Kconfig | 32 --
drivers/media/v4l2-core/Makefile | 7 -
fs/compat_ioctl.c | 5 +
include/uapi/linux/dvb/dmx.h | 63 ++-
27 files changed, 1290 insertions(+), 86 deletions(-)
create mode 100644 Documentation/media/uapi/dvb/dmx-expbuf.rst
create mode 100644 Documentation/media/uapi/dvb/dmx-mmap.rst
create mode 100644 Documentation/media/uapi/dvb/dmx-munmap.rst
create mode 100644 Documentation/media/uapi/dvb/dmx-qbuf.rst
create mode 100644 Documentation/media/uapi/dvb/dmx-querybuf.rst
create mode 100644 Documentation/media/uapi/dvb/dmx-reqbufs.rst
create mode 100644 drivers/media/common/videobuf/Kconfig
create mode 100644 drivers/media/common/videobuf/Makefile
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-core.c (98%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-dma-contig.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-dma-sg.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-dvb.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-memops.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-v4l2.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-vmalloc.c (100%)
create mode 100644 drivers/media/dvb-core/dvb_vb2.c
create mode 100644 drivers/media/dvb-core/dvb_vb2.h
--
2.14.3
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 01/11] media: vb2-core: Fix a bug about unnecessary calls to queue cancel and free
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O Mauro Carvalho Chehab
` (9 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Satendra Singh Thakur, Mauro Carvalho Chehab,
Linux Doc Mailing List, Pawel Osciak, Marek Szyprowski,
Kyungmin Park, Mauro Carvalho Chehab
From: Satendra Singh Thakur <satendra.t@samsung.com>
When the func vb2_core_reqbufs is called first time after
vb2_core_queue_init(), the condition q->memory != memory always gets
satisfied, since q->memory was set to 0 at vb2_core_queue_init().
After the condition is true, unnecessary calls to __vb2_queue_cancel()
and __vb2_queue_free() are done. in such case, *count is non-zero,
q->num_buffers is zero and q->memory is 0, which is not equal to
memory field *count=N, q->num_buffers=0, q->memory != memory.
[mchehab@s-opensource.com: fix checkpatch issues]
Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/v4l2-core/videobuf2-core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index cb115ba6a1d2..21017b478a34 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -662,7 +662,8 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
return -EBUSY;
}
- if (*count == 0 || q->num_buffers != 0 || q->memory != memory) {
+ if (*count == 0 || q->num_buffers != 0 ||
+ (q->memory && q->memory != memory)) {
/*
* We already have buffers allocated, so first check if they
* are not in use and can be freed.
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 01/11] media: vb2-core: Fix a bug about unnecessary calls to queue cancel and free Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2018-01-08 13:54 ` Hans Verkuil
2017-12-21 16:18 ` [PATCH 03/11] media: vb2-core: add pr_fmt() macro Mauro Carvalho Chehab
` (8 subsequent siblings)
10 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Satendra Singh Thakur, Mauro Carvalho Chehab,
Linux Doc Mailing List, Seung-Woo Kim, Greg Kroah-Hartman,
Geunyoung Kim, Philippe Ombredanne, Inki Dae, Thomas Gleixner,
Junghak Sung, devendra sharma, Sakari Ailus,
Mauro Carvalho Chehab
From: Satendra Singh Thakur <satendra.t@samsung.com>
Adds a new uAPI for DVB to use streaming I/O which is implemented
based on videobuf2, using those new ioctls:
- DMX_REQBUFS: Request kernel to allocate buffers which count and size
are dedicated by user.
- DMX_QUERYBUF: Get the buffer information like a memory offset which
will mmap() and be shared with user-space.
- DMX_EXPBUF: Just for testing whether buffer-exporting success or not.
- DMX_QBUF: Pass the buffer to kernel-space.
- DMX_DQBUF: Get back the buffer which may contain TS data.
Originally developed by: Junghak Sung <jh1009.sung@samsung.com>, as
seen at:
https://patchwork.linuxtv.org/patch/31613/
https://patchwork.kernel.org/patch/7334301/
The original patch was written before merging VB2-core functionalities
upstream. When such series was added, several adjustments were made,
fixing some issues with V4L2, causing the original patch to be
non-trivially rebased.
After rebased, a few bugs in the patch were fixed. The patch was
also enhanced it and polling functionality got added.
The main changes over the original patch are:
dvb_vb2_fill_buffer():
- Set the size of the outgoing buffer after while loop using
vb2_set_plane_payload;
- Added NULL check for source buffer as per normal convention
of demux driver, this is called twice, first time with valid
buffer second time with NULL pointer, if its not handled,
it will result in crash
- Restricted spinlock for only list_* operations
dvb_vb2_init():
- Restricted q->io_modes to only VB2_MMAP as its the only
supported mode
dvb_vb2_release():
- Replaced the && in if condiion with &, because otherwise
it was always getting satisfied.
dvb_vb2_stream_off():
- Added list_del code for enqueud buffers upon stream off
dvb_vb2_poll():
- Added this new function in order to support polling
dvb_demux_poll() and dvb_dvr_poll()
- dvb_vb2_poll() is now called from these functions
- Ported this patch and latest videobuf2 to lower kernel versions and
tested auto scan.
[mchehab@s-opensource.com: checkpatch fixes]
Co-developed-by: Junghak Sung <jh1009.sung@samsung.com>
Signed-off-by: Junghak Sung <jh1009.sung@samsung.com>
Signed-off-by: Geunyoung Kim <nenggun.kim@samsung.com>
Acked-by: Seung-Woo Kim <sw0312.kim@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/dvb-core/Makefile | 2 +-
drivers/media/dvb-core/dmxdev.c | 196 +++++++++++++++---
drivers/media/dvb-core/dmxdev.h | 4 +
drivers/media/dvb-core/dvb_vb2.c | 423 +++++++++++++++++++++++++++++++++++++++
drivers/media/dvb-core/dvb_vb2.h | 72 +++++++
include/uapi/linux/dvb/dmx.h | 66 +++++-
6 files changed, 733 insertions(+), 30 deletions(-)
create mode 100644 drivers/media/dvb-core/dvb_vb2.c
create mode 100644 drivers/media/dvb-core/dvb_vb2.h
diff --git a/drivers/media/dvb-core/Makefile b/drivers/media/dvb-core/Makefile
index 47e2e391bfb8..bbc65dfa0a8e 100644
--- a/drivers/media/dvb-core/Makefile
+++ b/drivers/media/dvb-core/Makefile
@@ -7,6 +7,6 @@ dvb-net-$(CONFIG_DVB_NET) := dvb_net.o
dvb-core-objs := dvbdev.o dmxdev.o dvb_demux.o \
dvb_ca_en50221.o dvb_frontend.o \
- $(dvb-net-y) dvb_ringbuffer.o dvb_math.o
+ $(dvb-net-y) dvb_ringbuffer.o dvb_vb2.o dvb_math.o
obj-$(CONFIG_DVB_CORE) += dvb-core.o
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 18e4230865be..4cbb9003a1ed 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -28,6 +28,7 @@
#include <linux/wait.h>
#include <linux/uaccess.h>
#include "dmxdev.h"
+#include "dvb_vb2.h"
static int debug;
@@ -138,14 +139,8 @@ static int dvb_dvr_open(struct inode *inode, struct file *file)
return -ENODEV;
}
- if ((file->f_flags & O_ACCMODE) == O_RDWR) {
- if (!(dmxdev->capabilities & DMXDEV_CAP_DUPLEX)) {
- mutex_unlock(&dmxdev->mutex);
- return -EOPNOTSUPP;
- }
- }
-
- if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
+ if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
+ ((file->f_flags & O_ACCMODE) == O_RDWR)) {
void *mem;
if (!dvbdev->readers) {
@@ -158,6 +153,8 @@ static int dvb_dvr_open(struct inode *inode, struct file *file)
return -ENOMEM;
}
dvb_ringbuffer_init(&dmxdev->dvr_buffer, mem, DVR_BUFFER_SIZE);
+ dvb_vb2_init(&dmxdev->dvr_vb2_ctx, "dvr",
+ file->f_flags & O_NONBLOCK);
dvbdev->readers--;
}
@@ -195,7 +192,11 @@ static int dvb_dvr_release(struct inode *inode, struct file *file)
dmxdev->demux->connect_frontend(dmxdev->demux,
dmxdev->dvr_orig_fe);
}
- if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
+ if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
+ ((file->f_flags & O_ACCMODE) == O_RDWR)) {
+ if (dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
+ dvb_vb2_stream_off(&dmxdev->dvr_vb2_ctx);
+ dvb_vb2_release(&dmxdev->dvr_vb2_ctx);
dvbdev->readers++;
if (dmxdev->dvr_buffer.data) {
void *mem = dmxdev->dvr_buffer.data;
@@ -360,8 +361,8 @@ static int dvb_dmxdev_section_callback(const u8 *buffer1, size_t buffer1_len,
{
struct dmxdev_filter *dmxdevfilter = filter->priv;
int ret;
-
- if (dmxdevfilter->buffer.error) {
+ if (!dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx) &&
+ dmxdevfilter->buffer.error) {
wake_up(&dmxdevfilter->buffer.queue);
return 0;
}
@@ -372,11 +373,19 @@ static int dvb_dmxdev_section_callback(const u8 *buffer1, size_t buffer1_len,
}
del_timer(&dmxdevfilter->timer);
dprintk("section callback %*ph\n", 6, buffer1);
- ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer, buffer1,
- buffer1_len);
- if (ret == buffer1_len) {
- ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer, buffer2,
- buffer2_len);
+ if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx)) {
+ ret = dvb_vb2_fill_buffer(&dmxdevfilter->vb2_ctx,
+ buffer1, buffer1_len);
+ if (ret == buffer1_len)
+ ret = dvb_vb2_fill_buffer(&dmxdevfilter->vb2_ctx,
+ buffer2, buffer2_len);
+ } else {
+ ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer,
+ buffer1, buffer1_len);
+ if (ret == buffer1_len) {
+ ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer,
+ buffer2, buffer2_len);
+ }
}
if (ret < 0)
dmxdevfilter->buffer.error = ret;
@@ -393,6 +402,7 @@ static int dvb_dmxdev_ts_callback(const u8 *buffer1, size_t buffer1_len,
{
struct dmxdev_filter *dmxdevfilter = feed->priv;
struct dvb_ringbuffer *buffer;
+ struct dvb_vb2_ctx *ctx;
int ret;
spin_lock(&dmxdevfilter->dev->lock);
@@ -401,19 +411,30 @@ static int dvb_dmxdev_ts_callback(const u8 *buffer1, size_t buffer1_len,
return 0;
}
- if (dmxdevfilter->params.pes.output == DMX_OUT_TAP
- || dmxdevfilter->params.pes.output == DMX_OUT_TSDEMUX_TAP)
+ if (dmxdevfilter->params.pes.output == DMX_OUT_TAP ||
+ dmxdevfilter->params.pes.output == DMX_OUT_TSDEMUX_TAP) {
buffer = &dmxdevfilter->buffer;
- else
+ ctx = &dmxdevfilter->vb2_ctx;
+ } else {
buffer = &dmxdevfilter->dev->dvr_buffer;
- if (buffer->error) {
- spin_unlock(&dmxdevfilter->dev->lock);
- wake_up(&buffer->queue);
- return 0;
+ ctx = &dmxdevfilter->dev->dvr_vb2_ctx;
+ }
+
+ if (dvb_vb2_is_streaming(ctx)) {
+ ret = dvb_vb2_fill_buffer(ctx, buffer1, buffer1_len);
+ if (ret == buffer1_len)
+ ret = dvb_vb2_fill_buffer(ctx, buffer2, buffer2_len);
+ } else {
+ if (buffer->error) {
+ spin_unlock(&dmxdevfilter->dev->lock);
+ wake_up(&buffer->queue);
+ return 0;
+ }
+ ret = dvb_dmxdev_buffer_write(buffer, buffer1, buffer1_len);
+ if (ret == buffer1_len)
+ ret = dvb_dmxdev_buffer_write(buffer,
+ buffer2, buffer2_len);
}
- ret = dvb_dmxdev_buffer_write(buffer, buffer1, buffer1_len);
- if (ret == buffer1_len)
- ret = dvb_dmxdev_buffer_write(buffer, buffer2, buffer2_len);
if (ret < 0)
buffer->error = ret;
spin_unlock(&dmxdevfilter->dev->lock);
@@ -752,6 +773,8 @@ static int dvb_demux_open(struct inode *inode, struct file *file)
file->private_data = dmxdevfilter;
dvb_ringbuffer_init(&dmxdevfilter->buffer, NULL, 8192);
+ dvb_vb2_init(&dmxdevfilter->vb2_ctx, "demux_filter",
+ file->f_flags & O_NONBLOCK);
dmxdevfilter->type = DMXDEV_TYPE_NONE;
dvb_dmxdev_filter_state_set(dmxdevfilter, DMXDEV_STATE_ALLOCATED);
init_timer(&dmxdevfilter->timer);
@@ -767,6 +790,10 @@ static int dvb_dmxdev_filter_free(struct dmxdev *dmxdev,
{
mutex_lock(&dmxdev->mutex);
mutex_lock(&dmxdevfilter->mutex);
+ if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
+ dvb_vb2_stream_off(&dmxdevfilter->vb2_ctx);
+ dvb_vb2_release(&dmxdevfilter->vb2_ctx);
+
dvb_dmxdev_filter_stop(dmxdevfilter);
dvb_dmxdev_filter_reset(dmxdevfilter);
@@ -1054,6 +1081,53 @@ static int dvb_demux_do_ioctl(struct file *file,
mutex_unlock(&dmxdevfilter->mutex);
break;
+ case DMX_REQBUFS:
+ if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
+ mutex_unlock(&dmxdev->mutex);
+ return -ERESTARTSYS;
+ }
+ ret = dvb_vb2_reqbufs(&dmxdevfilter->vb2_ctx, parg);
+ mutex_unlock(&dmxdevfilter->mutex);
+ break;
+
+ case DMX_QUERYBUF:
+ if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
+ mutex_unlock(&dmxdev->mutex);
+ return -ERESTARTSYS;
+ }
+ ret = dvb_vb2_querybuf(&dmxdevfilter->vb2_ctx, parg);
+ mutex_unlock(&dmxdevfilter->mutex);
+ break;
+
+ case DMX_EXPBUF:
+ if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
+ mutex_unlock(&dmxdev->mutex);
+ return -ERESTARTSYS;
+ }
+ ret = dvb_vb2_expbuf(&dmxdevfilter->vb2_ctx, parg);
+ mutex_unlock(&dmxdevfilter->mutex);
+ break;
+
+ case DMX_QBUF:
+ if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
+ mutex_unlock(&dmxdev->mutex);
+ return -ERESTARTSYS;
+ }
+ ret = dvb_vb2_qbuf(&dmxdevfilter->vb2_ctx, parg);
+ if (ret == 0 && !dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
+ ret = dvb_vb2_stream_on(&dmxdevfilter->vb2_ctx);
+ mutex_unlock(&dmxdevfilter->mutex);
+ break;
+
+ case DMX_DQBUF:
+ if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
+ mutex_unlock(&dmxdev->mutex);
+ return -ERESTARTSYS;
+ }
+ ret = dvb_vb2_dqbuf(&dmxdevfilter->vb2_ctx, parg);
+ mutex_unlock(&dmxdevfilter->mutex);
+ break;
+
default:
ret = -EINVAL;
break;
@@ -1075,6 +1149,8 @@ static unsigned int dvb_demux_poll(struct file *file, poll_table *wait)
if ((!dmxdevfilter) || dmxdevfilter->dev->exit)
return POLLERR;
+ if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
+ return dvb_vb2_poll(&dmxdevfilter->vb2_ctx, file, wait);
poll_wait(file, &dmxdevfilter->buffer.queue, wait);
@@ -1092,11 +1168,31 @@ static unsigned int dvb_demux_poll(struct file *file, poll_table *wait)
return mask;
}
+static int dvb_demux_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ struct dmxdev_filter *dmxdevfilter = file->private_data;
+ struct dmxdev *dmxdev = dmxdevfilter->dev;
+ int ret;
+
+ if (mutex_lock_interruptible(&dmxdev->mutex))
+ return -ERESTARTSYS;
+
+ if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
+ mutex_unlock(&dmxdev->mutex);
+ return -ERESTARTSYS;
+ }
+ ret = dvb_vb2_mmap(&dmxdevfilter->vb2_ctx, vma);
+
+ mutex_unlock(&dmxdevfilter->mutex);
+ mutex_unlock(&dmxdev->mutex);
+
+ return ret;
+}
+
static int dvb_demux_release(struct inode *inode, struct file *file)
{
struct dmxdev_filter *dmxdevfilter = file->private_data;
struct dmxdev *dmxdev = dmxdevfilter->dev;
-
int ret;
ret = dvb_dmxdev_filter_free(dmxdev, dmxdevfilter);
@@ -1120,6 +1216,7 @@ static const struct file_operations dvb_demux_fops = {
.release = dvb_demux_release,
.poll = dvb_demux_poll,
.llseek = default_llseek,
+ .mmap = dvb_demux_mmap,
};
static const struct dvb_device dvbdev_demux = {
@@ -1148,6 +1245,28 @@ static int dvb_dvr_do_ioctl(struct file *file,
ret = dvb_dvr_set_buffer_size(dmxdev, arg);
break;
+ case DMX_REQBUFS:
+ ret = dvb_vb2_reqbufs(&dmxdev->dvr_vb2_ctx, parg);
+ break;
+
+ case DMX_QUERYBUF:
+ ret = dvb_vb2_querybuf(&dmxdev->dvr_vb2_ctx, parg);
+ break;
+
+ case DMX_EXPBUF:
+ ret = dvb_vb2_expbuf(&dmxdev->dvr_vb2_ctx, parg);
+ break;
+
+ case DMX_QBUF:
+ ret = dvb_vb2_qbuf(&dmxdev->dvr_vb2_ctx, parg);
+ if (ret == 0 && !dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
+ ret = dvb_vb2_stream_on(&dmxdev->dvr_vb2_ctx);
+ break;
+
+ case DMX_DQBUF:
+ ret = dvb_vb2_dqbuf(&dmxdev->dvr_vb2_ctx, parg);
+ break;
+
default:
ret = -EINVAL;
break;
@@ -1172,10 +1291,13 @@ static unsigned int dvb_dvr_poll(struct file *file, poll_table *wait)
if (dmxdev->exit)
return POLLERR;
+ if (dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
+ return dvb_vb2_poll(&dmxdev->dvr_vb2_ctx, file, wait);
poll_wait(file, &dmxdev->dvr_buffer.queue, wait);
- if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
+ if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
+ ((file->f_flags & O_ACCMODE) == O_RDWR)) {
if (dmxdev->dvr_buffer.error)
mask |= (POLLIN | POLLRDNORM | POLLPRI | POLLERR);
@@ -1187,6 +1309,23 @@ static unsigned int dvb_dvr_poll(struct file *file, poll_table *wait)
return mask;
}
+static int dvb_dvr_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ struct dvb_device *dvbdev = file->private_data;
+ struct dmxdev *dmxdev = dvbdev->priv;
+ int ret;
+
+ if (dmxdev->exit)
+ return -ENODEV;
+
+ if (mutex_lock_interruptible(&dmxdev->mutex))
+ return -ERESTARTSYS;
+
+ ret = dvb_vb2_mmap(&dmxdev->dvr_vb2_ctx, vma);
+ mutex_unlock(&dmxdev->mutex);
+ return ret;
+}
+
static const struct file_operations dvb_dvr_fops = {
.owner = THIS_MODULE,
.read = dvb_dvr_read,
@@ -1196,6 +1335,7 @@ static const struct file_operations dvb_dvr_fops = {
.release = dvb_dvr_release,
.poll = dvb_dvr_poll,
.llseek = default_llseek,
+ .mmap = dvb_dvr_mmap,
};
static const struct dvb_device dvbdev_dvr = {
diff --git a/drivers/media/dvb-core/dmxdev.h b/drivers/media/dvb-core/dmxdev.h
index 054fd4eb6192..6b6aa80db22b 100644
--- a/drivers/media/dvb-core/dmxdev.h
+++ b/drivers/media/dvb-core/dmxdev.h
@@ -35,6 +35,7 @@
#include "dvbdev.h"
#include "demux.h"
#include "dvb_ringbuffer.h"
+#include "dvb_vb2.h"
enum dmxdev_type {
DMXDEV_TYPE_NONE,
@@ -77,6 +78,7 @@ struct dmxdev_filter {
enum dmxdev_state state;
struct dmxdev *dev;
struct dvb_ringbuffer buffer;
+ struct dvb_vb2_ctx vb2_ctx;
struct mutex mutex;
@@ -104,6 +106,8 @@ struct dmxdev {
struct dvb_ringbuffer dvr_buffer;
#define DVR_BUFFER_SIZE (10*188*1024)
+ struct dvb_vb2_ctx dvr_vb2_ctx;
+
struct mutex mutex;
spinlock_t lock;
};
diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
new file mode 100644
index 000000000000..34193a4acc47
--- /dev/null
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -0,0 +1,423 @@
+/*
+ * dvb-vb2.c - dvb-vb2
+ *
+ * Copyright (C) 2015 Samsung Electronics
+ *
+ * Author: jh1009.sung@samsung.com
+ *
+ * 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.
+ */
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mm.h>
+
+#include "dvbdev.h"
+#include "dvb_vb2.h"
+
+static int vb2_debug;
+module_param(vb2_debug, int, 0644);
+
+#define dprintk(level, fmt, arg...) \
+ do { \
+ if (vb2_debug >= level) \
+ pr_info("vb2: %s: " fmt, __func__, ## arg); \
+ } while (0)
+
+static int _queue_setup(struct vb2_queue *vq,
+ unsigned int *nbuffers, unsigned int *nplanes,
+ unsigned int sizes[], struct device *alloc_devs[])
+{
+ struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
+
+ *nbuffers = ctx->buf_cnt;
+ *nplanes = 1;
+ sizes[0] = ctx->buf_siz;
+
+ /*
+ * videobuf2-vmalloc allocator is context-less so no need to set
+ * alloc_ctxs array.
+ */
+
+ dprintk(3, "[%s] count=%d, size=%d\n", ctx->name,
+ *nbuffers, sizes[0]);
+
+ return 0;
+}
+
+static int _buffer_prepare(struct vb2_buffer *vb)
+{
+ struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+ unsigned long size = ctx->buf_siz;
+
+ if (vb2_plane_size(vb, 0) < size) {
+ dprintk(1, "[%s] data will not fit into plane (%lu < %lu)\n",
+ ctx->name, vb2_plane_size(vb, 0), size);
+ return -EINVAL;
+ }
+
+ vb2_set_plane_payload(vb, 0, size);
+ dprintk(3, "[%s]\n", ctx->name);
+
+ return 0;
+}
+
+static void _buffer_queue(struct vb2_buffer *vb)
+{
+ struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+ struct dvb_buffer *buf = container_of(vb, struct dvb_buffer, vb);
+ unsigned long flags = 0;
+
+ spin_lock_irqsave(&ctx->slock, flags);
+ list_add_tail(&buf->list, &ctx->dvb_q);
+ spin_unlock_irqrestore(&ctx->slock, flags);
+
+ dprintk(3, "[%s]\n", ctx->name);
+}
+
+static int _start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+ struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
+
+ dprintk(3, "[%s] count=%d\n", ctx->name, count);
+ return 0;
+}
+
+static void _stop_streaming(struct vb2_queue *vq)
+{
+ struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
+
+ dprintk(3, "[%s]\n", ctx->name);
+}
+
+static void _dmxdev_lock(struct vb2_queue *vq)
+{
+ struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
+
+ mutex_lock(&ctx->mutex);
+ dprintk(3, "[%s]\n", ctx->name);
+}
+
+static void _dmxdev_unlock(struct vb2_queue *vq)
+{
+ struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
+
+ if (mutex_is_locked(&ctx->mutex))
+ mutex_unlock(&ctx->mutex);
+ dprintk(3, "[%s]\n", ctx->name);
+}
+
+static const struct vb2_ops dvb_vb2_qops = {
+ .queue_setup = _queue_setup,
+ .buf_prepare = _buffer_prepare,
+ .buf_queue = _buffer_queue,
+ .start_streaming = _start_streaming,
+ .stop_streaming = _stop_streaming,
+ .wait_prepare = _dmxdev_unlock,
+ .wait_finish = _dmxdev_lock,
+};
+
+static void _fill_dmx_buffer(struct vb2_buffer *vb, void *pb)
+{
+ struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+ struct dmx_buffer *b = pb;
+
+ b->index = vb->index;
+ b->length = vb->planes[0].length;
+ b->bytesused = vb->planes[0].bytesused;
+ b->offset = vb->planes[0].m.offset;
+ memset(b->reserved, 0, sizeof(b->reserved));
+ dprintk(3, "[%s]\n", ctx->name);
+}
+
+static int _fill_vb2_buffer(struct vb2_buffer *vb,
+ const void *pb, struct vb2_plane *planes)
+{
+ struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+
+ planes[0].bytesused = 0;
+ dprintk(3, "[%s]\n", ctx->name);
+
+ return 0;
+}
+
+static const struct vb2_buf_ops dvb_vb2_buf_ops = {
+ .fill_user_buffer = _fill_dmx_buffer,
+ .fill_vb2_buffer = _fill_vb2_buffer,
+};
+
+/*
+ * Videobuf operations
+ */
+int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int nonblocking)
+{
+ struct vb2_queue *q = &ctx->vb_q;
+ int ret;
+
+ memset(ctx, 0, sizeof(struct dvb_vb2_ctx));
+ q->type = DVB_BUF_TYPE_CAPTURE;
+ /**capture type*/
+ q->is_output = 0;
+ /**only mmap is supported currently*/
+ q->io_modes = VB2_MMAP;
+ q->drv_priv = ctx;
+ q->buf_struct_size = sizeof(struct dvb_buffer);
+ q->min_buffers_needed = 1;
+ q->ops = &dvb_vb2_qops;
+ q->mem_ops = &vb2_vmalloc_memops;
+ q->buf_ops = &dvb_vb2_buf_ops;
+ q->num_buffers = 0;
+ ret = vb2_core_queue_init(q);
+ if (ret) {
+ ctx->state = DVB_VB2_STATE_NONE;
+ dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
+ return ret;
+ }
+
+ mutex_init(&ctx->mutex);
+ spin_lock_init(&ctx->slock);
+ INIT_LIST_HEAD(&ctx->dvb_q);
+
+ strncpy(ctx->name, name, DVB_VB2_NAME_MAX);
+ ctx->nonblocking = nonblocking;
+ ctx->state = DVB_VB2_STATE_INIT;
+
+ dprintk(3, "[%s]\n", ctx->name);
+
+ return 0;
+}
+
+int dvb_vb2_release(struct dvb_vb2_ctx *ctx)
+{
+ struct vb2_queue *q = (struct vb2_queue *)&ctx->vb_q;
+
+ if (ctx->state & DVB_VB2_STATE_INIT)
+ vb2_core_queue_release(q);
+
+ ctx->state = DVB_VB2_STATE_NONE;
+ dprintk(3, "[%s]\n", ctx->name);
+
+ return 0;
+}
+
+int dvb_vb2_stream_on(struct dvb_vb2_ctx *ctx)
+{
+ struct vb2_queue *q = &ctx->vb_q;
+ int ret;
+
+ ret = vb2_core_streamon(q, q->type);
+ if (ret) {
+ ctx->state = DVB_VB2_STATE_NONE;
+ dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
+ return ret;
+ }
+ ctx->state |= DVB_VB2_STATE_STREAMON;
+ dprintk(3, "[%s]\n", ctx->name);
+
+ return 0;
+}
+
+int dvb_vb2_stream_off(struct dvb_vb2_ctx *ctx)
+{
+ struct vb2_queue *q = (struct vb2_queue *)&ctx->vb_q;
+ int ret;
+ unsigned long flags = 0;
+
+ ctx->state &= ~DVB_VB2_STATE_STREAMON;
+ spin_lock_irqsave(&ctx->slock, flags);
+ while (!list_empty(&ctx->dvb_q)) {
+ struct dvb_buffer *buf;
+
+ buf = list_entry(ctx->dvb_q.next,
+ struct dvb_buffer, list);
+ list_del(&buf->list);
+ spin_unlock_irqrestore(&ctx->slock, flags);
+ vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
+ spin_lock_irqsave(&ctx->slock, flags);
+ }
+ spin_unlock_irqrestore(&ctx->slock, flags);
+ ret = vb2_core_streamoff(q, q->type);
+ if (ret) {
+ ctx->state = DVB_VB2_STATE_NONE;
+ dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
+ return ret;
+ }
+ dprintk(3, "[%s]\n", ctx->name);
+
+ return 0;
+}
+
+int dvb_vb2_is_streaming(struct dvb_vb2_ctx *ctx)
+{
+ return (ctx->state & DVB_VB2_STATE_STREAMON);
+}
+
+int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
+ const unsigned char *src, int len)
+{
+ unsigned long flags = 0;
+ void *vbuf = NULL;
+ int todo = len;
+ unsigned char *psrc = (unsigned char *)src;
+ int ll = 0;
+
+ dprintk(3, "[%s] %d bytes are rcvd\n", ctx->name, len);
+ if (!src) {
+ dprintk(3, "[%s]:NULL pointer src\n", ctx->name);
+ /**normal case: This func is called twice from demux driver
+ * once with valid src pointer, second time with NULL pointer
+ */
+ return 0;
+ }
+ while (todo) {
+ if (!ctx->buf) {
+ spin_lock_irqsave(&ctx->slock, flags);
+ if (list_empty(&ctx->dvb_q)) {
+ spin_unlock_irqrestore(&ctx->slock, flags);
+ dprintk(3, "[%s] Buffer overflow!!!\n",
+ ctx->name);
+ break;
+ }
+
+ ctx->buf = list_entry(ctx->dvb_q.next,
+ struct dvb_buffer, list);
+ list_del(&ctx->buf->list);
+ spin_unlock_irqrestore(&ctx->slock, flags);
+ ctx->remain = vb2_plane_size(&ctx->buf->vb, 0);
+ ctx->offset = 0;
+ }
+
+ if (!dvb_vb2_is_streaming(ctx)) {
+ vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_ERROR);
+ ctx->buf = NULL;
+ break;
+ }
+
+ /* Fill buffer */
+ ll = min(todo, ctx->remain);
+ vbuf = vb2_plane_vaddr(&ctx->buf->vb, 0);
+ memcpy(vbuf + ctx->offset, psrc, ll);
+ todo -= ll;
+ psrc += ll;
+
+ ctx->remain -= ll;
+ ctx->offset += ll;
+
+ if (ctx->remain == 0) {
+ vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
+ ctx->buf = NULL;
+ }
+ }
+
+ if (ctx->nonblocking && ctx->buf) {
+ vb2_set_plane_payload(&ctx->buf->vb, 0, ll);
+ vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
+ ctx->buf = NULL;
+ }
+
+ if (todo)
+ dprintk(1, "[%s] %d bytes are dropped.\n", ctx->name, todo);
+ else
+ dprintk(3, "[%s]\n", ctx->name);
+
+ dprintk(3, "[%s] %d bytes are copied\n", ctx->name, len - todo);
+ return (len - todo);
+}
+
+int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req)
+{
+ int ret;
+
+ ctx->buf_siz = req->size;
+ ctx->buf_cnt = req->count;
+ ret = vb2_core_reqbufs(&ctx->vb_q, VB2_MEMORY_MMAP, &req->count);
+ if (ret) {
+ ctx->state = DVB_VB2_STATE_NONE;
+ dprintk(1, "[%s] count=%d size=%d errno=%d\n", ctx->name,
+ ctx->buf_cnt, ctx->buf_siz, ret);
+ return ret;
+ }
+ ctx->state |= DVB_VB2_STATE_REQBUFS;
+ dprintk(3, "[%s] count=%d size=%d\n", ctx->name,
+ ctx->buf_cnt, ctx->buf_siz);
+
+ return 0;
+}
+
+int dvb_vb2_querybuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
+{
+ vb2_core_querybuf(&ctx->vb_q, b->index, b);
+ dprintk(3, "[%s] index=%d\n", ctx->name, b->index);
+ return 0;
+}
+
+int dvb_vb2_expbuf(struct dvb_vb2_ctx *ctx, struct dmx_exportbuffer *exp)
+{
+ struct vb2_queue *q = &ctx->vb_q;
+ int ret;
+
+ ret = vb2_core_expbuf(&ctx->vb_q, &exp->fd, q->type, exp->index,
+ 0, exp->flags);
+ if (ret) {
+ dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,
+ exp->index, ret);
+ return ret;
+ }
+ dprintk(3, "[%s] index=%d fd=%d\n", ctx->name, exp->index, exp->fd);
+
+ return 0;
+}
+
+int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
+{
+ int ret;
+
+ ret = vb2_core_qbuf(&ctx->vb_q, b->index, b);
+ if (ret) {
+ dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,
+ b->index, ret);
+ return ret;
+ }
+ dprintk(5, "[%s] index=%d\n", ctx->name, b->index);
+
+ return 0;
+}
+
+int dvb_vb2_dqbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
+{
+ int ret;
+
+ ret = vb2_core_dqbuf(&ctx->vb_q, &b->index, b, ctx->nonblocking);
+ if (ret) {
+ dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
+ return ret;
+ }
+ dprintk(5, "[%s] index=%d\n", ctx->name, b->index);
+
+ return 0;
+}
+
+int dvb_vb2_mmap(struct dvb_vb2_ctx *ctx, struct vm_area_struct *vma)
+{
+ int ret;
+
+ ret = vb2_mmap(&ctx->vb_q, vma);
+ if (ret) {
+ dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
+ return ret;
+ }
+ dprintk(3, "[%s] ret=%d\n", ctx->name, ret);
+
+ return 0;
+}
+
+unsigned int dvb_vb2_poll(struct dvb_vb2_ctx *ctx, struct file *file,
+ poll_table *wait)
+{
+ dprintk(3, "[%s]\n", ctx->name);
+ return vb2_core_poll(&ctx->vb_q, file, wait);
+}
+
diff --git a/drivers/media/dvb-core/dvb_vb2.h b/drivers/media/dvb-core/dvb_vb2.h
new file mode 100644
index 000000000000..d68653926d91
--- /dev/null
+++ b/drivers/media/dvb-core/dvb_vb2.h
@@ -0,0 +1,72 @@
+/*
+ * dvb-vb2.h - DVB driver helper framework for streaming I/O
+ *
+ * Copyright (C) 2015 Samsung Electronics
+ *
+ * Author: jh1009.sung@samsung.com
+ *
+ * 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.
+ */
+
+#ifndef _DVB_VB2_H
+#define _DVB_VB2_H
+
+#include <linux/mutex.h>
+#include <linux/poll.h>
+#include <linux/dvb/dmx.h>
+#include <media/videobuf2-core.h>
+#include <media/videobuf2-dma-contig.h>
+#include <media/videobuf2-vmalloc.h>
+
+enum dvb_buf_type {
+ DVB_BUF_TYPE_CAPTURE = 1,
+ DVB_BUF_TYPE_OUTPUT = 2,
+};
+
+#define DVB_VB2_STATE_NONE (0x0)
+#define DVB_VB2_STATE_INIT (0x1)
+#define DVB_VB2_STATE_REQBUFS (0x2)
+#define DVB_VB2_STATE_STREAMON (0x4)
+
+#define DVB_VB2_NAME_MAX (20)
+
+struct dvb_buffer {
+ struct vb2_buffer vb;
+ struct list_head list;
+};
+
+struct dvb_vb2_ctx {
+ struct vb2_queue vb_q;
+ struct mutex mutex;
+ spinlock_t slock;
+ struct list_head dvb_q;
+ struct dvb_buffer *buf;
+ int offset;
+ int remain;
+ int state;
+ int buf_siz;
+ int buf_cnt;
+ int nonblocking;
+ char name[DVB_VB2_NAME_MAX + 1];
+};
+
+int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int non_blocking);
+int dvb_vb2_release(struct dvb_vb2_ctx *ctx);
+int dvb_vb2_stream_on(struct dvb_vb2_ctx *ctx);
+int dvb_vb2_stream_off(struct dvb_vb2_ctx *ctx);
+int dvb_vb2_is_streaming(struct dvb_vb2_ctx *ctx);
+int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
+ const unsigned char *src, int len);
+
+int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req);
+int dvb_vb2_querybuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b);
+int dvb_vb2_expbuf(struct dvb_vb2_ctx *ctx, struct dmx_exportbuffer *exp);
+int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b);
+int dvb_vb2_dqbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b);
+int dvb_vb2_mmap(struct dvb_vb2_ctx *ctx, struct vm_area_struct *vma);
+unsigned int dvb_vb2_poll(struct dvb_vb2_ctx *ctx, struct file *file,
+ poll_table *wait);
+
+#endif /* _DVB_VB2_H */
diff --git a/include/uapi/linux/dvb/dmx.h b/include/uapi/linux/dvb/dmx.h
index c10f1324b4ca..e212aa18ad78 100644
--- a/include/uapi/linux/dvb/dmx.h
+++ b/include/uapi/linux/dvb/dmx.h
@@ -211,6 +211,64 @@ struct dmx_stc {
__u64 stc;
};
+/**
+ * struct dmx_buffer - dmx buffer info
+ *
+ * @index: id number of the buffer
+ * @bytesused: number of bytes occupied by data in the buffer (payload);
+ * @offset: for buffers with memory == DMX_MEMORY_MMAP;
+ * offset from the start of the device memory for this plane,
+ * (or a "cookie" that should be passed to mmap() as offset)
+ * @length: size in bytes of the buffer
+ *
+ * Contains data exchanged by application and driver using one of the streaming
+ * I/O methods.
+ */
+struct dmx_buffer {
+ __u32 index;
+ __u32 bytesused;
+ __u32 offset;
+ __u32 length;
+ __u32 reserved[4];
+};
+
+/**
+ * struct dmx_requestbuffers - request dmx buffer information
+ *
+ * @count: number of requested buffers,
+ * @size: size in bytes of the requested buffer
+ *
+ * Contains data used for requesting a dmx buffer.
+ * All reserved fields must be set to zero.
+ */
+struct dmx_requestbuffers {
+ __u32 count;
+ __u32 size;
+ __u32 reserved[2];
+};
+
+/**
+ * struct dmx_exportbuffer - export of dmx buffer as DMABUF file descriptor
+ *
+ * @index: id number of the buffer
+ * @flags: flags for newly created file, currently only O_CLOEXEC is
+ * supported, refer to manual of open syscall for more details
+ * @fd: file descriptor associated with DMABUF (set by driver)
+ *
+ * Contains data used for exporting a dmx buffer as DMABUF file descriptor.
+ * The buffer is identified by a 'cookie' returned by DMX_QUERYBUF
+ * (identical to the cookie used to mmap() the buffer to userspace). All
+ * reserved fields must be set to zero. The field reserved0 is expected to
+ * become a structure 'type' allowing an alternative layout of the structure
+ * content. Therefore this field should not be used for any other extensions.
+ */
+struct dmx_exportbuffer {
+ __u32 index;
+ __u32 flags;
+ __s32 fd;
+ __u32 reserved[5];
+};
+
#define DMX_START _IO('o', 41)
#define DMX_STOP _IO('o', 42)
#define DMX_SET_FILTER _IOW('o', 43, struct dmx_sct_filter_params)
@@ -231,4 +289,10 @@ typedef struct dmx_filter dmx_filter_t;
#endif
-#endif /* _UAPI_DVBDMX_H_ */
+#define DMX_REQBUFS _IOWR('o', 60, struct dmx_requestbuffers)
+#define DMX_QUERYBUF _IOWR('o', 61, struct dmx_buffer)
+#define DMX_EXPBUF _IOWR('o', 62, struct dmx_exportbuffer)
+#define DMX_QBUF _IOWR('o', 63, struct dmx_buffer)
+#define DMX_DQBUF _IOWR('o', 64, struct dmx_buffer)
+
+#endif /* _DVBDMX_H_ */
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 03/11] media: vb2-core: add pr_fmt() macro
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 01/11] media: vb2-core: Fix a bug about unnecessary calls to queue cancel and free Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 04/11] media: vb2-core: add a new warning about pending buffers Mauro Carvalho Chehab
` (7 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List, Pawel Osciak, Marek Szyprowski,
Kyungmin Park
Simplify the pr_foo() macros by adding a pr_fmt() macro.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/v4l2-core/videobuf2-core.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 21017b478a34..319ab8bf220f 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -14,6 +14,8 @@
* the Free Software Foundation.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/err.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -32,10 +34,10 @@
static int debug;
module_param(debug, int, 0644);
-#define dprintk(level, fmt, arg...) \
- do { \
- if (debug >= level) \
- pr_info("vb2-core: %s: " fmt, __func__, ## arg); \
+#define dprintk(level, fmt, arg...) \
+ do { \
+ if (debug >= level) \
+ pr_info("%s: " fmt, __func__, ## arg); \
} while (0)
#ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -460,12 +462,12 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
q->cnt_wait_prepare != q->cnt_wait_finish;
if (unbalanced || debug) {
- pr_info("vb2: counters for queue %p:%s\n", q,
+ pr_info("counters for queue %p:%s\n", q,
unbalanced ? " UNBALANCED!" : "");
- pr_info("vb2: setup: %u start_streaming: %u stop_streaming: %u\n",
+ 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("vb2: wait_prepare: %u wait_finish: %u\n",
+ pr_info(" wait_prepare: %u wait_finish: %u\n",
q->cnt_wait_prepare, q->cnt_wait_finish);
}
q->cnt_queue_setup = 0;
@@ -486,23 +488,23 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
vb->cnt_buf_init != vb->cnt_buf_cleanup;
if (unbalanced || debug) {
- pr_info("vb2: counters for queue %p, buffer %d:%s\n",
+ pr_info(" counters for queue %p, buffer %d:%s\n",
q, buffer, unbalanced ? " UNBALANCED!" : "");
- pr_info("vb2: buf_init: %u buf_cleanup: %u buf_prepare: %u buf_finish: %u\n",
+ pr_info(" buf_init: %u buf_cleanup: %u buf_prepare: %u buf_finish: %u\n",
vb->cnt_buf_init, vb->cnt_buf_cleanup,
vb->cnt_buf_prepare, vb->cnt_buf_finish);
- pr_info("vb2: buf_queue: %u buf_done: %u\n",
+ pr_info(" buf_queue: %u buf_done: %u\n",
vb->cnt_buf_queue, vb->cnt_buf_done);
- pr_info("vb2: alloc: %u put: %u prepare: %u finish: %u mmap: %u\n",
+ pr_info(" alloc: %u put: %u prepare: %u finish: %u mmap: %u\n",
vb->cnt_mem_alloc, vb->cnt_mem_put,
vb->cnt_mem_prepare, vb->cnt_mem_finish,
vb->cnt_mem_mmap);
- pr_info("vb2: get_userptr: %u put_userptr: %u\n",
+ pr_info(" get_userptr: %u put_userptr: %u\n",
vb->cnt_mem_get_userptr, vb->cnt_mem_put_userptr);
- pr_info("vb2: attach_dmabuf: %u detach_dmabuf: %u map_dmabuf: %u unmap_dmabuf: %u\n",
+ pr_info(" attach_dmabuf: %u detach_dmabuf: %u map_dmabuf: %u unmap_dmabuf: %u\n",
vb->cnt_mem_attach_dmabuf, vb->cnt_mem_detach_dmabuf,
vb->cnt_mem_map_dmabuf, vb->cnt_mem_unmap_dmabuf);
- pr_info("vb2: get_dmabuf: %u num_users: %u vaddr: %u cookie: %u\n",
+ pr_info(" get_dmabuf: %u num_users: %u vaddr: %u cookie: %u\n",
vb->cnt_mem_get_dmabuf,
vb->cnt_mem_num_users,
vb->cnt_mem_vaddr,
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 04/11] media: vb2-core: add a new warning about pending buffers
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
` (2 preceding siblings ...)
2017-12-21 16:18 ` [PATCH 03/11] media: vb2-core: add pr_fmt() macro Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 05/11] media: dvb_vb2: fix a warning about streamoff logic Mauro Carvalho Chehab
` (6 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List, Pawel Osciak, Marek Szyprowski,
Kyungmin Park
There's a logic at the VB2 core that produces a WARN_ON if
there are still buffers waiting to be filled. However, it doesn't
indicate what buffers are still opened, with makes harder to
identify issues inside caller drivers.
So, add a new pr_warn() pointing to such buffers. That, together
with debug instrumentation inside the drivers can make easier to
identify where the problem is.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/v4l2-core/videobuf2-core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 319ab8bf220f..064d3c6f1e74 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1653,8 +1653,11 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
*/
if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
for (i = 0; i < q->num_buffers; ++i)
- if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE)
+ if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) {
+ pr_warn("driver bug: stop_streaming operation is leaving buf %p in active state\n",
+ q->bufs[i]);
vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR);
+ }
/* Must be zero now */
WARN_ON(atomic_read(&q->owned_by_drv_count));
}
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 05/11] media: dvb_vb2: fix a warning about streamoff logic
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
` (3 preceding siblings ...)
2017-12-21 16:18 ` [PATCH 04/11] media: vb2-core: add a new warning about pending buffers Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2017-12-22 15:48 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 06/11] media: move videobuf2 to drivers/media/common Mauro Carvalho Chehab
` (5 subsequent siblings)
10 siblings, 1 reply; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List, Satendra Singh Thakur, Inki Dae,
Seung-Woo Kim, Junghak Sung
The streamoff logic is causing those warnings:
WARNING: CPU: 3 PID: 3382 at drivers/media/v4l2-core/videobuf2-core.c:1652 __vb2_queue_cancel+0x177/0x250 [videobuf2_core]
Modules linked in: bnep fuse xt_CHECKSUM iptable_mangle tun ebtable_filter ebtables ip6table_filter ip6_tables xt_physdev br_netfilter bluetooth bridge rfkill ecdh_generic stp llc nf_log_ipv4 nf_log_common xt_LOG xt_conntrack ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c sunrpc vfat fat snd_hda_codec_hdmi rc_dib0700_nec i915 rc_pinnacle_pctv_hd em28xx_rc a8293 ts2020 m88ds3103 i2c_mux em28xx_dvb dib8000 dvb_usb_dib0700 dib0070 dib7000m dib0090 dvb_usb dvb_core uvcvideo snd_usb_audio videobuf2_v4l2 dib3000mc videobuf2_vmalloc videobuf2_memops dibx000_common videobuf2_core rc_core snd_usbmidi_lib snd_rawmidi em28xx tveeprom v4l2_common videodev media intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel
kvm_intel snd_hda_codec kvm snd_hwdep snd_hda_core snd_seq irqbypass crct10dif_pclmul crc32_pclmul i2c_algo_bit ghash_clmulni_intel snd_seq_device drm_kms_helper snd_pcm intel_cstate intel_uncore snd_timer tpm_tis drm mei_wdt iTCO_wdt iTCO_vendor_support tpm_tis_core snd intel_rapl_perf mei_me mei tpm i2c_i801 soundcore lpc_ich video binfmt_misc hid_logitech_hidpp hid_logitech_dj e1000e crc32c_intel ptp pps_core analog gameport joydev
CPU: 3 PID: 3382 Comm: lt-dvbv5-zap Not tainted 4.14.0+ #3
Hardware name: /D53427RKE, BIOS RKPPT10H.86A.0048.2017.0506.1545 05/06/2017
task: ffff94b93bbe1e40 task.stack: ffffb7a98320c000
RIP: 0010:__vb2_queue_cancel+0x177/0x250 [videobuf2_core]
RSP: 0018:ffffb7a98320fd40 EFLAGS: 00010202
RAX: 0000000000000001 RBX: ffff94b92ff72428 RCX: 0000000000000000
RDX: 0000000000000001 RSI: 0000000000000001 RDI: ffff94b92ff72428
RBP: ffffb7a98320fd68 R08: ffff94b92ff725d8 R09: ffffb7a98320fcc8
R10: ffff94b978003d98 R11: ffff94b92ff72428 R12: ffff94b92ff72428
R13: 0000000000000282 R14: ffff94b92059ae20 R15: dead000000000100
FS: 0000000000000000(0000) GS:ffff94b99e380000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000555953007d70 CR3: 000000012be09004 CR4: 00000000001606e0
Call Trace:
vb2_core_streamoff+0x28/0x90 [videobuf2_core]
dvb_vb2_stream_off+0xd1/0x150 [dvb_core]
dvb_dvr_release+0x114/0x120 [dvb_core]
__fput+0xdf/0x1e0
____fput+0xe/0x10
task_work_run+0x94/0xc0
do_exit+0x2dc/0xba0
do_group_exit+0x47/0xb0
SyS_exit_group+0x14/0x20
entry_SYSCALL_64_fastpath+0x1a/0xa5
RIP: 0033:0x7f775e931ed8
RSP: 002b:00007fff07019d68 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000001d02690 RCX: 00007f775e931ed8
RDX: 0000000000000001 RSI: 000000000000003c RDI: 0000000000000001
RBP: 00007fff0701a500 R08: 00000000000000e7 R09: ffffffffffffff70
R10: 00007f775e854dd8 R11: 0000000000000246 R12: 0000000000000000
R13: 00000000035fa000 R14: 000000000000000a R15: 000000000000000a
Code: 00 00 04 74 1c 44 89 e8 49 83 c5 01 41 39 84 24 88 01 00 00 77 8a 5b 41 5c 41 5d 41 5e 41 5f 5d c3 48 89 df e8 bb fd ff ff eb da <0f> ff 41 8b b4 24 88 01 00 00 85 f6 74 34 bb 01 00 00 00 eb 10
There are actually two issues here:
1) list_del() should be called when changing the buffer state;
2) The logic with marks the buffers as done is at the wrong place.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/dvb-core/dvb_vb2.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
index 34193a4acc47..277d33b83089 100644
--- a/drivers/media/dvb-core/dvb_vb2.c
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -89,8 +89,19 @@ static int _start_streaming(struct vb2_queue *vq, unsigned int count)
static void _stop_streaming(struct vb2_queue *vq)
{
struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
+ struct dvb_buffer *buf;
+ unsigned long flags = 0;
dprintk(3, "[%s]\n", ctx->name);
+
+ spin_lock_irqsave(&ctx->slock, flags);
+ while (!list_empty(&ctx->dvb_q)) {
+ buf = list_entry(ctx->dvb_q.next,
+ struct dvb_buffer, list);
+ vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
+ list_del(&buf->list);
+ }
+ spin_unlock_irqrestore(&ctx->slock, flags);
}
static void _dmxdev_lock(struct vb2_queue *vq)
@@ -224,21 +235,8 @@ int dvb_vb2_stream_off(struct dvb_vb2_ctx *ctx)
{
struct vb2_queue *q = (struct vb2_queue *)&ctx->vb_q;
int ret;
- unsigned long flags = 0;
ctx->state &= ~DVB_VB2_STATE_STREAMON;
- spin_lock_irqsave(&ctx->slock, flags);
- while (!list_empty(&ctx->dvb_q)) {
- struct dvb_buffer *buf;
-
- buf = list_entry(ctx->dvb_q.next,
- struct dvb_buffer, list);
- list_del(&buf->list);
- spin_unlock_irqrestore(&ctx->slock, flags);
- vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
- spin_lock_irqsave(&ctx->slock, flags);
- }
- spin_unlock_irqrestore(&ctx->slock, flags);
ret = vb2_core_streamoff(q, q->type);
if (ret) {
ctx->state = DVB_VB2_STATE_NONE;
@@ -291,7 +289,10 @@ int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
}
if (!dvb_vb2_is_streaming(ctx)) {
+ spin_lock_irqsave(&ctx->slock, flags);
vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_ERROR);
+ list_del(&ctx->buf->list);
+ spin_unlock_irqrestore(&ctx->slock, flags);
ctx->buf = NULL;
break;
}
@@ -307,14 +308,20 @@ int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
ctx->offset += ll;
if (ctx->remain == 0) {
+ spin_lock_irqsave(&ctx->slock, flags);
vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
+ list_del(&ctx->buf->list);
+ spin_unlock_irqrestore(&ctx->slock, flags);
ctx->buf = NULL;
}
}
if (ctx->nonblocking && ctx->buf) {
+ spin_lock_irqsave(&ctx->slock, flags);
vb2_set_plane_payload(&ctx->buf->vb, 0, ll);
vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
+ list_del(&ctx->buf->list);
+ spin_unlock_irqrestore(&ctx->slock, flags);
ctx->buf = NULL;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 06/11] media: move videobuf2 to drivers/media/common
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
` (4 preceding siblings ...)
2017-12-21 16:18 ` [PATCH 05/11] media: dvb_vb2: fix a warning about streamoff logic Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 07/11] media: dvb uAPI docs: document demux mmap/munmap syscalls Mauro Carvalho Chehab
` (4 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List, Sakari Ailus, Kate Stewart,
Laurent Pinchart, Thomas Gleixner, Greg Kroah-Hartman
Now that VB2 is used by both V4L2 and DVB core, move it to
the common part of the subsystem.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/common/Kconfig | 1 +
drivers/media/common/Makefile | 2 +-
drivers/media/common/videobuf/Kconfig | 31 +++++++++++++++++++++
drivers/media/common/videobuf/Makefile | 7 +++++
.../videobuf}/videobuf2-core.c | 0
.../videobuf}/videobuf2-dma-contig.c | 0
.../videobuf}/videobuf2-dma-sg.c | 0
.../{v4l2-core => common/videobuf}/videobuf2-dvb.c | 0
.../videobuf}/videobuf2-memops.c | 0
.../videobuf}/videobuf2-v4l2.c | 0
.../videobuf}/videobuf2-vmalloc.c | 0
drivers/media/v4l2-core/Kconfig | 32 ----------------------
drivers/media/v4l2-core/Makefile | 7 -----
13 files changed, 40 insertions(+), 40 deletions(-)
create mode 100644 drivers/media/common/videobuf/Kconfig
create mode 100644 drivers/media/common/videobuf/Makefile
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-core.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-dma-contig.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-dma-sg.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-dvb.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-memops.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-v4l2.c (100%)
rename drivers/media/{v4l2-core => common/videobuf}/videobuf2-vmalloc.c (100%)
diff --git a/drivers/media/common/Kconfig b/drivers/media/common/Kconfig
index 326df0ad75c0..cdfc905967dc 100644
--- a/drivers/media/common/Kconfig
+++ b/drivers/media/common/Kconfig
@@ -16,6 +16,7 @@ config CYPRESS_FIRMWARE
tristate "Cypress firmware helper routines"
depends on USB
+source "drivers/media/common/videobuf/Kconfig"
source "drivers/media/common/b2c2/Kconfig"
source "drivers/media/common/saa7146/Kconfig"
source "drivers/media/common/siano/Kconfig"
diff --git a/drivers/media/common/Makefile b/drivers/media/common/Makefile
index 2d1b0a025084..f24b5ed39982 100644
--- a/drivers/media/common/Makefile
+++ b/drivers/media/common/Makefile
@@ -1,4 +1,4 @@
-obj-y += b2c2/ saa7146/ siano/ v4l2-tpg/
+obj-y += b2c2/ saa7146/ siano/ v4l2-tpg/ videobuf/
obj-$(CONFIG_VIDEO_CX2341X) += cx2341x.o
obj-$(CONFIG_VIDEO_TVEEPROM) += tveeprom.o
obj-$(CONFIG_CYPRESS_FIRMWARE) += cypress_firmware.o
diff --git a/drivers/media/common/videobuf/Kconfig b/drivers/media/common/videobuf/Kconfig
new file mode 100644
index 000000000000..5df05250de94
--- /dev/null
+++ b/drivers/media/common/videobuf/Kconfig
@@ -0,0 +1,31 @@
+# Used by drivers that need Videobuf2 modules
+config VIDEOBUF2_CORE
+ select DMA_SHARED_BUFFER
+ tristate
+
+config VIDEOBUF2_MEMOPS
+ tristate
+ select FRAME_VECTOR
+
+config VIDEOBUF2_DMA_CONTIG
+ tristate
+ depends on HAS_DMA
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_MEMOPS
+ select DMA_SHARED_BUFFER
+
+config VIDEOBUF2_VMALLOC
+ tristate
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_MEMOPS
+ select DMA_SHARED_BUFFER
+
+config VIDEOBUF2_DMA_SG
+ tristate
+ depends on HAS_DMA
+ select VIDEOBUF2_CORE
+ select VIDEOBUF2_MEMOPS
+
+config VIDEOBUF2_DVB
+ tristate
+ select VIDEOBUF2_CORE
diff --git a/drivers/media/common/videobuf/Makefile b/drivers/media/common/videobuf/Makefile
new file mode 100644
index 000000000000..19de5ccda20b
--- /dev/null
+++ b/drivers/media/common/videobuf/Makefile
@@ -0,0 +1,7 @@
+
+obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-core.o videobuf2-v4l2.o
+obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
+obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
+obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
+obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
+obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/common/videobuf/videobuf2-core.c
similarity index 100%
rename from drivers/media/v4l2-core/videobuf2-core.c
rename to drivers/media/common/videobuf/videobuf2-core.c
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/common/videobuf/videobuf2-dma-contig.c
similarity index 100%
rename from drivers/media/v4l2-core/videobuf2-dma-contig.c
rename to drivers/media/common/videobuf/videobuf2-dma-contig.c
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/common/videobuf/videobuf2-dma-sg.c
similarity index 100%
rename from drivers/media/v4l2-core/videobuf2-dma-sg.c
rename to drivers/media/common/videobuf/videobuf2-dma-sg.c
diff --git a/drivers/media/v4l2-core/videobuf2-dvb.c b/drivers/media/common/videobuf/videobuf2-dvb.c
similarity index 100%
rename from drivers/media/v4l2-core/videobuf2-dvb.c
rename to drivers/media/common/videobuf/videobuf2-dvb.c
diff --git a/drivers/media/v4l2-core/videobuf2-memops.c b/drivers/media/common/videobuf/videobuf2-memops.c
similarity index 100%
rename from drivers/media/v4l2-core/videobuf2-memops.c
rename to drivers/media/common/videobuf/videobuf2-memops.c
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/common/videobuf/videobuf2-v4l2.c
similarity index 100%
rename from drivers/media/v4l2-core/videobuf2-v4l2.c
rename to drivers/media/common/videobuf/videobuf2-v4l2.c
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/common/videobuf/videobuf2-vmalloc.c
similarity index 100%
rename from drivers/media/v4l2-core/videobuf2-vmalloc.c
rename to drivers/media/common/videobuf/videobuf2-vmalloc.c
diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index a35c33686abf..fbcb275e867b 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -79,35 +79,3 @@ config VIDEOBUF_DMA_CONTIG
config VIDEOBUF_DVB
tristate
select VIDEOBUF_GEN
-
-# Used by drivers that need Videobuf2 modules
-config VIDEOBUF2_CORE
- select DMA_SHARED_BUFFER
- tristate
-
-config VIDEOBUF2_MEMOPS
- tristate
- select FRAME_VECTOR
-
-config VIDEOBUF2_DMA_CONTIG
- tristate
- depends on HAS_DMA
- select VIDEOBUF2_CORE
- select VIDEOBUF2_MEMOPS
- select DMA_SHARED_BUFFER
-
-config VIDEOBUF2_VMALLOC
- tristate
- select VIDEOBUF2_CORE
- select VIDEOBUF2_MEMOPS
- select DMA_SHARED_BUFFER
-
-config VIDEOBUF2_DMA_SG
- tristate
- depends on HAS_DMA
- select VIDEOBUF2_CORE
- select VIDEOBUF2_MEMOPS
-
-config VIDEOBUF2_DVB
- tristate
- select VIDEOBUF2_CORE
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index 77303286aef7..1618ce984674 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -33,13 +33,6 @@ obj-$(CONFIG_VIDEOBUF_DMA_CONTIG) += videobuf-dma-contig.o
obj-$(CONFIG_VIDEOBUF_VMALLOC) += videobuf-vmalloc.o
obj-$(CONFIG_VIDEOBUF_DVB) += videobuf-dvb.o
-obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-core.o videobuf2-v4l2.o
-obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
-obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
-obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
-obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
-obj-$(CONFIG_VIDEOBUF2_DVB) += videobuf2-dvb.o
-
ccflags-y += -I$(srctree)/drivers/media/dvb-core
ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
ccflags-y += -I$(srctree)/drivers/media/tuners
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 07/11] media: dvb uAPI docs: document demux mmap/munmap syscalls
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
` (5 preceding siblings ...)
2017-12-21 16:18 ` [PATCH 06/11] media: move videobuf2 to drivers/media/common Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 08/11] media: dvb uAPI docs: document mmap-related ioctls Mauro Carvalho Chehab
` (3 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List
With the new dmx mmap interface, those two syscalls are now
handled by the subsystem. Document them.
This patch is based on the V4L2 text for those ioctls.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
Documentation/media/uapi/dvb/dmx-mmap.rst | 116 ++++++++++++++++++++++++++++
Documentation/media/uapi/dvb/dmx-munmap.rst | 54 +++++++++++++
Documentation/media/uapi/dvb/dmx_fcalls.rst | 2 +
3 files changed, 172 insertions(+)
create mode 100644 Documentation/media/uapi/dvb/dmx-mmap.rst
create mode 100644 Documentation/media/uapi/dvb/dmx-munmap.rst
diff --git a/Documentation/media/uapi/dvb/dmx-mmap.rst b/Documentation/media/uapi/dvb/dmx-mmap.rst
new file mode 100644
index 000000000000..15d107348b9f
--- /dev/null
+++ b/Documentation/media/uapi/dvb/dmx-mmap.rst
@@ -0,0 +1,116 @@
+.. _dmx-mmap:
+
+*****************
+Digital TV mmap()
+*****************
+
+Name
+====
+
+dmx-mmap - Map device memory into application address space
+
+.. warning:: this API is still experimental
+
+Synopsis
+========
+
+.. code-block:: c
+
+ #include <unistd.h>
+ #include <sys/mman.h>
+
+
+.. c:function:: void *mmap( void *start, size_t length, int prot, int flags, int fd, off_t offset )
+ :name: dmx-mmap
+
+Arguments
+=========
+
+``start``
+ Map the buffer to this address in the application's address space.
+ When the ``MAP_FIXED`` flag is specified, ``start`` must be a
+ multiple of the pagesize and mmap will fail when the specified
+ address cannot be used. Use of this option is discouraged;
+ applications should just specify a ``NULL`` pointer here.
+
+``length``
+ Length of the memory area to map. This must be a multiple of the
+ DVB packet length (188, on most drivers).
+
+``prot``
+ The ``prot`` argument describes the desired memory protection.
+ Regardless of the device type and the direction of data exchange it
+ should be set to ``PROT_READ`` | ``PROT_WRITE``, permitting read
+ and write access to image buffers. Drivers should support at least
+ this combination of flags.
+
+``flags``
+ The ``flags`` parameter specifies the type of the mapped object,
+ mapping options and whether modifications made to the mapped copy of
+ the page are private to the process or are to be shared with other
+ references.
+
+ ``MAP_FIXED`` requests that the driver selects no other address than
+ the one specified. If the specified address cannot be used,
+ :ref:`mmap() <dmx-mmap>` will fail. If ``MAP_FIXED`` is specified,
+ ``start`` must be a multiple of the pagesize. Use of this option is
+ discouraged.
+
+ One of the ``MAP_SHARED`` or ``MAP_PRIVATE`` flags must be set.
+ ``MAP_SHARED`` allows applications to share the mapped memory with
+ other (e. g. child-) processes.
+
+ .. note::
+
+ The Linux Digital TV applications should not set the
+ ``MAP_PRIVATE``, ``MAP_DENYWRITE``, ``MAP_EXECUTABLE`` or ``MAP_ANON``
+ flags.
+
+``fd``
+ File descriptor returned by :ref:`open() <dmx_fopen>`.
+
+``offset``
+ Offset of the buffer in device memory, as returned by
+ :ref:`DMX_QUERYBUF` ioctl.
+
+
+Description
+===========
+
+The :ref:`mmap() <dmx-mmap>` function asks to map ``length`` bytes starting at
+``offset`` in the memory of the device specified by ``fd`` into the
+application address space, preferably at address ``start``. This latter
+address is a hint only, and is usually specified as 0.
+
+Suitable length and offset parameters are queried with the
+:ref:`DMX_QUERYBUF` ioctl. Buffers must be allocated with the
+:ref:`DMX_REQBUFS` ioctl before they can be queried.
+
+To unmap buffers the :ref:`munmap() <dmx-munmap>` function is used.
+
+
+Return Value
+============
+
+On success :ref:`mmap() <dmx-mmap>` returns a pointer to the mapped buffer. On
+error ``MAP_FAILED`` (-1) is returned, and the ``errno`` variable is set
+appropriately. Possible error codes are:
+
+EBADF
+ ``fd`` is not a valid file descriptor.
+
+EACCES
+ ``fd`` is not open for reading and writing.
+
+EINVAL
+ The ``start`` or ``length`` or ``offset`` are not suitable. (E. g.
+ they are too large, or not aligned on a ``PAGESIZE`` boundary.)
+
+ The ``flags`` or ``prot`` value is not supported.
+
+ No buffers have been allocated with the
+ :ref:`DMX_REQBUFS` ioctl.
+
+ENOMEM
+ Not enough physical or virtual memory was available to complete the
+ request.
diff --git a/Documentation/media/uapi/dvb/dmx-munmap.rst b/Documentation/media/uapi/dvb/dmx-munmap.rst
new file mode 100644
index 000000000000..d77218732bb6
--- /dev/null
+++ b/Documentation/media/uapi/dvb/dmx-munmap.rst
@@ -0,0 +1,54 @@
+.. _dmx-munmap:
+
+************
+DVB munmap()
+************
+
+Name
+====
+
+dmx-munmap - Unmap device memory
+
+.. warning:: This API is still experimental.
+
+
+Synopsis
+========
+
+.. code-block:: c
+
+ #include <unistd.h>
+ #include <sys/mman.h>
+
+
+.. c:function:: int munmap( void *start, size_t length )
+ :name: dmx-munmap
+
+Arguments
+=========
+
+``start``
+ Address of the mapped buffer as returned by the
+ :ref:`mmap() <dmx-mmap>` function.
+
+``length``
+ Length of the mapped buffer. This must be the same value as given to
+ :ref:`mmap() <dmx-mmap>`.
+
+
+Description
+===========
+
+Unmaps a previously with the :ref:`mmap() <dmx-mmap>` function mapped
+buffer and frees it, if possible.
+
+
+Return Value
+============
+
+On success :ref:`munmap() <dmx-munmap>` returns 0, on failure -1 and the
+``errno`` variable is set appropriately:
+
+EINVAL
+ The ``start`` or ``length`` is incorrect, or no buffers have been
+ mapped yet.
diff --git a/Documentation/media/uapi/dvb/dmx_fcalls.rst b/Documentation/media/uapi/dvb/dmx_fcalls.rst
index a17289143220..db19df6dbf70 100644
--- a/Documentation/media/uapi/dvb/dmx_fcalls.rst
+++ b/Documentation/media/uapi/dvb/dmx_fcalls.rst
@@ -13,6 +13,8 @@ Demux Function Calls
dmx-fclose
dmx-fread
dmx-fwrite
+ dmx-mmap
+ dmx-munmap
dmx-start
dmx-stop
dmx-set-filter
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 08/11] media: dvb uAPI docs: document mmap-related ioctls
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
` (6 preceding siblings ...)
2017-12-21 16:18 ` [PATCH 07/11] media: dvb uAPI docs: document demux mmap/munmap syscalls Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 09/11] media: dvb-core: get rid of mmap reserved field Mauro Carvalho Chehab
` (2 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List
5 new ioctls were added to the DVB demux API, in order to
handle memory maped I/O. Add documentation for them.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
Documentation/media/uapi/dvb/dmx-expbuf.rst | 90 +++++++++++++++++++++++++++
Documentation/media/uapi/dvb/dmx-qbuf.rst | 85 +++++++++++++++++++++++++
Documentation/media/uapi/dvb/dmx-querybuf.rst | 65 +++++++++++++++++++
Documentation/media/uapi/dvb/dmx-reqbufs.rst | 76 ++++++++++++++++++++++
Documentation/media/uapi/dvb/dmx_fcalls.rst | 4 ++
5 files changed, 320 insertions(+)
create mode 100644 Documentation/media/uapi/dvb/dmx-expbuf.rst
create mode 100644 Documentation/media/uapi/dvb/dmx-qbuf.rst
create mode 100644 Documentation/media/uapi/dvb/dmx-querybuf.rst
create mode 100644 Documentation/media/uapi/dvb/dmx-reqbufs.rst
diff --git a/Documentation/media/uapi/dvb/dmx-expbuf.rst b/Documentation/media/uapi/dvb/dmx-expbuf.rst
new file mode 100644
index 000000000000..51df34c6fb59
--- /dev/null
+++ b/Documentation/media/uapi/dvb/dmx-expbuf.rst
@@ -0,0 +1,90 @@
+.. _DMX_EXPBUF:
+
+****************
+ioctl DMX_EXPBUF
+****************
+
+Name
+====
+
+DMX_EXPBUF - Export a buffer as a DMABUF file descriptor.
+
+.. warning:: this API is still experimental
+
+
+Synopsis
+========
+
+.. c:function:: int ioctl( int fd, DMX_EXPBUF, struct dmx_exportbuffer *argp )
+ :name: DMX_EXPBUF
+
+
+Arguments
+=========
+
+``fd``
+ File descriptor returned by :ref:`open() <dmx_fopen>`.
+
+``argp``
+ Pointer to struct :c:type:`dmx_exportbuffer`.
+
+
+Description
+===========
+
+This ioctl is an extension to the memory mapping I/O method.
+It can be used to export a buffer as a DMABUF file at any time after
+buffers have been allocated with the :ref:`DMX_REQBUFS` ioctl.
+
+The ``reserved`` array must be zeroed before calling it.
+
+To export a buffer, applications fill struct :c:type:`dmx_exportbuffer`.
+Applications must set the ``index`` field. Valid index numbers
+range from zero to the number of buffers allocated with :ref:`DMX_REQBUFS`
+(struct :c:type:`dmx_requestbuffers` ``count``) minus one.
+Additional flags may be posted in the ``flags`` field. Refer to a manual
+for open() for details. Currently only O_CLOEXEC, O_RDONLY, O_WRONLY,
+and O_RDWR are supported.
+All other fields must be set to zero. In the
+case of multi-planar API, every plane is exported separately using
+multiple :ref:`DMX_EXPBUF` calls.
+
+After calling :ref:`DMX_EXPBUF` the ``fd`` field will be set by a
+driver, on success. This is a DMABUF file descriptor. The application may
+pass it to other DMABUF-aware devices. It is recommended to close a DMABUF
+file when it is no longer used to allow the associated memory to be reclaimed.
+
+
+Examples
+========
+
+
+.. code-block:: c
+
+ int buffer_export(int v4lfd, enum dmx_buf_type bt, int index, int *dmafd)
+ {
+ struct dmx_exportbuffer expbuf;
+
+ memset(&expbuf, 0, sizeof(expbuf));
+ expbuf.type = bt;
+ expbuf.index = index;
+ if (ioctl(v4lfd, DMX_EXPBUF, &expbuf) == -1) {
+ perror("DMX_EXPBUF");
+ return -1;
+ }
+
+ *dmafd = expbuf.fd;
+
+ return 0;
+ }
+
+Return Value
+============
+
+On success 0 is returned, on error -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes <gen-errors>` chapter.
+
+EINVAL
+ A queue is not in MMAP mode or DMABUF exporting is not supported or
+ ``flags`` or ``index`` fields are invalid.
diff --git a/Documentation/media/uapi/dvb/dmx-qbuf.rst b/Documentation/media/uapi/dvb/dmx-qbuf.rst
new file mode 100644
index 000000000000..b20b8153d48d
--- /dev/null
+++ b/Documentation/media/uapi/dvb/dmx-qbuf.rst
@@ -0,0 +1,85 @@
+.. _DMX_QBUF:
+
+*************************
+ioctl DMX_QBUF, DMX_DQBUF
+*************************
+
+Name
+====
+
+DMX_QBUF - DMX_DQBUF - Exchange a buffer with the driver
+
+.. warning:: this API is still experimental
+
+
+Synopsis
+========
+
+.. c:function:: int ioctl( int fd, DMX_QBUF, struct dmx_buffer *argp )
+ :name: DMX_QBUF
+
+.. c:function:: int ioctl( int fd, DMX_DQBUF, struct dmx_buffer *argp )
+ :name: DMX_DQBUF
+
+
+Arguments
+=========
+
+``fd``
+ File descriptor returned by :ref:`open() <dmx_fopen>`.
+
+``argp``
+ Pointer to struct :c:type:`dmx_buffer`.
+
+
+Description
+===========
+
+Applications call the ``DMX_QBUF`` ioctl to enqueue an empty
+(capturing) or filled (output) buffer in the driver's incoming queue.
+The semantics depend on the selected I/O method.
+
+To enqueue a buffer applications set the ``index`` field. Valid index
+numbers range from zero to the number of buffers allocated with
+:ref:`DMX_REQBUFS` (struct :c:type:`dmx_requestbuffers` ``count``) minus
+one. The contents of the struct :c:type:`dmx_buffer` returned
+by a :ref:`DMX_QUERYBUF` ioctl will do as well.
+
+The and ``reserved`` field must be set to 0.
+
+When ``DMX_QBUF`` is called with a pointer to this structure, it locks the
+memory pages of the buffer in physical memory, so they cannot be swapped
+out to disk. Buffers remain locked until dequeued, until the
+the device is closed.
+
+Applications call the ``DMX_DQBUF`` ioctl to dequeue a filled
+(capturing) buffer from the driver's outgoing queue. They just set the ``reserved`` field array to zero. When ``DMX_DQBUF`` is called with a
+pointer to this structure, the driver fills the remaining fields or
+returns an error code.
+
+By default ``DMX_DQBUF`` blocks when no buffer is in the outgoing
+queue. When the ``O_NONBLOCK`` flag was given to the
+:ref:`open() <dmx_fopen>` function, ``DMX_DQBUF`` returns
+immediately with an ``EAGAIN`` error code when no buffer is available.
+
+The struct :c:type:`dmx_buffer` structure is specified in
+:ref:`buffer`.
+
+
+Return Value
+============
+
+On success 0 is returned, on error -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes <gen-errors>` chapter.
+
+EAGAIN
+ Non-blocking I/O has been selected using ``O_NONBLOCK`` and no
+ buffer was in the outgoing queue.
+
+EINVAL
+ The ``index`` is out of bounds, or no buffers have been allocated yet.
+
+EIO
+ ``DMX_DQBUF`` failed due to an internal error. Can also indicate
+ temporary problems like signal loss or CRC errors.
diff --git a/Documentation/media/uapi/dvb/dmx-querybuf.rst b/Documentation/media/uapi/dvb/dmx-querybuf.rst
new file mode 100644
index 000000000000..46a50f191b10
--- /dev/null
+++ b/Documentation/media/uapi/dvb/dmx-querybuf.rst
@@ -0,0 +1,65 @@
+.. _DMX_QUERYBUF:
+
+******************
+ioctl DMX_QUERYBUF
+******************
+
+Name
+====
+
+DMX_QUERYBUF - Query the status of a buffer
+
+.. warning:: this API is still experimental
+
+
+Synopsis
+========
+
+.. c:function:: int ioctl( int fd, DMX_QUERYBUF, struct dvb_buffer *argp )
+ :name: DMX_QUERYBUF
+
+
+Arguments
+=========
+
+``fd``
+ File descriptor returned by :ref:`open() <dmx_fopen>`.
+
+``argp``
+ Pointer to struct :c:type:`dvb_buffer`.
+
+
+Description
+===========
+
+This ioctl is part of the mmap streaming I/O method. It can
+be used to query the status of a buffer at any time after buffers have
+been allocated with the :ref:`DMX_REQBUFS` ioctl.
+
+The ``reserved`` array must be zeroed before calling it.
+
+Applications set the ``index`` field. Valid index numbers range from zero
+to the number of buffers allocated with :ref:`DMX_REQBUFS`
+(struct :c:type:`dvb_requestbuffers` ``count``) minus one.
+
+After calling :ref:`DMX_QUERYBUF` with a pointer to this structure,
+drivers return an error code or fill the rest of the structure.
+
+On success, the ``offset`` will contain the offset of the buffer from the
+start of the device memory, the ``length`` field its size, and the
+``bytesused`` the number of bytes occupied by data in the buffer (payload).
+
+Return Value
+============
+
+On success 0 is returned, the ``offset`` will contain the offset of the
+buffer from the start of the device memory, the ``length`` field its size,
+and the ``bytesused`` the number of bytes occupied by data in the buffer
+(payload).
+
+On error it returns -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes <gen-errors>` chapter.
+
+EINVAL
+ The ``index`` is out of bounds.
diff --git a/Documentation/media/uapi/dvb/dmx-reqbufs.rst b/Documentation/media/uapi/dvb/dmx-reqbufs.rst
new file mode 100644
index 000000000000..0749623d9d83
--- /dev/null
+++ b/Documentation/media/uapi/dvb/dmx-reqbufs.rst
@@ -0,0 +1,76 @@
+.. _DMX_REQBUFS:
+
+*****************
+ioctl DMX_REQBUFS
+*****************
+
+Name
+====
+
+DMX_REQBUFS - Initiate Memory Mapping and/or DMA buffer I/O
+
+.. warning:: this API is still experimental
+
+
+Synopsis
+========
+
+.. c:function:: int ioctl( int fd, DMX_REQBUFS, struct dmx_requestbuffers *argp )
+ :name: DMX_REQBUFS
+
+
+Arguments
+=========
+
+``fd``
+ File descriptor returned by :ref:`open() <dmx_fopen>`.
+
+``argp``
+ Pointer to struct :c:type:`dmx_requestbuffers`.
+
+Description
+===========
+
+This ioctl is used to initiate a memory mapped or DMABUF based demux I/O.
+
+Memory mapped buffers are located in device memory and must be allocated
+with this ioctl before they can be mapped into the application's address
+space. User buffers are allocated by applications themselves, and this
+ioctl is merely used to switch the driver into user pointer I/O mode and
+to setup some internal structures. Similarly, DMABUF buffers are
+allocated by applications through a device driver, and this ioctl only
+configures the driver into DMABUF I/O mode without performing any direct
+allocation.
+
+The ``reserved`` array must be zeroed before calling it.
+
+To allocate device buffers applications initialize all fields of the
+struct :c:type:`dmx_requestbuffers` structure. They set the ``count`` field
+to the desired number of buffers, and ``size`` to the size of each
+buffer.
+
+When the ioctl is called with a pointer to this structure, the driver will
+attempt to allocate the requested number of buffers and it stores the actual
+number allocated in the ``count`` field. The ``count`` can be smaller than the number requested, even zero, when the driver runs out of free memory. A larger
+number is also possible when the driver requires more buffers to
+function correctly. The actual allocated buffer size can is returned
+at ``size``, and can be smaller than what's requested.
+
+When this I/O method is not supported, the ioctl returns an ``EOPNOTSUPP``
+error code.
+
+Applications can call :ref:`DMX_REQBUFS` again to change the number of
+buffers, however this cannot succeed when any buffers are still mapped.
+A ``count`` value of zero frees all buffers, after aborting or finishing
+any DMA in progress.
+
+
+Return Value
+============
+
+On success 0 is returned, on error -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes <gen-errors>` chapter.
+
+EOPNOTSUPP
+ The the requested I/O method is not supported.
diff --git a/Documentation/media/uapi/dvb/dmx_fcalls.rst b/Documentation/media/uapi/dvb/dmx_fcalls.rst
index db19df6dbf70..4c391cf2554f 100644
--- a/Documentation/media/uapi/dvb/dmx_fcalls.rst
+++ b/Documentation/media/uapi/dvb/dmx_fcalls.rst
@@ -24,3 +24,7 @@ Demux Function Calls
dmx-get-pes-pids
dmx-add-pid
dmx-remove-pid
+ dmx-reqbufs
+ dmx-querybuf
+ dmx-expbuf
+ dmx-qbuf
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 09/11] media: dvb-core: get rid of mmap reserved field
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
` (7 preceding siblings ...)
2017-12-21 16:18 ` [PATCH 08/11] media: dvb uAPI docs: document mmap-related ioctls Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 10/11] fs: compat_ioctl: add new DVB demux ioctls Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 11/11] media: dvb_vb2: add SPDX headers Mauro Carvalho Chehab
10 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List, Seung-Woo Kim, Geunyoung Kim,
Junghak Sung, Satendra Singh Thakur
The "reserved" field was a way, used at V4L2 API, to add new
data to existing structs without breaking userspace. However,
there are now clever ways of doing that, without needing to add
an uneeded overhead. So, get rid of them.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
Documentation/media/uapi/dvb/dmx-expbuf.rst | 2 --
Documentation/media/uapi/dvb/dmx-qbuf.rst | 2 --
Documentation/media/uapi/dvb/dmx-querybuf.rst | 2 --
Documentation/media/uapi/dvb/dmx-reqbufs.rst | 2 --
drivers/media/dvb-core/dvb_vb2.c | 1 -
include/uapi/linux/dvb/dmx.h | 3 ---
6 files changed, 12 deletions(-)
diff --git a/Documentation/media/uapi/dvb/dmx-expbuf.rst b/Documentation/media/uapi/dvb/dmx-expbuf.rst
index 51df34c6fb59..2d96cfe891df 100644
--- a/Documentation/media/uapi/dvb/dmx-expbuf.rst
+++ b/Documentation/media/uapi/dvb/dmx-expbuf.rst
@@ -36,8 +36,6 @@ This ioctl is an extension to the memory mapping I/O method.
It can be used to export a buffer as a DMABUF file at any time after
buffers have been allocated with the :ref:`DMX_REQBUFS` ioctl.
-The ``reserved`` array must be zeroed before calling it.
-
To export a buffer, applications fill struct :c:type:`dmx_exportbuffer`.
Applications must set the ``index`` field. Valid index numbers
range from zero to the number of buffers allocated with :ref:`DMX_REQBUFS`
diff --git a/Documentation/media/uapi/dvb/dmx-qbuf.rst b/Documentation/media/uapi/dvb/dmx-qbuf.rst
index b20b8153d48d..b48c4931658e 100644
--- a/Documentation/media/uapi/dvb/dmx-qbuf.rst
+++ b/Documentation/media/uapi/dvb/dmx-qbuf.rst
@@ -45,8 +45,6 @@ numbers range from zero to the number of buffers allocated with
one. The contents of the struct :c:type:`dmx_buffer` returned
by a :ref:`DMX_QUERYBUF` ioctl will do as well.
-The and ``reserved`` field must be set to 0.
-
When ``DMX_QBUF`` is called with a pointer to this structure, it locks the
memory pages of the buffer in physical memory, so they cannot be swapped
out to disk. Buffers remain locked until dequeued, until the
diff --git a/Documentation/media/uapi/dvb/dmx-querybuf.rst b/Documentation/media/uapi/dvb/dmx-querybuf.rst
index 46a50f191b10..89481e24bb86 100644
--- a/Documentation/media/uapi/dvb/dmx-querybuf.rst
+++ b/Documentation/media/uapi/dvb/dmx-querybuf.rst
@@ -36,8 +36,6 @@ This ioctl is part of the mmap streaming I/O method. It can
be used to query the status of a buffer at any time after buffers have
been allocated with the :ref:`DMX_REQBUFS` ioctl.
-The ``reserved`` array must be zeroed before calling it.
-
Applications set the ``index`` field. Valid index numbers range from zero
to the number of buffers allocated with :ref:`DMX_REQBUFS`
(struct :c:type:`dvb_requestbuffers` ``count``) minus one.
diff --git a/Documentation/media/uapi/dvb/dmx-reqbufs.rst b/Documentation/media/uapi/dvb/dmx-reqbufs.rst
index 0749623d9d83..14b80d60bf35 100644
--- a/Documentation/media/uapi/dvb/dmx-reqbufs.rst
+++ b/Documentation/media/uapi/dvb/dmx-reqbufs.rst
@@ -42,8 +42,6 @@ allocated by applications through a device driver, and this ioctl only
configures the driver into DMABUF I/O mode without performing any direct
allocation.
-The ``reserved`` array must be zeroed before calling it.
-
To allocate device buffers applications initialize all fields of the
struct :c:type:`dmx_requestbuffers` structure. They set the ``count`` field
to the desired number of buffers, and ``size`` to the size of each
diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
index 277d33b83089..7b1663f64e84 100644
--- a/drivers/media/dvb-core/dvb_vb2.c
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -140,7 +140,6 @@ static void _fill_dmx_buffer(struct vb2_buffer *vb, void *pb)
b->length = vb->planes[0].length;
b->bytesused = vb->planes[0].bytesused;
b->offset = vb->planes[0].m.offset;
- memset(b->reserved, 0, sizeof(b->reserved));
dprintk(3, "[%s]\n", ctx->name);
}
diff --git a/include/uapi/linux/dvb/dmx.h b/include/uapi/linux/dvb/dmx.h
index e212aa18ad78..5f3c5a918f00 100644
--- a/include/uapi/linux/dvb/dmx.h
+++ b/include/uapi/linux/dvb/dmx.h
@@ -229,7 +229,6 @@ struct dmx_buffer {
__u32 bytesused;
__u32 offset;
__u32 length;
- __u32 reserved[4];
};
/**
@@ -244,7 +243,6 @@ struct dmx_buffer {
struct dmx_requestbuffers {
__u32 count;
__u32 size;
- __u32 reserved[2];
};
/**
@@ -266,7 +264,6 @@ struct dmx_exportbuffer {
__u32 index;
__u32 flags;
__s32 fd;
- __u32 reserved[5];
};
#define DMX_START _IO('o', 41)
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 10/11] fs: compat_ioctl: add new DVB demux ioctls
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
` (8 preceding siblings ...)
2017-12-21 16:18 ` [PATCH 09/11] media: dvb-core: get rid of mmap reserved field Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 11/11] media: dvb_vb2: add SPDX headers Mauro Carvalho Chehab
10 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List, Alexander Viro, linux-fsdevel
Use trivial handling for the new DVB demux ioctls, as none
of them passes a pointer inside their structures.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
fs/compat_ioctl.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index bd5d91e119ca..cc71c3676ce2 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -1333,6 +1333,11 @@ COMPATIBLE_IOCTL(DMX_SET_PES_FILTER)
COMPATIBLE_IOCTL(DMX_SET_BUFFER_SIZE)
COMPATIBLE_IOCTL(DMX_GET_PES_PIDS)
COMPATIBLE_IOCTL(DMX_GET_STC)
+COMPATIBLE_IOCTL(DMX_REQBUFS)
+COMPATIBLE_IOCTL(DMX_QUERYBUF)
+COMPATIBLE_IOCTL(DMX_EXPBUF)
+COMPATIBLE_IOCTL(DMX_QBUF)
+COMPATIBLE_IOCTL(DMX_DQBUF)
COMPATIBLE_IOCTL(FE_GET_INFO)
COMPATIBLE_IOCTL(FE_DISEQC_RESET_OVERLOAD)
COMPATIBLE_IOCTL(FE_DISEQC_SEND_MASTER_CMD)
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 11/11] media: dvb_vb2: add SPDX headers
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
` (9 preceding siblings ...)
2017-12-21 16:18 ` [PATCH 10/11] fs: compat_ioctl: add new DVB demux ioctls Mauro Carvalho Chehab
@ 2017-12-21 16:18 ` Mauro Carvalho Chehab
10 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-21 16:18 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab,
Linux Doc Mailing List, Seung-Woo Kim, Geunyoung Kim,
Satendra Singh Thakur, Inki Dae, Junghak Sung
This code is released under GPL. Add the corresponding SPDX
headers.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
drivers/media/dvb-core/dvb_vb2.c | 1 +
drivers/media/dvb-core/dvb_vb2.h | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
index 7b1663f64e84..10d8f627af3a 100644
--- a/drivers/media/dvb-core/dvb_vb2.c
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL
/*
* dvb-vb2.c - dvb-vb2
*
diff --git a/drivers/media/dvb-core/dvb_vb2.h b/drivers/media/dvb-core/dvb_vb2.h
index d68653926d91..a5164effee16 100644
--- a/drivers/media/dvb-core/dvb_vb2.h
+++ b/drivers/media/dvb-core/dvb_vb2.h
@@ -1,4 +1,6 @@
/*
+ * SPDX-License-Identifier: GPL
+ *
* dvb-vb2.h - DVB driver helper framework for streaming I/O
*
* Copyright (C) 2015 Samsung Electronics
--
2.14.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 05/11] media: dvb_vb2: fix a warning about streamoff logic
2017-12-21 16:18 ` [PATCH 05/11] media: dvb_vb2: fix a warning about streamoff logic Mauro Carvalho Chehab
@ 2017-12-22 15:48 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2017-12-22 15:48 UTC (permalink / raw)
To: Linux Media Mailing List, Jonathan Corbet
Cc: Mauro Carvalho Chehab, Linux Doc Mailing List,
Satendra Singh Thakur, Inki Dae, Seung-Woo Kim, Junghak Sung
Em Thu, 21 Dec 2017 14:18:04 -0200
Mauro Carvalho Chehab <mchehab@s-opensource.com> escreveu:
> The streamoff logic is causing those warnings:
>
> WARNING: CPU: 3 PID: 3382 at drivers/media/v4l2-core/videobuf2-core.c:1652 __vb2_queue_cancel+0x177/0x250 [videobuf2_core]
> Modules linked in: bnep fuse xt_CHECKSUM iptable_mangle tun ebtable_filter ebtables ip6table_filter ip6_tables xt_physdev br_netfilter bluetooth bridge rfkill ecdh_generic stp llc nf_log_ipv4 nf_log_common xt_LOG xt_conntrack ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c sunrpc vfat fat snd_hda_codec_hdmi rc_dib0700_nec i915 rc_pinnacle_pctv_hd em28xx_rc a8293 ts2020 m88ds3103 i2c_mux em28xx_dvb dib8000 dvb_usb_dib0700 dib0070 dib7000m dib0090 dvb_usb dvb_core uvcvideo snd_usb_audio videobuf2_v4l2 dib3000mc videobuf2_vmalloc videobuf2_memops dibx000_common videobuf2_core rc_core snd_usbmidi_lib snd_rawmidi em28xx tveeprom v4l2_common videodev media intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel
> kvm_intel snd_hda_codec kvm snd_hwdep snd_hda_core snd_seq irqbypass crct10dif_pclmul crc32_pclmul i2c_algo_bit ghash_clmulni_intel snd_seq_device drm_kms_helper snd_pcm intel_cstate intel_uncore snd_timer tpm_tis drm mei_wdt iTCO_wdt iTCO_vendor_support tpm_tis_core snd intel_rapl_perf mei_me mei tpm i2c_i801 soundcore lpc_ich video binfmt_misc hid_logitech_hidpp hid_logitech_dj e1000e crc32c_intel ptp pps_core analog gameport joydev
> CPU: 3 PID: 3382 Comm: lt-dvbv5-zap Not tainted 4.14.0+ #3
> Hardware name: /D53427RKE, BIOS RKPPT10H.86A.0048.2017.0506.1545 05/06/2017
> task: ffff94b93bbe1e40 task.stack: ffffb7a98320c000
> RIP: 0010:__vb2_queue_cancel+0x177/0x250 [videobuf2_core]
> RSP: 0018:ffffb7a98320fd40 EFLAGS: 00010202
> RAX: 0000000000000001 RBX: ffff94b92ff72428 RCX: 0000000000000000
> RDX: 0000000000000001 RSI: 0000000000000001 RDI: ffff94b92ff72428
> RBP: ffffb7a98320fd68 R08: ffff94b92ff725d8 R09: ffffb7a98320fcc8
> R10: ffff94b978003d98 R11: ffff94b92ff72428 R12: ffff94b92ff72428
> R13: 0000000000000282 R14: ffff94b92059ae20 R15: dead000000000100
> FS: 0000000000000000(0000) GS:ffff94b99e380000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000555953007d70 CR3: 000000012be09004 CR4: 00000000001606e0
> Call Trace:
> vb2_core_streamoff+0x28/0x90 [videobuf2_core]
> dvb_vb2_stream_off+0xd1/0x150 [dvb_core]
> dvb_dvr_release+0x114/0x120 [dvb_core]
> __fput+0xdf/0x1e0
> ____fput+0xe/0x10
> task_work_run+0x94/0xc0
> do_exit+0x2dc/0xba0
> do_group_exit+0x47/0xb0
> SyS_exit_group+0x14/0x20
> entry_SYSCALL_64_fastpath+0x1a/0xa5
> RIP: 0033:0x7f775e931ed8
> RSP: 002b:00007fff07019d68 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
> RAX: ffffffffffffffda RBX: 0000000001d02690 RCX: 00007f775e931ed8
> RDX: 0000000000000001 RSI: 000000000000003c RDI: 0000000000000001
> RBP: 00007fff0701a500 R08: 00000000000000e7 R09: ffffffffffffff70
> R10: 00007f775e854dd8 R11: 0000000000000246 R12: 0000000000000000
> R13: 00000000035fa000 R14: 000000000000000a R15: 000000000000000a
> Code: 00 00 04 74 1c 44 89 e8 49 83 c5 01 41 39 84 24 88 01 00 00 77 8a 5b 41 5c 41 5d 41 5e 41 5f 5d c3 48 89 df e8 bb fd ff ff eb da <0f> ff 41 8b b4 24 88 01 00 00 85 f6 74 34 bb 01 00 00 00 eb 10
>
> There are actually two issues here:
>
> 1) list_del() should be called when changing the buffer state;
>
> 2) The logic with marks the buffers as done is at the wrong place.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
I ended by sending a wrong version. The one I sent causes a list
corruption, as it deletes a queued value without marking the buffer
as done:
[ 627.233534] list_del corruption, ffffa0aa01182e20->next is LIST_POISON1 (dead000000000100)
Regards,
Mauro
[PATCH] media: dvb_vb2: fix a warning about streamoff logic
The streamoff logic is causing those warnings:
WARNING: CPU: 3 PID: 3382 at drivers/media/v4l2-core/videobuf2-core.c:1652 __vb2_queue_cancel+0x177/0x250 [videobuf2_core]
Modules linked in: bnep fuse xt_CHECKSUM iptable_mangle tun ebtable_filter ebtables ip6table_filter ip6_tables xt_physdev br_netfilter bluetooth bridge rfkill ecdh_generic stp llc nf_log_ipv4 nf_log_common xt_LOG xt_conntrack ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c sunrpc vfat fat snd_hda_codec_hdmi rc_dib0700_nec i915 rc_pinnacle_pctv_hd em28xx_rc a8293 ts2020 m88ds3103 i2c_mux em28xx_dvb dib8000 dvb_usb_dib0700 dib0070 dib7000m dib0090 dvb_usb dvb_core uvcvideo snd_usb_audio videobuf2_v4l2 dib3000mc videobuf2_vmalloc videobuf2_memops dibx000_common videobuf2_core rc_core snd_usbmidi_lib snd_rawmidi em28xx tveeprom v4l2_common videodev media intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel
kvm_intel snd_hda_codec kvm snd_hwdep snd_hda_core snd_seq irqbypass crct10dif_pclmul crc32_pclmul i2c_algo_bit ghash_clmulni_intel snd_seq_device drm_kms_helper snd_pcm intel_cstate intel_uncore snd_timer tpm_tis drm mei_wdt iTCO_wdt iTCO_vendor_support tpm_tis_core snd intel_rapl_perf mei_me mei tpm i2c_i801 soundcore lpc_ich video binfmt_misc hid_logitech_hidpp hid_logitech_dj e1000e crc32c_intel ptp pps_core analog gameport joydev
CPU: 3 PID: 3382 Comm: lt-dvbv5-zap Not tainted 4.14.0+ #3
Hardware name: /D53427RKE, BIOS RKPPT10H.86A.0048.2017.0506.1545 05/06/2017
task: ffff94b93bbe1e40 task.stack: ffffb7a98320c000
RIP: 0010:__vb2_queue_cancel+0x177/0x250 [videobuf2_core]
RSP: 0018:ffffb7a98320fd40 EFLAGS: 00010202
RAX: 0000000000000001 RBX: ffff94b92ff72428 RCX: 0000000000000000
RDX: 0000000000000001 RSI: 0000000000000001 RDI: ffff94b92ff72428
RBP: ffffb7a98320fd68 R08: ffff94b92ff725d8 R09: ffffb7a98320fcc8
R10: ffff94b978003d98 R11: ffff94b92ff72428 R12: ffff94b92ff72428
R13: 0000000000000282 R14: ffff94b92059ae20 R15: dead000000000100
FS: 0000000000000000(0000) GS:ffff94b99e380000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000555953007d70 CR3: 000000012be09004 CR4: 00000000001606e0
Call Trace:
vb2_core_streamoff+0x28/0x90 [videobuf2_core]
dvb_vb2_stream_off+0xd1/0x150 [dvb_core]
dvb_dvr_release+0x114/0x120 [dvb_core]
__fput+0xdf/0x1e0
____fput+0xe/0x10
task_work_run+0x94/0xc0
do_exit+0x2dc/0xba0
do_group_exit+0x47/0xb0
SyS_exit_group+0x14/0x20
entry_SYSCALL_64_fastpath+0x1a/0xa5
RIP: 0033:0x7f775e931ed8
RSP: 002b:00007fff07019d68 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000001d02690 RCX: 00007f775e931ed8
RDX: 0000000000000001 RSI: 000000000000003c RDI: 0000000000000001
RBP: 00007fff0701a500 R08: 00000000000000e7 R09: ffffffffffffff70
R10: 00007f775e854dd8 R11: 0000000000000246 R12: 0000000000000000
R13: 00000000035fa000 R14: 000000000000000a R15: 000000000000000a
Code: 00 00 04 74 1c 44 89 e8 49 83 c5 01 41 39 84 24 88 01 00 00 77 8a 5b 41 5c 41 5d 41 5e 41 5f 5d c3 48 89 df e8 bb fd ff ff eb da <0f> ff 41 8b b4 24 88 01 00 00 85 f6 74 34 bb 01 00 00 00 eb 10
There are actually two issues here:
1) list_del() should be called when changing the buffer state;
2) The logic with marks the buffers as done is at the wrong place.
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
index 01424e67b42e..0588c5520419 100644
--- a/drivers/media/dvb-core/dvb_vb2.c
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -90,8 +90,19 @@ static int _start_streaming(struct vb2_queue *vq, unsigned int count)
static void _stop_streaming(struct vb2_queue *vq)
{
struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
+ struct dvb_buffer *buf;
+ unsigned long flags = 0;
dprintk(3, "[%s]\n", ctx->name);
+
+ spin_lock_irqsave(&ctx->slock, flags);
+ while (!list_empty(&ctx->dvb_q)) {
+ buf = list_entry(ctx->dvb_q.next,
+ struct dvb_buffer, list);
+ vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
+ list_del(&buf->list);
+ }
+ spin_unlock_irqrestore(&ctx->slock, flags);
}
static void _dmxdev_lock(struct vb2_queue *vq)
@@ -225,21 +236,8 @@ int dvb_vb2_stream_off(struct dvb_vb2_ctx *ctx)
{
struct vb2_queue *q = (struct vb2_queue *)&ctx->vb_q;
int ret;
- unsigned long flags = 0;
ctx->state &= ~DVB_VB2_STATE_STREAMON;
- spin_lock_irqsave(&ctx->slock, flags);
- while (!list_empty(&ctx->dvb_q)) {
- struct dvb_buffer *buf;
-
- buf = list_entry(ctx->dvb_q.next,
- struct dvb_buffer, list);
- list_del(&buf->list);
- spin_unlock_irqrestore(&ctx->slock, flags);
- vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
- spin_lock_irqsave(&ctx->slock, flags);
- }
- spin_unlock_irqrestore(&ctx->slock, flags);
ret = vb2_core_streamoff(q, q->type);
if (ret) {
ctx->state = DVB_VB2_STATE_NONE;
@@ -273,11 +271,10 @@ int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
*/
return 0;
}
+ spin_lock_irqsave(&ctx->slock, flags);
while (todo) {
if (!ctx->buf) {
- spin_lock_irqsave(&ctx->slock, flags);
if (list_empty(&ctx->dvb_q)) {
- spin_unlock_irqrestore(&ctx->slock, flags);
dprintk(3, "[%s] Buffer overflow!!!\n",
ctx->name);
break;
@@ -285,14 +282,13 @@ int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
ctx->buf = list_entry(ctx->dvb_q.next,
struct dvb_buffer, list);
- list_del(&ctx->buf->list);
- spin_unlock_irqrestore(&ctx->slock, flags);
ctx->remain = vb2_plane_size(&ctx->buf->vb, 0);
ctx->offset = 0;
}
if (!dvb_vb2_is_streaming(ctx)) {
vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_ERROR);
+ list_del(&ctx->buf->list);
ctx->buf = NULL;
break;
}
@@ -309,6 +305,7 @@ int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
if (ctx->remain == 0) {
vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
+ list_del(&ctx->buf->list);
ctx->buf = NULL;
}
}
@@ -316,8 +313,10 @@ int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
if (ctx->nonblocking && ctx->buf) {
vb2_set_plane_payload(&ctx->buf->vb, 0, ll);
vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
+ list_del(&ctx->buf->list);
ctx->buf = NULL;
}
+ spin_unlock_irqrestore(&ctx->slock, flags);
if (todo)
dprintk(1, "[%s] %d bytes are dropped.\n", ctx->name, todo);
Thanks,
Mauro
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O
2017-12-21 16:18 ` [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O Mauro Carvalho Chehab
@ 2018-01-08 13:54 ` Hans Verkuil
2018-01-08 14:26 ` Hans Verkuil
2018-01-08 14:27 ` Mauro Carvalho Chehab
0 siblings, 2 replies; 17+ messages in thread
From: Hans Verkuil @ 2018-01-08 13:54 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List, Jonathan Corbet
Cc: Satendra Singh Thakur, Mauro Carvalho Chehab,
Linux Doc Mailing List, Seung-Woo Kim, Greg Kroah-Hartman,
Geunyoung Kim, Philippe Ombredanne, Inki Dae, Thomas Gleixner,
Junghak Sung, devendra sharma, Sakari Ailus
Hi Mauro,
Thanks for this patch series, nice to see this moving forward.
Some comments below (note the wrong queue_setup code in particular since that
relates to my comment about the '[PATCH v3] media: videobuf2-core: don't go out
of the buffer range' patch).
On 12/21/2017 05:18 PM, Mauro Carvalho Chehab wrote:
> From: Satendra Singh Thakur <satendra.t@samsung.com>
>
> Adds a new uAPI for DVB to use streaming I/O which is implemented
> based on videobuf2, using those new ioctls:
>
> - DMX_REQBUFS: Request kernel to allocate buffers which count and size
> are dedicated by user.
> - DMX_QUERYBUF: Get the buffer information like a memory offset which
> will mmap() and be shared with user-space.
> - DMX_EXPBUF: Just for testing whether buffer-exporting success or not.
> - DMX_QBUF: Pass the buffer to kernel-space.
> - DMX_DQBUF: Get back the buffer which may contain TS data.
>
> Originally developed by: Junghak Sung <jh1009.sung@samsung.com>, as
> seen at:
> https://patchwork.linuxtv.org/patch/31613/
> https://patchwork.kernel.org/patch/7334301/
>
> The original patch was written before merging VB2-core functionalities
> upstream. When such series was added, several adjustments were made,
> fixing some issues with V4L2, causing the original patch to be
> non-trivially rebased.
>
> After rebased, a few bugs in the patch were fixed. The patch was
> also enhanced it and polling functionality got added.
>
> The main changes over the original patch are:
>
> dvb_vb2_fill_buffer():
> - Set the size of the outgoing buffer after while loop using
> vb2_set_plane_payload;
>
> - Added NULL check for source buffer as per normal convention
> of demux driver, this is called twice, first time with valid
> buffer second time with NULL pointer, if its not handled,
its -> it's
> it will result in crash
>
> - Restricted spinlock for only list_* operations
>
> dvb_vb2_init():
> - Restricted q->io_modes to only VB2_MMAP as its the only
> supported mode
>
> dvb_vb2_release():
> - Replaced the && in if condiion with &, because otherwise
condition
> it was always getting satisfied.
>
> dvb_vb2_stream_off():
> - Added list_del code for enqueud buffers upon stream off
enqueued
>
> dvb_vb2_poll():
> - Added this new function in order to support polling
>
> dvb_demux_poll() and dvb_dvr_poll()
> - dvb_vb2_poll() is now called from these functions
>
> - Ported this patch and latest videobuf2 to lower kernel versions and
> tested auto scan.
>
> [mchehab@s-opensource.com: checkpatch fixes]
> Co-developed-by: Junghak Sung <jh1009.sung@samsung.com>
> Signed-off-by: Junghak Sung <jh1009.sung@samsung.com>
> Signed-off-by: Geunyoung Kim <nenggun.kim@samsung.com>
> Acked-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> drivers/media/dvb-core/Makefile | 2 +-
> drivers/media/dvb-core/dmxdev.c | 196 +++++++++++++++---
> drivers/media/dvb-core/dmxdev.h | 4 +
> drivers/media/dvb-core/dvb_vb2.c | 423 +++++++++++++++++++++++++++++++++++++++
> drivers/media/dvb-core/dvb_vb2.h | 72 +++++++
> include/uapi/linux/dvb/dmx.h | 66 +++++-
> 6 files changed, 733 insertions(+), 30 deletions(-)
> create mode 100644 drivers/media/dvb-core/dvb_vb2.c
> create mode 100644 drivers/media/dvb-core/dvb_vb2.h
>
> diff --git a/drivers/media/dvb-core/Makefile b/drivers/media/dvb-core/Makefile
> index 47e2e391bfb8..bbc65dfa0a8e 100644
> --- a/drivers/media/dvb-core/Makefile
> +++ b/drivers/media/dvb-core/Makefile
> @@ -7,6 +7,6 @@ dvb-net-$(CONFIG_DVB_NET) := dvb_net.o
>
> dvb-core-objs := dvbdev.o dmxdev.o dvb_demux.o \
> dvb_ca_en50221.o dvb_frontend.o \
> - $(dvb-net-y) dvb_ringbuffer.o dvb_math.o
> + $(dvb-net-y) dvb_ringbuffer.o dvb_vb2.o dvb_math.o
>
> obj-$(CONFIG_DVB_CORE) += dvb-core.o
> diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
> index 18e4230865be..4cbb9003a1ed 100644
> --- a/drivers/media/dvb-core/dmxdev.c
> +++ b/drivers/media/dvb-core/dmxdev.c
> @@ -28,6 +28,7 @@
> #include <linux/wait.h>
> #include <linux/uaccess.h>
> #include "dmxdev.h"
> +#include "dvb_vb2.h"
>
> static int debug;
>
> @@ -138,14 +139,8 @@ static int dvb_dvr_open(struct inode *inode, struct file *file)
> return -ENODEV;
> }
>
> - if ((file->f_flags & O_ACCMODE) == O_RDWR) {
> - if (!(dmxdev->capabilities & DMXDEV_CAP_DUPLEX)) {
> - mutex_unlock(&dmxdev->mutex);
> - return -EOPNOTSUPP;
> - }
> - }
> -
> - if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
> + if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
> + ((file->f_flags & O_ACCMODE) == O_RDWR)) {
> void *mem;
>
> if (!dvbdev->readers) {
> @@ -158,6 +153,8 @@ static int dvb_dvr_open(struct inode *inode, struct file *file)
> return -ENOMEM;
> }
> dvb_ringbuffer_init(&dmxdev->dvr_buffer, mem, DVR_BUFFER_SIZE);
> + dvb_vb2_init(&dmxdev->dvr_vb2_ctx, "dvr",
> + file->f_flags & O_NONBLOCK);
> dvbdev->readers--;
> }
>
> @@ -195,7 +192,11 @@ static int dvb_dvr_release(struct inode *inode, struct file *file)
> dmxdev->demux->connect_frontend(dmxdev->demux,
> dmxdev->dvr_orig_fe);
> }
> - if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
> + if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
> + ((file->f_flags & O_ACCMODE) == O_RDWR)) {
> + if (dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
> + dvb_vb2_stream_off(&dmxdev->dvr_vb2_ctx);
> + dvb_vb2_release(&dmxdev->dvr_vb2_ctx);
> dvbdev->readers++;
> if (dmxdev->dvr_buffer.data) {
> void *mem = dmxdev->dvr_buffer.data;
> @@ -360,8 +361,8 @@ static int dvb_dmxdev_section_callback(const u8 *buffer1, size_t buffer1_len,
> {
> struct dmxdev_filter *dmxdevfilter = filter->priv;
> int ret;
> -
> - if (dmxdevfilter->buffer.error) {
> + if (!dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx) &&
> + dmxdevfilter->buffer.error) {
> wake_up(&dmxdevfilter->buffer.queue);
> return 0;
> }
> @@ -372,11 +373,19 @@ static int dvb_dmxdev_section_callback(const u8 *buffer1, size_t buffer1_len,
> }
> del_timer(&dmxdevfilter->timer);
> dprintk("section callback %*ph\n", 6, buffer1);
> - ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer, buffer1,
> - buffer1_len);
> - if (ret == buffer1_len) {
> - ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer, buffer2,
> - buffer2_len);
> + if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx)) {
> + ret = dvb_vb2_fill_buffer(&dmxdevfilter->vb2_ctx,
> + buffer1, buffer1_len);
> + if (ret == buffer1_len)
> + ret = dvb_vb2_fill_buffer(&dmxdevfilter->vb2_ctx,
> + buffer2, buffer2_len);
> + } else {
> + ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer,
> + buffer1, buffer1_len);
> + if (ret == buffer1_len) {
> + ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer,
> + buffer2, buffer2_len);
> + }
> }
> if (ret < 0)
> dmxdevfilter->buffer.error = ret;
> @@ -393,6 +402,7 @@ static int dvb_dmxdev_ts_callback(const u8 *buffer1, size_t buffer1_len,
> {
> struct dmxdev_filter *dmxdevfilter = feed->priv;
> struct dvb_ringbuffer *buffer;
> + struct dvb_vb2_ctx *ctx;
> int ret;
>
> spin_lock(&dmxdevfilter->dev->lock);
> @@ -401,19 +411,30 @@ static int dvb_dmxdev_ts_callback(const u8 *buffer1, size_t buffer1_len,
> return 0;
> }
>
> - if (dmxdevfilter->params.pes.output == DMX_OUT_TAP
> - || dmxdevfilter->params.pes.output == DMX_OUT_TSDEMUX_TAP)
> + if (dmxdevfilter->params.pes.output == DMX_OUT_TAP ||
> + dmxdevfilter->params.pes.output == DMX_OUT_TSDEMUX_TAP) {
> buffer = &dmxdevfilter->buffer;
> - else
> + ctx = &dmxdevfilter->vb2_ctx;
> + } else {
> buffer = &dmxdevfilter->dev->dvr_buffer;
> - if (buffer->error) {
> - spin_unlock(&dmxdevfilter->dev->lock);
> - wake_up(&buffer->queue);
> - return 0;
> + ctx = &dmxdevfilter->dev->dvr_vb2_ctx;
> + }
> +
> + if (dvb_vb2_is_streaming(ctx)) {
> + ret = dvb_vb2_fill_buffer(ctx, buffer1, buffer1_len);
> + if (ret == buffer1_len)
> + ret = dvb_vb2_fill_buffer(ctx, buffer2, buffer2_len);
> + } else {
> + if (buffer->error) {
> + spin_unlock(&dmxdevfilter->dev->lock);
> + wake_up(&buffer->queue);
> + return 0;
> + }
> + ret = dvb_dmxdev_buffer_write(buffer, buffer1, buffer1_len);
> + if (ret == buffer1_len)
> + ret = dvb_dmxdev_buffer_write(buffer,
> + buffer2, buffer2_len);
> }
> - ret = dvb_dmxdev_buffer_write(buffer, buffer1, buffer1_len);
> - if (ret == buffer1_len)
> - ret = dvb_dmxdev_buffer_write(buffer, buffer2, buffer2_len);
> if (ret < 0)
> buffer->error = ret;
> spin_unlock(&dmxdevfilter->dev->lock);
> @@ -752,6 +773,8 @@ static int dvb_demux_open(struct inode *inode, struct file *file)
> file->private_data = dmxdevfilter;
>
> dvb_ringbuffer_init(&dmxdevfilter->buffer, NULL, 8192);
> + dvb_vb2_init(&dmxdevfilter->vb2_ctx, "demux_filter",
> + file->f_flags & O_NONBLOCK);
> dmxdevfilter->type = DMXDEV_TYPE_NONE;
> dvb_dmxdev_filter_state_set(dmxdevfilter, DMXDEV_STATE_ALLOCATED);
> init_timer(&dmxdevfilter->timer);
> @@ -767,6 +790,10 @@ static int dvb_dmxdev_filter_free(struct dmxdev *dmxdev,
> {
> mutex_lock(&dmxdev->mutex);
> mutex_lock(&dmxdevfilter->mutex);
> + if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
> + dvb_vb2_stream_off(&dmxdevfilter->vb2_ctx);
> + dvb_vb2_release(&dmxdevfilter->vb2_ctx);
> +
>
> dvb_dmxdev_filter_stop(dmxdevfilter);
> dvb_dmxdev_filter_reset(dmxdevfilter);
> @@ -1054,6 +1081,53 @@ static int dvb_demux_do_ioctl(struct file *file,
> mutex_unlock(&dmxdevfilter->mutex);
> break;
>
> + case DMX_REQBUFS:
> + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> + mutex_unlock(&dmxdev->mutex);
> + return -ERESTARTSYS;
> + }
> + ret = dvb_vb2_reqbufs(&dmxdevfilter->vb2_ctx, parg);
> + mutex_unlock(&dmxdevfilter->mutex);
> + break;
> +
> + case DMX_QUERYBUF:
> + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> + mutex_unlock(&dmxdev->mutex);
> + return -ERESTARTSYS;
> + }
> + ret = dvb_vb2_querybuf(&dmxdevfilter->vb2_ctx, parg);
> + mutex_unlock(&dmxdevfilter->mutex);
> + break;
> +
> + case DMX_EXPBUF:
> + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> + mutex_unlock(&dmxdev->mutex);
> + return -ERESTARTSYS;
> + }
> + ret = dvb_vb2_expbuf(&dmxdevfilter->vb2_ctx, parg);
> + mutex_unlock(&dmxdevfilter->mutex);
> + break;
> +
> + case DMX_QBUF:
> + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> + mutex_unlock(&dmxdev->mutex);
> + return -ERESTARTSYS;
> + }
> + ret = dvb_vb2_qbuf(&dmxdevfilter->vb2_ctx, parg);
> + if (ret == 0 && !dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
> + ret = dvb_vb2_stream_on(&dmxdevfilter->vb2_ctx);
> + mutex_unlock(&dmxdevfilter->mutex);
> + break;
> +
> + case DMX_DQBUF:
> + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> + mutex_unlock(&dmxdev->mutex);
> + return -ERESTARTSYS;
> + }
> + ret = dvb_vb2_dqbuf(&dmxdevfilter->vb2_ctx, parg);
> + mutex_unlock(&dmxdevfilter->mutex);
> + break;
> +
> default:
> ret = -EINVAL;
> break;
> @@ -1075,6 +1149,8 @@ static unsigned int dvb_demux_poll(struct file *file, poll_table *wait)
>
> if ((!dmxdevfilter) || dmxdevfilter->dev->exit)
> return POLLERR;
> + if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
> + return dvb_vb2_poll(&dmxdevfilter->vb2_ctx, file, wait);
>
> poll_wait(file, &dmxdevfilter->buffer.queue, wait);
>
> @@ -1092,11 +1168,31 @@ static unsigned int dvb_demux_poll(struct file *file, poll_table *wait)
> return mask;
> }
>
> +static int dvb_demux_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct dmxdev_filter *dmxdevfilter = file->private_data;
> + struct dmxdev *dmxdev = dmxdevfilter->dev;
> + int ret;
> +
> + if (mutex_lock_interruptible(&dmxdev->mutex))
> + return -ERESTARTSYS;
> +
> + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> + mutex_unlock(&dmxdev->mutex);
> + return -ERESTARTSYS;
> + }
> + ret = dvb_vb2_mmap(&dmxdevfilter->vb2_ctx, vma);
> +
> + mutex_unlock(&dmxdevfilter->mutex);
> + mutex_unlock(&dmxdev->mutex);
> +
> + return ret;
> +}
Has this been tested with lockdep?
See also the extensive commit log of patch f035eb4e976ef5a059e30bc91cfd310ff030a7d3.
> +
> static int dvb_demux_release(struct inode *inode, struct file *file)
> {
> struct dmxdev_filter *dmxdevfilter = file->private_data;
> struct dmxdev *dmxdev = dmxdevfilter->dev;
> -
> int ret;
>
> ret = dvb_dmxdev_filter_free(dmxdev, dmxdevfilter);
> @@ -1120,6 +1216,7 @@ static const struct file_operations dvb_demux_fops = {
> .release = dvb_demux_release,
> .poll = dvb_demux_poll,
> .llseek = default_llseek,
> + .mmap = dvb_demux_mmap,
> };
>
> static const struct dvb_device dvbdev_demux = {
> @@ -1148,6 +1245,28 @@ static int dvb_dvr_do_ioctl(struct file *file,
> ret = dvb_dvr_set_buffer_size(dmxdev, arg);
> break;
>
> + case DMX_REQBUFS:
> + ret = dvb_vb2_reqbufs(&dmxdev->dvr_vb2_ctx, parg);
> + break;
> +
> + case DMX_QUERYBUF:
> + ret = dvb_vb2_querybuf(&dmxdev->dvr_vb2_ctx, parg);
> + break;
> +
> + case DMX_EXPBUF:
> + ret = dvb_vb2_expbuf(&dmxdev->dvr_vb2_ctx, parg);
> + break;
> +
> + case DMX_QBUF:
> + ret = dvb_vb2_qbuf(&dmxdev->dvr_vb2_ctx, parg);
> + if (ret == 0 && !dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
> + ret = dvb_vb2_stream_on(&dmxdev->dvr_vb2_ctx);
> + break;
> +
> + case DMX_DQBUF:
> + ret = dvb_vb2_dqbuf(&dmxdev->dvr_vb2_ctx, parg);
> + break;
> +
> default:
> ret = -EINVAL;
> break;
> @@ -1172,10 +1291,13 @@ static unsigned int dvb_dvr_poll(struct file *file, poll_table *wait)
>
> if (dmxdev->exit)
> return POLLERR;
> + if (dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
> + return dvb_vb2_poll(&dmxdev->dvr_vb2_ctx, file, wait);
>
> poll_wait(file, &dmxdev->dvr_buffer.queue, wait);
>
> - if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
> + if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
> + ((file->f_flags & O_ACCMODE) == O_RDWR)) {
> if (dmxdev->dvr_buffer.error)
> mask |= (POLLIN | POLLRDNORM | POLLPRI | POLLERR);
>
> @@ -1187,6 +1309,23 @@ static unsigned int dvb_dvr_poll(struct file *file, poll_table *wait)
> return mask;
> }
>
> +static int dvb_dvr_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct dvb_device *dvbdev = file->private_data;
> + struct dmxdev *dmxdev = dvbdev->priv;
> + int ret;
> +
> + if (dmxdev->exit)
> + return -ENODEV;
> +
> + if (mutex_lock_interruptible(&dmxdev->mutex))
> + return -ERESTARTSYS;
> +
> + ret = dvb_vb2_mmap(&dmxdev->dvr_vb2_ctx, vma);
> + mutex_unlock(&dmxdev->mutex);
> + return ret;
> +}
> +
> static const struct file_operations dvb_dvr_fops = {
> .owner = THIS_MODULE,
> .read = dvb_dvr_read,
> @@ -1196,6 +1335,7 @@ static const struct file_operations dvb_dvr_fops = {
> .release = dvb_dvr_release,
> .poll = dvb_dvr_poll,
> .llseek = default_llseek,
> + .mmap = dvb_dvr_mmap,
> };
>
> static const struct dvb_device dvbdev_dvr = {
> diff --git a/drivers/media/dvb-core/dmxdev.h b/drivers/media/dvb-core/dmxdev.h
> index 054fd4eb6192..6b6aa80db22b 100644
> --- a/drivers/media/dvb-core/dmxdev.h
> +++ b/drivers/media/dvb-core/dmxdev.h
> @@ -35,6 +35,7 @@
> #include "dvbdev.h"
> #include "demux.h"
> #include "dvb_ringbuffer.h"
> +#include "dvb_vb2.h"
>
> enum dmxdev_type {
> DMXDEV_TYPE_NONE,
> @@ -77,6 +78,7 @@ struct dmxdev_filter {
> enum dmxdev_state state;
> struct dmxdev *dev;
> struct dvb_ringbuffer buffer;
> + struct dvb_vb2_ctx vb2_ctx;
>
> struct mutex mutex;
>
> @@ -104,6 +106,8 @@ struct dmxdev {
> struct dvb_ringbuffer dvr_buffer;
> #define DVR_BUFFER_SIZE (10*188*1024)
>
> + struct dvb_vb2_ctx dvr_vb2_ctx;
> +
> struct mutex mutex;
> spinlock_t lock;
> };
> diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
> new file mode 100644
> index 000000000000..34193a4acc47
> --- /dev/null
> +++ b/drivers/media/dvb-core/dvb_vb2.c
> @@ -0,0 +1,423 @@
> +/*
> + * dvb-vb2.c - dvb-vb2
> + *
> + * Copyright (C) 2015 Samsung Electronics
> + *
> + * Author: jh1009.sung@samsung.com
> + *
> + * 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.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mm.h>
> +
> +#include "dvbdev.h"
> +#include "dvb_vb2.h"
> +
> +static int vb2_debug;
> +module_param(vb2_debug, int, 0644);
> +
> +#define dprintk(level, fmt, arg...) \
> + do { \
> + if (vb2_debug >= level) \
> + pr_info("vb2: %s: " fmt, __func__, ## arg); \
> + } while (0)
> +
> +static int _queue_setup(struct vb2_queue *vq,
> + unsigned int *nbuffers, unsigned int *nplanes,
> + unsigned int sizes[], struct device *alloc_devs[])
> +{
> + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> +
> + *nbuffers = ctx->buf_cnt;
This is wrong and it is the reason for the kernel oops.
*nbuffers has already been constrained to the correct buffer range but by
assigning ctx->buf_cnt to it that is now all undone. You actually want to do
this:
ctx->buf_cnt = *nbuffers;
and drop the 'ctx->buf_cnt = req->count;' in dvb_vb2_reqbufs().
> + *nplanes = 1;
> + sizes[0] = ctx->buf_siz;
> +
> + /*
> + * videobuf2-vmalloc allocator is context-less so no need to set
> + * alloc_ctxs array.
> + */
> +
> + dprintk(3, "[%s] count=%d, size=%d\n", ctx->name,
> + *nbuffers, sizes[0]);
> +
> + return 0;
> +}
> +
> +static int _buffer_prepare(struct vb2_buffer *vb)
> +{
> + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> + unsigned long size = ctx->buf_siz;
> +
> + if (vb2_plane_size(vb, 0) < size) {
> + dprintk(1, "[%s] data will not fit into plane (%lu < %lu)\n",
> + ctx->name, vb2_plane_size(vb, 0), size);
> + return -EINVAL;
> + }
> +
> + vb2_set_plane_payload(vb, 0, size);
> + dprintk(3, "[%s]\n", ctx->name);
> +
> + return 0;
> +}
> +
> +static void _buffer_queue(struct vb2_buffer *vb)
> +{
> + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> + struct dvb_buffer *buf = container_of(vb, struct dvb_buffer, vb);
> + unsigned long flags = 0;
> +
> + spin_lock_irqsave(&ctx->slock, flags);
> + list_add_tail(&buf->list, &ctx->dvb_q);
> + spin_unlock_irqrestore(&ctx->slock, flags);
> +
> + dprintk(3, "[%s]\n", ctx->name);
> +}
> +
> +static int _start_streaming(struct vb2_queue *vq, unsigned int count)
> +{
> + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> +
> + dprintk(3, "[%s] count=%d\n", ctx->name, count);
> + return 0;
> +}
> +
> +static void _stop_streaming(struct vb2_queue *vq)
> +{
> + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> +
> + dprintk(3, "[%s]\n", ctx->name);
> +}
> +
> +static void _dmxdev_lock(struct vb2_queue *vq)
> +{
> + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> +
> + mutex_lock(&ctx->mutex);
> + dprintk(3, "[%s]\n", ctx->name);
> +}
> +
> +static void _dmxdev_unlock(struct vb2_queue *vq)
> +{
> + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> +
> + if (mutex_is_locked(&ctx->mutex))
> + mutex_unlock(&ctx->mutex);
> + dprintk(3, "[%s]\n", ctx->name);
> +}
> +
> +static const struct vb2_ops dvb_vb2_qops = {
> + .queue_setup = _queue_setup,
> + .buf_prepare = _buffer_prepare,
> + .buf_queue = _buffer_queue,
> + .start_streaming = _start_streaming,
> + .stop_streaming = _stop_streaming,
> + .wait_prepare = _dmxdev_unlock,
> + .wait_finish = _dmxdev_lock,
> +};
> +
> +static void _fill_dmx_buffer(struct vb2_buffer *vb, void *pb)
> +{
> + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> + struct dmx_buffer *b = pb;
> +
> + b->index = vb->index;
> + b->length = vb->planes[0].length;
> + b->bytesused = vb->planes[0].bytesused;
> + b->offset = vb->planes[0].m.offset;
> + memset(b->reserved, 0, sizeof(b->reserved));
> + dprintk(3, "[%s]\n", ctx->name);
> +}
> +
> +static int _fill_vb2_buffer(struct vb2_buffer *vb,
> + const void *pb, struct vb2_plane *planes)
> +{
> + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> + planes[0].bytesused = 0;
> + dprintk(3, "[%s]\n", ctx->name);
> +
> + return 0;
> +}
> +
> +static const struct vb2_buf_ops dvb_vb2_buf_ops = {
> + .fill_user_buffer = _fill_dmx_buffer,
> + .fill_vb2_buffer = _fill_vb2_buffer,
> +};
> +
> +/*
> + * Videobuf operations
> + */
> +int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int nonblocking)
> +{
> + struct vb2_queue *q = &ctx->vb_q;
> + int ret;
> +
> + memset(ctx, 0, sizeof(struct dvb_vb2_ctx));
> + q->type = DVB_BUF_TYPE_CAPTURE;
We don't support DVB_BUF_TYPE_OUTPUT? Shouldn't this information come from the
driver?
> + /**capture type*/
Why /** ?
> + q->is_output = 0;
Can be dropped unless we support DVB_BUF_TYPE_OUTPUT.
> + /**only mmap is supported currently*/
/** ?
> + q->io_modes = VB2_MMAP;
> + q->drv_priv = ctx;
> + q->buf_struct_size = sizeof(struct dvb_buffer);
> + q->min_buffers_needed = 1;
> + q->ops = &dvb_vb2_qops;
> + q->mem_ops = &vb2_vmalloc_memops;
> + q->buf_ops = &dvb_vb2_buf_ops;
> + q->num_buffers = 0;
Not needed, is zeroed in the memset above.
I'm also missing q->timestamp_flags: should be set to V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC.
> + ret = vb2_core_queue_init(q);
> + if (ret) {
> + ctx->state = DVB_VB2_STATE_NONE;
> + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> + return ret;
> + }
> +
> + mutex_init(&ctx->mutex);
> + spin_lock_init(&ctx->slock);
> + INIT_LIST_HEAD(&ctx->dvb_q);
> +
> + strncpy(ctx->name, name, DVB_VB2_NAME_MAX);
I believe strlcpy is recommended.
> + ctx->nonblocking = nonblocking;
> + ctx->state = DVB_VB2_STATE_INIT;
> +
> + dprintk(3, "[%s]\n", ctx->name);
> +
> + return 0;
> +}
> +
> +int dvb_vb2_release(struct dvb_vb2_ctx *ctx)
> +{
> + struct vb2_queue *q = (struct vb2_queue *)&ctx->vb_q;
> +
> + if (ctx->state & DVB_VB2_STATE_INIT)
> + vb2_core_queue_release(q);
> +
> + ctx->state = DVB_VB2_STATE_NONE;
> + dprintk(3, "[%s]\n", ctx->name);
> +
> + return 0;
> +}
> +
> +int dvb_vb2_stream_on(struct dvb_vb2_ctx *ctx)
> +{
> + struct vb2_queue *q = &ctx->vb_q;
> + int ret;
> +
> + ret = vb2_core_streamon(q, q->type);
> + if (ret) {
> + ctx->state = DVB_VB2_STATE_NONE;
> + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> + return ret;
> + }
> + ctx->state |= DVB_VB2_STATE_STREAMON;
> + dprintk(3, "[%s]\n", ctx->name);
> +
> + return 0;
> +}
> +
> +int dvb_vb2_stream_off(struct dvb_vb2_ctx *ctx)
> +{
> + struct vb2_queue *q = (struct vb2_queue *)&ctx->vb_q;
> + int ret;
> + unsigned long flags = 0;
> +
> + ctx->state &= ~DVB_VB2_STATE_STREAMON;
> + spin_lock_irqsave(&ctx->slock, flags);
> + while (!list_empty(&ctx->dvb_q)) {
> + struct dvb_buffer *buf;
> +
> + buf = list_entry(ctx->dvb_q.next,
> + struct dvb_buffer, list);
> + list_del(&buf->list);
> + spin_unlock_irqrestore(&ctx->slock, flags);
> + vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
> + spin_lock_irqsave(&ctx->slock, flags);
> + }
> + spin_unlock_irqrestore(&ctx->slock, flags);
> + ret = vb2_core_streamoff(q, q->type);
> + if (ret) {
> + ctx->state = DVB_VB2_STATE_NONE;
> + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> + return ret;
> + }
> + dprintk(3, "[%s]\n", ctx->name);
> +
> + return 0;
> +}
> +
> +int dvb_vb2_is_streaming(struct dvb_vb2_ctx *ctx)
> +{
> + return (ctx->state & DVB_VB2_STATE_STREAMON);
> +}
> +
> +int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
> + const unsigned char *src, int len)
> +{
> + unsigned long flags = 0;
> + void *vbuf = NULL;
> + int todo = len;
> + unsigned char *psrc = (unsigned char *)src;
> + int ll = 0;
'll'? That's a bit vague as a variable name.
> +
> + dprintk(3, "[%s] %d bytes are rcvd\n", ctx->name, len);
> + if (!src) {
> + dprintk(3, "[%s]:NULL pointer src\n", ctx->name);
> + /**normal case: This func is called twice from demux driver
> + * once with valid src pointer, second time with NULL pointer
> + */
> + return 0;
> + }
> + while (todo) {
> + if (!ctx->buf) {
> + spin_lock_irqsave(&ctx->slock, flags);
> + if (list_empty(&ctx->dvb_q)) {
> + spin_unlock_irqrestore(&ctx->slock, flags);
> + dprintk(3, "[%s] Buffer overflow!!!\n",
> + ctx->name);
> + break;
> + }
> +
> + ctx->buf = list_entry(ctx->dvb_q.next,
> + struct dvb_buffer, list);
> + list_del(&ctx->buf->list);
> + spin_unlock_irqrestore(&ctx->slock, flags);
> + ctx->remain = vb2_plane_size(&ctx->buf->vb, 0);
> + ctx->offset = 0;
> + }
> +
> + if (!dvb_vb2_is_streaming(ctx)) {
> + vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_ERROR);
> + ctx->buf = NULL;
> + break;
> + }
> +
> + /* Fill buffer */
> + ll = min(todo, ctx->remain);
> + vbuf = vb2_plane_vaddr(&ctx->buf->vb, 0);
> + memcpy(vbuf + ctx->offset, psrc, ll);
> + todo -= ll;
> + psrc += ll;
> +
> + ctx->remain -= ll;
> + ctx->offset += ll;
> +
> + if (ctx->remain == 0) {
> + vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
> + ctx->buf = NULL;
> + }
> + }
> +
> + if (ctx->nonblocking && ctx->buf) {
> + vb2_set_plane_payload(&ctx->buf->vb, 0, ll);
> + vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
> + ctx->buf = NULL;
> + }
> +
> + if (todo)
> + dprintk(1, "[%s] %d bytes are dropped.\n", ctx->name, todo);
> + else
> + dprintk(3, "[%s]\n", ctx->name);
> +
> + dprintk(3, "[%s] %d bytes are copied\n", ctx->name, len - todo);
> + return (len - todo);
> +}
> +
> +int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req)
> +{
> + int ret;
> +
> + ctx->buf_siz = req->size;
> + ctx->buf_cnt = req->count;
> + ret = vb2_core_reqbufs(&ctx->vb_q, VB2_MEMORY_MMAP, &req->count);
> + if (ret) {
> + ctx->state = DVB_VB2_STATE_NONE;
> + dprintk(1, "[%s] count=%d size=%d errno=%d\n", ctx->name,
> + ctx->buf_cnt, ctx->buf_siz, ret);
> + return ret;
> + }
> + ctx->state |= DVB_VB2_STATE_REQBUFS;
> + dprintk(3, "[%s] count=%d size=%d\n", ctx->name,
> + ctx->buf_cnt, ctx->buf_siz);
> +
> + return 0;
> +}
> +
> +int dvb_vb2_querybuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
> +{
> + vb2_core_querybuf(&ctx->vb_q, b->index, b);
> + dprintk(3, "[%s] index=%d\n", ctx->name, b->index);
> + return 0;
> +}
> +
> +int dvb_vb2_expbuf(struct dvb_vb2_ctx *ctx, struct dmx_exportbuffer *exp)
> +{
> + struct vb2_queue *q = &ctx->vb_q;
> + int ret;
> +
> + ret = vb2_core_expbuf(&ctx->vb_q, &exp->fd, q->type, exp->index,
> + 0, exp->flags);
> + if (ret) {
> + dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,
> + exp->index, ret);
> + return ret;
> + }
> + dprintk(3, "[%s] index=%d fd=%d\n", ctx->name, exp->index, exp->fd);
> +
> + return 0;
> +}
> +
> +int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
> +{
> + int ret;
> +
> + ret = vb2_core_qbuf(&ctx->vb_q, b->index, b);
> + if (ret) {
> + dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,
> + b->index, ret);
> + return ret;
> + }
> + dprintk(5, "[%s] index=%d\n", ctx->name, b->index);
> +
> + return 0;
> +}
> +
> +int dvb_vb2_dqbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
> +{
> + int ret;
> +
> + ret = vb2_core_dqbuf(&ctx->vb_q, &b->index, b, ctx->nonblocking);
> + if (ret) {
> + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> + return ret;
> + }
> + dprintk(5, "[%s] index=%d\n", ctx->name, b->index);
> +
> + return 0;
> +}
> +
> +int dvb_vb2_mmap(struct dvb_vb2_ctx *ctx, struct vm_area_struct *vma)
> +{
> + int ret;
> +
> + ret = vb2_mmap(&ctx->vb_q, vma);
> + if (ret) {
> + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> + return ret;
> + }
> + dprintk(3, "[%s] ret=%d\n", ctx->name, ret);
> +
> + return 0;
> +}
> +
> +unsigned int dvb_vb2_poll(struct dvb_vb2_ctx *ctx, struct file *file,
> + poll_table *wait)
> +{
> + dprintk(3, "[%s]\n", ctx->name);
> + return vb2_core_poll(&ctx->vb_q, file, wait);
> +}
> +
> diff --git a/drivers/media/dvb-core/dvb_vb2.h b/drivers/media/dvb-core/dvb_vb2.h
> new file mode 100644
> index 000000000000..d68653926d91
> --- /dev/null
> +++ b/drivers/media/dvb-core/dvb_vb2.h
> @@ -0,0 +1,72 @@
> +/*
> + * dvb-vb2.h - DVB driver helper framework for streaming I/O
> + *
> + * Copyright (C) 2015 Samsung Electronics
> + *
> + * Author: jh1009.sung@samsung.com
> + *
> + * 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.
> + */
> +
> +#ifndef _DVB_VB2_H
> +#define _DVB_VB2_H
> +
> +#include <linux/mutex.h>
> +#include <linux/poll.h>
> +#include <linux/dvb/dmx.h>
> +#include <media/videobuf2-core.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/videobuf2-vmalloc.h>
> +
> +enum dvb_buf_type {
> + DVB_BUF_TYPE_CAPTURE = 1,
> + DVB_BUF_TYPE_OUTPUT = 2,
> +};
> +
> +#define DVB_VB2_STATE_NONE (0x0)
> +#define DVB_VB2_STATE_INIT (0x1)
> +#define DVB_VB2_STATE_REQBUFS (0x2)
> +#define DVB_VB2_STATE_STREAMON (0x4)
> +
> +#define DVB_VB2_NAME_MAX (20)
> +
> +struct dvb_buffer {
> + struct vb2_buffer vb;
> + struct list_head list;
> +};
> +
> +struct dvb_vb2_ctx {
> + struct vb2_queue vb_q;
> + struct mutex mutex;
> + spinlock_t slock;
> + struct list_head dvb_q;
> + struct dvb_buffer *buf;
> + int offset;
> + int remain;
> + int state;
> + int buf_siz;
> + int buf_cnt;
> + int nonblocking;
> + char name[DVB_VB2_NAME_MAX + 1];
> +};
> +
> +int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int non_blocking);
> +int dvb_vb2_release(struct dvb_vb2_ctx *ctx);
> +int dvb_vb2_stream_on(struct dvb_vb2_ctx *ctx);
> +int dvb_vb2_stream_off(struct dvb_vb2_ctx *ctx);
> +int dvb_vb2_is_streaming(struct dvb_vb2_ctx *ctx);
> +int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
> + const unsigned char *src, int len);
> +
> +int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req);
> +int dvb_vb2_querybuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b);
> +int dvb_vb2_expbuf(struct dvb_vb2_ctx *ctx, struct dmx_exportbuffer *exp);
> +int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b);
> +int dvb_vb2_dqbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b);
> +int dvb_vb2_mmap(struct dvb_vb2_ctx *ctx, struct vm_area_struct *vma);
> +unsigned int dvb_vb2_poll(struct dvb_vb2_ctx *ctx, struct file *file,
> + poll_table *wait);
> +
> +#endif /* _DVB_VB2_H */
> diff --git a/include/uapi/linux/dvb/dmx.h b/include/uapi/linux/dvb/dmx.h
> index c10f1324b4ca..e212aa18ad78 100644
> --- a/include/uapi/linux/dvb/dmx.h
> +++ b/include/uapi/linux/dvb/dmx.h
> @@ -211,6 +211,64 @@ struct dmx_stc {
> __u64 stc;
> };
>
> +/**
> + * struct dmx_buffer - dmx buffer info
> + *
> + * @index: id number of the buffer
> + * @bytesused: number of bytes occupied by data in the buffer (payload);
> + * @offset: for buffers with memory == DMX_MEMORY_MMAP;
> + * offset from the start of the device memory for this plane,
> + * (or a "cookie" that should be passed to mmap() as offset)
Since we only support MMAP this is always a 'cookie' in practice. So I think this
should just be:
A "cookie" that should be passed to mmap() as offset.
> + * @length: size in bytes of the buffer
> + *
> + * Contains data exchanged by application and driver using one of the streaming
> + * I/O methods.
> + */
> +struct dmx_buffer {
> + __u32 index;
> + __u32 bytesused;
> + __u32 offset;
I suggest making this a __u64: that way we can handle pointers as well in
the future if we need them (as we do for the USERPTR case for V4L2).
Should this also be wrapped in a union? Useful when adding dmabuf support in the
future.
Do you think there is any use-case for multiplanar formats in the future?
With perhaps meta data in a separate plane? Having to add support for this later
has proven to be very painful, so we need to be as certain as possible that
this isn't going to happen. Otherwise it's better to prepare for this right now.
> + __u32 length;
> + __u32 reserved[4];
I do believe you need a memory field as well. It's only MMAP today, but in
the future DMABUF will most likely be supported as well and you need to be
able to tell what memory mode is being used.
> +};
> +
> +/**
> + * struct dmx_requestbuffers - request dmx buffer information
> + *
> + * @count: number of requested buffers,
> + * @size: size in bytes of the requested buffer
> + *
> + * Contains data used for requesting a dmx buffer.
> + * All reserved fields must be set to zero.
> + */
> +struct dmx_requestbuffers {
> + __u32 count;
> + __u32 size;
> + __u32 reserved[2];
> +};
Ditto: add a memory field.
One thing that is a pain in V4L2 today is that there is no easy way to detect
which streaming modes are supported. I think it would be a good idea to think
what would be the best approach in DVB to expose that to userspace.
The other comment is whether it might be a good idea to use create_bufs instead
of reqbufs (or better, design an improved API).
Due to historical reasons the V4L2 API to create/delete buffers is weird, and
I think we can improve on that for DVB.
> +
> +/**
> + * struct dmx_exportbuffer - export of dmx buffer as DMABUF file descriptor
> + *
> + * @index: id number of the buffer
> + * @flags: flags for newly created file, currently only O_CLOEXEC is
> + * supported, refer to manual of open syscall for more details
> + * @fd: file descriptor associated with DMABUF (set by driver)
> + *
> + * Contains data used for exporting a dmx buffer as DMABUF file descriptor.
> + * The buffer is identified by a 'cookie' returned by DMX_QUERYBUF
> + * (identical to the cookie used to mmap() the buffer to userspace). All
> + * reserved fields must be set to zero. The field reserved0 is expected to
> + * become a structure 'type' allowing an alternative layout of the structure
> + * content. Therefore this field should not be used for any other extensions.
> + */
> +struct dmx_exportbuffer {
> + __u32 index;
> + __u32 flags;
> + __s32 fd;
> + __u32 reserved[5];
> +};
> +
> #define DMX_START _IO('o', 41)
> #define DMX_STOP _IO('o', 42)
> #define DMX_SET_FILTER _IOW('o', 43, struct dmx_sct_filter_params)
> @@ -231,4 +289,10 @@ typedef struct dmx_filter dmx_filter_t;
>
> #endif
>
> -#endif /* _UAPI_DVBDMX_H_ */
> +#define DMX_REQBUFS _IOWR('o', 60, struct dmx_requestbuffers)
> +#define DMX_QUERYBUF _IOWR('o', 61, struct dmx_buffer)
> +#define DMX_EXPBUF _IOWR('o', 62, struct dmx_exportbuffer)
> +#define DMX_QBUF _IOWR('o', 63, struct dmx_buffer)
> +#define DMX_DQBUF _IOWR('o', 64, struct dmx_buffer)
> +
> +#endif /* _DVBDMX_H_ */
>
Regards,
Hans
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O
2018-01-08 13:54 ` Hans Verkuil
@ 2018-01-08 14:26 ` Hans Verkuil
2018-01-08 14:38 ` Mauro Carvalho Chehab
2018-01-08 14:27 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2018-01-08 14:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List, Jonathan Corbet
Cc: Satendra Singh Thakur, Mauro Carvalho Chehab,
Linux Doc Mailing List, Seung-Woo Kim, Greg Kroah-Hartman,
Geunyoung Kim, Philippe Ombredanne, Inki Dae, Thomas Gleixner,
Junghak Sung, devendra sharma, Sakari Ailus
A quick follow-up:
On 01/08/2018 02:54 PM, Hans Verkuil wrote:
>> +/*
>> + * Videobuf operations
>> + */
>> +int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int nonblocking)
>> +{
>> + struct vb2_queue *q = &ctx->vb_q;
>> + int ret;
>> +
>> + memset(ctx, 0, sizeof(struct dvb_vb2_ctx));
>> + q->type = DVB_BUF_TYPE_CAPTURE;
>
> We don't support DVB_BUF_TYPE_OUTPUT? Shouldn't this information come from the
> driver?
>
>> + /**capture type*/
>
> Why /** ?
>
>> + q->is_output = 0;
>
> Can be dropped unless we support DVB_BUF_TYPE_OUTPUT.
>
>> + /**only mmap is supported currently*/
>
> /** ?
>
>> + q->io_modes = VB2_MMAP;
>> + q->drv_priv = ctx;
>> + q->buf_struct_size = sizeof(struct dvb_buffer);
>> + q->min_buffers_needed = 1;
>> + q->ops = &dvb_vb2_qops;
>> + q->mem_ops = &vb2_vmalloc_memops;
>> + q->buf_ops = &dvb_vb2_buf_ops;
>> + q->num_buffers = 0;
>
> Not needed, is zeroed in the memset above.
>
> I'm also missing q->timestamp_flags: should be set to V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC.
Ignore this, see my comments later on.
>
>> + ret = vb2_core_queue_init(q);
>> + if (ret) {
>> + ctx->state = DVB_VB2_STATE_NONE;
>> + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
>> + return ret;
>> + }
>> +
>> + mutex_init(&ctx->mutex);
>> + spin_lock_init(&ctx->slock);
>> + INIT_LIST_HEAD(&ctx->dvb_q);
>> +
>> + strncpy(ctx->name, name, DVB_VB2_NAME_MAX);
>
> I believe strlcpy is recommended.
>
>> + ctx->nonblocking = nonblocking;
>> + ctx->state = DVB_VB2_STATE_INIT;
>> +
>> + dprintk(3, "[%s]\n", ctx->name);
>> +
>> + return 0;
>> +}
<snip>
>> diff --git a/include/uapi/linux/dvb/dmx.h b/include/uapi/linux/dvb/dmx.h
>> index c10f1324b4ca..e212aa18ad78 100644
>> --- a/include/uapi/linux/dvb/dmx.h
>> +++ b/include/uapi/linux/dvb/dmx.h
>> @@ -211,6 +211,64 @@ struct dmx_stc {
>> __u64 stc;
>> };
>>
>> +/**
>> + * struct dmx_buffer - dmx buffer info
>> + *
>> + * @index: id number of the buffer
>> + * @bytesused: number of bytes occupied by data in the buffer (payload);
>> + * @offset: for buffers with memory == DMX_MEMORY_MMAP;
>> + * offset from the start of the device memory for this plane,
>> + * (or a "cookie" that should be passed to mmap() as offset)
>
> Since we only support MMAP this is always a 'cookie' in practice. So I think this
> should just be:
>
> A "cookie" that should be passed to mmap() as offset.
>
>> + * @length: size in bytes of the buffer
>> + *
>> + * Contains data exchanged by application and driver using one of the streaming
>> + * I/O methods.
>> + */
>> +struct dmx_buffer {
>> + __u32 index;
>> + __u32 bytesused;
>> + __u32 offset;
>
> I suggest making this a __u64: that way we can handle pointers as well in
> the future if we need them (as we do for the USERPTR case for V4L2).
>
> Should this also be wrapped in a union? Useful when adding dmabuf support in the
> future.
>
> Do you think there is any use-case for multiplanar formats in the future?
> With perhaps meta data in a separate plane? Having to add support for this later
> has proven to be very painful, so we need to be as certain as possible that
> this isn't going to happen. Otherwise it's better to prepare for this right now.
>
>> + __u32 length;
>> + __u32 reserved[4];
>
> I do believe you need a memory field as well. It's only MMAP today, but in
> the future DMABUF will most likely be supported as well and you need to be
> able to tell what memory mode is being used.
>
>> +};
There is no 'flags' field here. But without that you cannot check the buffer
states, esp. the ERROR state. Or can that never happen?
Would a timestamp field be useful, if only for debugging?
Regards,
Hans
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O
2018-01-08 13:54 ` Hans Verkuil
2018-01-08 14:26 ` Hans Verkuil
@ 2018-01-08 14:27 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2018-01-08 14:27 UTC (permalink / raw)
To: Hans Verkuil
Cc: Linux Media Mailing List, Jonathan Corbet, Satendra Singh Thakur,
Mauro Carvalho Chehab, Linux Doc Mailing List, Seung-Woo Kim,
Greg Kroah-Hartman, Geunyoung Kim, Philippe Ombredanne, Inki Dae,
Thomas Gleixner, Junghak Sung, devendra sharma, Sakari Ailus
Em Mon, 8 Jan 2018 14:54:51 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> Hi Mauro,
>
> Thanks for this patch series, nice to see this moving forward.
>
> Some comments below (note the wrong queue_setup code in particular since that
> relates to my comment about the '[PATCH v3] media: videobuf2-core: don't go out
> of the buffer range' patch).
>
> On 12/21/2017 05:18 PM, Mauro Carvalho Chehab wrote:
> > From: Satendra Singh Thakur <satendra.t@samsung.com>
> >
> > Adds a new uAPI for DVB to use streaming I/O which is implemented
> > based on videobuf2, using those new ioctls:
> >
> > - DMX_REQBUFS: Request kernel to allocate buffers which count and size
> > are dedicated by user.
> > - DMX_QUERYBUF: Get the buffer information like a memory offset which
> > will mmap() and be shared with user-space.
> > - DMX_EXPBUF: Just for testing whether buffer-exporting success or not.
> > - DMX_QBUF: Pass the buffer to kernel-space.
> > - DMX_DQBUF: Get back the buffer which may contain TS data.
> >
> > Originally developed by: Junghak Sung <jh1009.sung@samsung.com>, as
> > seen at:
> > https://patchwork.linuxtv.org/patch/31613/
> > https://patchwork.kernel.org/patch/7334301/
> >
> > The original patch was written before merging VB2-core functionalities
> > upstream. When such series was added, several adjustments were made,
> > fixing some issues with V4L2, causing the original patch to be
> > non-trivially rebased.
> >
> > After rebased, a few bugs in the patch were fixed. The patch was
> > also enhanced it and polling functionality got added.
> >
> > The main changes over the original patch are:
> >
> > dvb_vb2_fill_buffer():
> > - Set the size of the outgoing buffer after while loop using
> > vb2_set_plane_payload;
> >
> > - Added NULL check for source buffer as per normal convention
> > of demux driver, this is called twice, first time with valid
> > buffer second time with NULL pointer, if its not handled,
>
> its -> it's
>
> > it will result in crash
> >
> > - Restricted spinlock for only list_* operations
> >
> > dvb_vb2_init():
> > - Restricted q->io_modes to only VB2_MMAP as its the only
> > supported mode
> >
> > dvb_vb2_release():
> > - Replaced the && in if condiion with &, because otherwise
>
> condition
>
> > it was always getting satisfied.
> >
> > dvb_vb2_stream_off():
> > - Added list_del code for enqueud buffers upon stream off
>
> enqueued
>
> >
> > dvb_vb2_poll():
> > - Added this new function in order to support polling
> >
> > dvb_demux_poll() and dvb_dvr_poll()
> > - dvb_vb2_poll() is now called from these functions
> >
> > - Ported this patch and latest videobuf2 to lower kernel versions and
> > tested auto scan.
Too late to change it, as the patch was already merged, as per
Michel's suggestion.
> > [mchehab@s-opensource.com: checkpatch fixes]
> > Co-developed-by: Junghak Sung <jh1009.sung@samsung.com>
> > Signed-off-by: Junghak Sung <jh1009.sung@samsung.com>
> > Signed-off-by: Geunyoung Kim <nenggun.kim@samsung.com>
> > Acked-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> > Acked-by: Inki Dae <inki.dae@samsung.com>
> > Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > ---
> > drivers/media/dvb-core/Makefile | 2 +-
> > drivers/media/dvb-core/dmxdev.c | 196 +++++++++++++++---
> > drivers/media/dvb-core/dmxdev.h | 4 +
> > drivers/media/dvb-core/dvb_vb2.c | 423 +++++++++++++++++++++++++++++++++++++++
> > drivers/media/dvb-core/dvb_vb2.h | 72 +++++++
> > include/uapi/linux/dvb/dmx.h | 66 +++++-
> > 6 files changed, 733 insertions(+), 30 deletions(-)
> > create mode 100644 drivers/media/dvb-core/dvb_vb2.c
> > create mode 100644 drivers/media/dvb-core/dvb_vb2.h
> >
> > diff --git a/drivers/media/dvb-core/Makefile b/drivers/media/dvb-core/Makefile
> > index 47e2e391bfb8..bbc65dfa0a8e 100644
> > --- a/drivers/media/dvb-core/Makefile
> > +++ b/drivers/media/dvb-core/Makefile
> > @@ -7,6 +7,6 @@ dvb-net-$(CONFIG_DVB_NET) := dvb_net.o
> >
> > dvb-core-objs := dvbdev.o dmxdev.o dvb_demux.o \
> > dvb_ca_en50221.o dvb_frontend.o \
> > - $(dvb-net-y) dvb_ringbuffer.o dvb_math.o
> > + $(dvb-net-y) dvb_ringbuffer.o dvb_vb2.o dvb_math.o
> >
> > obj-$(CONFIG_DVB_CORE) += dvb-core.o
> > diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
> > index 18e4230865be..4cbb9003a1ed 100644
> > --- a/drivers/media/dvb-core/dmxdev.c
> > +++ b/drivers/media/dvb-core/dmxdev.c
> > @@ -28,6 +28,7 @@
> > #include <linux/wait.h>
> > #include <linux/uaccess.h>
> > #include "dmxdev.h"
> > +#include "dvb_vb2.h"
> >
> > static int debug;
> >
> > @@ -138,14 +139,8 @@ static int dvb_dvr_open(struct inode *inode, struct file *file)
> > return -ENODEV;
> > }
> >
> > - if ((file->f_flags & O_ACCMODE) == O_RDWR) {
> > - if (!(dmxdev->capabilities & DMXDEV_CAP_DUPLEX)) {
> > - mutex_unlock(&dmxdev->mutex);
> > - return -EOPNOTSUPP;
> > - }
> > - }
> > -
> > - if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
> > + if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
> > + ((file->f_flags & O_ACCMODE) == O_RDWR)) {
> > void *mem;
> >
> > if (!dvbdev->readers) {
> > @@ -158,6 +153,8 @@ static int dvb_dvr_open(struct inode *inode, struct file *file)
> > return -ENOMEM;
> > }
> > dvb_ringbuffer_init(&dmxdev->dvr_buffer, mem, DVR_BUFFER_SIZE);
> > + dvb_vb2_init(&dmxdev->dvr_vb2_ctx, "dvr",
> > + file->f_flags & O_NONBLOCK);
> > dvbdev->readers--;
> > }
> >
> > @@ -195,7 +192,11 @@ static int dvb_dvr_release(struct inode *inode, struct file *file)
> > dmxdev->demux->connect_frontend(dmxdev->demux,
> > dmxdev->dvr_orig_fe);
> > }
> > - if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
> > + if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
> > + ((file->f_flags & O_ACCMODE) == O_RDWR)) {
> > + if (dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
> > + dvb_vb2_stream_off(&dmxdev->dvr_vb2_ctx);
> > + dvb_vb2_release(&dmxdev->dvr_vb2_ctx);
> > dvbdev->readers++;
> > if (dmxdev->dvr_buffer.data) {
> > void *mem = dmxdev->dvr_buffer.data;
> > @@ -360,8 +361,8 @@ static int dvb_dmxdev_section_callback(const u8 *buffer1, size_t buffer1_len,
> > {
> > struct dmxdev_filter *dmxdevfilter = filter->priv;
> > int ret;
> > -
> > - if (dmxdevfilter->buffer.error) {
> > + if (!dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx) &&
> > + dmxdevfilter->buffer.error) {
> > wake_up(&dmxdevfilter->buffer.queue);
> > return 0;
> > }
> > @@ -372,11 +373,19 @@ static int dvb_dmxdev_section_callback(const u8 *buffer1, size_t buffer1_len,
> > }
> > del_timer(&dmxdevfilter->timer);
> > dprintk("section callback %*ph\n", 6, buffer1);
> > - ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer, buffer1,
> > - buffer1_len);
> > - if (ret == buffer1_len) {
> > - ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer, buffer2,
> > - buffer2_len);
> > + if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx)) {
> > + ret = dvb_vb2_fill_buffer(&dmxdevfilter->vb2_ctx,
> > + buffer1, buffer1_len);
> > + if (ret == buffer1_len)
> > + ret = dvb_vb2_fill_buffer(&dmxdevfilter->vb2_ctx,
> > + buffer2, buffer2_len);
> > + } else {
> > + ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer,
> > + buffer1, buffer1_len);
> > + if (ret == buffer1_len) {
> > + ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer,
> > + buffer2, buffer2_len);
> > + }
> > }
> > if (ret < 0)
> > dmxdevfilter->buffer.error = ret;
> > @@ -393,6 +402,7 @@ static int dvb_dmxdev_ts_callback(const u8 *buffer1, size_t buffer1_len,
> > {
> > struct dmxdev_filter *dmxdevfilter = feed->priv;
> > struct dvb_ringbuffer *buffer;
> > + struct dvb_vb2_ctx *ctx;
> > int ret;
> >
> > spin_lock(&dmxdevfilter->dev->lock);
> > @@ -401,19 +411,30 @@ static int dvb_dmxdev_ts_callback(const u8 *buffer1, size_t buffer1_len,
> > return 0;
> > }
> >
> > - if (dmxdevfilter->params.pes.output == DMX_OUT_TAP
> > - || dmxdevfilter->params.pes.output == DMX_OUT_TSDEMUX_TAP)
> > + if (dmxdevfilter->params.pes.output == DMX_OUT_TAP ||
> > + dmxdevfilter->params.pes.output == DMX_OUT_TSDEMUX_TAP) {
> > buffer = &dmxdevfilter->buffer;
> > - else
> > + ctx = &dmxdevfilter->vb2_ctx;
> > + } else {
> > buffer = &dmxdevfilter->dev->dvr_buffer;
> > - if (buffer->error) {
> > - spin_unlock(&dmxdevfilter->dev->lock);
> > - wake_up(&buffer->queue);
> > - return 0;
> > + ctx = &dmxdevfilter->dev->dvr_vb2_ctx;
> > + }
> > +
> > + if (dvb_vb2_is_streaming(ctx)) {
> > + ret = dvb_vb2_fill_buffer(ctx, buffer1, buffer1_len);
> > + if (ret == buffer1_len)
> > + ret = dvb_vb2_fill_buffer(ctx, buffer2, buffer2_len);
> > + } else {
> > + if (buffer->error) {
> > + spin_unlock(&dmxdevfilter->dev->lock);
> > + wake_up(&buffer->queue);
> > + return 0;
> > + }
> > + ret = dvb_dmxdev_buffer_write(buffer, buffer1, buffer1_len);
> > + if (ret == buffer1_len)
> > + ret = dvb_dmxdev_buffer_write(buffer,
> > + buffer2, buffer2_len);
> > }
> > - ret = dvb_dmxdev_buffer_write(buffer, buffer1, buffer1_len);
> > - if (ret == buffer1_len)
> > - ret = dvb_dmxdev_buffer_write(buffer, buffer2, buffer2_len);
> > if (ret < 0)
> > buffer->error = ret;
> > spin_unlock(&dmxdevfilter->dev->lock);
> > @@ -752,6 +773,8 @@ static int dvb_demux_open(struct inode *inode, struct file *file)
> > file->private_data = dmxdevfilter;
> >
> > dvb_ringbuffer_init(&dmxdevfilter->buffer, NULL, 8192);
> > + dvb_vb2_init(&dmxdevfilter->vb2_ctx, "demux_filter",
> > + file->f_flags & O_NONBLOCK);
> > dmxdevfilter->type = DMXDEV_TYPE_NONE;
> > dvb_dmxdev_filter_state_set(dmxdevfilter, DMXDEV_STATE_ALLOCATED);
> > init_timer(&dmxdevfilter->timer);
> > @@ -767,6 +790,10 @@ static int dvb_dmxdev_filter_free(struct dmxdev *dmxdev,
> > {
> > mutex_lock(&dmxdev->mutex);
> > mutex_lock(&dmxdevfilter->mutex);
> > + if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
> > + dvb_vb2_stream_off(&dmxdevfilter->vb2_ctx);
> > + dvb_vb2_release(&dmxdevfilter->vb2_ctx);
> > +
> >
> > dvb_dmxdev_filter_stop(dmxdevfilter);
> > dvb_dmxdev_filter_reset(dmxdevfilter);
> > @@ -1054,6 +1081,53 @@ static int dvb_demux_do_ioctl(struct file *file,
> > mutex_unlock(&dmxdevfilter->mutex);
> > break;
> >
> > + case DMX_REQBUFS:
> > + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> > + mutex_unlock(&dmxdev->mutex);
> > + return -ERESTARTSYS;
> > + }
> > + ret = dvb_vb2_reqbufs(&dmxdevfilter->vb2_ctx, parg);
> > + mutex_unlock(&dmxdevfilter->mutex);
> > + break;
> > +
> > + case DMX_QUERYBUF:
> > + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> > + mutex_unlock(&dmxdev->mutex);
> > + return -ERESTARTSYS;
> > + }
> > + ret = dvb_vb2_querybuf(&dmxdevfilter->vb2_ctx, parg);
> > + mutex_unlock(&dmxdevfilter->mutex);
> > + break;
> > +
> > + case DMX_EXPBUF:
> > + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> > + mutex_unlock(&dmxdev->mutex);
> > + return -ERESTARTSYS;
> > + }
> > + ret = dvb_vb2_expbuf(&dmxdevfilter->vb2_ctx, parg);
> > + mutex_unlock(&dmxdevfilter->mutex);
> > + break;
> > +
> > + case DMX_QBUF:
> > + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> > + mutex_unlock(&dmxdev->mutex);
> > + return -ERESTARTSYS;
> > + }
> > + ret = dvb_vb2_qbuf(&dmxdevfilter->vb2_ctx, parg);
> > + if (ret == 0 && !dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
> > + ret = dvb_vb2_stream_on(&dmxdevfilter->vb2_ctx);
> > + mutex_unlock(&dmxdevfilter->mutex);
> > + break;
> > +
> > + case DMX_DQBUF:
> > + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> > + mutex_unlock(&dmxdev->mutex);
> > + return -ERESTARTSYS;
> > + }
> > + ret = dvb_vb2_dqbuf(&dmxdevfilter->vb2_ctx, parg);
> > + mutex_unlock(&dmxdevfilter->mutex);
> > + break;
> > +
> > default:
> > ret = -EINVAL;
> > break;
> > @@ -1075,6 +1149,8 @@ static unsigned int dvb_demux_poll(struct file *file, poll_table *wait)
> >
> > if ((!dmxdevfilter) || dmxdevfilter->dev->exit)
> > return POLLERR;
> > + if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx))
> > + return dvb_vb2_poll(&dmxdevfilter->vb2_ctx, file, wait);
> >
> > poll_wait(file, &dmxdevfilter->buffer.queue, wait);
> >
> > @@ -1092,11 +1168,31 @@ static unsigned int dvb_demux_poll(struct file *file, poll_table *wait)
> > return mask;
> > }
> >
> > +static int dvb_demux_mmap(struct file *file, struct vm_area_struct *vma)
> > +{
> > + struct dmxdev_filter *dmxdevfilter = file->private_data;
> > + struct dmxdev *dmxdev = dmxdevfilter->dev;
> > + int ret;
> > +
> > + if (mutex_lock_interruptible(&dmxdev->mutex))
> > + return -ERESTARTSYS;
> > +
> > + if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
> > + mutex_unlock(&dmxdev->mutex);
> > + return -ERESTARTSYS;
> > + }
> > + ret = dvb_vb2_mmap(&dmxdevfilter->vb2_ctx, vma);
> > +
> > + mutex_unlock(&dmxdevfilter->mutex);
> > + mutex_unlock(&dmxdev->mutex);
> > +
> > + return ret;
> > +}
>
> Has this been tested with lockdep?
>
> See also the extensive commit log of patch f035eb4e976ef5a059e30bc91cfd310ff030a7d3.
Good point. I didn't test.
>
> > +
> > static int dvb_demux_release(struct inode *inode, struct file *file)
> > {
> > struct dmxdev_filter *dmxdevfilter = file->private_data;
> > struct dmxdev *dmxdev = dmxdevfilter->dev;
> > -
> > int ret;
> >
> > ret = dvb_dmxdev_filter_free(dmxdev, dmxdevfilter);
> > @@ -1120,6 +1216,7 @@ static const struct file_operations dvb_demux_fops = {
> > .release = dvb_demux_release,
> > .poll = dvb_demux_poll,
> > .llseek = default_llseek,
> > + .mmap = dvb_demux_mmap,
> > };
> >
> > static const struct dvb_device dvbdev_demux = {
> > @@ -1148,6 +1245,28 @@ static int dvb_dvr_do_ioctl(struct file *file,
> > ret = dvb_dvr_set_buffer_size(dmxdev, arg);
> > break;
> >
> > + case DMX_REQBUFS:
> > + ret = dvb_vb2_reqbufs(&dmxdev->dvr_vb2_ctx, parg);
> > + break;
> > +
> > + case DMX_QUERYBUF:
> > + ret = dvb_vb2_querybuf(&dmxdev->dvr_vb2_ctx, parg);
> > + break;
> > +
> > + case DMX_EXPBUF:
> > + ret = dvb_vb2_expbuf(&dmxdev->dvr_vb2_ctx, parg);
> > + break;
> > +
> > + case DMX_QBUF:
> > + ret = dvb_vb2_qbuf(&dmxdev->dvr_vb2_ctx, parg);
> > + if (ret == 0 && !dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
> > + ret = dvb_vb2_stream_on(&dmxdev->dvr_vb2_ctx);
> > + break;
> > +
> > + case DMX_DQBUF:
> > + ret = dvb_vb2_dqbuf(&dmxdev->dvr_vb2_ctx, parg);
> > + break;
> > +
> > default:
> > ret = -EINVAL;
> > break;
> > @@ -1172,10 +1291,13 @@ static unsigned int dvb_dvr_poll(struct file *file, poll_table *wait)
> >
> > if (dmxdev->exit)
> > return POLLERR;
> > + if (dvb_vb2_is_streaming(&dmxdev->dvr_vb2_ctx))
> > + return dvb_vb2_poll(&dmxdev->dvr_vb2_ctx, file, wait);
> >
> > poll_wait(file, &dmxdev->dvr_buffer.queue, wait);
> >
> > - if ((file->f_flags & O_ACCMODE) == O_RDONLY) {
> > + if (((file->f_flags & O_ACCMODE) == O_RDONLY) ||
> > + ((file->f_flags & O_ACCMODE) == O_RDWR)) {
> > if (dmxdev->dvr_buffer.error)
> > mask |= (POLLIN | POLLRDNORM | POLLPRI | POLLERR);
> >
> > @@ -1187,6 +1309,23 @@ static unsigned int dvb_dvr_poll(struct file *file, poll_table *wait)
> > return mask;
> > }
> >
> > +static int dvb_dvr_mmap(struct file *file, struct vm_area_struct *vma)
> > +{
> > + struct dvb_device *dvbdev = file->private_data;
> > + struct dmxdev *dmxdev = dvbdev->priv;
> > + int ret;
> > +
> > + if (dmxdev->exit)
> > + return -ENODEV;
> > +
> > + if (mutex_lock_interruptible(&dmxdev->mutex))
> > + return -ERESTARTSYS;
> > +
> > + ret = dvb_vb2_mmap(&dmxdev->dvr_vb2_ctx, vma);
> > + mutex_unlock(&dmxdev->mutex);
> > + return ret;
> > +}
> > +
> > static const struct file_operations dvb_dvr_fops = {
> > .owner = THIS_MODULE,
> > .read = dvb_dvr_read,
> > @@ -1196,6 +1335,7 @@ static const struct file_operations dvb_dvr_fops = {
> > .release = dvb_dvr_release,
> > .poll = dvb_dvr_poll,
> > .llseek = default_llseek,
> > + .mmap = dvb_dvr_mmap,
> > };
> >
> > static const struct dvb_device dvbdev_dvr = {
> > diff --git a/drivers/media/dvb-core/dmxdev.h b/drivers/media/dvb-core/dmxdev.h
> > index 054fd4eb6192..6b6aa80db22b 100644
> > --- a/drivers/media/dvb-core/dmxdev.h
> > +++ b/drivers/media/dvb-core/dmxdev.h
> > @@ -35,6 +35,7 @@
> > #include "dvbdev.h"
> > #include "demux.h"
> > #include "dvb_ringbuffer.h"
> > +#include "dvb_vb2.h"
> >
> > enum dmxdev_type {
> > DMXDEV_TYPE_NONE,
> > @@ -77,6 +78,7 @@ struct dmxdev_filter {
> > enum dmxdev_state state;
> > struct dmxdev *dev;
> > struct dvb_ringbuffer buffer;
> > + struct dvb_vb2_ctx vb2_ctx;
> >
> > struct mutex mutex;
> >
> > @@ -104,6 +106,8 @@ struct dmxdev {
> > struct dvb_ringbuffer dvr_buffer;
> > #define DVR_BUFFER_SIZE (10*188*1024)
> >
> > + struct dvb_vb2_ctx dvr_vb2_ctx;
> > +
> > struct mutex mutex;
> > spinlock_t lock;
> > };
> > diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
> > new file mode 100644
> > index 000000000000..34193a4acc47
> > --- /dev/null
> > +++ b/drivers/media/dvb-core/dvb_vb2.c
> > @@ -0,0 +1,423 @@
> > +/*
> > + * dvb-vb2.c - dvb-vb2
> > + *
> > + * Copyright (C) 2015 Samsung Electronics
> > + *
> > + * Author: jh1009.sung@samsung.com
> > + *
> > + * 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.
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/mm.h>
> > +
> > +#include "dvbdev.h"
> > +#include "dvb_vb2.h"
> > +
> > +static int vb2_debug;
> > +module_param(vb2_debug, int, 0644);
> > +
> > +#define dprintk(level, fmt, arg...) \
> > + do { \
> > + if (vb2_debug >= level) \
> > + pr_info("vb2: %s: " fmt, __func__, ## arg); \
> > + } while (0)
> > +
> > +static int _queue_setup(struct vb2_queue *vq,
> > + unsigned int *nbuffers, unsigned int *nplanes,
> > + unsigned int sizes[], struct device *alloc_devs[])
> > +{
> > + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> > +
> > + *nbuffers = ctx->buf_cnt;
>
> This is wrong and it is the reason for the kernel oops.
>
> *nbuffers has already been constrained to the correct buffer range but by
> assigning ctx->buf_cnt to it that is now all undone. You actually want to do
> this:
>
> ctx->buf_cnt = *nbuffers;
Yes, this was changed on a separate patchset. I didn't want to mangle
with the original patch.
> and drop the 'ctx->buf_cnt = req->count;' in dvb_vb2_reqbufs().
>
> > + *nplanes = 1;
> > + sizes[0] = ctx->buf_siz;
> > +
> > + /*
> > + * videobuf2-vmalloc allocator is context-less so no need to set
> > + * alloc_ctxs array.
> > + */
> > +
> > + dprintk(3, "[%s] count=%d, size=%d\n", ctx->name,
> > + *nbuffers, sizes[0]);
> > +
> > + return 0;
> > +}
> > +
> > +static int _buffer_prepare(struct vb2_buffer *vb)
> > +{
> > + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > + unsigned long size = ctx->buf_siz;
> > +
> > + if (vb2_plane_size(vb, 0) < size) {
> > + dprintk(1, "[%s] data will not fit into plane (%lu < %lu)\n",
> > + ctx->name, vb2_plane_size(vb, 0), size);
> > + return -EINVAL;
> > + }
> > +
> > + vb2_set_plane_payload(vb, 0, size);
> > + dprintk(3, "[%s]\n", ctx->name);
> > +
> > + return 0;
> > +}
> > +
> > +static void _buffer_queue(struct vb2_buffer *vb)
> > +{
> > + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > + struct dvb_buffer *buf = container_of(vb, struct dvb_buffer, vb);
> > + unsigned long flags = 0;
> > +
> > + spin_lock_irqsave(&ctx->slock, flags);
> > + list_add_tail(&buf->list, &ctx->dvb_q);
> > + spin_unlock_irqrestore(&ctx->slock, flags);
> > +
> > + dprintk(3, "[%s]\n", ctx->name);
> > +}
> > +
> > +static int _start_streaming(struct vb2_queue *vq, unsigned int count)
> > +{
> > + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> > +
> > + dprintk(3, "[%s] count=%d\n", ctx->name, count);
> > + return 0;
> > +}
> > +
> > +static void _stop_streaming(struct vb2_queue *vq)
> > +{
> > + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> > +
> > + dprintk(3, "[%s]\n", ctx->name);
> > +}
> > +
> > +static void _dmxdev_lock(struct vb2_queue *vq)
> > +{
> > + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> > +
> > + mutex_lock(&ctx->mutex);
> > + dprintk(3, "[%s]\n", ctx->name);
> > +}
> > +
> > +static void _dmxdev_unlock(struct vb2_queue *vq)
> > +{
> > + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
> > +
> > + if (mutex_is_locked(&ctx->mutex))
> > + mutex_unlock(&ctx->mutex);
> > + dprintk(3, "[%s]\n", ctx->name);
> > +}
> > +
> > +static const struct vb2_ops dvb_vb2_qops = {
> > + .queue_setup = _queue_setup,
> > + .buf_prepare = _buffer_prepare,
> > + .buf_queue = _buffer_queue,
> > + .start_streaming = _start_streaming,
> > + .stop_streaming = _stop_streaming,
> > + .wait_prepare = _dmxdev_unlock,
> > + .wait_finish = _dmxdev_lock,
> > +};
> > +
> > +static void _fill_dmx_buffer(struct vb2_buffer *vb, void *pb)
> > +{
> > + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > + struct dmx_buffer *b = pb;
> > +
> > + b->index = vb->index;
> > + b->length = vb->planes[0].length;
> > + b->bytesused = vb->planes[0].bytesused;
> > + b->offset = vb->planes[0].m.offset;
> > + memset(b->reserved, 0, sizeof(b->reserved));
> > + dprintk(3, "[%s]\n", ctx->name);
> > +}
> > +
> > +static int _fill_vb2_buffer(struct vb2_buffer *vb,
> > + const void *pb, struct vb2_plane *planes)
> > +{
> > + struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > +
> > + planes[0].bytesused = 0;
> > + dprintk(3, "[%s]\n", ctx->name);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct vb2_buf_ops dvb_vb2_buf_ops = {
> > + .fill_user_buffer = _fill_dmx_buffer,
> > + .fill_vb2_buffer = _fill_vb2_buffer,
> > +};
> > +
> > +/*
> > + * Videobuf operations
> > + */
> > +int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int nonblocking)
> > +{
> > + struct vb2_queue *q = &ctx->vb_q;
> > + int ret;
> > +
> > + memset(ctx, 0, sizeof(struct dvb_vb2_ctx));
> > + q->type = DVB_BUF_TYPE_CAPTURE;
>
> We don't support DVB_BUF_TYPE_OUTPUT? Shouldn't this information come from the
> driver?
For now, we don't. I removed it from the uAPI on a separate patch.
>
> > + /**capture type*/
>
> Why /** ?
I remember I saw the same issue on other vb2 drivers: on several places,
it was using /** for comments. I'll fix it on a separate patch.
>
> > + q->is_output = 0;
>
> Can be dropped unless we support DVB_BUF_TYPE_OUTPUT.
The plan is to support, but more work is likely required at DVB side.
On DVB, almost everything is related to receive stuff, nowadays.
So, testing output is more complex, as most hardware don't support.
> > + /**only mmap is supported currently*/
>
> /** ?
>
> > + q->io_modes = VB2_MMAP;
> > + q->drv_priv = ctx;
> > + q->buf_struct_size = sizeof(struct dvb_buffer);
> > + q->min_buffers_needed = 1;
> > + q->ops = &dvb_vb2_qops;
> > + q->mem_ops = &vb2_vmalloc_memops;
> > + q->buf_ops = &dvb_vb2_buf_ops;
> > + q->num_buffers = 0;
>
> Not needed, is zeroed in the memset above.
True.
>
> I'm also missing q->timestamp_flags: should be set to V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC.
DVB doesn't care or use timestamps from the time buffers are filled.
Instead, applications use timestamps inside the MPEG-TS stream, if needed.
Btw,
$ git grep timestamp_flags drivers/media/common/
returns nothing, so I suspect VB2 core itself doesn't actually use it.
The implementation is, instead, at:
drivers/media/v4l2-core/videobuf2-v4l2.c
Besides that, passing any V4L2_foo inside it seems really weird.
What it could be done, instead, would be to move timestamp_flags to a
v4l2-specific struct as a cleanup.
>
> > + ret = vb2_core_queue_init(q);
> > + if (ret) {
> > + ctx->state = DVB_VB2_STATE_NONE;
> > + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> > + return ret;
> > + }
> > +
> > + mutex_init(&ctx->mutex);
> > + spin_lock_init(&ctx->slock);
> > + INIT_LIST_HEAD(&ctx->dvb_q);
> > +
> > + strncpy(ctx->name, name, DVB_VB2_NAME_MAX);
>
> I believe strlcpy is recommended.
Yes. I wrote/merged a patch doing that already :-)
>
> > + ctx->nonblocking = nonblocking;
> > + ctx->state = DVB_VB2_STATE_INIT;
> > +
> > + dprintk(3, "[%s]\n", ctx->name);
> > +
> > + return 0;
> > +}
> > +
> > +int dvb_vb2_release(struct dvb_vb2_ctx *ctx)
> > +{
> > + struct vb2_queue *q = (struct vb2_queue *)&ctx->vb_q;
> > +
> > + if (ctx->state & DVB_VB2_STATE_INIT)
> > + vb2_core_queue_release(q);
> > +
> > + ctx->state = DVB_VB2_STATE_NONE;
> > + dprintk(3, "[%s]\n", ctx->name);
> > +
> > + return 0;
> > +}
> > +
> > +int dvb_vb2_stream_on(struct dvb_vb2_ctx *ctx)
> > +{
> > + struct vb2_queue *q = &ctx->vb_q;
> > + int ret;
> > +
> > + ret = vb2_core_streamon(q, q->type);
> > + if (ret) {
> > + ctx->state = DVB_VB2_STATE_NONE;
> > + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> > + return ret;
> > + }
> > + ctx->state |= DVB_VB2_STATE_STREAMON;
> > + dprintk(3, "[%s]\n", ctx->name);
> > +
> > + return 0;
> > +}
> > +
> > +int dvb_vb2_stream_off(struct dvb_vb2_ctx *ctx)
> > +{
> > + struct vb2_queue *q = (struct vb2_queue *)&ctx->vb_q;
> > + int ret;
> > + unsigned long flags = 0;
> > +
> > + ctx->state &= ~DVB_VB2_STATE_STREAMON;
> > + spin_lock_irqsave(&ctx->slock, flags);
> > + while (!list_empty(&ctx->dvb_q)) {
> > + struct dvb_buffer *buf;
> > +
> > + buf = list_entry(ctx->dvb_q.next,
> > + struct dvb_buffer, list);
> > + list_del(&buf->list);
> > + spin_unlock_irqrestore(&ctx->slock, flags);
> > + vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
> > + spin_lock_irqsave(&ctx->slock, flags);
> > + }
> > + spin_unlock_irqrestore(&ctx->slock, flags);
> > + ret = vb2_core_streamoff(q, q->type);
> > + if (ret) {
> > + ctx->state = DVB_VB2_STATE_NONE;
> > + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> > + return ret;
> > + }
> > + dprintk(3, "[%s]\n", ctx->name);
> > +
> > + return 0;
> > +}
> > +
> > +int dvb_vb2_is_streaming(struct dvb_vb2_ctx *ctx)
> > +{
> > + return (ctx->state & DVB_VB2_STATE_STREAMON);
> > +}
> > +
> > +int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
> > + const unsigned char *src, int len)
> > +{
> > + unsigned long flags = 0;
> > + void *vbuf = NULL;
> > + int todo = len;
> > + unsigned char *psrc = (unsigned char *)src;
> > + int ll = 0;
>
> 'll'? That's a bit vague as a variable name.
Agreed.
>
> > +
> > + dprintk(3, "[%s] %d bytes are rcvd\n", ctx->name, len);
> > + if (!src) {
> > + dprintk(3, "[%s]:NULL pointer src\n", ctx->name);
> > + /**normal case: This func is called twice from demux driver
> > + * once with valid src pointer, second time with NULL pointer
> > + */
> > + return 0;
> > + }
> > + while (todo) {
> > + if (!ctx->buf) {
> > + spin_lock_irqsave(&ctx->slock, flags);
> > + if (list_empty(&ctx->dvb_q)) {
> > + spin_unlock_irqrestore(&ctx->slock, flags);
> > + dprintk(3, "[%s] Buffer overflow!!!\n",
> > + ctx->name);
> > + break;
> > + }
> > +
> > + ctx->buf = list_entry(ctx->dvb_q.next,
> > + struct dvb_buffer, list);
> > + list_del(&ctx->buf->list);
> > + spin_unlock_irqrestore(&ctx->slock, flags);
> > + ctx->remain = vb2_plane_size(&ctx->buf->vb, 0);
> > + ctx->offset = 0;
> > + }
> > +
> > + if (!dvb_vb2_is_streaming(ctx)) {
> > + vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_ERROR);
> > + ctx->buf = NULL;
> > + break;
> > + }
> > +
> > + /* Fill buffer */
> > + ll = min(todo, ctx->remain);
> > + vbuf = vb2_plane_vaddr(&ctx->buf->vb, 0);
> > + memcpy(vbuf + ctx->offset, psrc, ll);
> > + todo -= ll;
> > + psrc += ll;
> > +
> > + ctx->remain -= ll;
> > + ctx->offset += ll;
> > +
> > + if (ctx->remain == 0) {
> > + vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
> > + ctx->buf = NULL;
> > + }
> > + }
> > +
> > + if (ctx->nonblocking && ctx->buf) {
> > + vb2_set_plane_payload(&ctx->buf->vb, 0, ll);
> > + vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
> > + ctx->buf = NULL;
> > + }
> > +
> > + if (todo)
> > + dprintk(1, "[%s] %d bytes are dropped.\n", ctx->name, todo);
> > + else
> > + dprintk(3, "[%s]\n", ctx->name);
> > +
> > + dprintk(3, "[%s] %d bytes are copied\n", ctx->name, len - todo);
> > + return (len - todo);
> > +}
> > +
> > +int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req)
> > +{
> > + int ret;
> > +
> > + ctx->buf_siz = req->size;
> > + ctx->buf_cnt = req->count;
> > + ret = vb2_core_reqbufs(&ctx->vb_q, VB2_MEMORY_MMAP, &req->count);
> > + if (ret) {
> > + ctx->state = DVB_VB2_STATE_NONE;
> > + dprintk(1, "[%s] count=%d size=%d errno=%d\n", ctx->name,
> > + ctx->buf_cnt, ctx->buf_siz, ret);
> > + return ret;
> > + }
> > + ctx->state |= DVB_VB2_STATE_REQBUFS;
> > + dprintk(3, "[%s] count=%d size=%d\n", ctx->name,
> > + ctx->buf_cnt, ctx->buf_siz);
> > +
> > + return 0;
> > +}
> > +
> > +int dvb_vb2_querybuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
> > +{
> > + vb2_core_querybuf(&ctx->vb_q, b->index, b);
> > + dprintk(3, "[%s] index=%d\n", ctx->name, b->index);
> > + return 0;
> > +}
> > +
> > +int dvb_vb2_expbuf(struct dvb_vb2_ctx *ctx, struct dmx_exportbuffer *exp)
> > +{
> > + struct vb2_queue *q = &ctx->vb_q;
> > + int ret;
> > +
> > + ret = vb2_core_expbuf(&ctx->vb_q, &exp->fd, q->type, exp->index,
> > + 0, exp->flags);
> > + if (ret) {
> > + dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,
> > + exp->index, ret);
> > + return ret;
> > + }
> > + dprintk(3, "[%s] index=%d fd=%d\n", ctx->name, exp->index, exp->fd);
> > +
> > + return 0;
> > +}
> > +
> > +int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
> > +{
> > + int ret;
> > +
> > + ret = vb2_core_qbuf(&ctx->vb_q, b->index, b);
> > + if (ret) {
> > + dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,
> > + b->index, ret);
> > + return ret;
> > + }
> > + dprintk(5, "[%s] index=%d\n", ctx->name, b->index);
> > +
> > + return 0;
> > +}
> > +
> > +int dvb_vb2_dqbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
> > +{
> > + int ret;
> > +
> > + ret = vb2_core_dqbuf(&ctx->vb_q, &b->index, b, ctx->nonblocking);
> > + if (ret) {
> > + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> > + return ret;
> > + }
> > + dprintk(5, "[%s] index=%d\n", ctx->name, b->index);
> > +
> > + return 0;
> > +}
> > +
> > +int dvb_vb2_mmap(struct dvb_vb2_ctx *ctx, struct vm_area_struct *vma)
> > +{
> > + int ret;
> > +
> > + ret = vb2_mmap(&ctx->vb_q, vma);
> > + if (ret) {
> > + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> > + return ret;
> > + }
> > + dprintk(3, "[%s] ret=%d\n", ctx->name, ret);
> > +
> > + return 0;
> > +}
> > +
> > +unsigned int dvb_vb2_poll(struct dvb_vb2_ctx *ctx, struct file *file,
> > + poll_table *wait)
> > +{
> > + dprintk(3, "[%s]\n", ctx->name);
> > + return vb2_core_poll(&ctx->vb_q, file, wait);
> > +}
> > +
> > diff --git a/drivers/media/dvb-core/dvb_vb2.h b/drivers/media/dvb-core/dvb_vb2.h
> > new file mode 100644
> > index 000000000000..d68653926d91
> > --- /dev/null
> > +++ b/drivers/media/dvb-core/dvb_vb2.h
> > @@ -0,0 +1,72 @@
> > +/*
> > + * dvb-vb2.h - DVB driver helper framework for streaming I/O
> > + *
> > + * Copyright (C) 2015 Samsung Electronics
> > + *
> > + * Author: jh1009.sung@samsung.com
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef _DVB_VB2_H
> > +#define _DVB_VB2_H
> > +
> > +#include <linux/mutex.h>
> > +#include <linux/poll.h>
> > +#include <linux/dvb/dmx.h>
> > +#include <media/videobuf2-core.h>
> > +#include <media/videobuf2-dma-contig.h>
> > +#include <media/videobuf2-vmalloc.h>
> > +
> > +enum dvb_buf_type {
> > + DVB_BUF_TYPE_CAPTURE = 1,
> > + DVB_BUF_TYPE_OUTPUT = 2,
> > +};
> > +
> > +#define DVB_VB2_STATE_NONE (0x0)
> > +#define DVB_VB2_STATE_INIT (0x1)
> > +#define DVB_VB2_STATE_REQBUFS (0x2)
> > +#define DVB_VB2_STATE_STREAMON (0x4)
> > +
> > +#define DVB_VB2_NAME_MAX (20)
> > +
> > +struct dvb_buffer {
> > + struct vb2_buffer vb;
> > + struct list_head list;
> > +};
> > +
> > +struct dvb_vb2_ctx {
> > + struct vb2_queue vb_q;
> > + struct mutex mutex;
> > + spinlock_t slock;
> > + struct list_head dvb_q;
> > + struct dvb_buffer *buf;
> > + int offset;
> > + int remain;
> > + int state;
> > + int buf_siz;
> > + int buf_cnt;
> > + int nonblocking;
> > + char name[DVB_VB2_NAME_MAX + 1];
> > +};
> > +
> > +int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int non_blocking);
> > +int dvb_vb2_release(struct dvb_vb2_ctx *ctx);
> > +int dvb_vb2_stream_on(struct dvb_vb2_ctx *ctx);
> > +int dvb_vb2_stream_off(struct dvb_vb2_ctx *ctx);
> > +int dvb_vb2_is_streaming(struct dvb_vb2_ctx *ctx);
> > +int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
> > + const unsigned char *src, int len);
> > +
> > +int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req);
> > +int dvb_vb2_querybuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b);
> > +int dvb_vb2_expbuf(struct dvb_vb2_ctx *ctx, struct dmx_exportbuffer *exp);
> > +int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b);
> > +int dvb_vb2_dqbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b);
> > +int dvb_vb2_mmap(struct dvb_vb2_ctx *ctx, struct vm_area_struct *vma);
> > +unsigned int dvb_vb2_poll(struct dvb_vb2_ctx *ctx, struct file *file,
> > + poll_table *wait);
> > +
> > +#endif /* _DVB_VB2_H */
> > diff --git a/include/uapi/linux/dvb/dmx.h b/include/uapi/linux/dvb/dmx.h
> > index c10f1324b4ca..e212aa18ad78 100644
> > --- a/include/uapi/linux/dvb/dmx.h
> > +++ b/include/uapi/linux/dvb/dmx.h
> > @@ -211,6 +211,64 @@ struct dmx_stc {
> > __u64 stc;
> > };
> >
> > +/**
> > + * struct dmx_buffer - dmx buffer info
> > + *
> > + * @index: id number of the buffer
> > + * @bytesused: number of bytes occupied by data in the buffer (payload);
> > + * @offset: for buffers with memory == DMX_MEMORY_MMAP;
> > + * offset from the start of the device memory for this plane,
> > + * (or a "cookie" that should be passed to mmap() as offset)
>
> Since we only support MMAP this is always a 'cookie' in practice. So I think this
> should just be:
>
> A "cookie" that should be passed to mmap() as offset.
Good point. Yeah, that "offset" field sounded weird to me too.
>
> > + * @length: size in bytes of the buffer
> > + *
> > + * Contains data exchanged by application and driver using one of the streaming
> > + * I/O methods.
> > + */
> > +struct dmx_buffer {
> > + __u32 index;
> > + __u32 bytesused;
> > + __u32 offset;
>
> I suggest making this a __u64: that way we can handle pointers as well in
> the future if we need them (as we do for the USERPTR case for V4L2).
Good point. I can't foresee any cases for USERPTR anymore that is not better
covered by DMABUF (neither at DVB or V4L2).
>
> Should this also be wrapped in a union? Useful when adding dmabuf support in the
> future.
Well, we can always add a unnamed union in the future.
> Do you think there is any use-case for multiplanar formats in the future?
> With perhaps meta data in a separate plane? Having to add support for this later
> has proven to be very painful, so we need to be as certain as possible that
> this isn't going to happen. Otherwise it's better to prepare for this right now.
>
> > + __u32 length;
> > + __u32 reserved[4];
>
> I do believe you need a memory field as well. It's only MMAP today, but in
> the future DMABUF will most likely be supported as well and you need to be
> able to tell what memory mode is being used.
What I actually did here were to remove "reserved" field. DVB ioctls
currently don't implement reserved fields. Adding support for longer
structs is easy, as the code can check for the ioctl size, providing
backward compatibility on an easier way (as what we're thinking for the
media controller v2 API).
>
> > +};
> > +
> > +/**
> > + * struct dmx_requestbuffers - request dmx buffer information
> > + *
> > + * @count: number of requested buffers,
> > + * @size: size in bytes of the requested buffer
> > + *
> > + * Contains data used for requesting a dmx buffer.
> > + * All reserved fields must be set to zero.
> > + */
> > +struct dmx_requestbuffers {
> > + __u32 count;
> > + __u32 size;
> > + __u32 reserved[2];
> > +};
>
> Ditto: add a memory field.
Ditto: I also removed reserved here.
> One thing that is a pain in V4L2 today is that there is no easy way to detect
> which streaming modes are supported. I think it would be a good idea to think
> what would be the best approach in DVB to expose that to userspace.
>
> The other comment is whether it might be a good idea to use create_bufs instead
> of reqbufs (or better, design an improved API).
>
> Due to historical reasons the V4L2 API to create/delete buffers is weird, and
> I think we can improve on that for DVB.
>
> > +
> > +/**
> > + * struct dmx_exportbuffer - export of dmx buffer as DMABUF file descriptor
> > + *
> > + * @index: id number of the buffer
> > + * @flags: flags for newly created file, currently only O_CLOEXEC is
> > + * supported, refer to manual of open syscall for more details
> > + * @fd: file descriptor associated with DMABUF (set by driver)
> > + *
> > + * Contains data used for exporting a dmx buffer as DMABUF file descriptor.
> > + * The buffer is identified by a 'cookie' returned by DMX_QUERYBUF
> > + * (identical to the cookie used to mmap() the buffer to userspace). All
> > + * reserved fields must be set to zero. The field reserved0 is expected to
> > + * become a structure 'type' allowing an alternative layout of the structure
> > + * content. Therefore this field should not be used for any other extensions.
> > + */
> > +struct dmx_exportbuffer {
> > + __u32 index;
> > + __u32 flags;
> > + __s32 fd;
> > + __u32 reserved[5];
> > +};
> > +
> > #define DMX_START _IO('o', 41)
> > #define DMX_STOP _IO('o', 42)
> > #define DMX_SET_FILTER _IOW('o', 43, struct dmx_sct_filter_params)
> > @@ -231,4 +289,10 @@ typedef struct dmx_filter dmx_filter_t;
> >
> > #endif
> >
> > -#endif /* _UAPI_DVBDMX_H_ */
> > +#define DMX_REQBUFS _IOWR('o', 60, struct dmx_requestbuffers)
> > +#define DMX_QUERYBUF _IOWR('o', 61, struct dmx_buffer)
> > +#define DMX_EXPBUF _IOWR('o', 62, struct dmx_exportbuffer)
> > +#define DMX_QBUF _IOWR('o', 63, struct dmx_buffer)
> > +#define DMX_DQBUF _IOWR('o', 64, struct dmx_buffer)
> > +
> > +#endif /* _DVBDMX_H_ */
> >
>
> Regards,
>
> Hans
Thanks,
Mauro
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O
2018-01-08 14:26 ` Hans Verkuil
@ 2018-01-08 14:38 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 17+ messages in thread
From: Mauro Carvalho Chehab @ 2018-01-08 14:38 UTC (permalink / raw)
To: Hans Verkuil
Cc: Linux Media Mailing List, Jonathan Corbet, Satendra Singh Thakur,
Mauro Carvalho Chehab, Linux Doc Mailing List, Seung-Woo Kim,
Greg Kroah-Hartman, Geunyoung Kim, Philippe Ombredanne, Inki Dae,
Thomas Gleixner, Junghak Sung, devendra sharma, Sakari Ailus
Em Mon, 8 Jan 2018 15:26:50 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> A quick follow-up:
>
> On 01/08/2018 02:54 PM, Hans Verkuil wrote:
> >> +/*
> >> + * Videobuf operations
> >> + */
> >> +int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int nonblocking)
> >> +{
> >> + struct vb2_queue *q = &ctx->vb_q;
> >> + int ret;
> >> +
> >> + memset(ctx, 0, sizeof(struct dvb_vb2_ctx));
> >> + q->type = DVB_BUF_TYPE_CAPTURE;
> >
> > We don't support DVB_BUF_TYPE_OUTPUT? Shouldn't this information come from the
> > driver?
> >
> >> + /**capture type*/
> >
> > Why /** ?
> >
> >> + q->is_output = 0;
> >
> > Can be dropped unless we support DVB_BUF_TYPE_OUTPUT.
> >
> >> + /**only mmap is supported currently*/
> >
> > /** ?
> >
> >> + q->io_modes = VB2_MMAP;
> >> + q->drv_priv = ctx;
> >> + q->buf_struct_size = sizeof(struct dvb_buffer);
> >> + q->min_buffers_needed = 1;
> >> + q->ops = &dvb_vb2_qops;
> >> + q->mem_ops = &vb2_vmalloc_memops;
> >> + q->buf_ops = &dvb_vb2_buf_ops;
> >> + q->num_buffers = 0;
> >
> > Not needed, is zeroed in the memset above.
> >
> > I'm also missing q->timestamp_flags: should be set to V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC.
>
> Ignore this, see my comments later on.
>
> >
> >> + ret = vb2_core_queue_init(q);
> >> + if (ret) {
> >> + ctx->state = DVB_VB2_STATE_NONE;
> >> + dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
> >> + return ret;
> >> + }
> >> +
> >> + mutex_init(&ctx->mutex);
> >> + spin_lock_init(&ctx->slock);
> >> + INIT_LIST_HEAD(&ctx->dvb_q);
> >> +
> >> + strncpy(ctx->name, name, DVB_VB2_NAME_MAX);
> >
> > I believe strlcpy is recommended.
> >
> >> + ctx->nonblocking = nonblocking;
> >> + ctx->state = DVB_VB2_STATE_INIT;
> >> +
> >> + dprintk(3, "[%s]\n", ctx->name);
> >> +
> >> + return 0;
> >> +}
>
> <snip>
>
> >> diff --git a/include/uapi/linux/dvb/dmx.h b/include/uapi/linux/dvb/dmx.h
> >> index c10f1324b4ca..e212aa18ad78 100644
> >> --- a/include/uapi/linux/dvb/dmx.h
> >> +++ b/include/uapi/linux/dvb/dmx.h
> >> @@ -211,6 +211,64 @@ struct dmx_stc {
> >> __u64 stc;
> >> };
> >>
> >> +/**
> >> + * struct dmx_buffer - dmx buffer info
> >> + *
> >> + * @index: id number of the buffer
> >> + * @bytesused: number of bytes occupied by data in the buffer (payload);
> >> + * @offset: for buffers with memory == DMX_MEMORY_MMAP;
> >> + * offset from the start of the device memory for this plane,
> >> + * (or a "cookie" that should be passed to mmap() as offset)
> >
> > Since we only support MMAP this is always a 'cookie' in practice. So I think this
> > should just be:
> >
> > A "cookie" that should be passed to mmap() as offset.
> >
> >> + * @length: size in bytes of the buffer
> >> + *
> >> + * Contains data exchanged by application and driver using one of the streaming
> >> + * I/O methods.
> >> + */
> >> +struct dmx_buffer {
> >> + __u32 index;
> >> + __u32 bytesused;
> >> + __u32 offset;
> >
> > I suggest making this a __u64: that way we can handle pointers as well in
> > the future if we need them (as we do for the USERPTR case for V4L2).
> >
> > Should this also be wrapped in a union? Useful when adding dmabuf support in the
> > future.
> >
> > Do you think there is any use-case for multiplanar formats in the future?
> > With perhaps meta data in a separate plane? Having to add support for this later
> > has proven to be very painful, so we need to be as certain as possible that
> > this isn't going to happen. Otherwise it's better to prepare for this right now.
> >
> >> + __u32 length;
> >> + __u32 reserved[4];
> >
> > I do believe you need a memory field as well. It's only MMAP today, but in
> > the future DMABUF will most likely be supported as well and you need to be
> > able to tell what memory mode is being used.
> >
> >> +};
>
> There is no 'flags' field here. But without that you cannot check the buffer
> states, esp. the ERROR state. Or can that never happen?
On DVB, there is a protocol stack there that allows checking errors,
either MPEG-TS (current standards) or IP/MMT - for the new, yet to be
implemented ATSC version 3 standard.
Even if we add an error state, userspace will just ignore, as it will
need to verify the packet CRC checks.
> Would a timestamp field be useful, if only for debugging?
I don't see how a timestamp will help here for debugging. Probably,
adding event trace points would work better, but we have those
already at vb2-core.
Anyway, as this is kAPI only, we can add it later as needed.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-01-08 14:38 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-21 16:17 [PATCH 00/11] dvb: add support for memory mapped I/O Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 01/11] media: vb2-core: Fix a bug about unnecessary calls to queue cancel and free Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O Mauro Carvalho Chehab
2018-01-08 13:54 ` Hans Verkuil
2018-01-08 14:26 ` Hans Verkuil
2018-01-08 14:38 ` Mauro Carvalho Chehab
2018-01-08 14:27 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 03/11] media: vb2-core: add pr_fmt() macro Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 04/11] media: vb2-core: add a new warning about pending buffers Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 05/11] media: dvb_vb2: fix a warning about streamoff logic Mauro Carvalho Chehab
2017-12-22 15:48 ` Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 06/11] media: move videobuf2 to drivers/media/common Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 07/11] media: dvb uAPI docs: document demux mmap/munmap syscalls Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 08/11] media: dvb uAPI docs: document mmap-related ioctls Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 09/11] media: dvb-core: get rid of mmap reserved field Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 10/11] fs: compat_ioctl: add new DVB demux ioctls Mauro Carvalho Chehab
2017-12-21 16:18 ` [PATCH 11/11] media: dvb_vb2: add SPDX headers Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).