Re: [PATCH/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
Hi Laurent, Thanks for the patch. Did you test this to work in fileio mode? Looks like it should, but would like to make sure. Thanks, Pawel On Thu, Jun 5, 2014 at 9:23 PM, Laurent Pinchart wrote: > When a fatal error occurs that render the device unusable, the only > options for a driver to signal the error condition to userspace is to > set the V4L2_BUF_FLAG_ERROR flag when dequeuing buffers and to return an > error from the buffer prepare handler when queuing buffers. > > The buffer error flag indicates a transient error and can't be used by > applications to detect fatal errors. Returning an error from vb2_qbuf() > is thus the only real indication that a fatal error occurred. However, > this is difficult to handle for multithreaded applications that requeue > buffers from a thread other than the control thread. In particular the > poll() call in the control thread will not notify userspace of the > error. > > This patch adds an explicit mechanism to report fatal errors to > userspace. Applications can call the vb2_queue_error() function to > signal a fatal error. From this moment on, buffer preparation will > return -EIO to userspace, and vb2_poll() will set the POLLERR flag and > return immediately. The error flag is cleared when cancelling the queue, > either at stream off time (through vb2_streamoff) or when releasing the > queue with vb2_queue_release(). > > Signed-off-by: Laurent Pinchart > --- > drivers/media/v4l2-core/videobuf2-core.c | 40 > +--- > include/media/videobuf2-core.h | 3 +++ > 2 files changed, 40 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c > b/drivers/media/v4l2-core/videobuf2-core.c > index fd428e0..c7aa07d 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -1582,6 +1582,11 @@ static int __buf_prepare(struct vb2_buffer *vb, const > struct v4l2_buffer *b) > return -EINVAL; > } > > + if (q->error) { > + dprintk(1, "fatal error occurred on queue\n"); > + return -EIO; > + } > + > vb->state = VB2_BUF_STATE_PREPARING; > vb->v4l2_buf.timestamp.tv_sec = 0; > vb->v4l2_buf.timestamp.tv_usec = 0; > @@ -1877,6 +1882,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, > int nonblocking) > return -EINVAL; > } > > + if (q->error) { > + dprintk(1, "Queue in error state, will not wait for > buffers\n"); > + return -EIO; > + } > + > if (!list_empty(&q->done_list)) { > /* > * Found a buffer that we were waiting for. > @@ -1902,7 +1912,8 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, > int nonblocking) > */ > dprintk(3, "will sleep waiting for buffers\n"); > ret = wait_event_interruptible(q->done_wq, > - !list_empty(&q->done_list) || !q->streaming); > + !list_empty(&q->done_list) || !q->streaming || > + q->error); > > /* > * We need to reevaluate both conditions again after > reacquiring > @@ -2099,6 +2110,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) > q->streaming = 0; > q->start_streaming_called = 0; > q->queued_count = 0; > + q->error = 0; > > /* > * Remove all buffers from videobuf's list... > @@ -2176,6 +2188,27 @@ static int vb2_internal_streamon(struct vb2_queue *q, > enum v4l2_buf_type type) > } > > /** > + * vb2_queue_error() - signal a fatal error on the queue > + * @q: videobuf2 queue > + * > + * Flag that a fatal unrecoverable error has occurred and wake up all > processes > + * waiting on the queue. Polling will now set POLLERR and queuing and > dequeuing > + * buffers will return -EIO. > + * > + * The error flag will be cleared when cancelling the queue, either from > + * vb2_streamoff or vb2_queue_release. Drivers should thus not call this > + * function before starting the stream, otherwise the error flag will remain > set > + * until the queue is released when closing the device node. > + */ > +void vb2_queue_error(struct vb2_queue *q) > +{ > + q->error = 1; > + > + wake_up_all(&q->done_wq); > +} > +EXPORT_SYMBOL_GPL(vb2_queue_error); > + > +/** > * vb2_streamon - start streaming > * @q: videobuf2 queue > * @type: type argument passed from userspace to vidioc_streamon handler > @@ -2533,9 +2566,10 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file > *file, poll_table *wait) > } > > /* > -* There is nothing to wait for if the queue isn't streaming. > +* There is nothing to wait for if the queue isn't streaming or if the > +* error flag i
Re: [PATCH/RFC v2 1/2] v4l: vb2: Don't return POLLERR during transient buffer underruns
On Thu, Jun 5, 2014 at 9:23 PM, Laurent Pinchart wrote: > The V4L2 specification states that > > "When the application did not call VIDIOC_QBUF or VIDIOC_STREAMON yet > the poll() function succeeds, but sets the POLLERR flag in the revents > field." > > The vb2_poll() function sets POLLERR when the queued buffers list is > empty, regardless of whether this is caused by the stream not being > active yet, or by a transient buffer underrun. > > Bring the implementation in line with the specification by returning > POLLERR only when the queue is not streaming. Buffer underruns during > streaming are not treated specially anymore and just result in poll() > blocking until the next event. > > Signed-off-by: Laurent Pinchart > Acked-by: Hans Verkuil Acked-by: Pawel Osciak > --- > drivers/media/v4l2-core/videobuf2-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c > b/drivers/media/v4l2-core/videobuf2-core.c > index 349e659..fd428e0 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -2533,9 +2533,9 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file > *file, poll_table *wait) > } > > /* > -* There is nothing to wait for if no buffers have already been > queued. > +* There is nothing to wait for if the queue isn't streaming. > */ > - if (list_empty(&q->queued_list)) > + if (!vb2_is_streaming(q)) > return res | POLLERR; > > if (list_empty(&q->done_list)) > -- > 1.8.5.5 > -- 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: Fri Jun 6 04:00:31 CEST 2014 git branch: test git hash: 5ea878796f0a1d9649fe43a6a09df53d3915c0ef gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: v0.5.0-11-g38d1124 host hardware: x86_64 host os:3.14-5.slh.3-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: 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.31.14-i686: 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-i686: OK linux-3.13-i686: OK linux-3.14-i686: OK linux-3.15-rc1-i686: OK linux-2.6.31.14-x86_64: 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-x86_64: OK linux-3.13-x86_64: OK linux-3.14-x86_64: OK linux-3.15-rc1-x86_64: OK apps: OK spec-git: OK sparse version: v0.5.0-11-g38d1124 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.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
Leadtek WinFast DTV Dongle Dual
Hi All, Recently purchased one of these (0413:6a05), the instructions @ http://www.linuxtv.org/wiki/index.php/Leadtek_WinFast_DTV_Dual_Dongle appear to be wrong. Running 3.14.5 I didn't need to patch drivers/media/usb/dvb-usb-v2/af9035.c, however the tuner wouldn't work (it would be detected, but not able to tune) After a lot of stuffing around, I ended up patching it913x.c instead and everything is working well, here is my dmesg output: [3.981516] usb 3-2: new high-speed USB device number 2 using xhci_hcd [4.149345] usb 3-2: New USB device found, idVendor=0413, idProduct=6a05 [4.149355] usb 3-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [4.149361] usb 3-2: Product: WinFast DTV Dongle Dual [4.149366] usb 3-2: Manufacturer: Leadtek [ 380.529378] it913x: Chip Version=02 Chip Type=9135 [ 380.530913] it913x: Remote propriety (raw) modeit913x: Dual mode=3 Tuner Type=38 [ 380.531310] it913x: Unknown tuner ID applying default 0x60it913x: Chip Version=02 Chip Type=9135 [ 380.637528] usb 3-2: dvb_usb_v2: found a 'Leadtek WinFast DTV Dongle Dual' in cold state [ 380.638195] usb 3-2: dvb_usb_v2: downloading firmware from file 'dvb-usb-it9135-02.fw' [ 380.638598] it913x: FRM Starting Firmware Download [ 381.130961] it913x: FRM Firmware Download Completed - Resetting Deviceit913x: Chip Version=02 Chip Type=9135 [ 381.164104] it913x: Firmware Version 52887808<6> [ 381.226204] usb 3-2: dvb_usb_v2: found a 'Leadtek WinFast DTV Dongle Dual' in warm state [ 381.226300] usb 3-2: dvb_usb_v2: will pass the complete MPEG2 transport stream to the software demuxer [ 381.226519] DVB: registering new adapter (Leadtek WinFast DTV Dongle Dual) [ 381.231125] it913x-fe: ADF table value :00 [ 381.234465] it913x-fe: Crystal Frequency :1200 Adc Frequency :2025 ADC X2: 01 [ 381.262283] it913x-fe: Tuner LNA type :60 [ 381.493014] usb 3-2: DVB: registering adapter 0 frontend 0 (Leadtek WinFast DTV Dongle Dual_1)... [ 381.493251] usb 3-2: dvb_usb_v2: will pass the complete MPEG2 transport stream to the software demuxer [ 381.493446] DVB: registering new adapter (Leadtek WinFast DTV Dongle Dual) [ 381.494234] it913x-fe: ADF table value :00 [ 381.519335] it913x-fe: Crystal Frequency :1200 Adc Frequency :2025 ADC X2: 01 [ 381.750290] it913x-fe: Tuner LNA type :60 [ 382.280671] usb 3-2: DVB: registering adapter 1 frontend 0 (Leadtek WinFast DTV Dongle Dual_2)... [ 382.304097] IR keymap rc-it913x-v2 not found [ 382.304282] input: Leadtek WinFast DTV Dongle Dual as /devices/pci:00/:00:14.0/usb3/3-2/rc/rc0/input9 [ 382.304443] rc0: Leadtek WinFast DTV Dongle Dual as /devices/pci:00/:00:14.0/usb3/3-2/rc/rc0 [ 382.304453] usb 3-2: dvb_usb_v2: schedule remote query interval to 250 msecs [ 382.304462] usb 3-2: dvb_usb_v2: 'Leadtek WinFast DTV Dongle Dual' successfully initialized and connected [ 382.304551] usbcore: registered new interface driver dvb_usb_it913x My kernel config: CONFIG_DVB_USB_V2=m CONFIG_DVB_USB_IT913X=m CONFIG_DVB_IT913X_FE=m Do you want the output when it was using the af9035 driver? Regards David -- 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 0/5] randconfig build fixes for staging
Hi Greg, Here are a couple of simple fixes from my backlog of ARM randconfig bugs. Nothing urgent, they can probably all go into next for 3.17 after the merge window, but see for yourself. Arnd Arnd Bergmann (5): staging: lirc: remove sa1100 support staging/iio: IIO_SIMPLE_DUMMY_BUFFER neds IIO_BUFFER staging: sn9c102 depends on USB staging: wlags49_h2: avoid PROFILE_ALL_BRANCHES warnings staging: rtl8712, rtl8712: avoid lots of build warnings drivers/staging/iio/Kconfig| 9 +- drivers/staging/media/lirc/lirc_sir.c | 301 + drivers/staging/media/sn9c102/Kconfig | 2 +- drivers/staging/rtl8192u/ieee80211/ieee80211.h | 10 +- drivers/staging/rtl8712/ieee80211.h| 4 +- drivers/staging/wlags49_h2/wl_internal.h | 4 +- 6 files changed, 17 insertions(+), 313 deletions(-) -- 1.8.3.2 Cc: Florian Schilhabel Cc: Henk de Groot Cc: Jarod Wilson Cc: Jonathan Cameron Cc: Larry Finger Cc: Luca Risolia Cc: Mauro Carvalho Chehab Cc: Tuomas Tynkkynen Cc: linux-...@vger.kernel.org Cc: linux-media@vger.kernel.org Cc: linux-...@vger.kernel.org -- 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 3/5] staging: sn9c102 depends on USB
If the USB code is a loadable module, this driver cannot be built-in. This adds an explicit dependency on CONFIG_USB so that Kconfig can force sn9c102 to be a module in this case. Signed-off-by: Arnd Bergmann Cc: Luca Risolia Cc: Mauro Carvalho Chehab Cc: linux-media@vger.kernel.org Cc: linux-...@vger.kernel.org --- drivers/staging/media/sn9c102/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/sn9c102/Kconfig b/drivers/staging/media/sn9c102/Kconfig index c9aba59..10f586b 100644 --- a/drivers/staging/media/sn9c102/Kconfig +++ b/drivers/staging/media/sn9c102/Kconfig @@ -1,6 +1,6 @@ config USB_SN9C102 tristate "USB SN9C1xx PC Camera Controller support (DEPRECATED)" - depends on VIDEO_V4L2 && MEDIA_USB_SUPPORT + depends on VIDEO_V4L2 && MEDIA_USB_SUPPORT && USB ---help--- This driver is DEPRECATED, please use the gspca sonixb and sonixj modules instead. -- 1.8.3.2 -- 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 1/5] staging: lirc: remove sa1100 support
The LIRC support for sa1100 appears to have never worked because it relies on header files that have never been present in git history. Actually trying to build the driver on an ARM sa1100 kernel fails, so let's just remove the broken support. Signed-off-by: Arnd Bergmann Cc: Jarod Wilson Cc: Mauro Carvalho Chehab Cc: Tuomas Tynkkynen Cc: linux-media@vger.kernel.org --- drivers/staging/media/lirc/lirc_sir.c | 301 +- 1 file changed, 2 insertions(+), 299 deletions(-) diff --git a/drivers/staging/media/lirc/lirc_sir.c b/drivers/staging/media/lirc/lirc_sir.c index e31cbb8..79da3ad 100644 --- a/drivers/staging/media/lirc/lirc_sir.c +++ b/drivers/staging/media/lirc/lirc_sir.c @@ -55,13 +55,6 @@ #include #include #include -#ifdef LIRC_ON_SA1100 -#include -#ifdef CONFIG_SA1100_COLLIE -#include -#include -#endif -#endif #include @@ -94,35 +87,6 @@ static void init_act200(void); static void init_act220(void); #endif -/*** SA1100 ***/ -#ifdef LIRC_ON_SA1100 -struct sa1100_ser2_registers { - /* HSSP control register */ - unsigned char hscr0; - /* UART registers */ - unsigned char utcr0; - unsigned char utcr1; - unsigned char utcr2; - unsigned char utcr3; - unsigned char utcr4; - unsigned char utdr; - unsigned char utsr0; - unsigned char utsr1; -} sr; - -static int irq = IRQ_Ser2ICP; - -#define LIRC_ON_SA1100_TRANSMITTER_LATENCY 0 - -/* pulse/space ratio of 50/50 */ -static unsigned long pulse_width = (13-LIRC_ON_SA1100_TRANSMITTER_LATENCY); -/* 100/freq-pulse_width */ -static unsigned long space_width = (13-LIRC_ON_SA1100_TRANSMITTER_LATENCY); -static unsigned int freq = 38000; /* modulation frequency */ -static unsigned int duty_cycle = 50; /* duty cycle of 50% */ - -#endif - #define RBUF_LEN 1024 #define WBUF_LEN 1024 @@ -205,17 +169,6 @@ static void drop_hardware(void); static int init_port(void); static void drop_port(void); -#ifdef LIRC_ON_SA1100 -static void on(void) -{ - PPSR |= PPC_TXD2; -} - -static void off(void) -{ - PPSR &= ~PPC_TXD2; -} -#else static inline unsigned int sinp(int offset) { return inb(io + offset); @@ -225,7 +178,6 @@ static inline void soutp(int offset, int value) { outb(value, io + offset); } -#endif #ifndef MAX_UDELAY_MS #define MAX_UDELAY_US 5000 @@ -305,10 +257,6 @@ static ssize_t lirc_write(struct file *file, const char __user *buf, size_t n, if (IS_ERR(tx_buf)) return PTR_ERR(tx_buf); i = 0; -#ifdef LIRC_ON_SA1100 - /* disable receiver */ - Ser2UTCR3 = 0; -#endif local_irq_save(flags); while (1) { if (i >= count) @@ -323,15 +271,6 @@ static ssize_t lirc_write(struct file *file, const char __user *buf, size_t n, i++; } local_irq_restore(flags); -#ifdef LIRC_ON_SA1100 - off(); - udelay(1000); /* wait 1ms for IR diode to recover */ - Ser2UTCR3 = 0; - /* clear status register to prevent unwanted interrupts */ - Ser2UTSR0 &= (UTSR0_RID | UTSR0_RBB | UTSR0_REB); - /* enable receiver */ - Ser2UTCR3 = UTCR3_RXE|UTCR3_RIE; -#endif kfree(tx_buf); return count; } @@ -341,25 +280,12 @@ static long lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) u32 __user *uptr = (u32 __user *)arg; int retval = 0; u32 value = 0; -#ifdef LIRC_ON_SA1100 - - if (cmd == LIRC_GET_FEATURES) - value = LIRC_CAN_SEND_PULSE | - LIRC_CAN_SET_SEND_DUTY_CYCLE | - LIRC_CAN_SET_SEND_CARRIER | - LIRC_CAN_REC_MODE2; - else if (cmd == LIRC_GET_SEND_MODE) - value = LIRC_MODE_PULSE; - else if (cmd == LIRC_GET_REC_MODE) - value = LIRC_MODE_MODE2; -#else if (cmd == LIRC_GET_FEATURES) value = LIRC_CAN_SEND_PULSE | LIRC_CAN_REC_MODE2; else if (cmd == LIRC_GET_SEND_MODE) value = LIRC_MODE_PULSE; else if (cmd == LIRC_GET_REC_MODE) value = LIRC_MODE_MODE2; -#endif switch (cmd) { case LIRC_GET_FEATURES: @@ -372,37 +298,6 @@ static long lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) case LIRC_SET_REC_MODE: retval = get_user(value, uptr); break; -#ifdef LIRC_ON_SA1100 - case LIRC_SET_SEND_DUTY_CYCLE: - retval = get_user(value, uptr); - if (retval) - return retval; - if (value <= 0 || value > 100) - return -EINVAL; - /* (value/100)*(100/freq) */ - duty_cycle = value; - pulse_width = (unsigned long) duty_cycle*1/freq; - space_width = (unsigned long) 100L/freq-pulse_width; - if (pulse_width >= LIRC_ON_SA1100_TRANS
Re: [PATCH/RFC 1/2] v4l2grab: Add threaded producer/consumer option
Em Thu, 05 Jun 2014 12:31:23 -0300 Thiago Santos escreveu: > Adds options to allow the buffer dqbuf to happen on one thread while > the qbuf happens on another. This is useful to test concurrency access to > the v4l2 features. To enable this, 3 new options were added: > > t: enable threaded mode (off by default and will use the loop) > b: enable blocking io mode (off by default > s: how much the consumer thread will sleep after reading a buffer, this is to >simulate the time that it takes to process a buffer in a real application >(in ms) This is not a full review. Just a comment: as the possible values for -t and -b are just 0 or 1, why to require an argument? IMHO, the better would be to keep thread disabled by default, and use -t to enable it, and do the same for -b. To make it clearer when this is called with --help, we can even name the long option as "--thread-enable" and "--blockmode-enable". Regards, Mauro > > For example, you can simulate an application that takes 1s to process a buffer > with: > > v4l2grab -t 1 -b 1 -s 1000 > > Signed-off-by: Thiago Santos > --- > contrib/test/Makefile.am | 2 +- > contrib/test/v4l2grab.c | 265 > +++ > 2 files changed, 223 insertions(+), 44 deletions(-) > > diff --git a/contrib/test/Makefile.am b/contrib/test/Makefile.am > index 80c7665..c2e3860 100644 > --- a/contrib/test/Makefile.am > +++ b/contrib/test/Makefile.am > @@ -25,7 +25,7 @@ pixfmt_test_CFLAGS = $(X11_CFLAGS) > pixfmt_test_LDFLAGS = $(X11_LIBS) > > v4l2grab_SOURCES = v4l2grab.c > -v4l2grab_LDADD = ../../lib/libv4l2/libv4l2.la > ../../lib/libv4lconvert/libv4lconvert.la > +v4l2grab_LDADD = ../../lib/libv4l2/libv4l2.la > ../../lib/libv4lconvert/libv4lconvert.la -lpthread > > v4l2gl_SOURCES = v4l2gl.c > v4l2gl_LDFLAGS = $(X11_LIBS) $(GL_LIBS) $(GLU_LIBS) > diff --git a/contrib/test/v4l2grab.c b/contrib/test/v4l2grab.c > index 674cbe7..2fa5bb8 100644 > --- a/contrib/test/v4l2grab.c > +++ b/contrib/test/v4l2grab.c > @@ -24,8 +24,10 @@ > #include > #include "../../lib/include/libv4l2.h" > #include > +#include > > -#define CLEAR(x) memset(&(x), 0, sizeof(x)) > +#define CLEAR_P(x,s) memset((x), 0, s) > +#define CLEAR(x) CLEAR_P(&(x), sizeof(x)) > > struct buffer { > void *start; > @@ -46,22 +48,206 @@ static void xioctl(int fh, unsigned long int request, > void *arg) > } > } > > +/* Used by the multi thread capture version */ > +struct buffer_queue { > + struct v4l2_buffer *buffers; > + int buffers_size; > + > + int read_pos; > + int write_pos; > + int n_frames; > + > + int fd; > + > + pthread_mutex_t mutex; > + pthread_cond_t buffer_cond; > +}; > + > +/* Gets a buffer and puts it in the buffers list at write position, then > + * notifies the consumer that a new buffer is ready to be used */ > +static void *produce_buffer (void * p) > +{ > + struct buffer_queue *bq; > + fd_set fds; > + struct timeval tv; > + int i; > + struct v4l2_buffer *buf; > + int r; > + > + bq = p; > + > + for (i = 0; i < bq->n_frames; i++) { > + printf ("Prod: %d\n", i); > + buf = &bq->buffers[bq->write_pos % bq->buffers_size]; > + do { > + FD_ZERO(&fds); > + FD_SET(bq->fd, &fds); > + > + /* Timeout. */ > + tv.tv_sec = 2; > + tv.tv_usec = 0; > + > + r = select(bq->fd + 1, &fds, NULL, NULL, &tv); > + } while ((r == -1 && (errno == EINTR))); > + if (r == -1) { > + perror("select"); > + pthread_exit (NULL); > + return NULL; > + } > + > + CLEAR_P(buf, sizeof(struct v4l2_buffer)); > + buf->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; > + buf->memory = V4L2_MEMORY_MMAP; > + xioctl(bq->fd, VIDIOC_DQBUF, buf); > + > + pthread_mutex_lock (&bq->mutex); > + bq->write_pos++; > + printf ("Prod: %d (done)\n", i); > + pthread_cond_signal (&bq->buffer_cond); > + pthread_mutex_unlock (&bq->mutex); > + > + } > + > + pthread_exit (NULL); > +} > + > +/* will create a separate thread that will produce the buffers and put > + * into a circular array while this same thread will get the buffers from > + * this array and 'render' them */ > +static int capture_threads (int fd, struct buffer *buffers, int bufpool_size, > + struct v4l2_format fmt, int n_frames, > + char *out_dir, int sleep_ms) > +{ > + struct v4l2_buffer buf; > + unsigned inti; > + struct buffer_queue buf_queue; > + pthread_t producer
Re: [GIT PULL FOR v3.16] Migrate the OMAP3 ISP driver to videobuf2
Hi Mauro, On Wed, Jun 04, 2014 at 08:53:10PM -0300, Mauro Carvalho Chehab wrote: > Hi Joerg, > > I have one topic branch here that depends on your branch: > git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git arm/omap > > Did you send already a pull request for Linus? If not, when are you planning > to do? > > My intention is to send my pull request after yours ;) Just sent my pull-request. In the future you can also let me know earlier and I can give you a topic-branch then from my tree to just pull into yours to avoid those dependencies :-) Joerg -- 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/RFC 1/2] v4l2grab: Add threaded producer/consumer option
Adds options to allow the buffer dqbuf to happen on one thread while the qbuf happens on another. This is useful to test concurrency access to the v4l2 features. To enable this, 3 new options were added: t: enable threaded mode (off by default and will use the loop) b: enable blocking io mode (off by default s: how much the consumer thread will sleep after reading a buffer, this is to simulate the time that it takes to process a buffer in a real application (in ms) For example, you can simulate an application that takes 1s to process a buffer with: v4l2grab -t 1 -b 1 -s 1000 Signed-off-by: Thiago Santos --- contrib/test/Makefile.am | 2 +- contrib/test/v4l2grab.c | 265 +++ 2 files changed, 223 insertions(+), 44 deletions(-) diff --git a/contrib/test/Makefile.am b/contrib/test/Makefile.am index 80c7665..c2e3860 100644 --- a/contrib/test/Makefile.am +++ b/contrib/test/Makefile.am @@ -25,7 +25,7 @@ pixfmt_test_CFLAGS = $(X11_CFLAGS) pixfmt_test_LDFLAGS = $(X11_LIBS) v4l2grab_SOURCES = v4l2grab.c -v4l2grab_LDADD = ../../lib/libv4l2/libv4l2.la ../../lib/libv4lconvert/libv4lconvert.la +v4l2grab_LDADD = ../../lib/libv4l2/libv4l2.la ../../lib/libv4lconvert/libv4lconvert.la -lpthread v4l2gl_SOURCES = v4l2gl.c v4l2gl_LDFLAGS = $(X11_LIBS) $(GL_LIBS) $(GLU_LIBS) diff --git a/contrib/test/v4l2grab.c b/contrib/test/v4l2grab.c index 674cbe7..2fa5bb8 100644 --- a/contrib/test/v4l2grab.c +++ b/contrib/test/v4l2grab.c @@ -24,8 +24,10 @@ #include #include "../../lib/include/libv4l2.h" #include +#include -#define CLEAR(x) memset(&(x), 0, sizeof(x)) +#define CLEAR_P(x,s) memset((x), 0, s) +#define CLEAR(x) CLEAR_P(&(x), sizeof(x)) struct buffer { void *start; @@ -46,22 +48,206 @@ static void xioctl(int fh, unsigned long int request, void *arg) } } +/* Used by the multi thread capture version */ +struct buffer_queue { + struct v4l2_buffer *buffers; + int buffers_size; + + int read_pos; + int write_pos; + int n_frames; + + int fd; + + pthread_mutex_t mutex; + pthread_cond_t buffer_cond; +}; + +/* Gets a buffer and puts it in the buffers list at write position, then + * notifies the consumer that a new buffer is ready to be used */ +static void *produce_buffer (void * p) +{ + struct buffer_queue *bq; + fd_set fds; + struct timeval tv; + int i; + struct v4l2_buffer *buf; + int r; + + bq = p; + + for (i = 0; i < bq->n_frames; i++) { + printf ("Prod: %d\n", i); + buf = &bq->buffers[bq->write_pos % bq->buffers_size]; + do { + FD_ZERO(&fds); + FD_SET(bq->fd, &fds); + + /* Timeout. */ + tv.tv_sec = 2; + tv.tv_usec = 0; + + r = select(bq->fd + 1, &fds, NULL, NULL, &tv); + } while ((r == -1 && (errno == EINTR))); + if (r == -1) { + perror("select"); + pthread_exit (NULL); + return NULL; + } + + CLEAR_P(buf, sizeof(struct v4l2_buffer)); + buf->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; + buf->memory = V4L2_MEMORY_MMAP; + xioctl(bq->fd, VIDIOC_DQBUF, buf); + + pthread_mutex_lock (&bq->mutex); + bq->write_pos++; + printf ("Prod: %d (done)\n", i); + pthread_cond_signal (&bq->buffer_cond); + pthread_mutex_unlock (&bq->mutex); + + } + + pthread_exit (NULL); +} + +/* will create a separate thread that will produce the buffers and put + * into a circular array while this same thread will get the buffers from + * this array and 'render' them */ +static int capture_threads (int fd, struct buffer *buffers, int bufpool_size, + struct v4l2_format fmt, int n_frames, + char *out_dir, int sleep_ms) +{ + struct v4l2_buffer buf; + unsigned inti; + struct buffer_queue buf_queue; + pthread_t producer; + charout_name[25 + strlen(out_dir)]; + FILE*fout; + struct timespec sleeptime; + + if (sleep_ms) { + sleeptime.tv_sec = sleep_ms / 1000; + sleeptime.tv_nsec = (sleep_ms % 1000) * 100; + } + + buf_queue.buffers_size = bufpool_size * 2; + buf_queue.buffers = calloc(buf_queue.buffers_size, + sizeof(struct v4l2_buffer)); + buf_queue.fd = fd; + buf_queue.read_pos = 0; + buf_queue.write_pos = 0; + buf_queue.n_f
[PATCH/RFC 0/2] libv4l2: fix deadlock when DQBUF in block mode
This patchset modifies v4l2grab to allow using 2 threads (one for qbuf and another for dqbuf) to simulate multithreaded v4l2 usage. This is done to show a issue when using libv4l2 in blocking mode, if a DQBUF is issued when there are no buffers available it will block waiting for one but, as it blocks holding the stream_lock, a QBUF will never happen and we have a deadlock. Thiago Santos (2): v4l2grab: Add threaded producer/consumer option libv4l2: release the lock before doing a DQBUF contrib/test/Makefile.am | 2 +- contrib/test/v4l2grab.c | 265 +++ lib/libv4l2/libv4l2.c| 2 + 3 files changed, 225 insertions(+), 44 deletions(-) -- 2.0.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
[PATCH/RFC 2/2] libv4l2: release the lock before doing a DQBUF
In blocking mode, if there are no buffers available the DQBUF will block waiting for a QBUF to be called but it will block holding the streaming lock which will prevent any QBUF from happening, causing a deadlock. Can be tested with: v4l2grab -t 1 -b 1 -s 2000 Signed-off-by: Thiago Santos --- lib/libv4l2/libv4l2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c index c4d69f7..5589fe0 100644 --- a/lib/libv4l2/libv4l2.c +++ b/lib/libv4l2/libv4l2.c @@ -290,9 +290,11 @@ static int v4l2_dequeue_and_convert(int index, struct v4l2_buffer *buf, return result; do { + pthread_mutex_unlock(&devices[index].stream_lock); result = devices[index].dev_ops->ioctl( devices[index].dev_ops_priv, devices[index].fd, VIDIOC_DQBUF, buf); + pthread_mutex_lock(&devices[index].stream_lock); if (result) { if (errno != EAGAIN) { int saved_err = errno; -- 2.0.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
[GIT PULL for 3.16-rc1] updates and DT support for adv7604
Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media topic/adv76xx For adv7604 driver updates, including DT support. Thanks! Mauro The following changes since commit e5e749dfa8606343fd7956868038bdde2e656ec1: [media] adv7604: Add missing include to linux/types.h (2014-05-25 12:48:47 -0300) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media topic/adv76xx for you to fetch changes up to 1b5ab8755ec7b6ac83bf8d09c9f908d94e36b9a4: [media] adv7604: Add LLC polarity configuration (2014-05-25 13:10:16 -0300) Lars-Peter Clausen (3): [media] adv7604: Add support for asynchronous probing [media] adv7604: Don't put info string arrays on the stack [media] adv7604: Add adv7611 support Laurent Pinchart (18): [media] adv7604: Add 16-bit read functions for CP and HDMI [media] adv7604: Cache register contents when reading multiple bits [media] adv7604: Remove subdev control handlers [media] adv7604: Add sink pads [media] adv7604: Make output format configurable through pad format operations [media] adv7604: Add pad-level DV timings support [media] adv7604: Remove deprecated video-level DV timings operations [media] v4l: subdev: Remove deprecated video-level DV timings operations [media] adv7604: Inline the to_sd function [media] adv7604: Store I2C addresses and clients in arrays [media] adv7604: Replace *_and_or() functions with *_clr_set() [media] adv7604: Sort headers alphabetically [media] adv7604: Support hot-plug detect control through a GPIO [media] adv7604: Specify the default input through platform data [media] adv7604: Add DT support [media] adv7604: Add endpoint properties to DT bindings [media] adv7604: Set HPD GPIO direction to output [media] adv7604: Add LLC polarity configuration .../devicetree/bindings/media/i2c/adv7604.txt | 70 + drivers/media/i2c/adv7604.c| 1464 ++-- include/media/adv7604.h| 122 +- include/media/v4l2-subdev.h|4 - 4 files changed, 1160 insertions(+), 500 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/adv7604.txt -- 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
[GIT PULL for 3.16-rc1] add DT support for vsp1
Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media topic/vsp1 for Device Tree support for the VSP1 driver. Thanks! Mauro - The following changes since commit ce9c22443e77594531be84ba8d523f4148ba09fe: [media] vb2: fix compiler warning (2014-04-23 10:13:57 -0300) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media topic/vsp1 for you to fetch changes up to 0b82fb95d9edf7bdfc6486c67a42dbf9b1e97765: [media] v4l: vsp1: Add DT support (2014-04-23 10:21:58 -0300) Laurent Pinchart (6): [media] v4l: vsp1: Remove unexisting rt clocks [media] v4l: vsp1: uds: Enable scaling of alpha layer [media] v4l: vsp1: Support multi-input entities [media] v4l: vsp1: Add BRU support [media] v4l: vsp1: Add DT bindings documentation [media] v4l: vsp1: Add DT support .../devicetree/bindings/media/renesas,vsp1.txt | 43 +++ drivers/media/platform/vsp1/Makefile | 2 +- drivers/media/platform/vsp1/vsp1.h | 3 +- drivers/media/platform/vsp1/vsp1_bru.c | 395 + drivers/media/platform/vsp1/vsp1_bru.h | 39 ++ drivers/media/platform/vsp1/vsp1_drv.c | 101 +++--- drivers/media/platform/vsp1/vsp1_entity.c | 57 +-- drivers/media/platform/vsp1/vsp1_entity.h | 24 +- drivers/media/platform/vsp1/vsp1_hsit.c| 7 +- drivers/media/platform/vsp1/vsp1_lif.c | 1 - drivers/media/platform/vsp1/vsp1_lut.c | 1 - drivers/media/platform/vsp1/vsp1_regs.h| 98 + drivers/media/platform/vsp1/vsp1_rpf.c | 7 +- drivers/media/platform/vsp1/vsp1_rwpf.h| 4 + drivers/media/platform/vsp1/vsp1_sru.c | 1 - drivers/media/platform/vsp1/vsp1_uds.c | 4 +- drivers/media/platform/vsp1/vsp1_video.c | 26 +- drivers/media/platform/vsp1/vsp1_video.h | 1 + drivers/media/platform/vsp1/vsp1_wpf.c | 13 +- 19 files changed, 733 insertions(+), 94 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/renesas,vsp1.txt create mode 100644 drivers/media/platform/vsp1/vsp1_bru.c create mode 100644 drivers/media/platform/vsp1/vsp1_bru.h -- 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: [GIT PULL FOR v3.16] uvcvideo fixes
Hi William, On Thursday 05 June 2014 10:12:38 William Manley wrote: > On 12/05/14 23:43, Laurent Pinchart wrote: > > On Friday 09 May 2014 14:33:57 William Manley wrote: > >> Hi Laurent > >> > >> Any chance of my patch fixing manual exposure mode for the Logitech > >> C920[1] going in for 3.16? > > > > I know I've been largely unresponsive regarding this patch, and I'm really > > ashamed for that. > > > > I would have liked to get feedback from Logitech on the issue, but they've > > been mostly silent so far. I'll thus apply your patch even though I'm > > still bothered by the issue, we can always revisit it later if needed. > > Now the 3.16 merge window is open can you confirm whether this is going > in. I've not seen it appear in linus' tree yet, but I'm not sure what > the process is. The patch had slipped through the cracks, I've just asked Mauro to push it to v3.16. I'm sorry for the delay, it's my fault :-/ -- Regards, Laurent Pinchart -- 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/RFC v2 0/2] vb2: Report POLLERR for fatal errors only
Hello, This patch set modifies the vb2 implementation of the poll() operation to set the POLLERR flag for fatal errors only. The rationale and implementation details are explained in the individual commit messages. Changes since to v1: - Rebased on top of the latest media tree master branch - Fixed typos Laurent Pinchart (2): v4l: vb2: Don't return POLLERR during transient buffer underruns v4l: vb2: Add fatal error condition flag drivers/media/v4l2-core/videobuf2-core.c | 40 +--- include/media/videobuf2-core.h | 3 +++ 2 files changed, 40 insertions(+), 3 deletions(-) -- Regards, Laurent Pinchart -- 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/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
When a fatal error occurs that render the device unusable, the only options for a driver to signal the error condition to userspace is to set the V4L2_BUF_FLAG_ERROR flag when dequeuing buffers and to return an error from the buffer prepare handler when queuing buffers. The buffer error flag indicates a transient error and can't be used by applications to detect fatal errors. Returning an error from vb2_qbuf() is thus the only real indication that a fatal error occurred. However, this is difficult to handle for multithreaded applications that requeue buffers from a thread other than the control thread. In particular the poll() call in the control thread will not notify userspace of the error. This patch adds an explicit mechanism to report fatal errors to userspace. Applications can call the vb2_queue_error() function to signal a fatal error. From this moment on, buffer preparation will return -EIO to userspace, and vb2_poll() will set the POLLERR flag and return immediately. The error flag is cleared when cancelling the queue, either at stream off time (through vb2_streamoff) or when releasing the queue with vb2_queue_release(). Signed-off-by: Laurent Pinchart --- drivers/media/v4l2-core/videobuf2-core.c | 40 +--- include/media/videobuf2-core.h | 3 +++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index fd428e0..c7aa07d 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1582,6 +1582,11 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b) return -EINVAL; } + if (q->error) { + dprintk(1, "fatal error occurred on queue\n"); + return -EIO; + } + vb->state = VB2_BUF_STATE_PREPARING; vb->v4l2_buf.timestamp.tv_sec = 0; vb->v4l2_buf.timestamp.tv_usec = 0; @@ -1877,6 +1882,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking) return -EINVAL; } + if (q->error) { + dprintk(1, "Queue in error state, will not wait for buffers\n"); + return -EIO; + } + if (!list_empty(&q->done_list)) { /* * Found a buffer that we were waiting for. @@ -1902,7 +1912,8 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking) */ dprintk(3, "will sleep waiting for buffers\n"); ret = wait_event_interruptible(q->done_wq, - !list_empty(&q->done_list) || !q->streaming); + !list_empty(&q->done_list) || !q->streaming || + q->error); /* * We need to reevaluate both conditions again after reacquiring @@ -2099,6 +2110,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) q->streaming = 0; q->start_streaming_called = 0; q->queued_count = 0; + q->error = 0; /* * Remove all buffers from videobuf's list... @@ -2176,6 +2188,27 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type) } /** + * vb2_queue_error() - signal a fatal error on the queue + * @q: videobuf2 queue + * + * Flag that a fatal unrecoverable error has occurred and wake up all processes + * waiting on the queue. Polling will now set POLLERR and queuing and dequeuing + * buffers will return -EIO. + * + * The error flag will be cleared when cancelling the queue, either from + * vb2_streamoff or vb2_queue_release. Drivers should thus not call this + * function before starting the stream, otherwise the error flag will remain set + * until the queue is released when closing the device node. + */ +void vb2_queue_error(struct vb2_queue *q) +{ + q->error = 1; + + wake_up_all(&q->done_wq); +} +EXPORT_SYMBOL_GPL(vb2_queue_error); + +/** * vb2_streamon - start streaming * @q: videobuf2 queue * @type: type argument passed from userspace to vidioc_streamon handler @@ -2533,9 +2566,10 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) } /* -* There is nothing to wait for if the queue isn't streaming. +* There is nothing to wait for if the queue isn't streaming or if the +* error flag is set. */ - if (!vb2_is_streaming(q)) + if (!vb2_is_streaming(q) || q->error) return res | POLLERR; if (list_empty(&q->done_list)) diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index bca25dc..5a67f31 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -375,6 +375,7 @@ struct v4l2_fh; * @streaming: current streamin
[PATCH/RFC v2 1/2] v4l: vb2: Don't return POLLERR during transient buffer underruns
The V4L2 specification states that "When the application did not call VIDIOC_QBUF or VIDIOC_STREAMON yet the poll() function succeeds, but sets the POLLERR flag in the revents field." The vb2_poll() function sets POLLERR when the queued buffers list is empty, regardless of whether this is caused by the stream not being active yet, or by a transient buffer underrun. Bring the implementation in line with the specification by returning POLLERR only when the queue is not streaming. Buffer underruns during streaming are not treated specially anymore and just result in poll() blocking until the next event. Signed-off-by: Laurent Pinchart Acked-by: Hans Verkuil --- drivers/media/v4l2-core/videobuf2-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 349e659..fd428e0 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2533,9 +2533,9 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) } /* -* There is nothing to wait for if no buffers have already been queued. +* There is nothing to wait for if the queue isn't streaming. */ - if (list_empty(&q->queued_list)) + if (!vb2_is_streaming(q)) return res | POLLERR; if (list_empty(&q->done_list)) -- 1.8.5.5 -- 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: [RFC ATTN] Cropping, composing, scaling and S_FMT
On 06/05/14 13:06, Mauro Carvalho Chehab wrote: > Em Thu, 05 Jun 2014 09:20:33 +0200 > Hans Verkuil escreveu: > >> On 06/04/2014 08:40 PM, Mauro Carvalho Chehab wrote: >>> Em Mon, 02 Jun 2014 10:28:18 +0200 >>> Hans Verkuil escreveu: >>> During the media mini-summit I went through all 8 combinations of cropping, composing and scaling (i.e. none of these features is present, or only cropping, only composing, etc.). In particular I showed what I thought should happen if you change a crop rectangle, compose rectangle or the format rectangle (VIDIOC_S_FMT). In my proposal the format rectangle would increase in size if you attempt to set the compose rectangle wholly or partially outside the current format rectangle. Most (all?) of the developers present didn't like that and I was asked to take another look at that. After looking at this some more I realized that there was no need for this and it is OK to constrain a compose rectangle to the current format rectangle. All you need to do if you want to place the compose rectangle outside of the format rectangle is to just change the format rectangle first. If the driver supports composition then increasing the format rectangle will not change anything else, so that is a safe operation without side-effects. >>> >>> Good! >>> However, changing the crop rectangle *can* change the format rectangle. In the simple case of hardware that just supports cropping this is obvious, since the crop and format rectangles must always be of the same size, so changing one will change the other. >>> >>> True, but, in such case, I'm in doubt if it is worth to implement crop API >>> support, as just format API support is enough. The drawback is that >>> userspace won't know how to differentiate between: >>> >>> 1) scaler, no-crop, where changing the format changes the scaler; >>> 2) crop, no scaler, where changing the format changes the crop region. >>> >>> That could easily be fixed with a new caps flag, to announce if a device >>> has scaler or not. >> >> Erm, the format just specifies a size, crop specifies a rectangle. You can't >> use S_FMT to specify the crop rectangle. > > You said above about the format rectangle, and not about the crop rectangle. > I think we need first to use a consistent glossary on those discussions ;) > > I'm understanding "format rectangle" as the one defined by S_FMT. Sorry for the ambiguous terminology. S_FMT sets a size, not a rectangle. So whenever I talk about a format rectangle, read format size. > >> Also, this case of crop and no scaler exists today in various drivers and >> works as described (I'm sure about vpfe_capture, vino and I believe that >> there >> are various exynos drivers as well). > > This is confusing, and some drivers actually set both format and crop > rectangles at the same time, on S_FMT. See, for example, set_res() on: > drivers/media/i2c/mt9v011.c > > This one explicitly does crop for a random resolution, but there are other > sensor drivers that have multiple resolutions that are actually doing > crop instead of scaling, when changing the resolution, and don't implement > the crop API (I think that this is the case, for example, of ov7670). > > This is also the case of the gspca driver, and most of their sub-drivers. > > I'd say that there are a lot more sensor drivers doing crop at S_FMT > than via crop/selection API. > > We need to decide what's the best way for apps to set it, and then > see an strategy to migrate the non-compliant drivers. Whatever > decision, we'll need to concern about backward compat. These drivers just do not implement cropping (probably because nobody ever needed it), instead they just center a crop window if the requested image size is smaller than the sensor size. I would consider this out of spec actually: if a sensor allows formats of a different size than the native size, then that implies the presence of a scaler unless crop functionality is present. > But if you throw in a scaler as well, you usually still have such constraints based on the scaler capabilities. So assuming a scaler that can only scale 4 times (or less) up or down in each direction, then setting a crop rectangle of 240x160 will require that the format rectangle has a width in the range of 240/4 - 240*4 (60-960) and a height in the range of 160/4 - 160*4 (40-640). Anything outside of that will have to be corrected. >>> >>> This can be done on two directions, e. g. rounding the crop area or >>> rounding the scaler area. >>> >>> I is not obvious at all (nor backward compat) to change the format >>> rectangle when the crop rea is changed. >>> >>> So, the best approach in this case is to round the crop rectangle to fit >>> into the scaler limits, preserving the format rectangle. >> >>
GOOD DAY,
My name is Mr Yao Yuta from Hong Kong, I want you to be my partner in a business project. If Interested Contact me back via my email address. Thank you, Mr. Yao Yuta. -- 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 ATTN] uvcvideo: Work around buggy Logitech C920 firmware
From: William Manley The uvcvideo webcam driver exposes the v4l2 control "Exposure (Absolute)" which allows the user to control the exposure time of the webcam, essentially controlling the brightness of the received image. By default the webcam automatically adjusts the exposure time automatically but the if you set the control "Exposure, Auto"="Manual Mode" the user can fix the exposure time. Unfortunately it seems that the Logitech C920 has a firmware bug where it will forget that it's in manual mode temporarily during initialisation. This means that the camera doesn't respect the exposure time that the user requested if they request it before starting to stream video. They end up with a video stream which is either too bright or too dark and must reset the controls after video starts streaming. This patch introduces the quirk UVC_QUIRK_RESTORE_CTRLS_ON_INIT which causes the cached controls to be re-uploaded to the camera immediately after initialising the camera. This quirk is applied to the C920 to work around this camera bug. Signed-off-by: William Manley Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 2 +- drivers/media/usb/uvc/uvc_driver.c | 11 ++- drivers/media/usb/uvc/uvc_video.c | 6 ++ drivers/media/usb/uvc/uvcvideo.h | 3 ++- 4 files changed, 19 insertions(+), 3 deletions(-) Hi Mauro, Here's a lone uvcvideo patch that managed to splip through the cracks of my v3.16 pull requests. If you still find time to process it during this merge window it would be greatly appreciated. diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 0eb82106..30f4fe9 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1795,7 +1795,7 @@ done: * - Handle restore order (Auto-Exposure Mode should be restored before * Exposure Time). */ -int uvc_ctrl_resume_device(struct uvc_device *dev) +int uvc_ctrl_restore_values(struct uvc_device *dev) { struct uvc_control *ctrl; struct uvc_entity *entity; diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index ad47c5c..895b004 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -2001,7 +2001,7 @@ static int __uvc_resume(struct usb_interface *intf, int reset) int ret = 0; if (reset) { - ret = uvc_ctrl_resume_device(dev); + ret = uvc_ctrl_restore_values(dev); if (ret < 0) return ret; } @@ -2176,6 +2176,15 @@ static struct usb_device_id uvc_ids[] = { .bInterfaceClass = USB_CLASS_VENDOR_SPEC, .bInterfaceSubClass = 1, .bInterfaceProtocol = 0 }, + /* Logitech HD Pro Webcam C920 */ + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE + | USB_DEVICE_ID_MATCH_INT_INFO, + .idVendor = 0x046d, + .idProduct= 0x082d, + .bInterfaceClass = USB_CLASS_VIDEO, + .bInterfaceSubClass = 1, + .bInterfaceProtocol = 0, + .driver_info = UVC_QUIRK_RESTORE_CTRLS_ON_INIT }, /* Chicony CNF7129 (Asus EEE 100HE) */ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE | USB_DEVICE_ID_MATCH_INT_INFO, diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 9144a2f..61eb857 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1678,6 +1678,12 @@ static int uvc_init_video(struct uvc_streaming *stream, gfp_t gfp_flags) } } + /* The Logitech C920 temporarily forgets that it should not be adjusting +* Exposure Absolute during init so restore controls to stored values. +*/ + if (stream->dev->quirks & UVC_QUIRK_RESTORE_CTRLS_ON_INIT) + uvc_ctrl_restore_values(stream->dev); + return 0; } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index b1f69a6..39c4f94 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -147,6 +147,7 @@ #define UVC_QUIRK_FIX_BANDWIDTH0x0080 #define UVC_QUIRK_PROBE_DEF0x0100 #define UVC_QUIRK_RESTRICT_FRAME_RATE 0x0200 +#define UVC_QUIRK_RESTORE_CTRLS_ON_INIT0x0400 /* Format flags */ #define UVC_FMT_FLAG_COMPRESSED0x0001 @@ -688,7 +689,7 @@ extern int uvc_ctrl_add_mapping(struct uvc_video_chain *chain, const struct uvc_control_mapping *mapping); extern int uvc_ctrl_init_device(struct uvc_device *dev); extern void uvc_ctrl_cleanup_device(struct uvc_device *dev); -extern int uvc_ctrl_resume_device(struct uvc_device *dev); +extern int uvc_ctrl_restore_values(struct uvc_device *dev); extern int uvc_ctrl_b
Re: [RFC ATTN] Cropping, composing, scaling and S_FMT
Em Thu, 05 Jun 2014 09:20:33 +0200 Hans Verkuil escreveu: > On 06/04/2014 08:40 PM, Mauro Carvalho Chehab wrote: > > Em Mon, 02 Jun 2014 10:28:18 +0200 > > Hans Verkuil escreveu: > > > >> During the media mini-summit I went through all 8 combinations of cropping, > >> composing and scaling (i.e. none of these features is present, or only > >> cropping, > >> only composing, etc.). > >> > >> In particular I showed what I thought should happen if you change a crop > >> rectangle, > >> compose rectangle or the format rectangle (VIDIOC_S_FMT). > >> > >> In my proposal the format rectangle would increase in size if you attempt > >> to set > >> the compose rectangle wholly or partially outside the current format > >> rectangle. > >> Most (all?) of the developers present didn't like that and I was asked to > >> take > >> another look at that. > >> > >> After looking at this some more I realized that there was no need for this > >> and > >> it is OK to constrain a compose rectangle to the current format rectangle. > >> All > >> you need to do if you want to place the compose rectangle outside of the > >> format > >> rectangle is to just change the format rectangle first. If the driver > >> supports > >> composition then increasing the format rectangle will not change anything > >> else, > >> so that is a safe operation without side-effects. > > > > Good! > > > >> However, changing the crop rectangle *can* change the format rectangle. In > >> the > >> simple case of hardware that just supports cropping this is obvious, since > >> the crop and format rectangles must always be of the same size, so changing > >> one will change the other. > > > > True, but, in such case, I'm in doubt if it is worth to implement crop API > > support, as just format API support is enough. The drawback is that > > userspace won't know how to differentiate between: > > > > 1) scaler, no-crop, where changing the format changes the scaler; > > 2) crop, no scaler, where changing the format changes the crop region. > > > > That could easily be fixed with a new caps flag, to announce if a device > > has scaler or not. > > Erm, the format just specifies a size, crop specifies a rectangle. You can't > use S_FMT to specify the crop rectangle. You said above about the format rectangle, and not about the crop rectangle. I think we need first to use a consistent glossary on those discussions ;) I'm understanding "format rectangle" as the one defined by S_FMT. > Also, this case of crop and no scaler exists today in various drivers and > works as described (I'm sure about vpfe_capture, vino and I believe that there > are various exynos drivers as well). This is confusing, and some drivers actually set both format and crop rectangles at the same time, on S_FMT. See, for example, set_res() on: drivers/media/i2c/mt9v011.c This one explicitly does crop for a random resolution, but there are other sensor drivers that have multiple resolutions that are actually doing crop instead of scaling, when changing the resolution, and don't implement the crop API (I think that this is the case, for example, of ov7670). This is also the case of the gspca driver, and most of their sub-drivers. I'd say that there are a lot more sensor drivers doing crop at S_FMT than via crop/selection API. We need to decide what's the best way for apps to set it, and then see an strategy to migrate the non-compliant drivers. Whatever decision, we'll need to concern about backward compat. > >> But if you throw in a scaler as well, you usually > >> still have such constraints based on the scaler capabilities. > >> > >> So assuming a scaler that can only scale 4 times (or less) up or down in > >> each > >> direction, then setting a crop rectangle of 240x160 will require that the > >> format rectangle has a width in the range of 240/4 - 240*4 (60-960) and a > >> height in the range of 160/4 - 160*4 (40-640). Anything outside of that > >> will > >> have to be corrected. > > > > This can be done on two directions, e. g. rounding the crop area or > > rounding the scaler area. > > > > I is not obvious at all (nor backward compat) to change the format > > rectangle when the crop rea is changed. > > > > So, the best approach in this case is to round the crop rectangle to fit > > into the scaler limits, preserving the format rectangle. > > I disagree with that for several reasons: > > 1) In the case of no-scaler the format is already changed by s_crop in > existing > drivers. That can't be changed. So doing something else if there is a scaler > is > inconsistent behavior. See above. The inconsistent behavior is already there. > 2) The spec clearly specifies that changing the crop rectangle may change the > format size. It has always said so. From the section "Image Cropping, > Insertion > and Scaling", "Scaling Adjustments": > > "Applications can change the source or the target rectangle first, as they may > prefer a particular image size or a c
Re: [GIT PULL FOR v3.16] uvcvideo fixes
Hi Laurant On 12/05/14 23:43, Laurent Pinchart wrote: > Hi William, > > On Friday 09 May 2014 14:33:57 William Manley wrote: >> Hi Laurent >> >> Any chance of my patch fixing manual exposure mode for the Logitech >> C920[1] going in for 3.16? > > I know I've been largely unresponsive regarding this patch, and I'm really > ashamed for that. > > I would have liked to get feedback from Logitech on the issue, but they've > been mostly silent so far. I'll thus apply your patch even though I'm still > bothered by the issue, we can always revisit it later if needed. Now the 3.16 merge window is open can you confirm whether this is going in. I've not seen it appear in linus' tree yet, but I'm not sure what the process is. Thanks Will -- 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/RFC 2/2] v4l: vb2: Add fatal error condition flag
Hi Laurent, There are some typos and the patch is against an old kernel. I'll wait with an Ack until I see the v2. Regards, Hans On 06/04/14 16:05, Laurent Pinchart wrote: > When a fatal error occurs that render the device unusable, the only > options for a driver to signal the error condition to userspace is to > set the V4L2_BUF_FLAG_ERROR flag when dequeuing buffers and to return an > error from the buffer prepare handler when queuing buffers. > > The buffer error flag indicates a transient error and can't be used by > applications to detect fatal errors. Returning an error from vb2_qbuf() > is thus the only real indication that a fatal error occured. However, > this is difficult to handle for multithreaded applications that requeue > buffers from a thread other than the control thread. In particular the > poll() call in the control thread will not notify userspace of the > error. > > This patch adds an explicit mechanism to report fatal errors to > userspace. Applications can call the vb2_queue_error() function to That should be 'Drivers'. > signal a fatal error. From this moment on, buffer preparation will > return -EIO to userspace, and vb2_poll() will set the POLLERR flag and > return immediately. The error flag is cleared when cancelling the queue, > either at stream off time (through vb2_streamoff) or when releasing the > queue with vb2_queue_release(). > > Signed-off-by: Laurent Pinchart > --- > drivers/media/video/videobuf2-core.c | 41 > +--- media/video? This patch and the previous one are against some old kernel. > include/media/videobuf2-core.h | 3 +++ > 2 files changed, 41 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/video/videobuf2-core.c > b/drivers/media/video/videobuf2-core.c > index 5f38774..76e3456 100644 > --- a/drivers/media/video/videobuf2-core.c > +++ b/drivers/media/video/videobuf2-core.c > @@ -1295,6 +1295,12 @@ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer > *b) > call_qop(q, wait_finish, q); > } > > + if (q->error) { > + dprintk(1, "qbuf: fatal error occured on queue\n"); typo: 'occurred' > + ret = -EIO; > + goto unlock; > + } > + > if (q->fileio) { > dprintk(1, "qbuf: file io in progress\n"); > ret = -EBUSY; > @@ -1393,6 +1399,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, > int nonblocking) > return -EINVAL; > } > > + if (q->error) { > + dprintk(1, "Queue in error state, will not wait for > buffers\n"); > + return -EIO; > + } > + > if (!list_empty(&q->done_list)) { > /* >* Found a buffer that we were waiting for. > @@ -1418,7 +1429,8 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, > int nonblocking) >*/ > dprintk(3, "Will sleep waiting for buffers\n"); > ret = wait_event_interruptible(q->done_wq, > - !list_empty(&q->done_list) || !q->streaming); > + !list_empty(&q->done_list) || !q->streaming || > + q->error); > > /* >* We need to reevaluate both conditions again after reacquiring > @@ -1602,6 +1614,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) > if (q->streaming) > call_qop(q, stop_streaming, q); > q->streaming = 0; > + q->error = 0; > > /* >* Remove all buffers from videobuf's list... > @@ -1623,6 +1636,27 @@ static void __vb2_queue_cancel(struct vb2_queue *q) > } > > /** > + * vb2_queue_error() - signal a fatal error on the queue > + * @q: videobuf2 queue > + * > + * Flag that a fatal unrecoverable error occured and wake up all processes > + * waiting on the queue. Polling will now set POLLERR and queuing and > dequeuing > + * buffers will return -EIO. > + * > + * The error flag will be cleared when cancelling the queue, either from > + * vb2_streamoff or vb2_queue_release. Drivers should thus not call this > + * function before starting the stream, otherwise the error flag will remain > set > + * until the queue is released when closing the device node. > + */ > +void vb2_queue_error(struct vb2_queue *q) > +{ > + q->error = 1; > + > + wake_up_all(&q->done_wq); > +} > +EXPORT_SYMBOL_GPL(vb2_queue_error); > + > +/** > * vb2_streamon - start streaming > * @q: videobuf2 queue > * @type:type argument passed from userspace to vidioc_streamon handler > @@ -1984,9 +2018,10 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file > *file, poll_table *wait) > } > > /* > - * There is nothing to wait for if the queue isn't streaming. > + * There is nothing to wait for if the queue isn't streaming
Re: [PATCH/RFC 1/2] v4l: vb2: Don't return POLLERR during transient buffer underruns
On 06/04/14 16:05, Laurent Pinchart wrote: > The V4L2 specification states that > > "When the application did not call VIDIOC_QBUF or VIDIOC_STREAMON yet > the poll() function succeeds, but sets the POLLERR flag in the revents > field." > > The vb2_poll() function sets POLLERR when the queued buffers list is > empty, regardless of whether this is caused by the stream not being > active yet, or by a transient buffer underrun. > > Bring the implementation in line with the specification by returning > POLLERR only when the queue is not streaming. Buffer underruns during > streaming are not treated specially anymore and just result in poll() > blocking until the next event. > > Signed-off-by: Laurent Pinchart Acked-by: Hans Verkuil Regards, Hans > --- > drivers/media/video/videobuf2-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/video/videobuf2-core.c > b/drivers/media/video/videobuf2-core.c > index 11d31bf..5f38774 100644 > --- a/drivers/media/video/videobuf2-core.c > +++ b/drivers/media/video/videobuf2-core.c > @@ -1984,9 +1984,9 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file > *file, poll_table *wait) > } > > /* > - * There is nothing to wait for if no buffers have already been queued. > + * There is nothing to wait for if the queue isn't streaming. >*/ > - if (list_empty(&q->queued_list)) > + if (!vb2_is_streaming(q)) > return res | POLLERR; > > poll_wait(file, &q->done_wq, wait); > -- 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: [RFC ATTN] Cropping, composing, scaling and S_FMT
On 06/04/2014 08:40 PM, Mauro Carvalho Chehab wrote: > Em Mon, 02 Jun 2014 10:28:18 +0200 > Hans Verkuil escreveu: > >> During the media mini-summit I went through all 8 combinations of cropping, >> composing and scaling (i.e. none of these features is present, or only >> cropping, >> only composing, etc.). >> >> In particular I showed what I thought should happen if you change a crop >> rectangle, >> compose rectangle or the format rectangle (VIDIOC_S_FMT). >> >> In my proposal the format rectangle would increase in size if you attempt to >> set >> the compose rectangle wholly or partially outside the current format >> rectangle. >> Most (all?) of the developers present didn't like that and I was asked to >> take >> another look at that. >> >> After looking at this some more I realized that there was no need for this >> and >> it is OK to constrain a compose rectangle to the current format rectangle. >> All >> you need to do if you want to place the compose rectangle outside of the >> format >> rectangle is to just change the format rectangle first. If the driver >> supports >> composition then increasing the format rectangle will not change anything >> else, >> so that is a safe operation without side-effects. > > Good! > >> However, changing the crop rectangle *can* change the format rectangle. In >> the >> simple case of hardware that just supports cropping this is obvious, since >> the crop and format rectangles must always be of the same size, so changing >> one will change the other. > > True, but, in such case, I'm in doubt if it is worth to implement crop API > support, as just format API support is enough. The drawback is that > userspace won't know how to differentiate between: > > 1) scaler, no-crop, where changing the format changes the scaler; > 2) crop, no scaler, where changing the format changes the crop region. > > That could easily be fixed with a new caps flag, to announce if a device > has scaler or not. Erm, the format just specifies a size, crop specifies a rectangle. You can't use S_FMT to specify the crop rectangle. Also, this case of crop and no scaler exists today in various drivers and works as described (I'm sure about vpfe_capture, vino and I believe that there are various exynos drivers as well). > >> But if you throw in a scaler as well, you usually >> still have such constraints based on the scaler capabilities. >> >> So assuming a scaler that can only scale 4 times (or less) up or down in each >> direction, then setting a crop rectangle of 240x160 will require that the >> format rectangle has a width in the range of 240/4 - 240*4 (60-960) and a >> height in the range of 160/4 - 160*4 (40-640). Anything outside of that will >> have to be corrected. > > This can be done on two directions, e. g. rounding the crop area or > rounding the scaler area. > > I is not obvious at all (nor backward compat) to change the format > rectangle when the crop rea is changed. > > So, the best approach in this case is to round the crop rectangle to fit > into the scaler limits, preserving the format rectangle. I disagree with that for several reasons: 1) In the case of no-scaler the format is already changed by s_crop in existing drivers. That can't be changed. So doing something else if there is a scaler is inconsistent behavior. 2) The spec clearly specifies that changing the crop rectangle may change the format size. It has always said so. From the section "Image Cropping, Insertion and Scaling", "Scaling Adjustments": "Applications can change the source or the target rectangle first, as they may prefer a particular image size or a certain area in the video signal. If the driver has to adjust both to satisfy hardware limitations, the last requested rectangle shall take priority, and the driver should preferably adjust the opposite one. The VIDIOC_TRY_FMT ioctl however shall not change the driver state and therefore only adjust the requested rectangle." The two following paragraphs actually describe exactly the crop+scaler case and how setting the crop rectangle can change the format size. 3) If an application desires a specific crop rectangle that is possible by the hardware but is changed just because the format size is not suitable, then it is hard (perhaps even impossible) for the application to figure out how to change the format so the crop request can be achieved. That's quite a different situation compared to the compose case where that is easy to decide. 4) This is actually how bttv behaves. So this is well-established behavior. Regards, Hans > >> >> In my opinion this is valid behavior, and the specification also clearly >> specifies in the VIDIOC_S_CROP and VIDIOC_S_SELECTION documentation that the >> format may change after changing the crop rectangle. >> >> Note that for output streams the role of crop and compose is swapped. So for >> output streams it is the crop rectangle that will always be constrained by >> the format rect
[PATCH] staging: omap4iss: copy paste error in iss_get_clocks
It makes more sense to return PTR_ERR(iss->iss_ctrlclk) here. The current code looks like an oversight in pasting the block just above this one. Signed-off-by: Vitaly Osipov --- drivers/staging/media/omap4iss/iss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/omap4iss/iss.c b/drivers/staging/media/omap4iss/iss.c index 2e422dd..4a9e444 100644 --- a/drivers/staging/media/omap4iss/iss.c +++ b/drivers/staging/media/omap4iss/iss.c @@ -1029,7 +1029,7 @@ static int iss_get_clocks(struct iss_device *iss) if (IS_ERR(iss->iss_ctrlclk)) { dev_err(iss->dev, "Unable to get iss_ctrlclk clock info\n"); iss_put_clocks(iss); - return PTR_ERR(iss->iss_fck); + return PTR_ERR(iss->iss_ctrlclk); } return 0; -- 1.9.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