Re: [Workshop-2011] Media Subsystem Workshop 2011
Hi all, On 08/04/2011 02:34 PM, Mauro Carvalho Chehab wrote: Em 03-08-2011 20:20, Theodore Kilgore escreveu: snip snip Yes, that kind of thing is an obvious problem. Actually, though, it may be that this had just better not happen. For some of the hardware that I know of, it could be a real problem no matter what approach would be taken. For example, certain specific dual-mode cameras will delete all data stored on the camera if the camera is fired up in webcam mode. To drop Gphoto suddenly in order to do the videoconf call would, on such cameras, result in the automatic deletion of all photos on the camera even if those photos had not yet been downloaded. Presumably, one would not want to do that. So, in other words, the Kernel driver should return -EBUSY if on such cameras, if there are photos stored on them, and someone tries to stream. Agreed. IMO, the right solution is to work on a proper snapshot mode, in kernelspace, and moving the drivers that have already a kernelspace out of Gphoto. Well, the problem with that is, a still camera and a webcam are entirely different beasts. Still photos stored in the memory of an external device, waiting to be downloaded, are not snapshots. Thus, access to those still photos is not access to snapshots. Things are not that simple. Yes, stored photos require a different API, as Hans pointed. I think that some cameras just export them as a USB storage. Erm, that is not what I tried to say, or do you mean another Hans? snip snip If I understood you well, there are 4 possible ways: 1) UVC + USB mass storage; 2) UVC + Vendor Class mass storage; 3) Vendor Class video + USB mass storage; 4) Vendor Class video + Vendor Class mass storage. Actually the cameras Theodore and I are talking about here all fall into category 4. I expect devices which do any of 1-3 to properly use different interfaces for this, actually the different class specifications mandate that they use different interfaces for this. This sounds to be a good theme for the Workshop, or even to KS/2011. Agreed, although we don't need to talk about this for very long, the solution is basically: 1) Define a still image retrieval API for v4l2 devices (there is only 1 interface for both functions on these devices, so only 1 driver, and to me it makes sense to extend the existing drivers to also do still image retrieval). 2) Modify existing kernel v4l2 drivers to provide this API 3) Write a new libgphoto driver which talks this interface (only need to do one driver since all dual mode cams will export the same API). 1) is something to discuss at the workshop. 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
Re: USB mini-summit at LinuxCon Vancouver
Am Freitag, 5. August 2011, 00:56:03 schrieb Greg KH: On Thu, Aug 04, 2011 at 07:21:47PM -0300, Mauro Carvalho Chehab wrote: I know that this problem were somewhat solved for 3G modems, with the usage of the userspace problem usb_modeswitch, and with some quirks for the USB storage driver, but I'm not sure if such tricks will scale forever, as more functions are seen on some USB devices. Well, no matter how it scales it needs to be done in userspace, like usb_modeswitch does. We made that decision a while ago, and it is working out very well. I see no reason why you can't do it in userspace as well as that is the easiest place to control this type of thing. I thought we had a long discussion about this topic a while ago and came to this very conclusion. Or am I mistaken? Circumstances change. We want to keep the stuff in user space as much and as long as we can. However user space has limitations: - it has by necessity a race between resumption and access by others - it cannot resume anything we run a (rw) filesystem over. Furthermore, today PM actions that lead to a loss of mode are initiated by user space. If we ever want to oportunistically suspend a system we also need to restore mode from inside the kernel. We could avoid all that trouble, if we persuaded vendors to use plain USB configurations for those purposes. Regards Oliver -- 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] OMAP_VOUT: Fix build break caused by update_mode removal in DSS2
The DSS2 driver does not support the configuration of the update_mode of a panel anymore. Remove the setting of update_mode done in omap_vout_probe(). Ignore configuration of TE since omap_vout driver doesn't support manual update displays anyway. Signed-off-by: Archit Taneja arc...@ti.com --- drivers/media/video/omap/omap_vout.c | 13 - 1 files changed, 0 insertions(+), 13 deletions(-) diff --git a/drivers/media/video/omap/omap_vout.c b/drivers/media/video/omap/omap_vout.c index b5ef362..b3a5ecd 100644 --- a/drivers/media/video/omap/omap_vout.c +++ b/drivers/media/video/omap/omap_vout.c @@ -2194,19 +2194,6 @@ static int __init omap_vout_probe(struct platform_device *pdev) '%s' Display already enabled\n, def_display-name); } - /* set the update mode */ - if (def_display-caps - OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE) { - if (dssdrv-enable_te) - dssdrv-enable_te(def_display, 0); - if (dssdrv-set_update_mode) - dssdrv-set_update_mode(def_display, - OMAP_DSS_UPDATE_MANUAL); - } else { - if (dssdrv-set_update_mode) - dssdrv-set_update_mode(def_display, - OMAP_DSS_UPDATE_AUTO); - } } } -- 1.7.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: mx2_camera driver on mx27ipcam: dma_alloc_coherent size failed
Hello Uwe, thank you for the hint. There was problem with insufficient memory. dma_alloc_from_coherent is called several times and when I allocated only 4MB, last call failed. When I passed 8MB to dma_declare_coherent_memory, it works. Unfortunately I do not understand why 4MB is not enough for 640x480 YUV image... regards Jan 2011/8/4 Uwe Kleine-König u.kleine-koe...@pengutronix.de: Hello Jan, On Thu, Aug 04, 2011 at 03:11:11PM +0200, Jan Pohanka wrote: Dear Uwe, could you please give me some advice once more? It seems I'm not able to make mx2_camera working by myself. I have tried dma memory allocation in my board file in several ways, but nothing seems to work. I use Video capture example for v4l2 for testing. regards Jan mx27ipcam_camera_power: 1 mx27ipcam_camera_reset mx2-camera mx2-camera.0: Camera driver attached to camera 0 mx2-camera mx2-camera.0: dma_alloc_coherent size 614400 failed mmap error 12, Cannot allocate memory mx2-camera mx2-camera.0: Camera driver detached from camera 0 mx27ipcam_camera_power: 0 Cannot say offhand. I'd instrument dma_alloc_from_coherent to check where it fails. The patch looks OK from a first glance. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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: mx2_camera driver on mx27ipcam: dma_alloc_coherent size failed
Hello Jan, On Fri, Aug 05, 2011 at 09:21:36AM +0200, Jan Pohanka wrote: Hello Uwe, thank you for the hint. There was problem with insufficient memory. dma_alloc_from_coherent is called several times and when I allocated only 4MB, last call failed. When I passed 8MB to dma_declare_coherent_memory, it works. Unfortunately I do not understand why 4MB is not enough for 640x480 YUV image... A YUV420 image of size 640x480 needs 1.5 * 640 * 480 bytes (I think). That's ~ 0.5MB. Now it depends how much buffers are allocated. For taking a photo a single buffer is enough, for a video it uses probably more than one. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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: USB mini-summit at LinuxCon Vancouver
Hi, On 08/05/2011 12:56 AM, Greg KH wrote: On Thu, Aug 04, 2011 at 07:21:47PM -0300, Mauro Carvalho Chehab wrote: I know that this problem were somewhat solved for 3G modems, with the usage of the userspace problem usb_modeswitch, and with some quirks for the USB storage driver, but I'm not sure if such tricks will scale forever, as more functions are seen on some USB devices. Well, no matter how it scales it needs to be done in userspace, like usb_modeswitch does. We made that decision a while ago, and it is working out very well. I see no reason why you can't do it in userspace as well as that is the easiest place to control this type of thing. I thought we had a long discussion about this topic a while ago and came to this very conclusion. Or am I mistaken? I think we've had multiple discussions about this, surrounding various topics / use cases. I would not call the do it in userspace a conclusion. I rather more have a feeling of getting stonewalled on this by various people surrounding the usb system. Me shutting up on this is basically a case of: Discussion not getting anywhere and I don't have the time to do a kernel proof of concept myself right now. To be clear about the stonewalling I'm not talking about the dual cam issue here, nor about the morphing devices thing which usb_modeswitch fixes. I think it is important to separate oranges from apples here, there are at least 3 different problem classes which all seem to have gotten thrown onto a pile here: 1) The reason Mauro suggested having some discussion on this at the USB summit is because of a discussion about dual mode cameras on the linux media list. Dual-mode cameras are (usually very cheap) digital photo cameras which can take still pictures in stand alone mode and store them in onboard memory, like regular digital photo cameras, this is one mode. The other mode is they can operate as a regular webcam, the hardware actually is more regular webcam hardware with some memory and a battery added (usually no viewfinder screen). These devices typically use 1 usb interface (and thus 1 driver) for both modes. We currently have drivers for both modes, but they are separate drivers, which leads to fighting for device ownership, with the stillmode driver always winning as that is userspace and it simply kicks of the kernel webcam /streaming mode driver making it see a device unplug, even if an app was streaming from the device at the time. The solution here is simple, move both functions to 1 driver, which can then properly return -EBUSY when the device is active in one mode and an app tries to use it in the other mode. This 1 driver will likely end up being a kernel driver, since doing streaming drivers in userspace is problematic, as the v4l2 API assumes a kernel device node, and we don't want to invent a new API, we want something which works with existing applications. 2) So called morphing devices. For example a USB 3G modem which initially shows up as a cdrom drive with windows software, only to reveal its real identity (with completely different usb descriptors) after some magic command. This is solved by the usb_modeswitch command, nothing to see here move along. 3) Re-direction of usb devices to virtual machines. This works by using the userspace usbfs interface from qemu / vmware / virtualbox / whatever. The basics of this work fine, but it lacks proper locking / safeguards for when a vm takes over a usb device from the in kernel driver. Example of the problem: usb mass storage devices is mounted, filesystem is in use and dirty, with writebacks pending, user redirects to vm, usb mass storage driver sees the cable beeing yanked out of the device. Now we've lost the pending writebacks and have a dirty, if not journaling possibly corrupt, fs on the device. This is the issue on which I feel a bit stonewalled. Simple putting your fingers in your ears and singing la la la do it in userspace is not going to cut it here. There is no way to do this race free in userspace, unless all possible callers of mount get modified. Moreover 99% of the necessary accounting for this is already done in the kernel. We already have the notion of a block device being in use. We simply need to add some code to pass this notion to the usb mass storage driver, and add a new try_disconnect callback for usb drivers. I'm not saying this is going to be completely straight forward, but it ain't rocket science either. And it so very obviously is the *right* thing to do, that I'm getting very tired of the do it in userspace song I keep hearing. 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
[PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management
A possibility to preallocate and initialise buffers of different sizes in V4L2 is required for an efficient implementation of asnapshot mode. This patch adds two new ioctl()s: VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF and defines respective data structures. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- v4: 1. CREATE_BUFS now takes an array of plane sizes and a fourcc code in its argument, instead of a frame format specification, including documentation update 2. documentation improvements, as suggested by Hans 3. increased reserved fields to 18, as suggested by Sakari Documentation/DocBook/media/v4l/io.xml | 17 ++ Documentation/DocBook/media/v4l/v4l2.xml |2 + .../DocBook/media/v4l/vidioc-create-bufs.xml | 161 .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 drivers/media/video/v4l2-compat-ioctl32.c |6 + drivers/media/video/v4l2-ioctl.c | 26 +++ include/linux/videodev2.h | 18 +++ include/media/v4l2-ioctl.h |2 + 8 files changed, 328 insertions(+), 0 deletions(-) create mode 100644 Documentation/DocBook/media/v4l/vidioc-create-bufs.xml create mode 100644 Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml index 227e7ac..ff03dd2 100644 --- a/Documentation/DocBook/media/v4l/io.xml +++ b/Documentation/DocBook/media/v4l/io.xml @@ -927,6 +927,23 @@ ioctl is called./entry Applications set or clear this flag before calling the constantVIDIOC_QBUF/constant ioctl./entry /row + row + entryconstantV4L2_BUF_FLAG_NO_CACHE_INVALIDATE/constant/entry + entry0x0400/entry + entryCaches do not have to be invalidated for this buffer. +Typically applications shall use this flag if the data captured in the buffer +is not going to be touched by the CPU, instead the buffer will, probably, be +passed on to a DMA-capable hardware unit for further processing or output. +/entry + /row + row + entryconstantV4L2_BUF_FLAG_NO_CACHE_CLEAN/constant/entry + entry0x0800/entry + entryCaches do not have to be cleaned for this buffer. +Typically applications shall use this flag for output buffers if the data +in this buffer has not been created by the CPU but by some DMA-capable unit, +in which case caches have not been used./entry + /row /tbody /tgroup /table diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml index 0d05e87..06bb179 100644 --- a/Documentation/DocBook/media/v4l/v4l2.xml +++ b/Documentation/DocBook/media/v4l/v4l2.xml @@ -462,6 +462,7 @@ and discussions on the V4L mailing list./revremark sub-close; sub-ioctl; !-- All ioctls go here. -- +sub-create-bufs; sub-cropcap; sub-dbg-g-chip-ident; sub-dbg-g-register; @@ -504,6 +505,7 @@ and discussions on the V4L mailing list./revremark sub-queryctrl; sub-query-dv-preset; sub-querystd; +sub-prepare-buf; sub-reqbufs; sub-s-hw-freq-seek; sub-streamon; diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml new file mode 100644 index 000..b37b9a4 --- /dev/null +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml @@ -0,0 +1,161 @@ +refentry id=vidioc-create-bufs + refmeta +refentrytitleioctl VIDIOC_CREATE_BUFS/refentrytitle +manvol; + /refmeta + + refnamediv +refnameVIDIOC_CREATE_BUFS/refname +refpurposeCreate buffers for Memory Mapped or User Pointer I/O/refpurpose + /refnamediv + + refsynopsisdiv +funcsynopsis + funcprototype + funcdefint functionioctl/function/funcdef + paramdefint parameterfd/parameter/paramdef + paramdefint parameterrequest/parameter/paramdef + paramdefstruct v4l2_create_buffers *parameterargp/parameter/paramdef + /funcprototype +/funcsynopsis + /refsynopsisdiv + + refsect1 +titleArguments/title + +variablelist + varlistentry + termparameterfd/parameter/term + listitem + parafd;/para + /listitem + /varlistentry + varlistentry + termparameterrequest/parameter/term + listitem + paraVIDIOC_CREATE_BUFS/para + /listitem + /varlistentry + varlistentry + termparameterargp/parameter/term + listitem + para/para + /listitem + /varlistentry +/variablelist + /refsect1 + + refsect1 +titleDescription/title + +paraThis ioctl is used to create buffers for link linkend=mmapmemory +mapped/link or link linkend=userpuser pointer/link +I/O. It can be used as an alternative or in addition to the +constantVIDIOC_REQBUFS/constant ioctl, when a tighter control over buffers +is required. This
[PATCH 3/6 v4] V4L: vb2: change .queue_setup() argument to unsigned int
In preparation for the forthcoming VIDIOC_CREATE_BUFS ioctl change the type of the sizes[] argument of the .queue_setup() vb2 operation from unsigned long to unsigned int to match with the __u32 type of buffer size variables elsewhere in V4L2. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- drivers/media/video/atmel-isi.c |2 +- drivers/media/video/marvell-ccic/mcam-core.c |2 +- drivers/media/video/mem2mem_testdev.c|2 +- drivers/media/video/mx3_camera.c |2 +- drivers/media/video/pwc/pwc-if.c |2 +- drivers/media/video/s5p-fimc/fimc-capture.c |2 +- drivers/media/video/s5p-fimc/fimc-core.c |2 +- drivers/media/video/s5p-mfc/s5p_mfc_dec.c|2 +- drivers/media/video/s5p-mfc/s5p_mfc_enc.c|2 +- drivers/media/video/s5p-tv/mixer_video.c |2 +- drivers/media/video/sh_mobile_ceu_camera.c |4 ++-- drivers/media/video/videobuf2-core.c |6 +++--- drivers/media/video/vivi.c |2 +- include/media/videobuf2-core.h |2 +- 14 files changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c index 3e3d4cc..1cc05e9 100644 --- a/drivers/media/video/atmel-isi.c +++ b/drivers/media/video/atmel-isi.c @@ -250,7 +250,7 @@ static int atmel_isi_wait_status(struct atmel_isi *isi, int wait_reset) Videobuf operations --*/ static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers, - unsigned int *nplanes, unsigned long sizes[], + unsigned int *nplanes, unsigned int sizes[], void *alloc_ctxs[]) { struct soc_camera_device *icd = soc_camera_from_vb2q(vq); diff --git a/drivers/media/video/marvell-ccic/mcam-core.c b/drivers/media/video/marvell-ccic/mcam-core.c index 83c1451..744cf37 100644 --- a/drivers/media/video/marvell-ccic/mcam-core.c +++ b/drivers/media/video/marvell-ccic/mcam-core.c @@ -884,7 +884,7 @@ static int mcam_read_setup(struct mcam_camera *cam) */ static int mcam_vb_queue_setup(struct vb2_queue *vq, unsigned int *nbufs, - unsigned int *num_planes, unsigned long sizes[], + unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]) { struct mcam_camera *cam = vb2_get_drv_priv(vq); diff --git a/drivers/media/video/mem2mem_testdev.c b/drivers/media/video/mem2mem_testdev.c index 166bf93..0d0c0d5 100644 --- a/drivers/media/video/mem2mem_testdev.c +++ b/drivers/media/video/mem2mem_testdev.c @@ -739,7 +739,7 @@ static const struct v4l2_ioctl_ops m2mtest_ioctl_ops = { */ static int m2mtest_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers, - unsigned int *nplanes, unsigned long sizes[], + unsigned int *nplanes, unsigned int sizes[], void *alloc_ctxs[]) { struct m2mtest_ctx *ctx = vb2_get_drv_priv(vq); diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c index 3f37522f..2902c02 100644 --- a/drivers/media/video/mx3_camera.c +++ b/drivers/media/video/mx3_camera.c @@ -192,7 +192,7 @@ static void mx3_cam_dma_done(void *arg) */ static int mx3_videobuf_setup(struct vb2_queue *vq, unsigned int *count, unsigned int *num_planes, - unsigned long sizes[], void *alloc_ctxs[]) + unsigned int sizes[], void *alloc_ctxs[]) { struct soc_camera_device *icd = soc_camera_from_vb2q(vq); struct soc_camera_host *ici = to_soc_camera_host(icd-parent); diff --git a/drivers/media/video/pwc/pwc-if.c b/drivers/media/video/pwc/pwc-if.c index 51ca358..a7e4f56 100644 --- a/drivers/media/video/pwc/pwc-if.c +++ b/drivers/media/video/pwc/pwc-if.c @@ -745,7 +745,7 @@ static int pwc_video_mmap(struct file *file, struct vm_area_struct *vma) /* Videobuf2 operations */ static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers, - unsigned int *nplanes, unsigned long sizes[], + unsigned int *nplanes, unsigned int sizes[], void *alloc_ctxs[]) { struct pwc_device *pdev = vb2_get_drv_priv(vq); diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c b/drivers/media/video/s5p-fimc/fimc-capture.c index 0d730e5..e6afe5f 100644 --- a/drivers/media/video/s5p-fimc/fimc-capture.c +++ b/drivers/media/video/s5p-fimc/fimc-capture.c @@ -265,7 +265,7 @@ static unsigned int get_plane_size(struct fimc_frame *fr, unsigned int plane) } static int queue_setup(struct vb2_queue *vq, unsigned int *num_buffers, - unsigned int *num_planes, unsigned long sizes[], + unsigned int *num_planes, unsigned int sizes[],
[PATCH 2/6 v4] V4L: add a new videobuf2 buffer state VB2_BUF_STATE_PREPARED
This patch prepares for a better separation of the buffer preparation stage. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- drivers/media/video/videobuf2-core.c | 59 + include/media/videobuf2-core.h |2 + 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index 3015e60..fb7a3ac 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -333,6 +333,7 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) b-flags |= V4L2_BUF_FLAG_DONE; break; case VB2_BUF_STATE_DEQUEUED: + case VB2_BUF_STATE_PREPARED: /* nothing */ break; } @@ -817,6 +818,31 @@ static void __enqueue_in_driver(struct vb2_buffer *vb) q-ops-buf_queue(vb); } +static int __buf_prepare(struct vb2_buffer *vb, struct v4l2_buffer *b) +{ + struct vb2_queue *q = vb-vb2_queue; + int ret; + + switch (q-memory) { + case V4L2_MEMORY_MMAP: + ret = __qbuf_mmap(vb, b); + break; + case V4L2_MEMORY_USERPTR: + ret = __qbuf_userptr(vb, b); + break; + default: + WARN(1, Invalid queue type\n); + ret = -EINVAL; + } + + if (!ret) + ret = call_qop(q, buf_prepare, vb); + if (ret) + dprintk(1, qbuf: buffer preparation failed: %d\n, ret); + + return ret; +} + /** * vb2_qbuf() - Queue a buffer from userspace * @q: videobuf2 queue @@ -826,8 +852,8 @@ static void __enqueue_in_driver(struct vb2_buffer *vb) * Should be called from vidioc_qbuf ioctl handler of a driver. * This function: * 1) verifies the passed buffer, - * 2) calls buf_prepare callback in the driver (if provided), in which - *driver-specific buffer initialization can be performed, + * 2) if necessary, calls buf_prepare callback in the driver (if provided), in + *which driver-specific buffer initialization can be performed, * 3) if streaming is on, queues the buffer in driver by the means of buf_queue *callback for processing. * @@ -837,7 +863,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb) int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b) { struct vb2_buffer *vb; - int ret = 0; + int ret; if (q-fileio) { dprintk(1, qbuf: file io in progress\n); @@ -866,29 +892,18 @@ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b) return -EINVAL; } - if (vb-state != VB2_BUF_STATE_DEQUEUED) { + switch (vb-state) { + case VB2_BUF_STATE_DEQUEUED: + ret = __buf_prepare(vb, b); + if (ret) + return ret; + case VB2_BUF_STATE_PREPARED: + break; + default: dprintk(1, qbuf: buffer already in use\n); return -EINVAL; } - if (q-memory == V4L2_MEMORY_MMAP) - ret = __qbuf_mmap(vb, b); - else if (q-memory == V4L2_MEMORY_USERPTR) - ret = __qbuf_userptr(vb, b); - else { - WARN(1, Invalid queue type\n); - return -EINVAL; - } - - if (ret) - return ret; - - ret = call_qop(q, buf_prepare, vb); - if (ret) { - dprintk(1, qbuf: buffer preparation failed\n); - return ret; - } - /* * Add to the queued buffers list, a buffer will stay on it until * dequeued in dqbuf. diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index f87472a..65946c5 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -106,6 +106,7 @@ enum vb2_fileio_flags { /** * enum vb2_buffer_state - current video buffer state * @VB2_BUF_STATE_DEQUEUED:buffer under userspace control + * @VB2_BUF_STATE_PREPARED:buffer prepared in videobuf and by the driver * @VB2_BUF_STATE_QUEUED: buffer queued in videobuf, but not in driver * @VB2_BUF_STATE_ACTIVE: buffer queued in driver and possibly used * in a hardware operation @@ -117,6 +118,7 @@ enum vb2_fileio_flags { */ enum vb2_buffer_state { VB2_BUF_STATE_DEQUEUED, + VB2_BUF_STATE_PREPARED, VB2_BUF_STATE_QUEUED, VB2_BUF_STATE_ACTIVE, VB2_BUF_STATE_DONE, -- 1.7.2.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
[PATCH 5/6 v4] V4L: sh-mobile-ceu-camera: prepare to support multi-size buffers
Prepare the sh_mobile_ceu_camera friver to support the new VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be able to handle buffer sizes, provided by the caller, and the .buf_prepare() operation must not use the currently configured frame format for its operation. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- drivers/media/video/sh_mobile_ceu_camera.c | 98 1 files changed, 57 insertions(+), 41 deletions(-) diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c index 26d0248..996f2a9 100644 --- a/drivers/media/video/sh_mobile_ceu_camera.c +++ b/drivers/media/video/sh_mobile_ceu_camera.c @@ -101,6 +101,7 @@ struct sh_mobile_ceu_dev { unsigned int irq; void __iomem *base; unsigned long video_limit; + unsigned long buf_total; spinlock_t lock;/* Protects video buffer lists */ struct list_head capture; @@ -192,6 +193,12 @@ static int sh_mobile_ceu_soft_reset(struct sh_mobile_ceu_dev *pcdev) /* * Videobuf operations */ + +/* + * .queue_setup() is called to check, whether the driver can accept the + * requested number of buffers and to fill in plane sizes + * for the current frame format if required + */ static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq, unsigned int *count, unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]) @@ -199,26 +206,39 @@ static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq, struct soc_camera_device *icd = container_of(vq, struct soc_camera_device, vb2_vidq); struct soc_camera_host *ici = to_soc_camera_host(icd-parent); struct sh_mobile_ceu_dev *pcdev = ici-priv; - int bytes_per_line = soc_mbus_bytes_per_line(icd-user_width, + ssize_t size; + + if (!sizes[0] || !*num_planes) { + /* Called from VIDIOC_REQBUFS or in compatibility mode */ + int bytes_per_line = soc_mbus_bytes_per_line(icd-user_width, icd-current_fmt-host_fmt); + if (bytes_per_line 0) + return bytes_per_line; - if (bytes_per_line 0) - return bytes_per_line; + sizes[0] = bytes_per_line * icd-user_height; - *num_planes = 1; + *num_planes = 1; + } - pcdev-sequence = 0; - sizes[0] = bytes_per_line * icd-user_height; alloc_ctxs[0] = pcdev-alloc_ctx; + if (!vq-num_buffers) + pcdev-sequence = 0; + if (!*count) *count = 2; - if (pcdev-video_limit) { - if (PAGE_ALIGN(sizes[0]) * *count pcdev-video_limit) - *count = pcdev-video_limit / PAGE_ALIGN(sizes[0]); + size = PAGE_ALIGN(sizes[0]) * *count; + + if (pcdev-video_limit + size + pcdev-buf_total pcdev-video_limit) { + *count = (pcdev-video_limit - pcdev-buf_total) / + PAGE_ALIGN(sizes[0]); + size = PAGE_ALIGN(sizes[0]) * *count; } + pcdev-buf_total += size; + dev_dbg(icd-parent, count=%d, size=%u\n, *count, sizes[0]); return 0; @@ -330,23 +350,40 @@ static int sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev) static int sh_mobile_ceu_videobuf_prepare(struct vb2_buffer *vb) { + struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb); + + /* Added list head initialization on alloc */ + WARN(!list_empty(buf-queue), Buffer %p on queue!\n, vb); + + return 0; +} + +static void sh_mobile_ceu_videobuf_queue(struct vb2_buffer *vb) +{ struct soc_camera_device *icd = container_of(vb-vb2_queue, struct soc_camera_device, vb2_vidq); - struct sh_mobile_ceu_buffer *buf; + struct soc_camera_host *ici = to_soc_camera_host(icd-parent); + struct sh_mobile_ceu_dev *pcdev = ici-priv; + struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb); + unsigned long size; int bytes_per_line = soc_mbus_bytes_per_line(icd-user_width, icd-current_fmt-host_fmt); - unsigned long size; if (bytes_per_line 0) - return bytes_per_line; + return; + + size = icd-user_height * bytes_per_line; + + if (vb2_plane_size(vb, 0) size) { + dev_err(icd-parent, Buffer #%d too small (%lu %lu)\n, + vb-v4l2_buf.index, vb2_plane_size(vb, 0), size); + return; + } - buf = to_ceu_vb(vb); + vb2_set_plane_payload(vb, 0, size); dev_dbg(icd-parent, %s (vb=0x%p) 0x%p %lu\n, __func__, vb, vb2_plane_vaddr(vb, 0), vb2_get_plane_payload(vb, 0)); - /* Added list head initialization on alloc */ -
[PATCH 0/6 v4] new ioctl()s and soc-camera implementation
Replying to Hans' request here's an updated RFC with a new vb2 soc-camera implementation. v4 refers only to the actual two new ioctl()s patch, the rest are being posted for the first or the second time only. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 6/6 v4] V4L: soc-camera: add 2 new ioctl() handlers
This patch adds two new ioctl() handlers: .vidioc_create_bufs() and .vidioc_prepare_buf() for compliant vb2 soc-camera hosts. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- Actually, there should be one more patch before this one - to convert mx3-camera to the new API, otherwise it will break, if anyone decides to use CREATE_BUFS / PREPARE_BUF with it. I'll work on that while these patches are being reviewed. drivers/media/video/soc_camera.c | 33 +++-- 1 files changed, 31 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index ac23916..088972d 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -318,6 +318,32 @@ static int soc_camera_dqbuf(struct file *file, void *priv, return vb2_dqbuf(icd-vb2_vidq, p, file-f_flags O_NONBLOCK); } +static int soc_camera_create_bufs(struct file *file, void *priv, + struct v4l2_create_buffers *create) +{ + struct soc_camera_device *icd = file-private_data; + struct soc_camera_host *ici = to_soc_camera_host(icd-parent); + + /* videobuf2 only */ + if (ici-ops-init_videobuf) + return -EINVAL; + else + return vb2_create_bufs(icd-vb2_vidq, create); +} + +static int soc_camera_prepare_buf(struct file *file, void *priv, + struct v4l2_buffer *b) +{ + struct soc_camera_device *icd = file-private_data; + struct soc_camera_host *ici = to_soc_camera_host(icd-parent); + + /* videobuf2 only */ + if (ici-ops-init_videobuf) + return -EINVAL; + else + return vb2_prepare_buf(icd-vb2_vidq, b); +} + /* Always entered with .video_lock held */ static int soc_camera_init_user_formats(struct soc_camera_device *icd) { @@ -1101,6 +1127,7 @@ static int soc_camera_probe(struct soc_camera_device *icd) if (!control || !control-driver || !dev_get_drvdata(control) || !try_module_get(control-driver-owner)) { icl-del_device(icd); + ret = -ENODEV; goto enodrv; } } @@ -1366,19 +1393,21 @@ static int soc_camera_device_register(struct soc_camera_device *icd) static const struct v4l2_ioctl_ops soc_camera_ioctl_ops = { .vidioc_querycap = soc_camera_querycap, + .vidioc_try_fmt_vid_cap = soc_camera_try_fmt_vid_cap, .vidioc_g_fmt_vid_cap= soc_camera_g_fmt_vid_cap, - .vidioc_enum_fmt_vid_cap = soc_camera_enum_fmt_vid_cap, .vidioc_s_fmt_vid_cap= soc_camera_s_fmt_vid_cap, + .vidioc_enum_fmt_vid_cap = soc_camera_enum_fmt_vid_cap, .vidioc_enum_input = soc_camera_enum_input, .vidioc_g_input = soc_camera_g_input, .vidioc_s_input = soc_camera_s_input, .vidioc_s_std= soc_camera_s_std, .vidioc_enum_framesizes = soc_camera_enum_fsizes, .vidioc_reqbufs = soc_camera_reqbufs, - .vidioc_try_fmt_vid_cap = soc_camera_try_fmt_vid_cap, .vidioc_querybuf = soc_camera_querybuf, .vidioc_qbuf = soc_camera_qbuf, .vidioc_dqbuf= soc_camera_dqbuf, + .vidioc_create_bufs = soc_camera_create_bufs, + .vidioc_prepare_buf = soc_camera_prepare_buf, .vidioc_streamon = soc_camera_streamon, .vidioc_streamoff= soc_camera_streamoff, .vidioc_queryctrl= soc_camera_queryctrl, -- 1.7.2.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
[PATCH 4/6 v4] V4L: vb2: add support for buffers of different sizes on a single queue
The two recently added ioctl()s VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF allow user-space applications to allocate video buffers of different sizes and hand them over to the driver for fast switching between different frame formats. This patch adds support for buffers of different sizes on the same buffer-queue to vb2. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- drivers/media/video/videobuf2-core.c | 266 +- include/media/videobuf2-core.h | 31 +++-- 2 files changed, 246 insertions(+), 51 deletions(-) diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index 7045d1f..6e7f9e5 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -44,7 +44,7 @@ module_param(debug, int, 0644); * __vb2_buf_mem_alloc() - allocate video memory for the given buffer */ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb, - unsigned int *plane_sizes) + const unsigned int *plane_sizes) { struct vb2_queue *q = vb-vb2_queue; void *mem_priv; @@ -110,13 +110,22 @@ static void __vb2_buf_userptr_put(struct vb2_buffer *vb) * __setup_offsets() - setup unique offsets (cookies) for every plane in * every buffer on the queue */ -static void __setup_offsets(struct vb2_queue *q) +static void __setup_offsets(struct vb2_queue *q, unsigned int n) { unsigned int buffer, plane; struct vb2_buffer *vb; - unsigned long off = 0; + unsigned long off; - for (buffer = 0; buffer q-num_buffers; ++buffer) { + if (q-num_buffers) { + struct v4l2_plane *p; + vb = q-bufs[q-num_buffers - 1]; + p = vb-v4l2_planes[vb-num_planes - 1]; + off = PAGE_ALIGN(p-m.mem_offset + p-length); + } else { + off = 0; + } + + for (buffer = q-num_buffers; buffer q-num_buffers + n; ++buffer) { vb = q-bufs[buffer]; if (!vb) continue; @@ -142,7 +151,7 @@ static void __setup_offsets(struct vb2_queue *q) */ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory, unsigned int num_buffers, unsigned int num_planes, -unsigned int plane_sizes[]) +const unsigned int *plane_sizes) { unsigned int buffer; struct vb2_buffer *vb; @@ -163,7 +172,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory, vb-state = VB2_BUF_STATE_DEQUEUED; vb-vb2_queue = q; vb-num_planes = num_planes; - vb-v4l2_buf.index = buffer; + vb-v4l2_buf.index = q-num_buffers + buffer; vb-v4l2_buf.type = q-type; vb-v4l2_buf.memory = memory; @@ -191,15 +200,13 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory, } } - q-bufs[buffer] = vb; + q-bufs[q-num_buffers + buffer] = vb; } - q-num_buffers = buffer; - - __setup_offsets(q); + __setup_offsets(q, buffer); dprintk(1, Allocated %d buffers, %d plane(s) each\n, - q-num_buffers, num_planes); + buffer, num_planes); return buffer; } @@ -207,12 +214,13 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory, /** * __vb2_free_mem() - release all video buffer memory for a given queue */ -static void __vb2_free_mem(struct vb2_queue *q) +static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers) { unsigned int buffer; struct vb2_buffer *vb; - for (buffer = 0; buffer q-num_buffers; ++buffer) { + for (buffer = q-num_buffers - buffers; buffer q-num_buffers; +++buffer) { vb = q-bufs[buffer]; if (!vb) continue; @@ -226,17 +234,18 @@ static void __vb2_free_mem(struct vb2_queue *q) } /** - * __vb2_queue_free() - free the queue - video memory and related information - * and return the queue to an uninitialized state. Might be called even if the - * queue has already been freed. + * __vb2_queue_free() - free buffers at the end of the queue - video memory and + * related information, if no buffers are left return the queue to an + * uninitialized state. Might be called even if the queue has already been freed. */ -static void __vb2_queue_free(struct vb2_queue *q) +static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) { unsigned int buffer; /* Call driver-provided cleanup function for each buffer, if provided */ if (q-ops-buf_cleanup) { - for (buffer = 0; buffer q-num_buffers; ++buffer) { + for (buffer = q-num_buffers - buffers;
Re: USB mini-summit at LinuxCon Vancouveroliver
Am Freitag, 5. August 2011, 09:45:56 schrieb Hans de Goede: This is the issue on which I feel a bit stonewalled. Simple putting your fingers in your ears and singing la la la do it in userspace is not going to cut it here. There is no way to do this race free in userspace, unless all possible callers of mount get modified. Moreover 99% of the necessary accounting for this is already done in the kernel. We already have the notion of a block device being in use. We simply need to add some code to pass this notion to the usb mass storage driver, and add a new try_disconnect callback for usb drivers. I'm not saying this is going to be completely straight forward, but it ain't rocket science either. And it so very obviously is the right thing to do, that I'm getting very tired of the do it in userspace song I keep hearing. Doing a try_disconnect() would also solve the dual camera issue. But it doesn't really interfere with the user space vs. kernel space issue. You simply have to expand the ioctl interface to have an ioctl that triggers this API call in the kernel. A V4L2 device would return an error if its device node is opened, otherwise disconnect. Regards Oliver -- 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: USB mini-summit at LinuxCon Vancouveroliver
Hi, On 08/05/2011 09:59 AM, Oliver Neukum wrote: Am Freitag, 5. August 2011, 09:45:56 schrieb Hans de Goede: This is the issue on which I feel a bit stonewalled. Simple putting your fingers in your ears and singing la la la do it in userspace is not going to cut it here. There is no way to do this race free in userspace, unless all possible callers of mount get modified. Moreover 99% of the necessary accounting for this is already done in the kernel. We already have the notion of a block device being in use. We simply need to add some code to pass this notion to the usb mass storage driver, and add a new try_disconnect callback for usb drivers. I'm not saying this is going to be completely straight forward, but it ain't rocket science either. And it so very obviously is the right thing to do, that I'm getting very tired of the do it in userspace song I keep hearing. Doing a try_disconnect() would also solve the dual camera issue. But it doesn't really interfere with the user space vs. kernel space issue. You simply have to expand the ioctl interface to have an ioctl that triggers this API call in the kernel. A V4L2 device would return an error if its device node is opened, otherwise disconnect. Getting a bit offtopic here, but no a try_disconnect will fix the userspace stillcam mode driver being able to disconnect the device while the webcam function is active. If the webcam is not active userspace will still win, and possibly never return the device back to the kernel driver (this already happens today with gvfs-gphoto creating a fuse mount and keeping the device open indefinitely, locking out the webcam function). Likewise a v4l2 control panel like app (think alsamixer for videodevs to set brightness / contrast, etc.) can keep the /dev/video node open indefinitely. Unless we rewrite most of userspace, we need to allow the device to be open in bode modes *at the same time* and only fail with -EBUSY when something really exclusive is requested (so not just having the device open, or setting contrast, but trying to stream and read/delete pictures from the stillcam memory at the same time). 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
Re: DiBxxxx: fixes for 3.1/3.0
Hi Mauro, On Wed, 3 Aug 2011, Patrick Boettcher wrote: Would you please pull from git://linuxtv.org/pb/media_tree.git for_v3.0 for the following to changesets: [media] dib0700: protect the dib0700 buffer access [media] DiBcom: protect the I2C bufer access I added a patch from Olivier which fixes wrongly used dprintks and replaces them by err()-calls. [media] dib0700: correct error message I herewith renew my PULL-request. The request now contains 3 changesets. best regards, -- Patrick -- 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
Possible issue in videobuf2 with buffer length check at QBUF time
Hi Marek and Pawel, While reviewing an OMAP3 ISP patch, I noticed that videobuf2 doesn't verify the buffer length field value when a new USERPTR buffer is queued. The length given by userspace is copied to the internal buffer length field. It seems to be up to drivers to verify that the value is equal to or higher than the minimum required to store the image, but that's not clearly documented. Should the buf_init operation be made mandatory for drivers that support USERPTR, and documented as required to implement a length check ? Alternatively the check could be performed in videobuf2-core, which would probably make sense as the check is required. videobuf2 doesn't store the minimum size for USERPTR buffers though (but that can of course be changed). -- 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
Re: [PATCH] [media] omap3isp: queue: fail QBUF if buffer is too small
Hi Michael, Thanks for the patch. On Thursday 04 August 2011 17:40:37 Michael Jones wrote: Add buffer length to sanity checks for QBUF. Signed-off-by: Michael Jones michael.jo...@matrix-vision.de --- drivers/media/video/omap3isp/ispqueue.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/omap3isp/ispqueue.c b/drivers/media/video/omap3isp/ispqueue.c index 9c31714..4f6876f 100644 --- a/drivers/media/video/omap3isp/ispqueue.c +++ b/drivers/media/video/omap3isp/ispqueue.c @@ -867,6 +867,9 @@ int omap3isp_video_queue_qbuf(struct isp_video_queue *queue, if (buf-state != ISP_BUF_STATE_IDLE) goto done; + if (vbuf-length buf-vbuf.length) + goto done; + The vbuf-length value passed from userspace isn't used by the driver, so I'm not sure if verifying it is really useful. We verify the memory itself instead, to make sure that enough pages can be accessed. The application can always lie about the length, so we can't relying on it anyway. if (vbuf-memory == V4L2_MEMORY_USERPTR vbuf-m.userptr != buf-vbuf.m.userptr) { isp_video_buffer_cleanup(buf); -- 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
Re: ISP CCDC freeze-up on STREAMON
Hi Laurent, On 07/20/2011 10:47 AM, Laurent Pinchart wrote: Hi Michael, Sorry for the late reply. Likewise :) On Thursday 30 June 2011 10:31:52 Michael Jones wrote: Hi Laurent, I'm observing a system freeze-up with the ISP when writing data to memory directly from the ccdc. Here's the sequence I'm using: 0. apply the patch I'm sending separate in this thread. 1. configure the ISP pipeline for the CCDC to deliver V4L2_PIX_FMT_GREY directly from the sensor to memory. 2. yavta -c10 /dev/video2 The patch is pretty self-explanatory. It introduces a loop (with ugly indenting to keep the patch simple) with 100 iterations leaving the device open between them. My system usually hangs up within the first 30 iterations. I've never made it to 100 successfully. I see the same behavior with user pointers and with mmap, but I don't see it when using data from the previewer. Can you please try this out with your setup? Even if you can't get 8-bit gray data from your sensor, hopefully you could observe it with any other format directly from the CCDC. I'll postpone further discussion until you confirm that you can reproduce the behavior. As the patch illustrates, it looks like it is hanging up in STREAMON. I've tested this with a serial CSI-2 sensor and a parallel sensor (MT9V032, in both 8-bit and 10-bit modes, albeit with SGRBG8 instead of GREY for the 8-bit mode), and I can't reproduce the issue. I thought I've asked you already but can't find this in my mailbox, so I apologize if I have, but could you try increasing vertical blanking and see if it helps ? I think that was the first time you suggested that. Indeed, if I stretch out the time between frames, the problem goes away. I haven't tested it precisely to see how long it needs to be to work correctly. But what does this tell me? This isn't a very appealing fix as 1) I would have to fish around for a minimum vertical blank time that works and 2) this would slow down the frame rate for the normal case, when frames are just being streamed uninterrupted. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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] [media] omap3isp: queue: fail QBUF if buffer is too small
Hi Laurent, On 08/05/2011 10:59 AM, Laurent Pinchart wrote: Hi Michael, Thanks for the patch. On Thursday 04 August 2011 17:40:37 Michael Jones wrote: Add buffer length to sanity checks for QBUF. Signed-off-by: Michael Jones michael.jo...@matrix-vision.de --- drivers/media/video/omap3isp/ispqueue.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/omap3isp/ispqueue.c b/drivers/media/video/omap3isp/ispqueue.c index 9c31714..4f6876f 100644 --- a/drivers/media/video/omap3isp/ispqueue.c +++ b/drivers/media/video/omap3isp/ispqueue.c @@ -867,6 +867,9 @@ int omap3isp_video_queue_qbuf(struct isp_video_queue *queue, if (buf-state != ISP_BUF_STATE_IDLE) goto done; +if (vbuf-length buf-vbuf.length) +goto done; + The vbuf-length value passed from userspace isn't used by the driver, so I'm not sure if verifying it is really useful. We verify the memory itself instead, to make sure that enough pages can be accessed. The application can always lie about the length, so we can't rely on it anyway. According to the spec, it's expected that the application set 'length': To enqueue a user pointer buffer applications set [...] length to its size. (Now that I say that, I realize I should only do this length check for USERPTR buffers.) If we don't at least sanity check it for the application, then it has no purpose at all on QBUF. If this is desirable, I would propose changing the spec. This patch was born of a mistake when my application set 624x480, which resulted in sizeimage=640x480=307200 but it used width height to calculate the buffer size rather than sizeimage or even to take bytesperline into account. It was then honest with QBUF, confessing that it wasn't providing enough space, but QBUF just went ahead. What followed were random crashes while data was DMA'd into memory not set aside for the buffer, while I assumed that the buffer size was OK because QBUF had succeeded and was looking elsewhere in the program for the culprit. I think it makes sense to give the app an error on QBUF in this situation. if (vbuf-memory == V4L2_MEMORY_USERPTR vbuf-m.userptr != buf-vbuf.m.userptr) { isp_video_buffer_cleanup(buf); MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier -- 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: USB mini-summit at LinuxCon Vancouver
Em 05-08-2011 04:45, Hans de Goede escreveu: Hi, On 08/05/2011 12:56 AM, Greg KH wrote: On Thu, Aug 04, 2011 at 07:21:47PM -0300, Mauro Carvalho Chehab wrote: I know that this problem were somewhat solved for 3G modems, with the usage of the userspace problem usb_modeswitch, and with some quirks for the USB storage driver, but I'm not sure if such tricks will scale forever, as more functions are seen on some USB devices. Well, no matter how it scales it needs to be done in userspace, like usb_modeswitch does. We made that decision a while ago, and it is working out very well. I see no reason why you can't do it in userspace as well as that is the easiest place to control this type of thing. I thought we had a long discussion about this topic a while ago and came to this very conclusion. Or am I mistaken? I think we've had multiple discussions about this, surrounding various topics / use cases. I would not call the do it in userspace a conclusion. I rather more have a feeling of getting stonewalled on this by various people surrounding the usb system. Me shutting up on this is basically a case of: Discussion not getting anywhere and I don't have the time to do a kernel proof of concept myself right now. To be clear about the stonewalling I'm not talking about the dual cam issue here, nor about the morphing devices thing which usb_modeswitch fixes. I think it is important to separate oranges from apples here, there are at least 3 different problem classes which all seem to have gotten thrown onto a pile here: 1) The reason Mauro suggested having some discussion on this at the USB summit is because of a discussion about dual mode cameras on the linux media list. Dual-mode cameras are (usually very cheap) digital photo cameras which can take still pictures in stand alone mode and store them in onboard memory, like regular digital photo cameras, this is one mode. The other mode is they can operate as a regular webcam, the hardware actually is more regular webcam hardware with some memory and a battery added (usually no viewfinder screen). Yes, that's my concern. These devices typically use 1 usb interface (and thus 1 driver) for both modes. I also want to cover the case where there are two drivers involved, as this is a typical trouble we need to do with all devices that have both analog and digital TV's. A typical media device exposes several different API's to userspace, each with its own character device, and sometimes implemented by a different driver. For example, Conexant cx23102 USB chipsets provide 2 USB interfaces, but require 3 different drivers, and exposes 4 different userspace API's to userspace: USB interface 0: Remote Controller event/input API - Handled by drivers/media/rc/mceusb.c USB interface 1: Audio/Video ALSA API - Handled by drivers/media/video/cx231xx/cx231xx-audio.c It is a separate driver, called cx231xx-alsa V4L2 API - Handled by drivers/media/video/cx231xx/cx231xx-video.c DVB API - Handled by drivers/media/video/cx231xx/cx231xx-dvb.c Both DVB and V4L2 are currently part of the same driver (cx231xx). The mceusb is completely independent of the other drivers. The cx231xx-alsa also is almost independent. It only shares a lock with the main cx231xx driver, and uses the I2C functions implemented by the main driver. Now, we're starting to see devices like ZTE MF845 that have TV, 3G and USB storage, exposing 2 more userspace API's to userspace (vfs and tty). I don't think that userspace usb_modeswitch-like type of locking scales such complex devices. Besides that, on a large amount of USB sticks, if you enable both V4L2 and DVB, the power consumption may exceed the 500mA power limit of USB, and can overheat the device. There are several reports of burnt devices due to that. Also, there's no way for both to work, as they share the same tuner, that can either be tuning an analog or a digital channel. To me, the right solution seems to add some sort of locking schema that will protect the common resources on complex hardware. Something like: int get_hw_resource(struct device, enum hw_resource_type); void put_hw_resource(struct device, enum hw_resource_type); enum hw_resource_type { HW_RES_USB, HW_RES_I2C, HW_RES_TUNER, HW_RES_USB, ... }; So that, when a device need, for example, an exclusive lock for the USB device, for example, it could be doing something like: rc = get_hw_resource(dev, HW_RES_USB); if (rc 0) return -EBUSY; and, after finishing, doing a put_hw_resource(). We currently have drivers for both modes, but they are separate drivers, which leads to fighting for device ownership, with the stillmode driver always winning as that is userspace and it simply kicks of the kernel webcam /streaming mode driver making it see a device unplug, even if an app was streaming from the device at the time. The solution
Re: Possible issue in videobuf2 with buffer length check at QBUF time
Hi Laurent, On Fri, Aug 5, 2011 at 01:55, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Marek and Pawel, While reviewing an OMAP3 ISP patch, I noticed that videobuf2 doesn't verify the buffer length field value when a new USERPTR buffer is queued. That's a good catch. We should definitely do something about it. The length given by userspace is copied to the internal buffer length field. It seems to be up to drivers to verify that the value is equal to or higher than the minimum required to store the image, but that's not clearly documented. Should the buf_init operation be made mandatory for drivers that support USERPTR, and documented as required to implement a length check ? Technically, drivers can do the length checks on buf_prepare if they don't allow format changes after REQBUFS. On the other hand though, if a driver supports USERPTR, the buffers queued from userspace have to be verified on qbuf and the only place to do that would be buf_init. So every driver that supports USERPTR would have to implement buf_init, as you said. Alternatively the check could be performed in videobuf2-core, which would probably make sense as the check is required. videobuf2 doesn't store the minimum size for USERPTR buffers though (but that can of course be changed). Let's say we make vb2 save minimum buffer size. This would have to be done on queue_setup I imagine. We could add a new field to vb2_buffer for that. One problem is that if the driver actually supports changing format after REQBUFS, we would need a new function in vb2 to change minimum buffer size. Actually, this has to be minimum plane sizes. So the alternatives are: 1. Make buf_init required for drivers that support USERPTR; or 2. Add minimum plane sizes to vb2_buffer,add a new return array argument to queue_setup to return minimum plane sizes that would be stored in vb2. Make vb2 verify sizes on qbuf of USERPTR. Add a new vb2 function for drivers to call when minimum sizes have to be changed. The first solution is way simpler for drivers that require this. The second solution is maybe a bit simpler for drivers that do not, as they would only have to return the sizes in queue_setup, but implementing buf_init instead wouldn't be a big of a difference I think. So I'm leaning towards the second solution. Any comments, did I miss something? -- Best regards, Pawel Osciak -- 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: Possible issue in videobuf2 with buffer length check at QBUF time
Sorry, just realized I mixed up callback names a bit in my previous response. Let me rephrase: Drivers may allow changing formats after buffers have been initialized. This means that it's possible to s_fmt, reqbufs, streamon, streamoff and s_fmt again without changing buffers. buf_init will still be called for USERPTR though, even if the same buffers are used after resume. So enforcing buf_init will always work. If buf_init were not to be required for USERPTR, we'd need the drivers to report change of minimum buffer (planes) size asynchronously to vb2 from the driver. And get them on alloc from the driver on queue_setup. So the two options I described in my first response are still standing, although I made a typo there, saying I'd prefer the second solution, I obviously meant the first one, i.e. enforcing buf_init(). Sorry. One additional detail to the second proposal below: if we were to have the new callback, it could only be allowed when not streaming. Otherwise we'd have to forcibly dequeue buffers if they became too small and this would be confusing for applications. Pawel On Fri, Aug 5, 2011 at 08:01, Pawel Osciak pa...@osciak.com wrote: Hi Laurent, On Fri, Aug 5, 2011 at 01:55, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Marek and Pawel, While reviewing an OMAP3 ISP patch, I noticed that videobuf2 doesn't verify the buffer length field value when a new USERPTR buffer is queued. That's a good catch. We should definitely do something about it. The length given by userspace is copied to the internal buffer length field. It seems to be up to drivers to verify that the value is equal to or higher than the minimum required to store the image, but that's not clearly documented. Should the buf_init operation be made mandatory for drivers that support USERPTR, and documented as required to implement a length check ? Technically, drivers can do the length checks on buf_prepare if they don't allow format changes after REQBUFS. On the other hand though, if a driver supports USERPTR, the buffers queued from userspace have to be verified on qbuf and the only place to do that would be buf_init. So every driver that supports USERPTR would have to implement buf_init, as you said. Alternatively the check could be performed in videobuf2-core, which would probably make sense as the check is required. videobuf2 doesn't store the minimum size for USERPTR buffers though (but that can of course be changed). Let's say we make vb2 save minimum buffer size. This would have to be done on queue_setup I imagine. We could add a new field to vb2_buffer for that. One problem is that if the driver actually supports changing format after REQBUFS, we would need a new function in vb2 to change minimum buffer size. Actually, this has to be minimum plane sizes. So the alternatives are: 1. Make buf_init required for drivers that support USERPTR; or 2. Add minimum plane sizes to vb2_buffer,add a new return array argument to queue_setup to return minimum plane sizes that would be stored in vb2. Make vb2 verify sizes on qbuf of USERPTR. Add a new vb2 function for drivers to call when minimum sizes have to be changed. The first solution is way simpler for drivers that require this. The second solution is maybe a bit simpler for drivers that do not, as they would only have to return the sizes in queue_setup, but implementing buf_init instead wouldn't be a big of a difference I think. So I'm leaning towards the second solution. Any comments, did I miss something? -- 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: [Workshop-2011] Media Subsystem Workshop 2011
On Fri, 5 Aug 2011, Hans de Goede wrote: Hi all, On 08/04/2011 02:34 PM, Mauro Carvalho Chehab wrote: Em 03-08-2011 20:20, Theodore Kilgore escreveu: snip snip Yes, that kind of thing is an obvious problem. Actually, though, it may be that this had just better not happen. For some of the hardware that I know of, it could be a real problem no matter what approach would be taken. For example, certain specific dual-mode cameras will delete all data stored on the camera if the camera is fired up in webcam mode. To drop Gphoto suddenly in order to do the videoconf call would, on such cameras, result in the automatic deletion of all photos on the camera even if those photos had not yet been downloaded. Presumably, one would not want to do that. So, in other words, the Kernel driver should return -EBUSY if on such cameras, if there are photos stored on them, and someone tries to stream. Agreed. Here, too. Not only that, but also -EBUSY needs to be returned if streaming is being done and someone tries to download photos (cf. yesterday's exchange between me and Adam Baker, where it was definitely established that this currently leads to bad stuff happening). IMO, the right solution is to work on a proper snapshot mode, in kernelspace, and moving the drivers that have already a kernelspace out of Gphoto. Well, the problem with that is, a still camera and a webcam are entirely different beasts. Still photos stored in the memory of an external device, waiting to be downloaded, are not snapshots. Thus, access to those still photos is not access to snapshots. Things are not that simple. Yes, stored photos require a different API, as Hans pointed. I think that some cameras just export them as a USB storage. Erm, that is not what I tried to say, or do you mean another Hans? For the record, this one didn't come from me, either. :-) snip snip If I understood you well, there are 4 possible ways: 1) UVC + USB mass storage; 2) UVC + Vendor Class mass storage; 3) Vendor Class video + USB mass storage; 4) Vendor Class video + Vendor Class mass storage. Actually the cameras Theodore and I are talking about here all fall into category 4. Currently true, yes. I expect devices which do any of 1-3 to properly use different interfaces for this, actually the different class specifications mandate that they use different interfaces for this. As is well known, *everybody* obeys the class specifications, too. Always did, and always will. And Linus says that he got the original kernel from the Tooth Fairy, and because he said that we all believe him. The point being, trouble will very likely come along. I think Mauro is right at least to consider the possibility. This sounds to be a good theme for the Workshop, or even to KS/2011. Agreed, although we don't need to talk about this for very long, the solution is basically: 1) Define a still image retrieval API for v4l2 devices (there is only 1 interface for both functions on these devices, True so only 1 driver, and to me it makes sense to extend the existing drivers to also do still image retrieval). 2) Modify existing kernel v4l2 drivers to provide this API 3) Write a new libgphoto driver which talks this interface (only need to do one driver since all dual mode cams will export the same API). Yes, we pretty much agree that this is probably a good way to proceed. However, my curiosity is aroused by something that Adam mentioned yesterday. Namely If you can solve the locking problem between devices in the kernel then it shouldn't matter if one of the kernel devices is the generic device that is used to support libusb. I am not completely sure of what he meant here. I am not intimately conversant with the internals of libusb. However, is there something here which could be used constructively? Could things be set up so that, for example, the kernel module hands the generic device over to libusb? If it were possible to do things that way, it might be the most minimally disruptive approach of all, since it might not require much if any changes in libgphoto2 access to cameras. 1) is something to discuss at the workshop. Regards, Hans Theodore Kilgore -- 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: USB mini-summit at LinuxCon Vancouver
On Fri, 5 Aug 2011, Oliver Neukum wrote: Am Freitag, 5. August 2011, 00:56:03 schrieb Greg KH: On Thu, Aug 04, 2011 at 07:21:47PM -0300, Mauro Carvalho Chehab wrote: I know that this problem were somewhat solved for 3G modems, with the usage of the userspace problem usb_modeswitch, and with some quirks for the USB storage driver, but I'm not sure if such tricks will scale forever, as more functions are seen on some USB devices. Well, no matter how it scales it needs to be done in userspace, like usb_modeswitch does. We made that decision a while ago, and it is working out very well. I see no reason why you can't do it in userspace as well as that is the easiest place to control this type of thing. I thought we had a long discussion about this topic a while ago and came to this very conclusion. Or am I mistaken? Circumstances change. We want to keep the stuff in user space as much and as long as we can. However user space has limitations: - it has by necessity a race between resumption and access by others - it cannot resume anything we run a (rw) filesystem over. Furthermore, today PM actions that lead to a loss of mode are initiated by user space. If we ever want to oportunistically suspend a system we also need to restore mode from inside the kernel. We could avoid all that trouble, if we persuaded vendors to use plain USB configurations for those purposes. But that would happen, I suspect, in an alternate universe. Better to anticipate the trouble, I suspect. :-{ Moreover, the spark for the current discussion was the problem of dual-mode cameras, which can work both as webcams and stillcams, not the 3G modems that you mention. The problems are analogous but not identical. -- dual-mode cameras are, typically, Class Proprietary devices in all of their functions. None of them that I know of are Mass Storage devices. Therefore, usb_modeswitch would have to be rewritten completely in order to be used for such hardware. As things stand right now, it has nothing at all to do with the problem. Not to say, of course, that the experience gained with usb_modeswitch is totally irrelevant. -- I don't have one of those modems, but I have the impression that the need to access the disk partition on one of them is basically a one-shot deal. Namely, one needs to load some firmware or so on the disk before the gadget can be used. The problem with a dual-mode camera is that the user ought to be able to switch at will between a download-the-pictures stillcam application and a stream application. While, of course, not being able to start one of these activities while the other is going on, because that would cause obvious trouble. The similarity of the modem-and-mass-storage device and the web-and-still camera is, of course, that both partake of being devices that will do more than one kind of thing and they need to be supported in that respect. Other hardware exists with similar characteristics, but sometimes the functionaliy is not dual but even triple, and one can reasonably suspect that more of this kind of thing is going to come at us in the future. I think it is a good occasion to sit back and think things over a bit. Theodore Kilgore -- 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] v4l-dvb daily build: ERRORS
This message is generated daily by a cron job that builds v4l-dvb for the kernels and architectures in the list below. Results of the daily build of v4l-dvb: date:Fri Aug 5 19:00:59 CEST 2011 git hash:46540f7ac646ada7f22912ea7ea9b761ff5c4718 gcc version: i686-linux-gcc (GCC) 4.5.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: WARNINGS linux-git-armv5-davinci: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-armv5-omap2: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: ERRORS linux-2.6.33-i686: ERRORS linux-2.6.34-i686: ERRORS linux-2.6.35.3-i686: ERRORS linux-2.6.36-i686: ERRORS linux-2.6.37-i686: ERRORS linux-2.6.38.2-i686: ERRORS linux-2.6.39.1-i686: ERRORS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: ERRORS linux-2.6.33-x86_64: ERRORS linux-2.6.34-x86_64: ERRORS linux-2.6.35.3-x86_64: ERRORS linux-2.6.36-x86_64: ERRORS linux-2.6.37-x86_64: ERRORS linux-2.6.38.2-x86_64: ERRORS linux-2.6.39.1-x86_64: ERRORS spec-git: ERRORS 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 V4L-DVB specification 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] [media] OMAP_VOUT: Fix build break caused by update_mode removal in DSS2
Op 5 aug 2011, om 09:19 heeft Archit Taneja het volgende geschreven: The DSS2 driver does not support the configuration of the update_mode of a panel anymore. Remove the setting of update_mode done in omap_vout_probe(). Ignore configuration of TE since omap_vout driver doesn't support manual update displays anyway. Signed-off-by: Archit Taneja arc...@ti.com Tested-by: Koen Kooi k...@dominion.thruhere.net --- drivers/media/video/omap/omap_vout.c | 13 - 1 files changed, 0 insertions(+), 13 deletions(-) diff --git a/drivers/media/video/omap/omap_vout.c b/drivers/media/ video/omap/omap_vout.c index b5ef362..b3a5ecd 100644 --- a/drivers/media/video/omap/omap_vout.c +++ b/drivers/media/video/omap/omap_vout.c @@ -2194,19 +2194,6 @@ static int __init omap_vout_probe(struct platform_device *pdev) '%s' Display already enabled\n, def_display-name); } - /* set the update mode */ - if (def_display-caps - OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE) { - if (dssdrv-enable_te) - dssdrv-enable_te(def_display, 0); - if (dssdrv-set_update_mode) - dssdrv-set_update_mode(def_display, - OMAP_DSS_UPDATE_MANUAL); - } else { - if (dssdrv-set_update_mode) - dssdrv-set_update_mode(def_display, - OMAP_DSS_UPDATE_AUTO); - } } } -- 1.7.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
Guidance regarding deferred I2C transactions
I'm looking for some guidance regarding a clean way to support a certain camera module. We are using the soc_camera framework, and in particular, the ov9740.c image sensor driver. This camera module has the camera activity status LED tied to the image sensor's power. This is meant as a security feature, so that there is no way to turn the camera on without the user being informed through the status LED. The problem with this is that any I2C transaction to the image sensor necessitates turning the image sensor on, which results in the status LED turning on. Various methods in the soc camera sensor drivers typically perform I2C transactions. For example, probe will check the image sensor registers to validate device presence. However, this results in the LED blinking during probe, which can be misconstrued as the camera having taken an actual picture. Opening the /dev/video node will also typically blink the status LED for similar reasons (in this case, calling the s_mbus_fmt video op), so any application probing for camera presence will cause the status LED to blink. One way to solve this can be to defer these I2C transactions in the image sensor driver all the way up to the time the image sensor is asked to start streaming frames. However, it seems to me that this breaks the spirit of the probe; applications will successfully probe for camera presence even though the camera isn't actually there. Is this okay? Is there a better way to do this? Maybe a more general thing we can add to the V4L2 framework? --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- 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: Guidance regarding deferred I2C transactions
Em 05-08-2011 17:18, Andrew Chew escreveu: I'm looking for some guidance regarding a clean way to support a certain camera module. We are using the soc_camera framework, and in particular, the ov9740.c image sensor driver. This camera module has the camera activity status LED tied to the image sensor's power. This is meant as a security feature, so that there is no way to turn the camera on without the user being informed through the status LED. The problem with this is that any I2C transaction to the image sensor necessitates turning the image sensor on, which results in the status LED turning on. Various methods in the soc camera sensor drivers typically perform I2C transactions. For example, probe will check the image sensor registers to validate device presence. However, this results in the LED blinking during probe, which can be misconstrued as the camera having taken an actual picture. Opening the /dev/video node will also typically blink the status LED for similar reasons (in this case, calling the s_mbus_fmt video op), so any application probing for camera presence will cause the status LED to blink. One way to solve this can be to defer these I2C transactions in the image sensor driver all the way up to the time the image sensor is asked to start streaming frames. However, it seems to me that this breaks the spirit of the probe; applications will successfully probe for camera presence even though the camera isn't actually there. Is this okay? Is there a better way to do this? Maybe a more general thing we can add to the V4L2 framework? Probing for the presence of the device hardware at driver init time seems to be the right thing to do, even when the LED blinks. PC keyboard LEDs also blinks during machine reset, and this is not really annoying. Even on some embedded devices like some cell phones, LEDs blink during the boot time. So, as a general rule, I'd say that the better is to keep the capability of probing the hardware at init time, especially since the same sensor may eventually be used by non SoC drivers. One strategy that several drivers do, and that solves the issue of blinking after the device reset is to have a shadow copy of the register contents. This way, you can defer the device register writes to occur only when you're actually streaming. E. g. you'll still have the blink at probe time (probably a longer one), but, after that, the driver can just work with the cached values, up to the moment it will really start streaming. Would that strategy work for you? Regards, Mauro -- 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: Guidance regarding deferred I2C transactions
One way to solve this can be to defer these I2C transactions in the image sensor driver all the way up to the time the image sensor is asked to start streaming frames. However, it seems to me that this breaks the spirit of the probe; applications will successfully probe for camera presence even though the camera isn't actually there. Is this okay? Is there a better way to do this? Maybe a more general thing we can add to the V4L2 framework? Probing for the presence of the device hardware at driver init time seems to be the right thing to do, even when the LED blinks. PC keyboard LEDs also blinks during machine reset, and this is not really annoying. Even on some embedded devices like some cell phones, LEDs blink during the boot time. It's a bit different when the camera LED blinks, though. The whole problem is that the user will have thought that the system took a picture of them without knowing. What the user sees will potentially be indistinguishable between expected behavior, and a system that has been compromised to make use of that blink to actually take a picture, leading to privacy concerns. So, as a general rule, I'd say that the better is to keep the capability of probing the hardware at init time, especially since the same sensor may eventually be used by non SoC drivers. I completely agree with you. I was just hoping that others have run into this as well, and that there was an officially endorsed method to solve this. Sounds like there isn't. One strategy that several drivers do, and that solves the issue of blinking after the device reset is to have a shadow copy of the register contents. This way, you can defer the device register writes to occur only when you're actually streaming. E. g. you'll still have the blink at probe time (probably a longer one), but, after that, the driver can just work with the cached values, up to the moment it will really start streaming. Yes, that's easy to do, and will completely solve the blink on open issue. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- 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 1/3] IT9137 driver for Kworld UB499-2T T09 (id 1b80:e409) - firmware details
On Mon, 2011-07-25 at 19:34 +0100, Malcolm Priestley wrote: Firmware information for Kworld UB499-2T T09 based on IT913x series. This device uses file dvb-usb-it9137-01.fw. Signed-off-by: Malcolm Priestley tvbox...@gmail.com --- Documentation/dvb/it9137.txt |9 + 1 files changed, 9 insertions(+), 0 deletions(-) create mode 100644 Documentation/dvb/it9137.txt diff --git a/Documentation/dvb/it9137.txt b/Documentation/dvb/it9137.txt new file mode 100644 index 000..9e6726e --- /dev/null +++ b/Documentation/dvb/it9137.txt @@ -0,0 +1,9 @@ +To extract firmware for Kworld UB499-2T (id 1b80:e409) you need to copy the +following file(s) to this directory. + +IT9135BDA.sys Dated Mon 22 Mar 2010 02:20:08 GMT + +extract using dd +dd if=IT9135BDA.sys ibs=1 skip=69632 count=5731 of=dvb-usb-it9137-01.fw + +copy to default firmware location. In light of today's patch from the chip vendor, I am changing the patchwork status to under review. There is no point in continuing with these patches at this stage. -- 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/6 v5] V4L: vb2: prepare to support multi-size buffers
In preparation for the forthcoming VIDIOC_CREATE_BUFS ioctl change the type of the sizes[] argument of the .queue_setup() vb2 operation from unsigned long to unsigned int to match with the __u32 type of buffer size variables elsewhere in V4L2. Drivers will also need the fourcc value, passed along with the new ioctl(), to correctly set up the queue, it has to be supplied with the .buf_init() vb2 method. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- v5: the fourcc value, that we now send with the CREATE_BUFS ioctl() has to be passed on to the drivers. Add it to the vb2 .buf_init() method. drivers/media/video/atmel-isi.c |4 ++-- drivers/media/video/marvell-ccic/mcam-core.c |4 ++-- drivers/media/video/mem2mem_testdev.c|2 +- drivers/media/video/mx3_camera.c |4 ++-- drivers/media/video/pwc/pwc-if.c |4 ++-- drivers/media/video/s5p-fimc/fimc-capture.c |2 +- drivers/media/video/s5p-fimc/fimc-core.c |2 +- drivers/media/video/s5p-mfc/s5p_mfc_dec.c|4 ++-- drivers/media/video/s5p-mfc/s5p_mfc_enc.c|4 ++-- drivers/media/video/s5p-tv/mixer_video.c |2 +- drivers/media/video/sh_mobile_ceu_camera.c |6 +++--- drivers/media/video/videobuf2-core.c | 10 +- drivers/media/video/vivi.c |4 ++-- include/media/videobuf2-core.h |4 ++-- 14 files changed, 28 insertions(+), 28 deletions(-) diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c index 3e3d4cc..11b6d54 100644 --- a/drivers/media/video/atmel-isi.c +++ b/drivers/media/video/atmel-isi.c @@ -250,7 +250,7 @@ static int atmel_isi_wait_status(struct atmel_isi *isi, int wait_reset) Videobuf operations --*/ static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers, - unsigned int *nplanes, unsigned long sizes[], + unsigned int *nplanes, unsigned int sizes[], void *alloc_ctxs[]) { struct soc_camera_device *icd = soc_camera_from_vb2q(vq); @@ -295,7 +295,7 @@ static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers, return 0; } -static int buffer_init(struct vb2_buffer *vb) +static int buffer_init(struct vb2_buffer *vb, u32 fourcc) { struct frame_buffer *buf = container_of(vb, struct frame_buffer, vb); diff --git a/drivers/media/video/marvell-ccic/mcam-core.c b/drivers/media/video/marvell-ccic/mcam-core.c index 83c1451..262b861 100644 --- a/drivers/media/video/marvell-ccic/mcam-core.c +++ b/drivers/media/video/marvell-ccic/mcam-core.c @@ -884,7 +884,7 @@ static int mcam_read_setup(struct mcam_camera *cam) */ static int mcam_vb_queue_setup(struct vb2_queue *vq, unsigned int *nbufs, - unsigned int *num_planes, unsigned long sizes[], + unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]) { struct mcam_camera *cam = vb2_get_drv_priv(vq); @@ -1000,7 +1000,7 @@ static const struct vb2_ops mcam_vb2_ops = { * Scatter/gather mode uses all of the above functions plus a * few extras to deal with DMA mapping. */ -static int mcam_vb_sg_buf_init(struct vb2_buffer *vb) +static int mcam_vb_sg_buf_init(struct vb2_buffer *vb, u32 fourcc) { struct mcam_vb_buffer *mvb = vb_to_mvb(vb); struct mcam_camera *cam = vb2_get_drv_priv(vb-vb2_queue); diff --git a/drivers/media/video/mem2mem_testdev.c b/drivers/media/video/mem2mem_testdev.c index 166bf93..0d0c0d5 100644 --- a/drivers/media/video/mem2mem_testdev.c +++ b/drivers/media/video/mem2mem_testdev.c @@ -739,7 +739,7 @@ static const struct v4l2_ioctl_ops m2mtest_ioctl_ops = { */ static int m2mtest_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers, - unsigned int *nplanes, unsigned long sizes[], + unsigned int *nplanes, unsigned int sizes[], void *alloc_ctxs[]) { struct m2mtest_ctx *ctx = vb2_get_drv_priv(vq); diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c index 3f37522f..b48b2a4 100644 --- a/drivers/media/video/mx3_camera.c +++ b/drivers/media/video/mx3_camera.c @@ -192,7 +192,7 @@ static void mx3_cam_dma_done(void *arg) */ static int mx3_videobuf_setup(struct vb2_queue *vq, unsigned int *count, unsigned int *num_planes, - unsigned long sizes[], void *alloc_ctxs[]) + unsigned int sizes[], void *alloc_ctxs[]) { struct soc_camera_device *icd = soc_camera_from_vb2q(vq); struct soc_camera_host *ici = to_soc_camera_host(icd-parent); @@ -387,7 +387,7 @@ static void mx3_videobuf_release(struct vb2_buffer *vb) spin_unlock_irqrestore(mx3_cam-lock, flags); } -static int
[PATCH 5/6 v5] V4L: sh-mobile-ceu-camera: prepare to support multi-size buffers
Prepare the sh_mobile_ceu_camera friver to support the new VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be able to handle buffer sizes, provided by the caller, and the .buf_prepare() operation must not use the currently configured frame format for its operation. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- v5: fix error handling in .buf_queue(), correct buffer accounting. drivers/media/video/sh_mobile_ceu_camera.c | 116 +--- 1 files changed, 71 insertions(+), 45 deletions(-) diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c index 583d771..6a39f9b 100644 --- a/drivers/media/video/sh_mobile_ceu_camera.c +++ b/drivers/media/video/sh_mobile_ceu_camera.c @@ -90,7 +90,6 @@ struct sh_mobile_ceu_buffer { struct vb2_buffer vb; /* v4l buffer must be first */ struct list_head queue; - enum v4l2_mbus_pixelcode code; }; struct sh_mobile_ceu_dev { @@ -100,7 +99,8 @@ struct sh_mobile_ceu_dev { unsigned int irq; void __iomem *base; - unsigned long video_limit; + size_t video_limit; + size_t buf_total; spinlock_t lock;/* Protects video buffer lists */ struct list_head capture; @@ -192,6 +192,12 @@ static int sh_mobile_ceu_soft_reset(struct sh_mobile_ceu_dev *pcdev) /* * Videobuf operations */ + +/* + * .queue_setup() is called to check, whether the driver can accept the + * requested number of buffers and to fill in plane sizes + * for the current frame format if required + */ static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq, unsigned int *count, unsigned int *num_planes, unsigned int sizes[], void *alloc_ctxs[]) @@ -199,25 +205,34 @@ static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq, struct soc_camera_device *icd = container_of(vq, struct soc_camera_device, vb2_vidq); struct soc_camera_host *ici = to_soc_camera_host(icd-parent); struct sh_mobile_ceu_dev *pcdev = ici-priv; - int bytes_per_line = soc_mbus_bytes_per_line(icd-user_width, + size_t size; + + if (!sizes[0] || !*num_planes) { + /* Called from VIDIOC_REQBUFS or in compatibility mode */ + int bytes_per_line = soc_mbus_bytes_per_line(icd-user_width, icd-current_fmt-host_fmt); + if (bytes_per_line 0) + return bytes_per_line; - if (bytes_per_line 0) - return bytes_per_line; + sizes[0] = bytes_per_line * icd-user_height; - *num_planes = 1; + *num_planes = 1; + } - pcdev-sequence = 0; - sizes[0] = bytes_per_line * icd-user_height; alloc_ctxs[0] = pcdev-alloc_ctx; + if (!vq-num_buffers) + pcdev-sequence = 0; + if (!*count) *count = 2; - if (pcdev-video_limit) { - if (PAGE_ALIGN(sizes[0]) * *count pcdev-video_limit) - *count = pcdev-video_limit / PAGE_ALIGN(sizes[0]); - } + size = PAGE_ALIGN(sizes[0]) * *count; + + if (pcdev-video_limit + size + pcdev-buf_total pcdev-video_limit) + *count = (pcdev-video_limit - pcdev-buf_total) / + PAGE_ALIGN(sizes[0]); dev_dbg(icd-parent, count=%d, size=%u\n, *count, sizes[0]); @@ -330,23 +345,40 @@ static int sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev) static int sh_mobile_ceu_videobuf_prepare(struct vb2_buffer *vb) { + struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb); + + /* Added list head initialization on alloc */ + WARN(!list_empty(buf-queue), Buffer %p on queue!\n, vb); + + return 0; +} + +static void sh_mobile_ceu_videobuf_queue(struct vb2_buffer *vb) +{ struct soc_camera_device *icd = container_of(vb-vb2_queue, struct soc_camera_device, vb2_vidq); - struct sh_mobile_ceu_buffer *buf; + struct soc_camera_host *ici = to_soc_camera_host(icd-parent); + struct sh_mobile_ceu_dev *pcdev = ici-priv; + struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb); + unsigned long size; int bytes_per_line = soc_mbus_bytes_per_line(icd-user_width, icd-current_fmt-host_fmt); - unsigned long size; if (bytes_per_line 0) - return bytes_per_line; + goto error; + + size = icd-user_height * bytes_per_line; - buf = to_ceu_vb(vb); + if (vb2_plane_size(vb, 0) size) { + dev_err(icd-parent, Buffer #%d too small (%lu %lu)\n, + vb-v4l2_buf.index, vb2_plane_size(vb, 0), size); + goto error; + } + + vb2_set_plane_payload(vb, 0, size);
RE: Guidance regarding deferred I2C transactions
On Fri, 5 Aug 2011, Andrew Chew wrote: One way to solve this can be to defer these I2C transactions in the image sensor driver all the way up to the time the image sensor is asked to start streaming frames. However, it seems to me that this breaks the spirit of the probe; applications will successfully probe for camera presence even though the camera isn't actually there. Is this okay? Is there a better way to do this? Maybe a more general thing we can add to the V4L2 framework? Probing for the presence of the device hardware at driver init time seems to be the right thing to do, even when the LED blinks. PC keyboard LEDs also blinks during machine reset, and this is not really annoying. Even on some embedded devices like some cell phones, LEDs blink during the boot time. It's a bit different when the camera LED blinks, though. The whole problem is that the user will have thought that the system took a picture of them without knowing. What the user sees will potentially be indistinguishable between expected behavior, and a system that has been compromised to make use of that blink to actually take a picture, leading to privacy concerns. So, as a general rule, I'd say that the better is to keep the capability of probing the hardware at init time, especially since the same sensor may eventually be used by non SoC drivers. I completely agree with you. I was just hoping that others have run into this as well, and that there was an officially endorsed method to solve this. Sounds like there isn't. One strategy that several drivers do, and that solves the issue of blinking after the device reset is to have a shadow copy of the register contents. This way, you can defer the device register writes to occur only when you're actually streaming. E. g. you'll still have the blink at probe time (probably a longer one), but, after that, the driver can just work with the cached values, up to the moment it will really start streaming. Yes, that's easy to do, and will completely solve the blink on open issue. This would require modifying each sensor driver. And not always these shadowing is desired. A simpler approach seems to be to only load the driver, when streaming is required. Yes, it would add a (considerable) delay to streaming begin, but you'd be completely honest to your user and privacy would be guaranteed. Another less secure approach would be to tie your LED to a different function, to one, that only activates, when actual data is transferred. Maybe you can tie it to one of sync or clock signals? Look whether your sensor has any pins, that only get activated, when video data is transferred. You can also trigger it from the software, but this might contradict your security policy. However, if you think about it, I don't think your users anyway have a chance to make really 100% sure, their privacy is not violated, so... Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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: Guidance regarding deferred I2C transactions
Em 05-08-2011 20:16, Guennadi Liakhovetski escreveu: On Fri, 5 Aug 2011, Andrew Chew wrote: One way to solve this can be to defer these I2C transactions in the image sensor driver all the way up to the time the image sensor is asked to start streaming frames. However, it seems to me that this breaks the spirit of the probe; applications will successfully probe for camera presence even though the camera isn't actually there. Is this okay? Is there a better way to do this? Maybe a more general thing we can add to the V4L2 framework? Probing for the presence of the device hardware at driver init time seems to be the right thing to do, even when the LED blinks. PC keyboard LEDs also blinks during machine reset, and this is not really annoying. Even on some embedded devices like some cell phones, LEDs blink during the boot time. It's a bit different when the camera LED blinks, though. The whole problem is that the user will have thought that the system took a picture of them without knowing. What the user sees will potentially be indistinguishable between expected behavior, and a system that has been compromised to make use of that blink to actually take a picture, leading to privacy concerns. So, as a general rule, I'd say that the better is to keep the capability of probing the hardware at init time, especially since the same sensor may eventually be used by non SoC drivers. I completely agree with you. I was just hoping that others have run into this as well, and that there was an officially endorsed method to solve this. Sounds like there isn't. One strategy that several drivers do, and that solves the issue of blinking after the device reset is to have a shadow copy of the register contents. This way, you can defer the device register writes to occur only when you're actually streaming. E. g. you'll still have the blink at probe time (probably a longer one), but, after that, the driver can just work with the cached values, up to the moment it will really start streaming. Yes, that's easy to do, and will completely solve the blink on open issue. This would require modifying each sensor driver. Why? Only the one(s) that have such trouble will need that. Also, a config option for the device init may enable or disable such feature. And not always these shadowing is desired. It should be checked case by case. On some cases where shadowing is used, developers have no option, as some registers are write-only. On other cases, this is needed, in order to support PM operations (like suspend/resume). A simpler approach seems to be to only load the driver, when streaming is required. Yes, it would add a (considerable) delay to streaming begin, but you'd be completely honest to your user and privacy would be guaranteed. The delay will likely be close to the one introduced by flushing the shadow registers: it will basically be the needed time to transfer all registers data via I2C. Another less secure approach would be to tie your LED to a different function, to one, that only activates, when actual data is transferred. Maybe you can tie it to one of sync or clock signals? Look whether your sensor has any pins, that only get activated, when video data is transferred. I agree. If the hardware design is not finished, this is the best way of doing that. It is safer and more honest than attaching the led to the I2C bus, as it will be monitoring the actual data transfer, an not the control bus. You can also trigger it from the software, but this might contradict your security policy. However, if you think about it, I don't think your users anyway have a chance to make really 100% sure, their privacy is not violated, so... Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 -- 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: linux-next: Tree for Aug 5 (media/radio/radio-sf16fmr2)
On Fri, 5 Aug 2011 14:31:03 +1000 Stephen Rothwell wrote: Hi all, [The kernel.org mirroring is running slowly today] Is media/radio/radio-sf16fmr2 an ISA driver or a PCI driver? ugh. Or is it an I2C driver? linux-next fails with (this is not a new failure): ERROR: snd_tea575x_init [drivers/media/radio/radio-sf16fmr2.ko] undefined! ERROR: snd_tea575x_exit [drivers/media/radio/radio-sf16fmr2.ko] undefined! The Kconfig entry for RADIO_SF16FMR2 is: config RADIO_SF16FMR2 tristate SF16FMR2 Radio depends on ISA VIDEO_V4L2 SND and the Kconfig entry for SND_TEA575X is (not user visible): config SND_TEA575X tristate depends on SND_FM801_TEA575X_BOOL || SND_ES1968_RADIO || RADIO_SF16FMR2 default SND_FM801 || SND_ES1968 || RADIO_SF16FMR2 This latter entry is in sound/pci/Kconfig and is under: if SND_PCI so it depends on PCI and SND_PCI. This build fails when CONFIG_PCI is not enabled. Suggestions? thanks, --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** -- 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: Guidance regarding deferred I2C transactions
A simpler approach seems to be to only load the driver, when streaming is required. Yes, it would add a (considerable) delay to streaming begin, but you'd be completely honest to your user and privacy would be guaranteed. The delay will likely be close to the one introduced by flushing the shadow registers: it will basically be the needed time to transfer all registers data via I2C. I would think the delay is potentially a lot worse, as the module has to be read from the filesystem (if the image sensor driver was built as a module). Another less secure approach would be to tie your LED to a different function, to one, that only activates, when actual data is transferred. Maybe you can tie it to one of sync or clock signals? Look whether your sensor has any pins, that only get activated, when video data is transferred. I agree. If the hardware design is not finished, this is the best way of doing that. It is safer and more honest than attaching the led to the I2C bus, as it will be monitoring the actual data transfer, an not the control bus. I completely agree on this as well. Sadly, that doesn't seem to be an option at this point. --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- 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: RTL2831U driver updates
Hi, With the latest driver my card never gets a signal lock, not even once. As before there are no error messages. It does always probe correctly now though. On 5 August 2011 00:43, Antti Palosaari cr...@iki.fi wrote: Hello, I have done some updates. MXL5005S based RTL2831U devices didn't worked due to bug. That's main visible change. Secondly I added basic support for RTL2832U to rtl28xx driver. And implemented I2C as I see it really is, I think it is almost perfect now. It works fine my RTL2832U device with stubbed demod and tuner drivers. -- Alistair Buxton a.j.bux...@gmail.com -- 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] usb-urb.c: usb_bulk_urb_init and usb_isoc_urb_init
If there is not enough memory the allocated urb_list will not be freed. Signed-off-by: Manoel Pinheiro pinus...@hotmail.com --- drivers/media/dvb/dvb-usb/usb-urb.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/dvb/dvb-usb/usb-urb.c b/drivers/media/dvb/dvb-usb/usb-urb.c index 86d6893..d62ee0f 100644 --- a/drivers/media/dvb/dvb-usb/usb-urb.c +++ b/drivers/media/dvb/dvb-usb/usb-urb.c @@ -148,7 +148,7 @@ static int usb_bulk_urb_init(struct usb_data_stream *stream) if (!stream-urb_list[i]) { deb_mem(not enough memory for urb_alloc_urb!.\n); for (j = 0; j i; j++) - usb_free_urb(stream-urb_list[i]); + usb_free_urb(stream-urb_list[j]); return -ENOMEM; } usb_fill_bulk_urb( stream-urb_list[i], stream-udev, @@ -181,7 +181,7 @@ static int usb_isoc_urb_init(struct usb_data_stream *stream) if (!stream-urb_list[i]) { deb_mem(not enough memory for urb_alloc_urb!\n); for (j = 0; j i; j++) - usb_free_urb(stream-urb_list[i]); + usb_free_urb(stream-urb_list[j]); return -ENOMEM; } -- 1.7.4.4 -- 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: RTL2831U driver updates
On 6 August 2011 04:56, Alistair Buxton a.j.bux...@gmail.com wrote: Hi, With the latest driver my card never gets a signal lock, not even once. As before there are no error messages. It does always probe correctly now though. I tracked this down to: http://git.linuxtv.org/anttip/media_tree.git/commit/e5d3e4f27f0cf71c29d12ce39752195d8994dea3 and to this specific change: @@ -459,21 +563,14 @@ static int rtl28xxu_power_ctrl(struct dvb_usb_device *d, int onoff) sys0 = sys0 0x0f; sys0 |= 0xe0; } else { - -#if 0 /* keep */ /* * FIXME: Use .fe_ioctl_override() to prevent demod -* IOCTLs in case of device is powered off. -* -* For now we cannot power off device because most FE IOCTLs -* can be performed only when device is powered. -* Using IOCTLs when device is powered off will result errors -* because register access to demod fails. +* IOCTLs in case of device is powered off. Or change +* RTL2830 demod not perform requestesd IOCTL IO when sleep. */ gpio = (~0x01); /* GPIO0 = 0 */ gpio |= 0x10; /* GPIO4 = 1 */ sys0 = sys0 (~0xc0); -#endif } deb_info(%s: WR SYS0=%02x GPIO_OUT_VAL=%02x\n, __func__, sys0, gpio); Commenting those three lines makes the driver work again. Don't know yet if it will keep working for longer than a couple of days. -- Alistair Buxton a.j.bux...@gmail.com -- 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