Re: [PATCH v1] media: videobuf2: Add new uAPI for DVB streaming I/O

2017-12-21 Thread Mauro Carvalho Chehab
Em Tue, 19 Dec 2017 09:05:53 +0530
Satendra Singh Thakur  escreveu:

> -Ported below mentioned patch to latest kernel:
>  commit ace52288edf0 ("Merge tag 'for-linus-20171218' of
>  git://git.infradead.org/linux-mtd")
> 
> -Fixed few bugs in the patch, enhanced it and added polling
> --dvb_vb2.c: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.c:dvb_vb2_init=>  
> Restricted q->io_modes to only VB2_MMAP as its the only
>  supported mode
> 
> --dvb_vb2.c:dvb_vb2_release=>Replaced the && in if condiion with &,  
>  because it was always getting satisfied.
> 
> --dvb_vb2.c:dvb_vb2_stream_off=>Added list_del code for enqueud buffers  
>  upon stream off
> 
> -Added polling functionality
> --dvb_vb2.c:dvb_vb2_poll=>added this function  
> --dmxdev.c:dvb_demux_poll, dvb_dvr_poll=>Called dvb_vb2_poll from
>  these functions
> 
> -Ported this patch and latest videobuf2 to lower kernel versions and
>  tested auto scan
> 
> -Original patch=>
> --https://patchwork.linuxtv.org/patch/31613/
> 
> Signed-off-by: Junghak Sung 
> Signed-off-by: Geunyoung Kim 
> Acked-by: Seung-Woo Kim 
> Acked-by: Inki Dae 
> Signed-off-by: Satendra Singh Thakur 

Thanks for the patch!

I applied it on the top of Kernel 4.14 and fixed one bug that were causing 
an annoying WARN_ON() print.

Btw, as you wrote a poll() function, I suspect that you used some app to
test it. Care to ship the patches for your test application?

-

For now, I added on my experimental repository:
https://git.linuxtv.org/mchehab/experimental.git/log/?h=dvb_mmap

I also applied the testing patch from 2015 on this tree:

https://git.linuxtv.org/mchehab/experimental-v4l-utils.git/log/?h=dvb_mmap

I ended by rewriting the patch description to make it clearer/more
complete.

And I took some statistics using "perf stats" command, using a DiBcom 8000
ISDB-T device (Prolink Microsystems Corp. PV-D231U(RN)-F), running on an
Intel NUC D53427RKE with an i5-3427U CPU @ 1.80GHz (2 cores, 2 threads/core),
equipped with SSD disks.

Those are the results:

1) Writing to a file, using read() syscalls

 Performance counter stats for 'dvbv5-zap -c /home/mchehab/dvb_channel.conf TV 
Brasilia RedeTV! -o /tmp/out -t120':

 117.253780  task-clock-msecs #  0.001 CPUs 
   4593  context-switches #  0.039 M/sec
425  CPU-migrations   #  0.004 M/sec
194  page-faults  #  0.002 M/sec
  284427111  cycles   #   2425.739 M/sec
  143947266  instructions #  0.506 IPC  
   11975002  cache-references #102.129 M/sec
3571404  cache-misses # 30.459 M/sec

  120.114989264  seconds time elapsed

2) Writing to a file, using mmap() syscalls

 Performance counter stats for 'dvbv5-zap -c /home/mchehab/dvb_channel.conf TV 
Brasilia RedeTV! -o /tmp/out -t120 -R':

  69.258796  task-clock-msecs #  0.001 CPUs 
999  context-switches #  0.014 M/sec
190  CPU-migrations   #  0.003 M/sec
182  page-faults  #  0.003 M/sec
  176921677  cycles   #   2554.501 M/sec
  125926300  instructions #  0.712 IPC  
5053616  cache-references # 72.967 M/sec
3123259  cache-misses # 45.095 M/sec

  120.235712497  seconds time elapsed

