Re: [Workshop-2011] Media Subsystem Workshop 2011

2011-08-05 Thread Hans de Goede

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

2011-08-05 Thread Oliver Neukum
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

2011-08-05 Thread Archit Taneja
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

2011-08-05 Thread Jan Pohanka
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

2011-08-05 Thread Uwe Kleine-König
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

2011-08-05 Thread Hans de Goede

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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Oliver Neukum
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

2011-08-05 Thread Hans de Goede

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

2011-08-05 Thread Patrick Boettcher

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

2011-08-05 Thread Laurent Pinchart
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

2011-08-05 Thread Laurent Pinchart
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

2011-08-05 Thread Michael Jones
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

2011-08-05 Thread Michael Jones
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

2011-08-05 Thread Mauro Carvalho Chehab
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

2011-08-05 Thread Pawel Osciak
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

2011-08-05 Thread Pawel Osciak
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

2011-08-05 Thread Theodore Kilgore


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

2011-08-05 Thread Theodore Kilgore


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

2011-08-05 Thread Hans Verkuil
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

2011-08-05 Thread Koen Kooi


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

2011-08-05 Thread Andrew Chew
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

2011-08-05 Thread Mauro Carvalho Chehab
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

2011-08-05 Thread Andrew Chew
  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

2011-08-05 Thread Malcolm Priestley
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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Guennadi Liakhovetski
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

2011-08-05 Thread Mauro Carvalho Chehab
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)

2011-08-05 Thread Randy Dunlap
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

2011-08-05 Thread Andrew Chew
  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

2011-08-05 Thread Alistair Buxton
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

2011-08-05 Thread Manoel Pinheiro
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

2011-08-05 Thread Alistair Buxton
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