[PATCH] [media] solo6x10: release vb2 buffers in solo_stop_streaming()

[PATCH] [media] solo6x10: release vb2 buffers in solo_stop_streaming()

[Anton Sviridenko]

Fixes warning that appears in dmesg after closing V4L2 userspace
application that plays video from the display device
(first device from V4L2 device nodes provided by solo, usually /dev/video0
when no other V4L2 devices are present). Encoder device nodes are not
affected. Can be reproduced by starting and closing

ffplay -f video4linux2  /dev/video0

[ 8130.281251] ------------[ cut here ]------------
[ 8130.281256] WARNING: CPU: 1 PID: 20414 at drivers/media/v4l2-core/videobuf2-core.c:1651 __vb2_queue_cancel+0x14b/0x230
[ 8130.281257] Modules linked in: ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat solo6x10 x86_pkg_temp_thermal vboxpci(O) vboxnetadp(O) vboxnetflt(O) vboxdrv(O)
[ 8130.281264] CPU: 1 PID: 20414 Comm: ffplay Tainted: G           O    4.10.0-gentoo #1
[ 8130.281264] Hardware name: ASUS All Series/B85M-E, BIOS 2301 03/30/2015
[ 8130.281265] Call Trace:
[ 8130.281267]  dump_stack+0x4f/0x72
[ 8130.281270]  __warn+0xc7/0xf0
[ 8130.281271]  warn_slowpath_null+0x18/0x20
[ 8130.281272]  __vb2_queue_cancel+0x14b/0x230
[ 8130.281273]  vb2_core_streamoff+0x23/0x90
[ 8130.281275]  vb2_streamoff+0x24/0x50
[ 8130.281276]  vb2_ioctl_streamoff+0x3d/0x50
[ 8130.281278]  v4l_streamoff+0x15/0x20
[ 8130.281279]  __video_do_ioctl+0x25e/0x2f0
[ 8130.281280]  video_usercopy+0x279/0x520
[ 8130.281282]  ? v4l_enum_fmt+0x1330/0x1330
[ 8130.281285]  ? unmap_region+0xdf/0x110
[ 8130.281285]  video_ioctl2+0x10/0x20
[ 8130.281286]  v4l2_ioctl+0xce/0xe0
[ 8130.281289]  do_vfs_ioctl+0x8b/0x5b0
[ 8130.281290]  ? __fget+0x72/0xa0
[ 8130.281291]  SyS_ioctl+0x74/0x80
[ 8130.281294]  entry_SYSCALL_64_fastpath+0x13/0x94
[ 8130.281295] RIP: 0033:0x7ff86fee6b27
[ 8130.281296] RSP: 002b:00007ffe467f6a08 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 8130.281297] RAX: ffffffffffffffda RBX: 00000000d1a4d788 RCX: 00007ff86fee6b27
[ 8130.281297] RDX: 00007ffe467f6a14 RSI: 0000000040045613 RDI: 0000000000000006
[ 8130.281298] RBP: 000000000373f8d0 R08: 00000000ffffffff R09: 00007ff860001140
[ 8130.281298] R10: 0000000000000243 R11: 0000000000000246 R12: 0000000000000000
[ 8130.281299] R13: 00000000000000a0 R14: 00007ffe467f6530 R15: 0000000001f32228
[ 8130.281300] ---[ end trace 00695dc96be646e7 ]---

Signed-off-by: Anton Sviridenko <anton@corp.bluecherry.net>
---
 drivers/media/pci/solo6x10/solo6x10-v4l2.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2.c b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
index 896bec6..4163103 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
@@ -341,6 +341,18 @@ static void solo_stop_streaming(struct vb2_queue *q)
 	struct solo_dev *solo_dev = vb2_get_drv_priv(q);
 
 	solo_stop_thread(solo_dev);
+
+	spin_lock(&solo_dev->slock);
+	while (!list_empty(&solo_dev->vidq_active)) {
+		struct solo_vb2_buf *buf = list_entry(
+				solo_dev->vidq_active.next,
+				struct solo_vb2_buf, list);
+
+		list_del(&buf->list);
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+		dbg_buf_cnt++;
+	}
+	spin_unlock(&solo_dev->slock);
 	INIT_LIST_HEAD(&solo_dev->vidq_active);
 }
 