All performance indicators reduced. The most impressive ones are
the number of context switches, that reduced by about 4.5x, and the 
number of cache references by about 2,5x! 

3) Writing to /dev/null, using read() syscalls

 Performance counter stats for 'dvbv5-zap -c /home/mchehab/dvb_channel.conf TV 
Brasilia RedeTV! -o /dev/null -t120':

  67.633678  task-clock-msecs #  0.001 CPUs 
   4583  context-switches #  0.068 M/sec
113  CPU-migrations   #  0.002 M/sec
199  page-faults  #  0.003 M/sec
  152635399  cycles   #   2256.796 M/sec
   30920291  instructions #  0.203 IPC  
6273962  cache-references # 92.764 M/sec
1666393  cache-misses # 24.639 M/sec


4) writing to /dev/null, using mmap() syscalls

 Performance counter stats for 'dvbv5-zap -c /home/mchehab/dvb_channel.conf TV 
Brasilia RedeTV! -o /dev/null -t120 -R':

  19.198192  task-clock-msecs #  0.000 CPUs 
991  

Re: [PATCH v1] media: videobuf2: Add new uAPI for DVB streaming I/O

2017-12-21 Thread Mauro Carvalho Chehab
Em Tue, 19 Dec 2017 09:05:53 +0530
Satendra Singh Thakur  escreveu:

> -Ported below mentioned patch to latest kernel:
>  commit ace52288edf0 ("Merge tag 'for-linus-20171218' of
>  git://git.infradead.org/linux-mtd")
> 
> -Fixed few bugs in the patch, enhanced it and added polling
> --dvb_vb2.c: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.c:dvb_vb2_init=>  
> Restricted q->io_modes to only VB2_MMAP as its the only
>  supported mode
> 
> --dvb_vb2.c:dvb_vb2_release=>Replaced the && in if condiion with &,  
>  because it was always getting satisfied.
> 
> --dvb_vb2.c:dvb_vb2_stream_off=>Added list_del code for enqueud buffers  
>  upon stream off
> 
> -Added polling functionality
> --dvb_vb2.c:dvb_vb2_poll=>added this function  
> --dmxdev.c:dvb_demux_poll, dvb_dvr_poll=>Called dvb_vb2_poll from
>  these functions
> 
> -Ported this patch and latest videobuf2 to lower kernel versions and
>  tested auto scan
> 
> -Original patch=>
> --https://patchwork.linuxtv.org/patch/31613/
> 
> Signed-off-by: Junghak Sung 
> Signed-off-by: Geunyoung Kim 
> Acked-by: Seung-Woo Kim 
> Acked-by: Inki Dae 
> Signed-off-by: Satendra Singh Thakur 

Thanks for the patch!

I applied it on the top of Kernel 4.14 and fixed one bug that were causing 
an annoying WARN_ON() print.

Btw, as you wrote a poll() function, I suspect that you used some app to
test it. Care to ship the patches for your test application?

-

For now, I added on my experimental repository:
https://git.linuxtv.org/mchehab/experimental.git/log/?h=dvb_mmap

I also applied the testing patch from 2015 on this tree:

https://git.linuxtv.org/mchehab/experimental-v4l-utils.git/log/?h=dvb_mmap

I ended by rewriting the patch description to make it clearer/more
complete.

And I took some statistics using "perf stats" command, using a DiBcom 8000
ISDB-T device (Prolink Microsystems Corp. PV-D231U(RN)-F), running on an
Intel NUC D53427RKE with an i5-3427U CPU @ 1.80GHz (2 cores, 2 threads/core),
equipped with SSD disks.

Those are the results:

1) Writing to a file, using read() syscalls

 Performance counter stats for 'dvbv5-zap -c /home/mchehab/dvb_channel.conf TV 
Brasilia RedeTV! -o /tmp/out -t120':

 117.253780  task-clock-msecs #  0.001 CPUs 
   4593  context-switches #  0.039 M/sec
