dvb: add support for memory mapped I/O
Subsystems (3)
cover dvb: add support for memory mapped I/O
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.hSignature · 1 line
2.14.3
PATCH 1/11 media: vb2-core: Fix a bug about unnecessary calls to queue cancel and free
From: Satendra Singh Thakur <[email protected]>
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.
--- drivers/media/v4l2-core/videobuf2-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/media/v4l2-core/videobuf2-core.c +2 -1
@@ -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.
Signature · 1 line
2.14.3
PATCH 2/11 media: videobuf2: Add new uAPI for DVB streaming I/O
In reply to Mauro Carvalho ChehabFrom: Satendra Singh Thakur <[email protected]>
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 <[email protected]>, 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.
--- 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
drivers/media/dvb-core/Makefile +1 -1
@@ -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
drivers/media/dvb-core/dmxdev.c +168 -28
314 lines not on this page · raw message
drivers/media/dvb-core/dmxdev.h +4 -0
@@ -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; };
drivers/media/dvb-core/dvb_vb2.c new file +423 -0
423 lines not on this page · raw message
drivers/media/dvb-core/dvb_vb2.h new file +72 -0
72 lines not on this page · raw message
include/uapi/linux/dvb/dmx.h +65 -1
75 lines not on this page · raw message
Signature · 1 line
2.14.3
Re: [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O
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).
Quoted 28 lines · Mauro Carvalho Chehab
From: Satendra Singh Thakur <[email protected]>
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 willmmap()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 <[email protected]>, 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:
Quoted 7 lines
dvb_vb2_fill_buffer():
- Set the size of the outgoing buffer after while loop usingvb2_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
Quoted 10 lines
it will result in crash
- Restricted spinlock for only list_* operations
dvb_vb2_init():
- Restrictedq->io_modesto onlyVB2_MMAPas its the only supported mode
dvb_vb2_release():
- Replaced the && in if condiion with &, because otherwise
condition
Quoted 4 lines
it was always getting satisfied.
dvb_vb2_stream_off():
- Addedlist_delcode for enqueud buffers upon stream off
enqueued
Quoted 274 lines
dvb_vb2_poll():
- Added this new function in order to support polling
dvb_demux_poll()anddvb_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.
[[email protected]: checkpatch fixes]Co-developed-by Junghak Sung <[email protected]>Signed-off-by Junghak Sung <[email protected]>Signed-off-by Geunyoung Kim <[email protected]>Acked-by Seung-Woo Kim <[email protected]>Acked-by Inki Dae <[email protected]>Signed-off-by Satendra Singh Thakur <[email protected]>Signed-off-by Mauro Carvalho Chehab <[email protected]>--- 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.hdrivers/media/dvb-core/Makefile +1
-1@@ -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.odrivers/media/dvb-core/dmxdev.c +112
-26@@ -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)) {
drivers/media/dvb-core/dmxdev.c static unsigned int dvb_demux_poll(struct file *file, poll_table *wait) · line 1190
Quoted 10 lines
drivers/media/dvb-core/dmxdev.c +10
-0+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.
Quoted 154 lines
drivers/media/dvb-core/dmxdev.c +46
-2+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 = {drivers/media/dvb-core/dmxdev.h +4
-0@@ -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; };drivers/media/dvb-core/dvb_vb2.c new file +26
-0@@ -0,0 +1,423 @@ +/* +* dvb-vb2.c - dvb-vb2 +* +* Copyright (C) 2015 Samsung Electronics +* +* Author: [email protected] +* +* 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) \
drivers/media/dvb-core/dvb_vb2.c line 36
Quoted 10 lines
drivers/media/dvb-core/dvb_vb2.c new file +10
-0+ 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().
Quoted 115 lines
drivers/media/dvb-core/dvb_vb2.c new file +115
-0+ *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, +}; +
drivers/media/dvb-core/dvb_vb2.c line 161
Quoted 10 lines
drivers/media/dvb-core/dvb_vb2.c new file +10
-0+/* +* 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?
drivers/media/dvb-core/dvb_vb2.c line 162
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+/**capture type*/
Why /** ?
drivers/media/dvb-core/dvb_vb2.c line 163
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+ q->is_output =0 ;
Can be dropped unless we support DVB_BUF_TYPE_OUTPUT.
drivers/media/dvb-core/dvb_vb2.c line 164
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+/**only mmap is supported currently*/
/** ?
drivers/media/dvb-core/dvb_vb2.c line 172
Quoted 8 lines
drivers/media/dvb-core/dvb_vb2.c new file +8
-0+ 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.
drivers/media/dvb-core/dvb_vb2.c line 184
Quoted 12 lines
drivers/media/dvb-core/dvb_vb2.c new file +12
-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 );
I believe strlcpy is recommended.
Quoted 71 lines
drivers/media/dvb-core/dvb_vb2.c new file +71
-0+ 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 );
drivers/media/dvb-core/dvb_vb2.c line 265
Quoted 10 lines
drivers/media/dvb-core/dvb_vb2.c new file +10
-0+} + +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.
Quoted 242 lines
drivers/media/dvb-core/dvb_vb2.c new file +158
-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); +} +drivers/media/dvb-core/dvb_vb2.h new file +72
-0@@ -0,0 +1,72 @@ +/* +* dvb-vb2.h - DVB driver helper framework for streaming I/O +* +* Copyright (C) 2015 Samsung Electronics +* +* Author: [email protected] +* +* 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 */ include/uapi/linux/dvb/dmx.h +0
-0@@ -211,6 +211,64 @@ struct dmx_stc {__u64 stc;
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 221
Quoted 10 lines
include/uapi/linux/dvb/dmx.h +8
-0}; +/** +* 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.
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 230
Quoted 9 lines
include/uapi/linux/dvb/dmx.h +9
-0+* @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.
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 232
Quoted 2 lines
include/uapi/linux/dvb/dmx.h +2
-0+__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.
Quoted 6 lines
include/uapi/linux/dvb/dmx.h +6
-0+}; + +/** +* struct dmx_requestbuffers - request dmx buffer information +* +* @count: number of requested buffers,
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 248
Quoted 10 lines
include/uapi/linux/dvb/dmx.h +10
-0+* @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.
Quoted 29 lines
include/uapi/linux/dvb/dmx.h +23
-0+ +/** +* 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;
include/uapi/linux/dvb/dmx.h typedef struct dmx_filter dmx_filter_t; · line 298
Quoted 10 lines
include/uapi/linux/dvb/dmx.h +7
-1#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
Re: [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O
A quick follow-up:
Quoted 37 lines · Hans Verkuil
drivers/media/dvb-core/dvb_vb2.c line 161
Quoted 10 lines
drivers/media/dvb-core/dvb_vb2.c new file +10
-0+/* +* 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?drivers/media/dvb-core/dvb_vb2.c line 162
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+/**capture type*/ Why /** ?
drivers/media/dvb-core/dvb_vb2.c line 163
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+ q->is_output =0 ;Can be dropped unless we support
DVB_BUF_TYPE_OUTPUT.drivers/media/dvb-core/dvb_vb2.c line 164
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+/**only mmap is supported currently*/ /** ?
drivers/media/dvb-core/dvb_vb2.c line 172
Quoted 8 lines
drivers/media/dvb-core/dvb_vb2.c new file +8
-0+ 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.
Quoted 1 line
I'm also missing
q->timestamp_flags: should be set toV4L2_BUF_FLAG_TIMESTAMP_MONOTONIC.
Ignore this, see my comments later on.
Quoted 23 lines
drivers/media/dvb-core/dvb_vb2.c line 184
Quoted 13 lines
drivers/media/dvb-core/dvb_vb2.c new file +12
-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 );I believe strlcpy is recommended.
Quoted 7 lines
drivers/media/dvb-core/dvb_vb2.c new file +7
-0+ ctx->nonblocking = nonblocking; + ctx->state =DVB_VB2_STATE_INIT ; + +dprintk (3 ,"[%s]\n" , ctx->name); + +return 0 ; +}
<snip>
Quoted 43 lines
Quoted 6 lines
include/uapi/linux/dvb/dmx.h +0
-0@@ -211,6 +211,64 @@ struct dmx_stc {__u64 stc;include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 221
Quoted 10 lines
include/uapi/linux/dvb/dmx.h +8
-0}; +/** +* 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.include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 230
Quoted 9 lines
include/uapi/linux/dvb/dmx.h +9
-0+* @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.
Quoted 8 lines
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 232
Quoted 2 lines
include/uapi/linux/dvb/dmx.h +2
-0+__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.
Quoted 1 line
include/uapi/linux/dvb/dmx.h +1
-0+};
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
Re: [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O
Em Mon, 8 Jan 2018 15:26:50 +0100
Hans Verkuil <[email protected]> escreveu:
Quoted 120 lines
A quick follow-up:
Quoted 37 lines · Hans Verkuil
drivers/media/dvb-core/dvb_vb2.c line 161
Quoted 10 lines
drivers/media/dvb-core/dvb_vb2.c new file +10
-0+/* +* 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?drivers/media/dvb-core/dvb_vb2.c line 162
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+/**capture type*/ Why /** ?
drivers/media/dvb-core/dvb_vb2.c line 163
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+ q->is_output =0 ;Can be dropped unless we support
DVB_BUF_TYPE_OUTPUT.drivers/media/dvb-core/dvb_vb2.c line 164
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+/**only mmap is supported currently*/ /** ?
drivers/media/dvb-core/dvb_vb2.c line 172
Quoted 8 lines
drivers/media/dvb-core/dvb_vb2.c new file +8
-0+ 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.
Quoted 1 line
I'm also missing
q->timestamp_flags: should be set toV4L2_BUF_FLAG_TIMESTAMP_MONOTONIC.Ignore this, see my comments later on.
Quoted 23 lines
drivers/media/dvb-core/dvb_vb2.c line 184
Quoted 13 lines
drivers/media/dvb-core/dvb_vb2.c new file +12
-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 );I believe strlcpy is recommended.
Quoted 7 lines
drivers/media/dvb-core/dvb_vb2.c new file +7
-0+ ctx->nonblocking = nonblocking; + ctx->state =DVB_VB2_STATE_INIT ; + +dprintk (3 ,"[%s]\n" , ctx->name); + +return 0 ; +}<snip>
Quoted 50 lines
Quoted 6 lines
include/uapi/linux/dvb/dmx.h +0
-0@@ -211,6 +211,64 @@ struct dmx_stc {__u64 stc;include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 221
Quoted 10 lines
include/uapi/linux/dvb/dmx.h +8
-0}; +/** +* 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.include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 230
Quoted 9 lines
include/uapi/linux/dvb/dmx.h +9
-0+* @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.
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 232
Quoted 2 lines
include/uapi/linux/dvb/dmx.h +2
-0+__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.
Quoted 4 lines
Quoted 1 line
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 233
Quoted 1 line
include/uapi/linux/dvb/dmx.h +1
-0+};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.
Quoted 1 line
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
Re: [PATCH 02/11] media: videobuf2: Add new uAPI for DVB streaming I/O
In reply to Hans VerkuilEm Mon, 8 Jan 2018 14:54:51 +0100
Hans Verkuil <[email protected]> escreveu:
Quoted 58 lines
Hi Mauro,
Thanks for this patch series, nice to see this moving forward.
Some comments below (note the wrong
queue_setupcode in particular since that relates to my comment about the '[PATCH v3] media: videobuf2-core: don't go out of the buffer range' patch).Quoted 28 lines · Mauro Carvalho Chehab
From: Satendra Singh Thakur <[email protected]>
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 willmmap()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 <[email protected]>, 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:
Quoted 7 lines
dvb_vb2_fill_buffer():
- Set the size of the outgoing buffer after while loop usingvb2_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
Quoted 10 lines
it will result in crash
- Restricted spinlock for only list_* operations
dvb_vb2_init():
- Restrictedq->io_modesto onlyVB2_MMAPas its the only supported mode
dvb_vb2_release():
- Replaced the && in if condiion with &, because otherwise
Quoted 18 lines
condition
Quoted 4 lines
it was always getting satisfied.
dvb_vb2_stream_off():
- Addedlist_delcode for enqueud buffers upon stream offenqueued
Quoted 9 lines
dvb_vb2_poll():
- Added this new function in order to support polling
dvb_demux_poll()anddvb_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.
Quoted 277 lines
Quoted 264 lines
[[email protected]: checkpatch fixes]Co-developed-by Junghak Sung <[email protected]>Signed-off-by Junghak Sung <[email protected]>Signed-off-by Geunyoung Kim <[email protected]>Acked-by Seung-Woo Kim <[email protected]>Acked-by Inki Dae <[email protected]>Signed-off-by Satendra Singh Thakur <[email protected]>Signed-off-by Mauro Carvalho Chehab <[email protected]>--- 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.hdrivers/media/dvb-core/Makefile +1
-1@@ -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.odrivers/media/dvb-core/dmxdev.c +112
-26@@ -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)) {drivers/media/dvb-core/dmxdev.c static unsigned int dvb_demux_poll(struct file *file, poll_table *wait) · line 1190
Quoted 10 lines
drivers/media/dvb-core/dmxdev.c +10
-0+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?
Quoted 1 line
See also the extensive commit log of patch
f035eb4e976ef5a059e30bc91cfd310ff030a7d3.
Good point. I didn't test.
Quoted 168 lines
Quoted 155 lines
drivers/media/dvb-core/dmxdev.c +46
-2+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 = {drivers/media/dvb-core/dmxdev.h +4
-0@@ -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; };drivers/media/dvb-core/dvb_vb2.c new file +26
-0@@ -0,0 +1,423 @@ +/* +* dvb-vb2.c - dvb-vb2 +* +* Copyright (C) 2015 Samsung Electronics +* +* Author: [email protected] +* +* 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) \drivers/media/dvb-core/dvb_vb2.c line 36
Quoted 10 lines
drivers/media/dvb-core/dvb_vb2.c new file +10
-0+ 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.
Quoted 5 lines
*nbuffers has already been constrained to the correct buffer range but by assigning
ctx->buf_cntto 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.
Quoted 122 lines
and drop the '
ctx->buf_cnt=req->count;' indvb_vb2_reqbufs().Quoted 120 lines
drivers/media/dvb-core/dvb_vb2.c new file +120
-0+ *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) +{
Quoted 8 lines
drivers/media/dvb-core/dvb_vb2.c line 161
Quoted 5 lines
drivers/media/dvb-core/dvb_vb2.c new file +5
-0+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.
Quoted 4 lines
drivers/media/dvb-core/dvb_vb2.c line 162
Quoted 2 lines
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+/**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.
Quoted 4 lines
drivers/media/dvb-core/dvb_vb2.c line 163
Quoted 2 lines
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+ 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.
Quoted 4 lines
drivers/media/dvb-core/dvb_vb2.c line 164
Quoted 1 line
drivers/media/dvb-core/dvb_vb2.c new file +1
-0+/**only mmap is supported currently*/ /** ?
Quoted 10 lines
drivers/media/dvb-core/dvb_vb2.c line 172
Quoted 8 lines
drivers/media/dvb-core/dvb_vb2.c new file +8
-0+ 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.
Quoted 2 lines
I'm also missing
q->timestamp_flags: should be set toV4L2_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.
Quoted 15 lines
drivers/media/dvb-core/dvb_vb2.c line 184
Quoted 13 lines
drivers/media/dvb-core/dvb_vb2.c new file +12
-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 );I believe strlcpy is recommended.
Yes. I wrote/merged a patch doing that already :-)
Quoted 76 lines
Quoted 76 lines
drivers/media/dvb-core/dvb_vb2.c new file +75
-0+ 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)
Quoted 8 lines
drivers/media/dvb-core/dvb_vb2.c line 265
Quoted 6 lines
drivers/media/dvb-core/dvb_vb2.c new file +6
-0+{ +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.
Quoted 257 lines
Quoted 243 lines
drivers/media/dvb-core/dvb_vb2.c new file +158
-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); +} +drivers/media/dvb-core/dvb_vb2.h new file +72
-0@@ -0,0 +1,72 @@ +/* +* dvb-vb2.h - DVB driver helper framework for streaming I/O +* +* Copyright (C) 2015 Samsung Electronics +* +* Author: [email protected] +* +* 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 */ include/uapi/linux/dvb/dmx.h +0
-0@@ -211,6 +211,64 @@ struct dmx_stc {__u64 stc;include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 221
Quoted 10 lines
include/uapi/linux/dvb/dmx.h +8
-0}; +/** +* 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:
Quoted 1 line
A "cookie" that should be passed to
mmap()as offset.
Good point. Yeah, that "offset" field sounded weird to me too.
Quoted 13 lines
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 230
Quoted 10 lines
include/uapi/linux/dvb/dmx.h +9
-0+* @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).
Quoted 3 lines
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.
Quoted 11 lines
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.
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 232
Quoted 2 lines
include/uapi/linux/dvb/dmx.h +2
-0+__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).
Quoted 11 lines
Quoted 11 lines
include/uapi/linux/dvb/dmx.h +10
-0+}; + +/** +* 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.
Quoted 8 lines
include/uapi/linux/dvb/dmx.h struct dmx_stc { · line 248
Quoted 6 lines
include/uapi/linux/dvb/dmx.h +6
-0+*/ +struct dmx_requestbuffers { +__u32 count; +__u32 size; +__u32 reserved[2 ]; +};Ditto: add a memory field.
Ditto: I also removed reserved here.
Quoted 52 lines
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_bufsinstead 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.
Quoted 29 lines
include/uapi/linux/dvb/dmx.h +23
-0+ +/** +* 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;include/uapi/linux/dvb/dmx.h typedef struct dmx_filter dmx_filter_t; · line 298
Quoted 10 lines
include/uapi/linux/dvb/dmx.h +7
-1#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,
Quoted 1 line
Hans
Thanks,
Mauro
PATCH 3/11 media: vb2-core: add pr_fmt() macro
In reply to Mauro Carvalho ChehabSimplify the pr_foo() macros by adding a pr_fmt() macro.
--- drivers/media/v4l2-core/videobuf2-core.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)
drivers/media/v4l2-core/videobuf2-core.c +16 -14
@@ -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,
Signature · 1 line
2.14.3
PATCH 4/11 media: vb2-core: add a new warning about pending buffers
In reply to Mauro Carvalho ChehabThere'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.
--- drivers/media/v4l2-core/videobuf2-core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/media/v4l2-core/videobuf2-core.c +4 -1
@@ -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)); }
Signature · 1 line
2.14.3
PATCH 5/11 media: dvb_vb2: fix a warning about streamoff logic
In reply to Mauro Carvalho ChehabThe 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.
--- drivers/media/dvb-core/dvb_vb2.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)
drivers/media/dvb-core/dvb_vb2.c +20 -13
@@ -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 ; }
Signature · 1 line
2.14.3
Re: [PATCH 05/11] media: dvb_vb2: fix a warning about streamoff logic
Em Thu, 21 Dec 2017 14:18:04 -0200
Mauro Carvalho Chehab <[email protected]> escreveu:
Quoted 38 lines
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
Quoted 7 lines
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 <[email protected]>
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.
drivers/media/dvb-core/dvb_vb2.c +16 -17
@@ -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
PATCH 6/11 media: move videobuf2 to drivers/media/common
In reply to Mauro Carvalho ChehabNow that VB2 is used by both V4L2 and DVB core, move it to the common part of the subsystem.
---
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%)drivers/media/common/Kconfig +1 -0
@@ -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"
drivers/media/common/Makefile +1 -1
@@ -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
drivers/media/common/videobuf/Kconfig new file +31 -0
@@ -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
drivers/media/common/videobuf/Makefile new file +7 -0
@@ -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
drivers/media/common/videobuf/videobuf2-core.c renamed from drivers/media/v4l2-core/videobuf2-core.c +0 -0
drivers/media/common/videobuf/videobuf2-dma-contig.c renamed from drivers/media/v4l2-core/videobuf2-dma-contig.c +0 -0
drivers/media/common/videobuf/videobuf2-dma-sg.c renamed from drivers/media/v4l2-core/videobuf2-dma-sg.c +0 -0
drivers/media/common/videobuf/videobuf2-dvb.c renamed from drivers/media/v4l2-core/videobuf2-dvb.c +0 -0
drivers/media/common/videobuf/videobuf2-memops.c renamed from drivers/media/v4l2-core/videobuf2-memops.c +0 -0
drivers/media/common/videobuf/videobuf2-v4l2.c renamed from drivers/media/v4l2-core/videobuf2-v4l2.c +0 -0
drivers/media/common/videobuf/videobuf2-vmalloc.c renamed from drivers/media/v4l2-core/videobuf2-vmalloc.c +0 -0
drivers/media/v4l2-core/Kconfig +0 -32
@@ -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
drivers/media/v4l2-core/Makefile +0 -7
@@ -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
Signature · 1 line
2.14.3
PATCH 7/11 media: dvb uAPI docs: document demux mmap/munmap syscalls
In reply to Mauro Carvalho ChehabWith 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.
--- 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
Documentation/media/uapi/dvb/dmx-mmap.rst new file +116 -0
@@ -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.
Documentation/media/uapi/dvb/dmx-munmap.rst new file +54 -0
54 lines not on this page · raw message
Documentation/media/uapi/dvb/dmx_fcalls.rst +2 -0
@@ -13,6 +13,8 @@ Demux Function Calls dmx-fclose dmx-fread dmx-fwrite + dmx-mmap + dmx-munmap dmx-start dmx-stop dmx-set-filter
Signature · 1 line
2.14.3
PATCH 8/11 media: dvb uAPI docs: document mmap-related ioctls
In reply to Mauro Carvalho Chehab5 new ioctls were added to the DVB demux API, in order to handle memory maped I/O. Add documentation for them.
--- 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
Documentation/media/uapi/dvb/dmx-expbuf.rst new file +90 -0
@@ -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.
Documentation/media/uapi/dvb/dmx-qbuf.rst new file +85 -0
85 lines not on this page · raw message
Documentation/media/uapi/dvb/dmx-querybuf.rst new file +65 -0
65 lines not on this page · raw message
Documentation/media/uapi/dvb/dmx-reqbufs.rst new file +76 -0
76 lines not on this page · raw message
Documentation/media/uapi/dvb/dmx_fcalls.rst +4 -0
@@ -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
Signature · 1 line
2.14.3
PATCH 9/11 media: dvb-core: get rid of mmap reserved field
In reply to Mauro Carvalho ChehabThe "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.
--- 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(-)
Documentation/media/uapi/dvb/dmx-expbuf.rst +0 -2
@@ -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`
Documentation/media/uapi/dvb/dmx-qbuf.rst +0 -2
@@ -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
Documentation/media/uapi/dvb/dmx-querybuf.rst +0 -2
@@ -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.
Documentation/media/uapi/dvb/dmx-reqbufs.rst +0 -2
@@ -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
drivers/media/dvb-core/dvb_vb2.c +0 -1
@@ -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); }
include/uapi/linux/dvb/dmx.h +0 -3
@@ -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)
Signature · 1 line
2.14.3
PATCH 10/11 fs: compat_ioctl: add new DVB demux ioctls
In reply to Mauro Carvalho ChehabUse trivial handling for the new DVB demux ioctls, as none of them passes a pointer inside their structures.
--- fs/compat_ioctl.c | 5 +++++ 1 file changed, 5 insertions(+)
fs/compat_ioctl.c +5 -0
@@ -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)
Signature · 1 line
2.14.3
PATCH 11/11 media: dvb_vb2: add SPDX headers
In reply to Mauro Carvalho ChehabThis code is released under GPL. Add the corresponding SPDX headers.
--- drivers/media/dvb-core/dvb_vb2.c | 1 + drivers/media/dvb-core/dvb_vb2.h | 2 ++ 2 files changed, 3 insertions(+)
drivers/media/dvb-core/dvb_vb2.c +1 -0
@@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL /* * dvb-vb2.c - dvb-vb2 *
drivers/media/dvb-core/dvb_vb2.h +2 -0
@@ -1,4 +1,6 @@/* +* SPDX-License-Identifier: GPL +* * dvb-vb2.h - DVB driver helper framework for streaming I/O * * Copyright (C) 2015 Samsung Electronics
Signature · 1 line
2.14.3