Re: [PATCH] rtl28xxu: fix control message flaws
On 10/15/2015 01:11 AM, Daniel Glöckner wrote: On Sat, Oct 10, 2015 at 07:45:30PM +0300, Antti Palosaari wrote: Add lock to prevent concurrent access for control message as control message function uses shared buffer. Without the lock there may be remote control polling which messes the buffer causing IO errors. This patch fixes the Problems I had with my Astrometa stick's I2C bus locking up at the end of each dvbv5-scan run until it is disconnected. There is another source of IO errors in the current driver, though. The delayed work closing the I2C gate to the tuner is often executed after rtl2832_power_ctrl has disabled the PLL. This will cause the USB transfer accessing the gate control register to fail with -EPIPE. I saw that few times too, but it does not cause any other harm than error printing. It went away when canceled that delayed gate closing timer during demod sleep. But that was device which doesn't have slave demod at all, so it does not apply to your case as integrated demod sleep is not called at all. I think some callback which does opposite than "enable_slave_ts()" is needed. Like "disable_slave_ts()" which kills that timer before demod is powered off. regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 69/79] include/uapi/linux/dvb/video.h: remove stdint.h include
Kernel headers should use linux/types.h instead. Signed-off-by: Mikko Rapeli --- include/uapi/linux/dvb/video.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h index d3d14a59..4939256 100644 --- a/include/uapi/linux/dvb/video.h +++ b/include/uapi/linux/dvb/video.h @@ -26,7 +26,6 @@ #include #ifndef __KERNEL__ -#include #include #endif -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Thu Oct 15 04:00:18 CEST 2015 git branch: test git hash: efe98010b80ec4516b2779e1b4e4a8ce16bf89fe gcc version:i686-linux-gcc (GCC) 5.1.0 sparse version: v0.5.0-51-ga53cea2 smatch version: 0.4.1-3153-g7d56ab3 host hardware: x86_64 host os:4.0.0-3.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: OK linux-3.14.9-i686: OK linux-3.15.2-i686: OK linux-3.16.7-i686: OK linux-3.17.8-i686: OK linux-3.18.7-i686: OK linux-3.19-i686: OK linux-4.0-i686: OK linux-4.1.1-i686: OK linux-4.2-i686: OK linux-4.3-rc1-i686: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: OK linux-3.14.9-x86_64: OK linux-3.15.2-x86_64: OK linux-3.16.7-x86_64: OK linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-x86_64: OK linux-4.1.1-x86_64: OK linux-4.2-x86_64: OK linux-4.3-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS smatch: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rtl28xxu: fix control message flaws
On Sat, Oct 10, 2015 at 07:45:30PM +0300, Antti Palosaari wrote: > Add lock to prevent concurrent access for control message as control > message function uses shared buffer. Without the lock there may be > remote control polling which messes the buffer causing IO errors. This patch fixes the Problems I had with my Astrometa stick's I2C bus locking up at the end of each dvbv5-scan run until it is disconnected. There is another source of IO errors in the current driver, though. The delayed work closing the I2C gate to the tuner is often executed after rtl2832_power_ctrl has disabled the PLL. This will cause the USB transfer accessing the gate control register to fail with -EPIPE. Best regards, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
MC Next Gen au0828_usb_disconnect() softlockups
I am seeing softlockups from au0828_usb_disconnect(). I am looking into this problem. Reporting just in case you saw this during your testing and problem is fixed. I am doing USB device removal test when I ran into this. Could this be an issue with the use of spin_lock() - should spin_lock_irq() a better choice in media_device_unregister_entity() Here is the dmesg snippet: [ 1316.127004] R13: 810d0a2e R14: 818005a1 R15: 810d0a2e [ 1316.127008] FS: 7f47d8fd9700() GS:88023ec8() knlGS: [ 1316.127012] CS: 0010 DS: ES: CR0: 8005003b [ 1316.127015] CR2: 7fff1917afa8 CR3: 000222b4b000 CR4: 000407e0 [ 1316.127017] Stack: [ 1316.127020] 88023407b958 813bf83f 88023407b988 810b4d4c [ 1316.127027] 880223329180 880223329180 880223385d58 880223385d78 [ 1316.127034] 88023407b9b8 817fefa9 8161199b 81611b18 [ 1316.127040] Call Trace: [ 1316.127049] [] __delay+0xf/0x20 [ 1316.127056] [] do_raw_spin_lock+0x8c/0x120 [ 1316.127062] [] _raw_spin_lock+0x39/0x40 [ 1316.127070] [] ? media_device_unregister_entity+0x3b/0x130 [ 1316.127075] [] ? media_device_unregister+0x88/0x150 [ 1316.127081] [] media_device_unregister_entity+0x3b/0x130 [ 1316.127087] [] media_device_unregister+0xbe/0x150 [ 1316.127097] [] au0828_unregister_media_device+0x45/0x60 [au0828] [ 1316.127105] [] au0828_usb_v4l2_release+0x6a/0x90 [au0828] [ 1316.127111] [] v4l2_device_release+0x1e/0x30 [ 1316.127116] [] v4l2_device_put+0x25/0x30 [ 1316.127123] [] au0828_usb_disconnect+0xb1/0xd0 [au0828] [ 1316.127129] [] usb_unbind_interface+0x86/0x280 [ 1316.127135] [] ? trace_hardirqs_on+0xd/0x10 [ 1316.127141] [] __device_release_driver+0x96/0x130 [ 1316.127146] [] device_release_driver+0x25/0x40 [ 1316.127150] [] bus_remove_device+0x11c/0x1a0 [ 1316.127157] [] device_del+0x139/0x250 [ 1316.127162] [] ? remove_intf_ep_devs+0x41/0x60 [ 1316.127168] [] usb_disable_device+0x89/0x280 [ 1316.127173] [] usb_disconnect+0x96/0x2b0 [ 1316.127178] [] hub_event+0x696/0x15a0 [ 1316.127185] [] process_one_work+0x1c0/0x4b0 [ 1316.127191] [] ? process_one_work+0x154/0x4b0 [ 1316.127196] [] worker_thread+0x4b/0x440 [ 1316.127202] [] ? rescuer_thread+0x2e0/0x2e0 [ 1316.127207] [] ? rescuer_thread+0x2e0/0x2e0 [ 1316.127212] [] kthread+0xe4/0x100 [ 1316.127219] [] ? kthread_create_on_node+0x220/0x220 [ 1316.127224] [] ret_from_fork+0x3f/0x70 [ 1316.127229] [] ? kthread_create_on_node+0x220/0x220 [ 1316.127232] Code: c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 e5 65 8b 35 80 a8 c4 7e 0f ae f0 0f 31 89 c1 0f ae f0 0f 31 <48> c1 e2 20 89 c0 48 09 c2 89 d0 29 ca 39 fa 73 1c f3 90 65 8b [ 1323.553480] device: '0:40': device_add [ 1323.553532] PM: Adding info for No Bus:0:40 -- Shuah Khan Sr. Linux Kernel Developer Open Source Innovation Group Samsung Research America (Silicon Valley) shua...@osg.samsung.com | (970) 217-8978 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: v4l2-ctrl is unable to set autogain to 0 with gspca/ov534
On Mon, 12 Oct 2015 09:56:08 +0200 Hans Verkuil wrote: > Hi Antonio, > > On 10/07/2015 10:05 AM, Antonio Ospite wrote: [...] > > After a little investigation I figured out some more details: in my use > > case the autogain is a master control in an auto cluster, and switching > > it from auto to manual does not work when using VIDIOC_S_CTRL i.e. when > > calling set_ctrl(). > > > > It works with qv4l2 because it uses VIDIOC_S_EXT_CTRLS. > > > > So the difference is between v4l2-ctrls.c::v4l2_s_ctrl() and > > v4l2-ctrls.c::v4l2_s_ext_ctrls(). > > > > Wrt. auto clusters going from auto to manual the two functions do > > basically this: > > > > > > v4l2_s_ctrl() > > set_ctrl_lock() > > user_to_new() > > set_ctrl() > > update_from_auto_cluster(master) > > try_or_set_cluster() > > cur_to_user() > > > > > > v4l2_s_ext_ctrls() > > try_set_ext_ctrls() > > update_from_auto_cluster(master) > > user_to_new() for each control > > try_or_set_cluster() > > new_to_user() > > > > > > I think the problem is that when update_from_auto_cluster(master) is > > called it overrides the new master control value from userspace by > > calling cur_to_new(). This also happens when calling VIDIOC_S_EXT_CTRLS > > (in try_set_ext_ctrls), but in that case, AFTER the call to > > update_from_auto_cluster(master), the code calls user_to_new() that sets > > back again the correct new value in the control before making the value > > permanent with try_or_set_cluster(). > > > > The regression may have been introduced in > > 5d0360a4f027576e5419d4a7c711c9ca0f1be8ca, in fact by just reverting > > these two interdependent commits: > > > > 7a7f1ab37dc8f66cf0ef10f3d3f1b79ac4bc67fc > > 5d0360a4f027576e5419d4a7c711c9ca0f1be8ca > > > > the problem goes away, so the regression is about user_to_new() not > > being called AFTER update_from_auto_cluster(master) anymore in > > set_ctrl(), as per 5d0360a4f027576e5419d4a7c711c9ca0f1be8ca. > > Excellent analysis! > I think we can make the code paths of v4l2_s_ctrl() and v4l2_s_ext_ctrls() more look alike to ease similar investigations. In v4l2_s_ext_ctrls(): 1. Call user_to_new() before update_from_auto_cluster(master), I am still not 100% sure if this can introduce regressions. 2. Use cur_to_user() instead of new_to_user(), to match the code path of v4l2_s_ctrl(), the effect of using either one or the other should be equivalent right _after_ a call to try_or_set_cluster(), shouldn't it? I'll try to test these changes as time permits, but if anyone can squeeze that in their paid time feel free to anticipate me. [...] > Thanks for looking at this! > FWIW I got interested by this in particular: $ strace v4l2-ctl --set-ctrl=gain_automatic=0 ... ioctl(3, VIDIOC_S_CTRL, {id=V4L2_CID_AUTOGAIN, value=1}) = 0 ^ Ciao ciao, Antonio -- Antonio Ospite http://ao2.it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] media/v4l2-ctrls: fix setting autocluster to manual with VIDIOC_S_CTRL
Since commit 5d0360a4f027576e5419d4a7c711c9ca0f1be8ca it's not possible anymore to set auto clusters from auto to manual using VIDIOC_S_CTRL. For example, setting autogain to manual with gspca/ov534 driver and this sequence of commands does not work: v4l2-ctl --set-ctrl=gain_automatic=1 v4l2-ctl --list-ctrls | grep gain_automatic # The following does not work v4l2-ctl --set-ctrl=gain_automatic=0 v4l2-ctl --list-ctrls | grep gain_automatic Changing the value using VIDIOC_S_EXT_CTRLS (like qv4l2 does) works fine. The apparent cause by looking at the changes in 5d0360a and comparing with the code path for VIDIOC_S_EXT_CTRLS seems to be that the code in v4l2-ctrls.c::set_ctrl() is not calling user_to_new() anymore after calling update_from_auto_cluster(master). However the root cause of the problem is that calling update_from_auto_cluster(master) overrides also the _master_ control state calling cur_to_new() while it was supposed to only update the volatile controls. Calling user_to_new() after update_from_auto_cluster(master) was just masking the original bug by restoring the correct new value of the master control before making the changes permanent. Fix the original bug by making update_from_auto_cluster() not override the new master control value. Signed-off-by: Antonio Ospite Cc: # for v3.17 and up --- Hi, I did check the patch with scripts/checkpatch.pl but it gives an error, I think it's a false positive. Ciao ciao, Antonio drivers/media/v4l2-core/v4l2-ctrls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index b6b7dcc..19fc06e 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -3043,7 +3043,7 @@ static void update_from_auto_cluster(struct v4l2_ctrl *master) { int i; - for (i = 0; i < master->ncontrols; i++) + for (i = 1; i < master->ncontrols; i++) cur_to_new(master->cluster[i]); if (!call_op(master, g_volatile_ctrl)) for (i = 1; i < master->ncontrols; i++) -- 2.6.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv9 06/15] rc: Add HDMI CEC protocol handling
On 10/14/2015 01:09 AM, Russell King - ARM Linux wrote: > On Mon, Oct 12, 2015 at 01:50:47PM +0200, Hans Verkuil wrote: >> On 10/06/2015 08:05 PM, Russell King - ARM Linux wrote: >>> On Mon, Sep 07, 2015 at 03:44:35PM +0200, Hans Verkuil wrote: From: Kamil Debski Add handling of remote control events coming from the HDMI CEC bus. This patch includes a new keymap that maps values found in the CEC messages to the keys pressed and released. Also, a new protocol has been added to the core. Signed-off-by: Kamil Debski Signed-off-by: Hans Verkuil >>> >>> (Added Mauro) >>> >>> Hmm, how is rc-cec supposed to be loaded? >> >> Is CONFIG_RC_MAP enabled in your config? Ran 'depmod -a'? (Sorry, I'm sure >> you've done >> that, just checking...) > > CONFIG_RC_MAP=m > > and yes, if depmod hadn't have been run, modprobing rc-cec would not > have worked - modprobe always looks up in the depmod information to > find out where the module is located, and also to determine any > dependencies. > >> It's optional as I understand it, since you could configure the keytable from >> userspace instead of using this module. >> >> For the record (just tried it), it does load fine on my setup. > > Immediately after boot, I have: > > # lsmod > Module Size Used by > ... > coda 54685 0 > v4l2_mem2mem 14517 1 coda > videobuf2_dma_contig 9478 1 coda > videobuf2_vmalloc 5529 1 coda > videobuf2_memops1888 2 videobuf2_dma_contig,videobuf2_vmalloc > cecd_dw_hdmi3129 0 > # modprobe rc-cec > # lsmod > Module Size Used by > rc_cec 1785 0 > ... > coda 54685 0 > v4l2_mem2mem 14517 1 coda > videobuf2_dma_contig 9478 1 coda > videobuf2_vmalloc 5529 1 coda > videobuf2_memops1888 2 videobuf2_dma_contig,videobuf2_vmalloc > cecd_dw_hdmi3129 0 > > So, rc-cec is perfectly loadable, it just doesn't get loaded at boot. > Manually loading it like this is useless though - I have to unload > cecd_dw_hdmi and then re-load it after rc-cec is loaded for rc-cec to > be seen. At that point, (and with the help of a userspace program) > things start working as expected. Did you compile and install the v4l-utils found here: http://git.linuxtv.org/cgit.cgi/hverkuil/v4l-utils.git/log/?h=cec I think that the rc_cec module is loaded through some udev rules and keytables that are installed by v4l-utils. The standard v4l-utils doesn't know about cec, but my repo above does. To be honest, I don't really understand how it works, but if you haven't installed it yet then try it and see if that solves the problem. >> BTW, I am still on the fence whether using the kernel RC subsystem is >> the right thing to do. There are a number of CEC RC commands that use >> extra parameters that cannot be mapped to the RC API, so you still >> need to handle those manually. > > Even though it is a remote control which is being forwarded for the > most part, but there are operation codes which aren't related to > key presses specified by the standard. I don't think there's anything > wrong with having a RC interface present, but allowing other interfaces > as a possibility is a good thing too - it allows a certain amount of > flexibility. > > For example, with rc-cec loaded and properly bound, I can control at > least rhythmbox within gnome using the TVs remote control with no > modifications - and that happens because the X server passes on the > events it receives via the event device. > > Given the range of media applications, I think that's key - it needs > to at least have the capability to plug into the existing ways of doing > things, even if those ways are not perfect. > >> Perhaps I should split it off into a separate patch and keep it out >> from the initial pull request once we're ready for that. > > I'm biased because it is an enablement feature - it allows CEC to work > out of the box with at least some existing media apps. :) > OK, useful feedback. I am considering putting the RC code under a kernel config option though. So if the RC core is not enabled or you don't want the RC part to be created, then you can opt to disable it. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html