425  CPU-migrations   #  0.004 M/sec
194  page-faults  #  0.002 M/sec
  284427111  cycles   #   2425.739 M/sec
  143947266  instructions #  0.506 IPC  
   11975002  cache-references #102.129 M/sec
3571404  cache-misses # 30.459 M/sec

  120.114989264  seconds time elapsed

2) Writing to a file, using mmap() syscalls

 Performance counter stats for 'dvbv5-zap -c /home/mchehab/dvb_channel.conf TV 
Brasilia RedeTV! -o /tmp/out -t120 -R':

  69.258796  task-clock-msecs #  0.001 CPUs 
999  context-switches #  0.014 M/sec
190  CPU-migrations   #  0.003 M/sec
182  page-faults  #  0.003 M/sec
  176921677  cycles   #   2554.501 M/sec
  125926300  instructions #  0.712 IPC  
5053616  cache-references # 72.967 M/sec
3123259  cache-misses # 45.095 M/sec

  120.235712497  seconds time elapsed

All performance indicators reduced. The most impressive ones are
the number of context switches, that reduced by about 4.5x, and the 
number of cache references by about 2,5x! 

3) Writing to /dev/null, using read() syscalls

 Performance counter stats for 'dvbv5-zap -c /home/mchehab/dvb_channel.conf TV 
Brasilia RedeTV! -o /dev/null -t120':

  67.633678  task-clock-msecs #  0.001 CPUs 
   4583  context-switches #  0.068 M/sec
113  CPU-migrations   #  0.002 M/sec
199  page-faults  #  0.003 M/sec
  152635399  cycles   #   2256.796 M/sec
   30920291  instructions #  0.203 IPC  
6273962  cache-references # 92.764 M/sec
1666393  cache-misses # 24.639 M/sec


4) writing to /dev/null, using mmap() syscalls

 Performance counter stats for 'dvbv5-zap -c /home/mchehab/dvb_channel.conf TV 
Brasilia RedeTV! -o /dev/null -t120 -R':

  19.198192  task-clock-msecs #  0.000 CPUs 
991  context-switches #  0.052 M/sec
196  CPU-migrations   #  0.010 M/sec
181  page-faults   

Re: [PATCH v1] media: videobuf2: Add new uAPI for DVB streaming I/O

2017-12-18 Thread Philippe Ombredanne
Dear Satendra,

On Tue, Dec 19, 2017 at 4:35 AM, Satendra Singh Thakur
 wrote:



> --- /dev/null
> +++ b/drivers/media/dvb-core/dvb_vb2.c
> @@ -0,0 +1,422 @@
> +/*
> + * dvb-vb2.c - dvb-vb2
> + *
> + * Copyright (C) 2015 Samsung Electronics
> + *
> + * Author: jh1009.s...@samsung.com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation.
> + */

Would you mind using the new SPDX tags documented in Thomas patch set
[1] rather than this fine but longer legalese?

And if you could spread the word to others in your team this would be very nice.
See also this fine article posted by Mauro on the Samsung Open Source
Group Blog [2]
Thank you!

[1] https://lkml.org/lkml/2017/12/4/934
[2] https://blogs.s-osg.org/linux-kernel-license-practices-revisited-spdx/
-- 
Cordially
Philippe Ombredanne


Re: [PATCH v1] media: videobuf2: Add new uAPI for DVB streaming I/O

2017-12-18 Thread Philippe Ombredanne
Dear Satendra,

On Tue, Dec 19, 2017 at 4:35 AM, Satendra Singh Thakur
 wrote:



> --- /dev/null
> +++ b/drivers/media/dvb-core/dvb_vb2.c
> @@ -0,0 +1,422 @@
> +/*
> + * dvb-vb2.c - dvb-vb2
> + *
> + * Copyright (C) 2015 Samsung Electronics
> + *
> + * Author: jh1009.s...@samsung.com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation.
> + */