-- 
2.10.2

Re: [PATCH] [media] solo6x10: release vb2 buffers in solo_stop_streaming()

[Andrey Utkin]

Signed-off-by: Andrey Utkin <andrey.utkin@corp.bluecherry.net>
Signed-off-by: Andrey Utkin <andrey_utkin@fastmail.com>

Please welcome Anton who is now in charge of solo6x10 and tw5864 support
and development in Bluecherry company, I have sent out to him the
hardware samples I possessed. (We will prepare the patch updating
MAINTAINERS file soon.)

If anybody has any outstanding complains, concerns or tasks regarding
solo6x10 and tw5864 drivers, I think now is good occasion to let us know
about it.

Re: [PATCH] [media] solo6x10: release vb2 buffers in solo_stop_streaming()

[Hans Verkuil]

On 08/03/17 18:47, Anton Sviridenko wrote:
> Fixes warning that appears in dmesg after closing V4L2 userspace
> application that plays video from the display device
> (first device from V4L2 device nodes provided by solo, usually /dev/video0
> when no other V4L2 devices are present). Encoder device nodes are not
> affected. Can be reproduced by starting and closing
> 
> ffplay -f video4linux2  /dev/video0
> 
> [ 8130.281251] ------------[ cut here ]------------
> [ 8130.281256] WARNING: CPU: 1 PID: 20414 at drivers/media/v4l2-core/videobuf2-core.c:1651 __vb2_queue_cancel+0x14b/0x230
> [ 8130.281257] Modules linked in: ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat solo6x10 x86_pkg_temp_thermal vboxpci(O) vboxnetadp(O) vboxnetflt(O) vboxdrv(O)
> [ 8130.281264] CPU: 1 PID: 20414 Comm: ffplay Tainted: G           O    4.10.0-gentoo #1
> [ 8130.281264] Hardware name: ASUS All Series/B85M-E, BIOS 2301 03/30/2015
> [ 8130.281265] Call Trace:
> [ 8130.281267]  dump_stack+0x4f/0x72
> [ 8130.281270]  __warn+0xc7/0xf0
> [ 8130.281271]  warn_slowpath_null+0x18/0x20
> [ 8130.281272]  __vb2_queue_cancel+0x14b/0x230
> [ 8130.281273]  vb2_core_streamoff+0x23/0x90
> [ 8130.281275]  vb2_streamoff+0x24/0x50
> [ 8130.281276]  vb2_ioctl_streamoff+0x3d/0x50
> [ 8130.281278]  v4l_streamoff+0x15/0x20
> [ 8130.281279]  __video_do_ioctl+0x25e/0x2f0
> [ 8130.281280]  video_usercopy+0x279/0x520
> [ 8130.281282]  ? v4l_enum_fmt+0x1330/0x1330
> [ 8130.281285]  ? unmap_region+0xdf/0x110
> [ 8130.281285]  video_ioctl2+0x10/0x20
> [ 8130.281286]  v4l2_ioctl+0xce/0xe0
> [ 8130.281289]  do_vfs_ioctl+0x8b/0x5b0
> [ 8130.281290]  ? __fget+0x72/0xa0
> [ 8130.281291]  SyS_ioctl+0x74/0x80
> [ 8130.281294]  entry_SYSCALL_64_fastpath+0x13/0x94
> [ 8130.281295] RIP: 0033:0x7ff86fee6b27
> [ 8130.281296] RSP: 002b:00007ffe467f6a08 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [ 8130.281297] RAX: ffffffffffffffda RBX: 00000000d1a4d788 RCX: 00007ff86fee6b27
> [ 8130.281297] RDX: 00007ffe467f6a14 RSI: 0000000040045613 RDI: 0000000000000006
> [ 8130.281298] RBP: 000000000373f8d0 R08: 00000000ffffffff R09: 00007ff860001140
> [ 8130.281298] R10: 0000000000000243 R11: 0000000000000246 R12: 0000000000000000
> [ 8130.281299] R13: 00000000000000a0 R14: 00007ffe467f6530 R15: 0000000001f32228
> [ 8130.281300] ---[ end trace 00695dc96be646e7 ]---
> 
> Signed-off-by: Anton Sviridenko <anton@corp.bluecherry.net>
> ---
>  drivers/media/pci/solo6x10/solo6x10-v4l2.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2.c b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
> index 896bec6..4163103 100644
> --- a/drivers/media/pci/solo6x10/solo6x10-v4l2.c
> +++ b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
> @@ -341,6 +341,18 @@ static void solo_stop_streaming(struct vb2_queue *q)
>  	struct solo_dev *solo_dev = vb2_get_drv_priv(q);
>  
>  	solo_stop_thread(solo_dev);
> +
> +	spin_lock(&solo_dev->slock);
> +	while (!list_empty(&solo_dev->vidq_active)) {
> +		struct solo_vb2_buf *buf = list_entry(
> +				solo_dev->vidq_active.next,
> +				struct solo_vb2_buf, list);
> +
> +		list_del(&buf->list);
> +		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> +		dbg_buf_cnt++;

Left-over from debugging? This variable doesn't exist in the mainline code, so
this patch doesn't compile.

Regards,

	Hans

> +	}
> +	spin_unlock(&solo_dev->slock);
>  	INIT_LIST_HEAD(&solo_dev->vidq_active);
>  }
>  
> 

