* [PATCH 1/2] hverkuil/go7007: staging: media: go7007: Restore b_frame control
@ 2013-03-12 15:09 Volokh Konstantin
2013-03-12 15:09 ` [PATCH 2/2] hverkuil/go7007: staging: media: go7007: rmmod firmware protection stuff Volokh Konstantin
2013-03-12 15:24 ` [PATCH 1/2] hverkuil/go7007: staging: media: go7007: Restore b_frame control Hans Verkuil
0 siblings, 2 replies; 4+ messages in thread
From: Volokh Konstantin @ 2013-03-12 15:09 UTC (permalink / raw)
To: hverkuil, linux-media; +Cc: Volokh Konstantin
Signed-off-by: Volokh Konstantin <volokh84@gmail.com>
---
drivers/staging/media/go7007/go7007-priv.h | 1 +
drivers/staging/media/go7007/go7007-v4l2.c | 7 +++++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/go7007/go7007-priv.h b/drivers/staging/media/go7007/go7007-priv.h
index 0914fa3..5f9b389 100644
--- a/drivers/staging/media/go7007/go7007-priv.h
+++ b/drivers/staging/media/go7007/go7007-priv.h
@@ -166,6 +166,7 @@ struct go7007 {
struct v4l2_ctrl *mpeg_video_gop_closure;
struct v4l2_ctrl *mpeg_video_bitrate;
struct v4l2_ctrl *mpeg_video_aspect_ratio;
+ struct v4l2_ctrl *mpeg_video_b_frames;
enum { STATUS_INIT, STATUS_ONLINE, STATUS_SHUTDOWN } status;
spinlock_t spinlock;
struct mutex hw_lock;
diff --git a/drivers/staging/media/go7007/go7007-v4l2.c b/drivers/staging/media/go7007/go7007-v4l2.c
index 3634580..06fc930 100644
--- a/drivers/staging/media/go7007/go7007-v4l2.c
+++ b/drivers/staging/media/go7007/go7007-v4l2.c
@@ -170,7 +170,7 @@ static void set_formatting(struct go7007 *go)
go->gop_size == 15 &&
go->closed_gop;
go->repeat_seqhead = go->dvd_mode;
- go->ipb = 0;
+ go->ipb = v4l2_ctrl_g_ctrl(go->mpeg_video_b_frames);
switch (v4l2_ctrl_g_ctrl(go->mpeg_video_aspect_ratio)) {
default:
@@ -935,7 +935,7 @@ int go7007_v4l2_ctrl_init(struct go7007 *go)
struct v4l2_ctrl_handler *hdl = &go->hdl;
struct v4l2_ctrl *ctrl;
- v4l2_ctrl_handler_init(hdl, 12);
+ v4l2_ctrl_handler_init(hdl, 13);
go->mpeg_video_gop_size = v4l2_ctrl_new_std(hdl, NULL,
V4L2_CID_MPEG_VIDEO_GOP_SIZE, 0, 34, 1, 15);
go->mpeg_video_gop_closure = v4l2_ctrl_new_std(hdl, NULL,
@@ -943,6 +943,9 @@ int go7007_v4l2_ctrl_init(struct go7007 *go)
go->mpeg_video_bitrate = v4l2_ctrl_new_std(hdl, NULL,
V4L2_CID_MPEG_VIDEO_BITRATE,
64000, 10000000, 1, 9800000);
+ go->mpeg_video_b_frames = v4l2_ctrl_new_std(hdl, NULL,
+ V4L2_CID_MPEG_VIDEO_B_FRAMES, 0, 2, 1, 0);
+
go->mpeg_video_aspect_ratio = v4l2_ctrl_new_std_menu(hdl, NULL,
V4L2_CID_MPEG_VIDEO_ASPECT,
V4L2_MPEG_VIDEO_ASPECT_16x9, 0,
--
1.7.7.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] hverkuil/go7007: staging: media: go7007: rmmod firmware protection stuff
2013-03-12 15:09 [PATCH 1/2] hverkuil/go7007: staging: media: go7007: Restore b_frame control Volokh Konstantin
@ 2013-03-12 15:09 ` Volokh Konstantin
2013-03-12 15:26 ` Hans Verkuil
2013-03-12 15:24 ` [PATCH 1/2] hverkuil/go7007: staging: media: go7007: Restore b_frame control Hans Verkuil
1 sibling, 1 reply; 4+ messages in thread
From: Volokh Konstantin @ 2013-03-12 15:09 UTC (permalink / raw)
To: hverkuil, linux-media; +Cc: Volokh Konstantin
If firmware wasn`t load, rmmod fail with oops:
usbcore: deregistering interface driver go7007
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffff81797d02>] __mutex_lock_slowpath+0xa2/0x140
PGD 13143b067 PUD 132a52067 PMD 0
Oops: 0002 [#1] SMP
Modules linked in: go7007_usb(C-) go7007(C) videobuf2_core bttv videobuf_dma_sg videobuf_core btcx_risc rc_core v4l2_common videodev videobuf2_vmalloc videobuf2_memops tveeprom
CPU 0
Pid: 3305, comm: rmmod Tainted: G C 3.9.0-rc1+ #4 To Be Filled By O.E.M. To Be Filled By O.E.M./H55MX-S Series
RIP: 0010:[<ffffffff81797d02>] [<ffffffff81797d02>] __mutex_lock_slowpath+0xa2/0x140
RSP: 0018:ffff88013220bd18 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff880132264c80 RCX: 00000000ffffffff
RDX: ffff88013220bd20 RSI: ffff880130d3ecc0 RDI: ffff880132264c84
RBP: ffff88013220bd68 R08: 0000000000000000 R09: ffffea0004c5c940
R10: ffffffff811a08d3 R11: 0000000000027854 R12: ffff880132264c84
R13: ffff8801325ec410 R14: 00000000ffffffff R15: ffff880132264c88
Mar 12 01:25:57 Video kernel: [ 128.939419] FS: 00007fb32649d700(0000) GS:ffff880137c00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000000000 CR3: 0000000130f1d000 CR4: 00000000000007f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process rmmod (pid: 3305, threadinfo ffff88013220a000, task ffff8801325ec410)
Stack:
ffff88013220bd48 ffff880132264c88 0000000000000000 ffff880131abf800
ffff880130d3ecc0 ffff880132264c80 ffff880132264c80 ffff880132264478
ffff880132264000 0000000000000000 ffff88013220bd88 ffffffff81797c05
Call Trace:
[<ffffffff81797c05>] mutex_lock+0x25/0x40
[<ffffffffa0090051>] go7007_usb_disconnect+0x41/0xb0 [go7007_usb]
[<ffffffff814b105b>] usb_unbind_interface+0x5b/0x130
[<ffffffff813f0a71>] __device_release_driver+0x61/0xd0
[<ffffffff813f1340>] driver_detach+0xb0/0xc0
[<ffffffff813f0859>] bus_remove_driver+0x79/0xd0
[<ffffffff813f19da>] driver_unregister+0x5a/0x90
[<ffffffff814b0d64>] usb_deregister+0x64/0xd0
[<ffffffffa00914dc>] go7007_usb_driver_exit+0x10/0x12 [go7007_usb]
[<ffffffff8109370c>] sys_delete_module+0x15c/0x240
[<ffffffff817a1d12>] system_call_fastpath+0x16/0x1b
Code: 00 4c 8d 63 04 4c 8d 7b 08 41 be ff ff ff ff 4c 89 e7 e8 92 26 00 00 48 8b 43 10 48 8d 55 b8 4c 89 7d b8 48 89 53 10 48 89 45 c0 <48> 89 10 44 89 f0 4c 89 6d c8 87 03 83 f8 01 75 1f eb 27 0f 1f
RIP [<ffffffff81797d02>] __mutex_lock_slowpath+0xa2/0x140
RSP <ffff88013220bd18>
CR2: 0000000000000000
Signed-off-by: Volokh Konstantin <volokh84@gmail.com>
---
drivers/staging/media/go7007/go7007-usb.c | 25 +++++++++++++------------
1 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/go7007/go7007-usb.c b/drivers/staging/media/go7007/go7007-usb.c
index bd23d7d..48ad01f 100644
--- a/drivers/staging/media/go7007/go7007-usb.c
+++ b/drivers/staging/media/go7007/go7007-usb.c
@@ -1316,18 +1316,19 @@ static void go7007_usb_disconnect(struct usb_interface *intf)
{
struct go7007 *go = to_go7007(usb_get_intfdata(intf));
- mutex_lock(&go->queue_lock);
- mutex_lock(&go->serialize_lock);
-
- if (go->audio_enabled)
- go7007_snd_remove(go);
-
- go->status = STATUS_SHUTDOWN;
- v4l2_device_disconnect(&go->v4l2_dev);
- video_unregister_device(&go->vdev);
- mutex_unlock(&go->serialize_lock);
- mutex_unlock(&go->queue_lock);
-
+ if (go->status == STATUS_ONLINE) {
+ mutex_lock(&go->queue_lock);
+ mutex_lock(&go->serialize_lock);
+
+ if (go->audio_enabled)
+ go7007_snd_remove(go);
+
+ go->status = STATUS_SHUTDOWN;
+ v4l2_device_disconnect(&go->v4l2_dev);
+ video_unregister_device(&go->vdev);
+ mutex_unlock(&go->serialize_lock);
+ mutex_unlock(&go->queue_lock);
+ }
v4l2_device_put(&go->v4l2_dev);
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] hverkuil/go7007: staging: media: go7007: Restore b_frame control
2013-03-12 15:09 [PATCH 1/2] hverkuil/go7007: staging: media: go7007: Restore b_frame control Volokh Konstantin
2013-03-12 15:09 ` [PATCH 2/2] hverkuil/go7007: staging: media: go7007: rmmod firmware protection stuff Volokh Konstantin
@ 2013-03-12 15:24 ` Hans Verkuil
1 sibling, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2013-03-12 15:24 UTC (permalink / raw)
To: Volokh Konstantin; +Cc: linux-media
On Tue 12 March 2013 16:09:29 Volokh Konstantin wrote:
> Signed-off-by: Volokh Konstantin <volokh84@gmail.com>
Just wondering: did you test this?
If you get the latest v4l-utils code and run:
v4l2-ctl --stream-mmap=3
(first set the format to MPEG2) you should see B frames appearing.
> ---
> drivers/staging/media/go7007/go7007-priv.h | 1 +
> drivers/staging/media/go7007/go7007-v4l2.c | 7 +++++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/go7007/go7007-priv.h b/drivers/staging/media/go7007/go7007-priv.h
> index 0914fa3..5f9b389 100644
> --- a/drivers/staging/media/go7007/go7007-priv.h
> +++ b/drivers/staging/media/go7007/go7007-priv.h
> @@ -166,6 +166,7 @@ struct go7007 {
> struct v4l2_ctrl *mpeg_video_gop_closure;
> struct v4l2_ctrl *mpeg_video_bitrate;
> struct v4l2_ctrl *mpeg_video_aspect_ratio;
> + struct v4l2_ctrl *mpeg_video_b_frames;
> enum { STATUS_INIT, STATUS_ONLINE, STATUS_SHUTDOWN } status;
> spinlock_t spinlock;
> struct mutex hw_lock;
> diff --git a/drivers/staging/media/go7007/go7007-v4l2.c b/drivers/staging/media/go7007/go7007-v4l2.c
> index 3634580..06fc930 100644
> --- a/drivers/staging/media/go7007/go7007-v4l2.c
> +++ b/drivers/staging/media/go7007/go7007-v4l2.c
> @@ -170,7 +170,7 @@ static void set_formatting(struct go7007 *go)
> go->gop_size == 15 &&
> go->closed_gop;
What should the ipb mode be for 'dvd_mode'? I think 0, but I'm not sure.
> go->repeat_seqhead = go->dvd_mode;
> - go->ipb = 0;
> + go->ipb = v4l2_ctrl_g_ctrl(go->mpeg_video_b_frames);
>
> switch (v4l2_ctrl_g_ctrl(go->mpeg_video_aspect_ratio)) {
> default:
> @@ -935,7 +935,7 @@ int go7007_v4l2_ctrl_init(struct go7007 *go)
> struct v4l2_ctrl_handler *hdl = &go->hdl;
> struct v4l2_ctrl *ctrl;
>
> - v4l2_ctrl_handler_init(hdl, 12);
> + v4l2_ctrl_handler_init(hdl, 13);
> go->mpeg_video_gop_size = v4l2_ctrl_new_std(hdl, NULL,
> V4L2_CID_MPEG_VIDEO_GOP_SIZE, 0, 34, 1, 15);
> go->mpeg_video_gop_closure = v4l2_ctrl_new_std(hdl, NULL,
> @@ -943,6 +943,9 @@ int go7007_v4l2_ctrl_init(struct go7007 *go)
> go->mpeg_video_bitrate = v4l2_ctrl_new_std(hdl, NULL,
> V4L2_CID_MPEG_VIDEO_BITRATE,
> 64000, 10000000, 1, 9800000);
> + go->mpeg_video_b_frames = v4l2_ctrl_new_std(hdl, NULL,
> + V4L2_CID_MPEG_VIDEO_B_FRAMES, 0, 2, 1, 0);
Set the step value to '2'. As I understand it it is either 0 or 2 and value 1
isn't supported.
> +
> go->mpeg_video_aspect_ratio = v4l2_ctrl_new_std_menu(hdl, NULL,
> V4L2_CID_MPEG_VIDEO_ASPECT,
> V4L2_MPEG_VIDEO_ASPECT_16x9, 0,
>
Regards,
Hans
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] hverkuil/go7007: staging: media: go7007: rmmod firmware protection stuff
2013-03-12 15:09 ` [PATCH 2/2] hverkuil/go7007: staging: media: go7007: rmmod firmware protection stuff Volokh Konstantin
@ 2013-03-12 15:26 ` Hans Verkuil
0 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2013-03-12 15:26 UTC (permalink / raw)
To: Volokh Konstantin; +Cc: linux-media
On Tue 12 March 2013 16:09:30 Volokh Konstantin wrote:
> If firmware wasn`t load, rmmod fail with oops:
>
> usbcore: deregistering interface driver go7007
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffff81797d02>] __mutex_lock_slowpath+0xa2/0x140
> PGD 13143b067 PUD 132a52067 PMD 0
> Oops: 0002 [#1] SMP
> Modules linked in: go7007_usb(C-) go7007(C) videobuf2_core bttv videobuf_dma_sg videobuf_core btcx_risc rc_core v4l2_common videodev videobuf2_vmalloc videobuf2_memops tveeprom
> CPU 0
> Pid: 3305, comm: rmmod Tainted: G C 3.9.0-rc1+ #4 To Be Filled By O.E.M. To Be Filled By O.E.M./H55MX-S Series
> RIP: 0010:[<ffffffff81797d02>] [<ffffffff81797d02>] __mutex_lock_slowpath+0xa2/0x140
> RSP: 0018:ffff88013220bd18 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff880132264c80 RCX: 00000000ffffffff
> RDX: ffff88013220bd20 RSI: ffff880130d3ecc0 RDI: ffff880132264c84
> RBP: ffff88013220bd68 R08: 0000000000000000 R09: ffffea0004c5c940
> R10: ffffffff811a08d3 R11: 0000000000027854 R12: ffff880132264c84
> R13: ffff8801325ec410 R14: 00000000ffffffff R15: ffff880132264c88
> Mar 12 01:25:57 Video kernel: [ 128.939419] FS: 00007fb32649d700(0000) GS:ffff880137c00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000000000000 CR3: 0000000130f1d000 CR4: 00000000000007f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process rmmod (pid: 3305, threadinfo ffff88013220a000, task ffff8801325ec410)
> Stack:
> ffff88013220bd48 ffff880132264c88 0000000000000000 ffff880131abf800
> ffff880130d3ecc0 ffff880132264c80 ffff880132264c80 ffff880132264478
> ffff880132264000 0000000000000000 ffff88013220bd88 ffffffff81797c05
> Call Trace:
> [<ffffffff81797c05>] mutex_lock+0x25/0x40
> [<ffffffffa0090051>] go7007_usb_disconnect+0x41/0xb0 [go7007_usb]
> [<ffffffff814b105b>] usb_unbind_interface+0x5b/0x130
> [<ffffffff813f0a71>] __device_release_driver+0x61/0xd0
> [<ffffffff813f1340>] driver_detach+0xb0/0xc0
> [<ffffffff813f0859>] bus_remove_driver+0x79/0xd0
> [<ffffffff813f19da>] driver_unregister+0x5a/0x90
> [<ffffffff814b0d64>] usb_deregister+0x64/0xd0
> [<ffffffffa00914dc>] go7007_usb_driver_exit+0x10/0x12 [go7007_usb]
> [<ffffffff8109370c>] sys_delete_module+0x15c/0x240
> [<ffffffff817a1d12>] system_call_fastpath+0x16/0x1b
> Code: 00 4c 8d 63 04 4c 8d 7b 08 41 be ff ff ff ff 4c 89 e7 e8 92 26 00 00 48 8b 43 10 48 8d 55 b8 4c 89 7d b8 48 89 53 10 48 89 45 c0 <48> 89 10 44 89 f0 4c 89 6d c8 87 03 83 f8 01 75 1f eb 27 0f 1f
> RIP [<ffffffff81797d02>] __mutex_lock_slowpath+0xa2/0x140
> RSP <ffff88013220bd18>
> CR2: 0000000000000000
>
It's clearly a bug, but I'm not sure this is the right approach. If the firmware
can't be loaded, then the probe() should just exit with an error.
I also need to check the use of the 'status' field, I never trust that sort of
thing.
Regards,
Hans
> Signed-off-by: Volokh Konstantin <volokh84@gmail.com>
> ---
> drivers/staging/media/go7007/go7007-usb.c | 25 +++++++++++++------------
> 1 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/media/go7007/go7007-usb.c b/drivers/staging/media/go7007/go7007-usb.c
> index bd23d7d..48ad01f 100644
> --- a/drivers/staging/media/go7007/go7007-usb.c
> +++ b/drivers/staging/media/go7007/go7007-usb.c
> @@ -1316,18 +1316,19 @@ static void go7007_usb_disconnect(struct usb_interface *intf)
> {
> struct go7007 *go = to_go7007(usb_get_intfdata(intf));
>
> - mutex_lock(&go->queue_lock);
> - mutex_lock(&go->serialize_lock);
> -
> - if (go->audio_enabled)
> - go7007_snd_remove(go);
> -
> - go->status = STATUS_SHUTDOWN;
> - v4l2_device_disconnect(&go->v4l2_dev);
> - video_unregister_device(&go->vdev);
> - mutex_unlock(&go->serialize_lock);
> - mutex_unlock(&go->queue_lock);
> -
> + if (go->status == STATUS_ONLINE) {
> + mutex_lock(&go->queue_lock);
> + mutex_lock(&go->serialize_lock);
> +
> + if (go->audio_enabled)
> + go7007_snd_remove(go);
> +
> + go->status = STATUS_SHUTDOWN;
> + v4l2_device_disconnect(&go->v4l2_dev);
> + video_unregister_device(&go->vdev);
> + mutex_unlock(&go->serialize_lock);
> + mutex_unlock(&go->queue_lock);
> + }
> v4l2_device_put(&go->v4l2_dev);
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-03-12 15:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-12 15:09 [PATCH 1/2] hverkuil/go7007: staging: media: go7007: Restore b_frame control Volokh Konstantin
2013-03-12 15:09 ` [PATCH 2/2] hverkuil/go7007: staging: media: go7007: rmmod firmware protection stuff Volokh Konstantin
2013-03-12 15:26 ` Hans Verkuil
2013-03-12 15:24 ` [PATCH 1/2] hverkuil/go7007: staging: media: go7007: Restore b_frame control Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.