Would you mind using the new SPDX tags documented in Thomas patch set
[1] rather than this fine but longer legalese?

And if you could spread the word to others in your team this would be very nice.
See also this fine article posted by Mauro on the Samsung Open Source
Group Blog [2]
Thank you!

[1] https://lkml.org/lkml/2017/12/4/934
[2] https://blogs.s-osg.org/linux-kernel-license-practices-revisited-spdx/
-- 
Cordially
Philippe Ombredanne


[PATCH v1] media: videobuf2: Add new uAPI for DVB streaming I/O

2017-12-18 Thread Satendra Singh Thakur
-Ported below mentioned patch to latest kernel:
 commit ace52288edf0 ("Merge tag 'for-linus-20171218' of
 git://git.infradead.org/linux-mtd")

-Fixed few bugs in the patch, enhanced it and added polling
--dvb_vb2.c: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.c:dvb_vb2_init=>
Restricted q->io_modes to only VB2_MMAP as its the only
 supported mode

--dvb_vb2.c:dvb_vb2_release=>Replaced the && in if condiion with &,
 because it was always getting satisfied.

--dvb_vb2.c:dvb_vb2_stream_off=>Added list_del code for enqueud buffers
 upon stream off

-Added polling functionality
--dvb_vb2.c:dvb_vb2_poll=>added this function
--dmxdev.c:dvb_demux_poll, dvb_dvr_poll=>Called dvb_vb2_poll from
 these functions

-Ported this patch and latest videobuf2 to lower kernel versions and
 tested auto scan

-Original patch=>
--https://patchwork.linuxtv.org/patch/31613/

Signed-off-by: Junghak Sung 
Signed-off-by: Geunyoung Kim 
Acked-by: Seung-Woo Kim 
Acked-by: Inki Dae 
Signed-off-by: Satendra Singh Thakur 
---
 drivers/media/dvb-core/Makefile  |   2 +-
 drivers/media/dvb-core/dmxdev.c  | 194 +++---
 drivers/media/dvb-core/dmxdev.h  |   4 +
 drivers/media/dvb-core/dvb_vb2.c | 422 +++
 drivers/media/dvb-core/dvb_vb2.h |  72 +++
 include/uapi/linux/dvb/dmx.h |  66 +-
 6 files changed, 731 insertions(+), 29 deletions(-)
 create mode 100644 drivers/media/dvb-core/dvb_vb2.c
 create mode 100644 drivers/media/dvb-core/dvb_vb2.h

diff --git a/drivers/media/dvb-core/Makefile b/drivers/media/dvb-core/Makefile
index 47e2e39..bbc65df 100644
--- a/drivers/media/dvb-core/Makefile
+++ b/drivers/media/dvb-core/Makefile
@@ -7,6 +7,6 @@ dvb-net-$(CONFIG_DVB_NET) := dvb_net.o
 
 dvb-core-objs := dvbdev.o dmxdev.o dvb_demux.o \
 dvb_ca_en50221.o dvb_frontend.o\
-$(dvb-net-y) dvb_ringbuffer.o dvb_math.o
+$(dvb-net-y) dvb_ringbuffer.o dvb_vb2.o dvb_math.o
 
 obj-$(CONFIG_DVB_CORE) += dvb-core.o
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 3ddd44e..7701cb0 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #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(>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(>dvr_buffer, mem, DVR_BUFFER_SIZE);
+   dvb_vb2_init(>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(>dvr_vb2_ctx))
+   dvb_vb2_stream_off(>dvr_vb2_ctx);
+   dvb_vb2_release(>dvr_vb2_ctx);
dvbdev->readers++;
if (dmxdev->dvr_buffer.data) {
void *mem = dmxdev->dvr_buffer.data;
@@ -358,8 +359,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(>vb2_ctx)
+   && dmxdevfilter->buffer.error) {
wake_up(>buffer.queue);
return 0;
}
@@ -370,11 +371,19 @@ static int dvb_dmxdev_section_callback(const u8 *buffer1, 
size_t buffer1_len,
}

[PATCH v1] media: videobuf2: Add new uAPI for DVB streaming I/O

2017-12-18 Thread Satendra Singh Thakur
-Ported below mentioned patch to latest kernel:
 commit ace52288edf0 ("Merge tag 'for-linus-20171218' of
 git://git.infradead.org/linux-mtd")

-Fixed few bugs in the patch, enhanced it and added polling
--dvb_vb2.c: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.c:dvb_vb2_init=>
Restricted q->io_modes to only VB2_MMAP as its the only
 supported mode

--dvb_vb2.c:dvb_vb2_release=>Replaced the && in if condiion with &,
 because it was always getting satisfied.

--dvb_vb2.c:dvb_vb2_stream_off=>Added list_del code for enqueud buffers
 upon stream off

-Added polling functionality
--dvb_vb2.c:dvb_vb2_poll=>added this function
--dmxdev.c:dvb_demux_poll, dvb_dvr_poll=>Called dvb_vb2_poll from
 these functions

-Ported this patch and latest videobuf2 to lower kernel versions and
 tested auto scan

-Original patch=>
--https://patchwork.linuxtv.org/patch/31613/

Signed-off-by: Junghak Sung 
Signed-off-by: Geunyoung Kim 
Acked-by: Seung-Woo Kim 
Acked-by: Inki Dae 
Signed-off-by: Satendra Singh Thakur 
---
 drivers/media/dvb-core/Makefile  |   2 +-
 drivers/media/dvb-core/dmxdev.c  | 194 +++---
 drivers/media/dvb-core/dmxdev.h  |   4 +
 drivers/media/dvb-core/dvb_vb2.c | 422 +++
 drivers/media/dvb-core/dvb_vb2.h |  72 +++
 include/uapi/linux/dvb/dmx.h |  66 +-
 6 files changed, 731 insertions(+), 29 deletions(-)
 create mode 100644 drivers/media/dvb-core/dvb_vb2.c
 create mode 100644 drivers/media/dvb-core/dvb_vb2.h

diff --git a/drivers/media/dvb-core/Makefile b/drivers/media/dvb-core/Makefile
index 47e2e39..bbc65df 100644
--- a/drivers/media/dvb-core/Makefile
+++ b/drivers/media/dvb-core/Makefile
@@ -7,6 +7,6 @@ dvb-net-$(CONFIG_DVB_NET) := dvb_net.o
 
 dvb-core-objs := dvbdev.o dmxdev.o dvb_demux.o \
 dvb_ca_en50221.o dvb_frontend.o\
-$(dvb-net-y) dvb_ringbuffer.o dvb_math.o
+$(dvb-net-y) dvb_ringbuffer.o dvb_vb2.o dvb_math.o
 
 obj-$(CONFIG_DVB_CORE) += dvb-core.o
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 3ddd44e..7701cb0 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #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(>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(>dvr_buffer, mem, DVR_BUFFER_SIZE);
+   dvb_vb2_init(>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(>dvr_vb2_ctx))
+   dvb_vb2_stream_off(>dvr_vb2_ctx);
+   dvb_vb2_release(>dvr_vb2_ctx);
dvbdev->readers++;
if (dmxdev->dvr_buffer.data) {
void *mem = dmxdev->dvr_buffer.data;
@@ -358,8 +359,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(>vb2_ctx)
+   && dmxdevfilter->buffer.error) {
wake_up(>buffer.queue);
return 0;
}
@@ -370,11 +371,19 @@ static int dvb_dmxdev_section_callback(const u8 *buffer1, 
size_t buffer1_len,
}
del_timer(>timer);
dprintk("section callback %*ph\n", 6, buffer1);
-   ret = dvb_dmxdev_buffer_write(>buffer,