Re: [PATCH] [media] solo6x10: release vb2 buffers in solo_stop_streaming()

[Anton Sviridenko]

On Thu, Mar 09, 2017 at 12:55:35PM +0100, Hans Verkuil wrote:
> > +		dbg_buf_cnt++;
> 
> Left-over from debugging? This variable doesn't exist in the mainline code, so
> this patch doesn't compile.
> 
> Regards,
> 
> 	Hans

Exactly, left-over from debugging, thank you. Going to resubmit fixed
patch.

Anton

Re: [PATCH] [media] solo6x10: release vb2 buffers in solo_stop_streaming()

[Ismael Luceno]

On 08/Mar/2017 21:59, Andrey Utkin wrote:
> Signed-off-by: Andrey Utkin <andrey.utkin@corp.bluecherry.net>
> Signed-off-by: Andrey Utkin <andrey_utkin@fastmail.com>
> 
> Please welcome Anton who is now in charge of solo6x10 and tw5864 support
> and development in Bluecherry company, I have sent out to him the
> hardware samples I possessed. (We will prepare the patch updating
> MAINTAINERS file soon.)
> 
> If anybody has any outstanding complains, concerns or tasks regarding
> solo6x10 and tw5864 drivers, I think now is good occasion to let us know
> about it.

Welcome Anton!

The first issue that comes to mind is that quantization matrices
need to be adjusted since forever. Also it needs more realistic
buffer sizes.

Re: [PATCH] [media] solo6x10: release vb2 buffers in solo_stop_streaming()

[Anton Sviridenko]

On Thu, Mar 09, 2017 at 06:58:45PM -0300, Ismael Luceno wrote:
> On 08/Mar/2017 21:59, Andrey Utkin wrote:
> > Signed-off-by: Andrey Utkin <andrey.utkin@corp.bluecherry.net>
> > Signed-off-by: Andrey Utkin <andrey_utkin@fastmail.com>
> > 
> > Please welcome Anton who is now in charge of solo6x10 and tw5864 support
> > and development in Bluecherry company, I have sent out to him the
> > hardware samples I possessed. (We will prepare the patch updating
> > MAINTAINERS file soon.)
> > 
> > If anybody has any outstanding complains, concerns or tasks regarding
> > solo6x10 and tw5864 drivers, I think now is good occasion to let us know
> > about it.
> 
> Welcome Anton!
> 
> The first issue that comes to mind is that quantization matrices
> need to be adjusted since forever. Also it needs more realistic
> buffer sizes.

Hi Ismael,

can you provide more details about quantization matrices issue and
buffer sizes?