Re: SoC i.mx35 userptr method failure while running capture-example utility

2012-06-18 Thread Prabhakar Lad
Hi Guennadi/Laurent,

On Wed, May 2, 2012 at 4:20 AM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 Hi Alex

 On Tue, 1 May 2012, Alex Gershgorin wrote:

 Hi everyone,

 I use user-space utility from  
 http://git.linuxtv.org/v4l-utils.git/blob/HEAD:/contrib/test/capture-example.c
 I made two small changes in this application and this is running on i.MX35 
 SoC

 fmt.fmt.pix.pixelformat = V4L2_PIX_FMT_RGB565;
 fmt.fmt.pix.field       = V4L2_FIELD_ANY;

 When MMAP method is used everything works fine, problem occurs when using 
 USERPTR method
 this can see bellow :

 ./capture-example -u -f -d /dev/video0
 mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
 Failed acquiring VMA for vaddr 0x76cd9008
 VIDIOC_QBUF error 22, Invalid arg

 It doesn't surprise me, that this doesn't work. capture-example allocates
 absolutely normal user-space buffers, when called with USERPTR, and those
 buffers are very likely discontiguous. Whereas mx3-camera needs physically
 contiguous buffers, so, this can only fail. This means, you either have to
 use MMAP or you need to allocate USERPTR buffers in a special way to
 guarantee their contiguity.

  Even I am facing a similar issue when ported to VB2 for USERPTR.
 How do we ensure the buffers not discontiguous. Would cmem assure it?

Thx,
--Prabhakar Lad

 Unable to handle kernel NULL pointer dereference at virtual address 

 This, however, is bad and is a bug in the driver. The capture-example
 should just fail nicely with no trouble. I'll add it to my TODO and will
 try to find some time to debug and fix this, however, I'd be more than
 happy if someone else would beat me on this ;-)

 Thanks
 Guennadi

 pgd = 80004000
 [] *pgd=
 Internal error: Oops: 817 [#1] ARM
 CPU: 0    Not tainted  (3.4.0-rc5+ #2283
 PC is at mx3_videobuf_release+0x9c/0x10c
 LR is at mx3_videobuf_release+0x20/0x10c
 pc : [802cd92c]    lr : [802cd8b0]    psr: 0093
 sp : 86db3e00  ip : 86db3e00  fp : 86db3e2c
 r10: 86ff6b20  r9 : 86817200  r8 : 
 r7 : 86ff568c  r6 :   r5 : 8801a000  r4 : 86da3000
 r3 : 6013  r2 : 86da3264  r1 :   r0 : 
 Flags: nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
 Control: 00c5387d  Table: 86dcc008  DAC: 0015
 Process capture-example (pid: 52, stack limit = 0x86db2268)
 Stack: (0x86db3e00 to 0x86db4000)
 3e00:  6013  86ff568c  0002 86ff56ac 
 3e20: 86db3e64 86db3e30 802c8978 802cd89c  80099414 86db3e84 86ff568c
 3e40: 86dc9a80 8801a03c 80491828  86817200 86ff6b20 86db3e7c 86db3e68
 3e60: 802c9a1c 802c8930 802ce048 86ff5600 86db3e9c 86db3e80 802cca14 802c9a00
 3e80: 86ff5800 86dc9a80 0008 86dc9a88 86db3eb4 86db3ea0 802b936c 802cc9d0
 3ea0: 86dc9a80 86ff6b20 86db3ef4 86db3eb8 80082f00 802b932c  
 3ec0:  86d35010 86d7f000 86dc9a80  86d59000 86d90120 000c
 3ee0: 86db2000  86db3f14 86db3ef8 8007ff58 80082df0  86d59000
 3f00:  0001 86db3f3c 86db3f18 8001c72c 8007fee4 86d59000 86d82000
 3f20: 0100 76ef1770 00f8 8000e564 86db3f4c 86db3f40 8001c7a8 8001c6b4
 3f40: 86db3f7c 86db3f50 8001dab4 8001c78c 7eb002b8 0001 0004 
 3f60: 86db3fa4 86db3f70 800824fc 00f8 86db3f94 86db3f80 8001dfc0 8001d8c4
 3f80:  000a3d78 86db3fa4 86db3f98 8001e004 8001df4c  86db3fa8
 3fa0: 8000e3e0 8001dff8 000a3d78 76ef1770 0001 000a3d64 0008 0001
 3fc0: 000a3d78 76ef1770 76ef1770 00f8 76e1d248  9ecc 7eb02954
 3fe0: 76f2e000 7eb02908 76de14dc 76e4f3d4 6010 0001  
 Backtrace:
 [802cd890] (mx3_videobuf_release+0x0/0x10c) from [802c8978] 
 (__vb2_queue_free+0x54/0x15c)
  r8: r7:86ff56ac r6:0002 r5: r4:86ff568c
 [802c8924] (__vb2_queue_free+0x0/0x15c) from [802c9a1c] 
 (vb2_queue_release+0x28/0x2c)
 [802c99f4] (vb2_queue_release+0x0/0x2c) from [802cca14] 
 (soc_camera_close+0x50/0xac)
  r4:86ff5600 r3:802ce048
 [802cc9c4] (soc_camera_close+0x0/0xac) from [802b936c] 
 (v4l2_release+0x4c/0x6c)
  r7:86dc9a88 r6:0008 r5:86dc9a80 r4:86ff5800
 [802b9320] (v4l2_release+0x0/0x6c) from [80082f00] (fput+0x11c/0x204)
  r5:86ff6b20 r4:86dc9a80
 [80082de4] (fput+0x0/0x204) from [8007ff58] (filp_close+0x80/0x8c)
 [8007fed8] (filp_close+0x0/0x8c) from [8001c72c] 
 (put_files_struct+0x84/0xd8)
  r6:0001 r5: r4:86d59000 r3:
 [8001c6a8] (put_files_struct+0x0/0xd8) from [8001c7a8] 
 (exit_files+0x28/0x2c)
  r8:8000e564 r7:00f8 r6:76ef1770 r5:0100 r4:86d82000
 r3:86d59000
 [8001c780] (exit_files+0x0/0x2c) from [8001dab4] (do_exit+0x1fc/0x688)
 [8001d8b8] (do_exit+0x0/0x688) from [8001dfc0] (do_group_exit+0x80/0xac)
  r7:00f8
 [8001df40] (do_group_exit+0x0/0xac) from [8001e004] 
 (sys_exit_group+0x18/0x24)
  r4:000a3d78 r3:
 [8001dfec] (sys_exit_group+0x0/0x24) from [8000e3e0] 
 (ret_fast_syscall+0x0/0x30)
 Code: 05852024 e5941268 e5940264 e2842f99 (e581)
 

Re: [RFC] [media] cx231xx: restore tuner settings on first open

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 06:49:58 David Dillow wrote:
 Without this patch, MythTV requires some workarounds to be able to
 capture analog TV on my HVR-850. I need to keep the v4l device open via
 'sleep 365d  /dev/video0' or some other mechanism or there will be no
 recording. Also, I need to run vq4l2 and change the standard away and
 back to NTSC or I'll get garbage. Both of these can be traced to the
 settings (or lack there of) on the tuner.
 
 The first problem is that we will tell applications that we are in NTSC
 mode for the HVR-850 when they open the device for the first time after
 plugging it in, but never actually set the TDA18271 to NTSC, so it
 defaults to PAL_I. Applications may decide to not change the signal
 standard, as they have been told it is already in the desired state.
 This could be resolved by setting it during init (as we claim in the
 query results), as it seems to survive putting the tuner into standby
 mode.

That's correct. Drivers must set an initial tuner standard and frequency.
It doesn't matter whether that's PAL or NTSC, or what frequency, as long
as there is something setup initially.

 The second problem is a bad interaction between driver behavior and
 MythTV. MythTV opens the video device, tunes to the desired channel,
 then closes the device. It then reopens the device and starts recording.
 While this works for my WinTV-FM PCI card, the cx231xx driver puts the
 tuner into standby after MythTV closes the device, and the tuner looses
 the frequency when waking back up.

That sounds like a bug in the tuner driver: it should remember and restore
the last set frequency when waking up.

 I solved both of these issues by just setting the standard and frequency
 when opening the device for the first user. This fixes the immediate
 problem, but I'm not sure it's the right fix, and I'm a bit
 uncomfortable faking a call into the ioctl() routines.
 
 What does the V4L2 API spec say about tuning frequency being persistent
 when there are no users of a video capture device? Is MythTV wrong to
 have that assumption, or is cx231xx wrong to not restore the frequency
 when a user first opens the device?

Tuner standards and frequencies must be persistent. So cx231xx is wrong.
Actually, all V4L2 settings must in general be persistent (there are
some per-filehandle settings when dealing with low-level subdev setups or
mem2mem devices).

 Either way, I think MythTV should keep the device open until it is done
 with it, as that would avoid added latency from putting the tuner to
 sleep and waking it back up. But, I think we should address the issue in
 the driver if it is not living up to the guarantees of the API.

From what I can tell it is a bug in the tda tuner (not restoring the frequency)
and cx231xx (not setting the initial standard and possibly frequency).

Regards,

Hans

 
 
 
 diff --git a/drivers/media/video/cx231xx/cx231xx-video.c 
 b/drivers/media/video/cx231xx/cx231xx-video.c
 index 7f916f0..2794396 100644
 --- a/drivers/media/video/cx231xx/cx231xx-video.c
 +++ b/drivers/media/video/cx231xx/cx231xx-video.c
 @@ -2190,6 +2190,12 @@ static int cx231xx_v4l2_open(struct file *filp)
   filp-private_data = fh;
  
   if (fh-type == V4L2_BUF_TYPE_VIDEO_CAPTURE  dev-users == 0) {
 + struct v4l2_frequency freq = {
 + .tuner = 0,
 + .type = V4L2_TUNER_ANALOG_TV,
 + .frequency = dev-ctl_freq,
 + };
 +
   dev-width = norm_maxw(dev);
   dev-height = norm_maxh(dev);
  
 @@ -2214,6 +2220,11 @@ static int cx231xx_v4l2_open(struct file *filp)
   /* device needs to be initialized before isoc transfer */
   dev-video_input = dev-video_input  2 ? 2 : dev-video_input;
  
 + /* Restore standard and channel, as they may be lost when
 +  * the tuner went to sleep.
 +  */
 + vidioc_s_std(filp, fh, dev-norm);
 + vidioc_s_frequency(filp, fh, freq);
   }
   if (fh-radio) {
   cx231xx_videodbg(video_open: setting radio device\n);
 
 
 
 --
 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


[PATCH] s5p-fimc: Fix control creation function

2012-06-18 Thread Kamil Debski
Fixed the size of the V4L2_CID_COLORFX control cluster.
Prior to this fix V4L2_CID_ROTATE was also icluded in
the cluster preventing applications from enabling rotation.

Reported-by: Marek Szyprowski m.szyprow...@samsung.com
Signed-off-by: Kamil Debski k.deb...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 drivers/media/video/s5p-fimc/fimc-core.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/fimc-core.c 
b/drivers/media/video/s5p-fimc/fimc-core.c
index fedcd56..92fc5a2 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -615,7 +615,7 @@ int fimc_ctrls_create(struct fimc_ctx *ctx)
ctx-effect.type = FIMC_REG_CIIMGEFF_FIN_BYPASS;
 
if (!handler-error) {
-   v4l2_ctrl_cluster(3, ctrls-colorfx);
+   v4l2_ctrl_cluster(2, ctrls-colorfx);
ctrls-ready = true;
}
 
-- 
1.7.0.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: [RFCv1 PATCH 04/32] v4l2-ioctl.c: v4l2-ioctl: add debug and callback/offset functionality.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

Thanks for the patch.

On Sunday 10 June 2012 12:25:26 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Add the necessary plumbing to make it possible to replace the switch by a
 table driven implementation.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/video/v4l2-ioctl.c |   91
 -- 1 file changed, 78 insertions(+), 13
 deletions(-)
 
 diff --git a/drivers/media/video/v4l2-ioctl.c
 b/drivers/media/video/v4l2-ioctl.c index a9602db..a4115ce 100644
 --- a/drivers/media/video/v4l2-ioctl.c
 +++ b/drivers/media/video/v4l2-ioctl.c
 @@ -396,12 +396,22 @@ struct v4l2_ioctl_info {
   unsigned int ioctl;
   u32 flags;
   const char * const name;
 + union {
 + u32 offset;
 + int (*func)(const struct v4l2_ioctl_ops *ops,
 + struct file *file, void *fh, void *p);
 + };
 + void (*debug)(const void *arg);
  };
 
  /* This control needs a priority check */
  #define INFO_FL_PRIO (1  0)
  /* This control can be valid if the filehandle passes a control handler. */
 #define INFO_FL_CTRL  (1  1)
 +/* This is a standard ioctl, no need for special code */
 +#define INFO_FL_STD  (1  2)
 +/* This is ioctl has its own function */
 +#define INFO_FL_FUNC (1  3)
  /* Zero struct from after the field to the end */
  #define INFO_FL_CLEAR(v4l2_struct, field)\
   ((offsetof(struct v4l2_struct, field) + \
 @@ -414,6 +424,24 @@ struct v4l2_ioctl_info {
   .name = #_ioctl,\
  }
 
 +#define IOCTL_INFO_STD(_ioctl, _vidioc, _debug, _flags)  
 \
 + [_IOC_NR(_ioctl)] = {   \
 + .ioctl = _ioctl,\
 + .flags = _flags | INFO_FL_STD,  \
 + .name = #_ioctl,\
 + .offset = offsetof(struct v4l2_ioctl_ops, _vidioc), \
 + .debug = _debug,\
 + }
 +
 +#define IOCTL_INFO_FNC(_ioctl, _func, _debug, _flags)
 \
 + [_IOC_NR(_ioctl)] = {   \
 + .ioctl = _ioctl,\
 + .flags = _flags | INFO_FL_FUNC, \
 + .name = #_ioctl,\
 + .func = _func,  \
 + .debug = _debug,\
 + }
 +
  static struct v4l2_ioctl_info v4l2_ioctls[] = {
   IOCTL_INFO(VIDIOC_QUERYCAP, 0),
   IOCTL_INFO(VIDIOC_ENUM_FMT, INFO_FL_CLEAR(v4l2_fmtdesc, type)),
 @@ -512,7 +540,7 @@ bool v4l2_is_known_ioctl(unsigned int cmd)
 external ioctl messages as well as internal V4L ioctl */
  void v4l_printk_ioctl(unsigned int cmd)
  {
 - char *dir, *type;
 + const char *dir, *type;
 
   switch (_IOC_TYPE(cmd)) {
   case 'd':
 @@ -523,10 +551,11 @@ void v4l_printk_ioctl(unsigned int cmd)
   type = v4l2;
   break;
   }
 - printk(%s, v4l2_ioctls[_IOC_NR(cmd)].name);
 + pr_cont(%s, v4l2_ioctls[_IOC_NR(cmd)].name);
   return;
   default:
   type = unknown;
 + break;
   }
 
   switch (_IOC_DIR(cmd)) {
 @@ -536,7 +565,7 @@ void v4l_printk_ioctl(unsigned int cmd)
   case _IOC_READ | _IOC_WRITE: dir = rw; break;
   default: dir = *ERR*; break;
   }
 - printk(%s ioctl '%c', dir=%s, #%d (0x%08x),
 + pr_cont(%s ioctl '%c', dir=%s, #%d (0x%08x),
   type, _IOC_TYPE(cmd), dir, _IOC_NR(cmd), cmd);
  }
  EXPORT_SYMBOL(v4l_printk_ioctl);
 @@ -546,6 +575,9 @@ static long __video_do_ioctl(struct file *file,
  {
   struct video_device *vfd = video_devdata(file);
   const struct v4l2_ioctl_ops *ops = vfd-ioctl_ops;
 + bool write_only = false;
 + struct v4l2_ioctl_info default_info;
 + const struct v4l2_ioctl_info *info;
   void *fh = file-private_data;
   struct v4l2_fh *vfh = NULL;
   int use_fh_prio = 0;
 @@ -563,23 +595,40 @@ static long __video_do_ioctl(struct file *file,
   }
 
   if (v4l2_is_known_ioctl(cmd)) {
 - struct v4l2_ioctl_info *info = v4l2_ioctls[_IOC_NR(cmd)];
 + info = v4l2_ioctls[_IOC_NR(cmd)];
 
   if (!test_bit(_IOC_NR(cmd), vfd-valid_ioctls) 
   !((info-flags  INFO_FL_CTRL)  vfh  vfh-ctrl_handler))
 - return -ENOTTY;
 + goto error;
 
   if (use_fh_prio  (info-flags  INFO_FL_PRIO)) {
   ret = v4l2_prio_check(vfd-prio, vfh-prio);
   if (ret)
 - return ret;
 + 

Re: [RFCv1 PATCH 19/32] v4l2-dev.c: add debug sysfs entry.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

Thanks for the patch.

On Sunday 10 June 2012 12:25:41 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/video/v4l2-dev.c |   24 
  1 file changed, 24 insertions(+)
 
 diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
 index 1500208..5c0bb18 100644
 --- a/drivers/media/video/v4l2-dev.c
 +++ b/drivers/media/video/v4l2-dev.c
 @@ -46,6 +46,29 @@ static ssize_t show_index(struct device *cd,
   return sprintf(buf, %i\n, vdev-index);
  }
 
 +static ssize_t show_debug(struct device *cd,
 +  struct device_attribute *attr, char *buf)
 +{
 + struct video_device *vdev = to_video_device(cd);
 +
 + return sprintf(buf, %i\n, vdev-debug);
 +}
 +
 +static ssize_t set_debug(struct device *cd, struct device_attribute *attr,
 +const char *buf, size_t len)
 +{
 + struct video_device *vdev = to_video_device(cd);
 + int res = 0;
 + u16 value;
 +
 + res = kstrtou16(buf, 0, value);
 + if (res)
 + return res;
 +
 + vdev-debug = value;

Can't this race with the various vdev-debug tests we have in the V4L core ?

 + return len;
 +}
 +
  static ssize_t show_name(struct device *cd,
struct device_attribute *attr, char *buf)
  {
 @@ -56,6 +79,7 @@ static ssize_t show_name(struct device *cd,
 
  static struct device_attribute video_device_attrs[] = {
   __ATTR(name, S_IRUGO, show_name, NULL),
 + __ATTR(debug, 0644, show_debug, set_debug),
   __ATTR(index, S_IRUGO, show_index, NULL),
   __ATTR_NULL
  };
-- 
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: [RFCv1 PATCH 20/32] v4l2-ioctl: remove v4l_(i2c_)print_ioctl

2012-06-18 Thread Laurent Pinchart
Hi Hans,

Thanks for the patch.

On Sunday 10 June 2012 12:25:42 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 v4l_i2c_print_ioctl wasn't used and v4l_print_ioctl could be replaced by
 v4l_i2c_printk_ioctl.

I suppose you meant v4l_printk_ioctl herE.

 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com

-- 
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: [RFCv1 PATCH 30/32] v4l2-ioctl.c: shorten the lines of the table.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

Thanks for the patch.

On Sunday 10 June 2012 12:25:52 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Use some macro magic to reduce the length of the lines in the table. This
 makes it more readable.

It indeed shortens the lines, but to be honest I find the result less 
readable.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com

-- 
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: [RFCv1 PATCH 29/32] v4l2-dev.c: also add debug support for the fops.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

Thanks for the patch.

On Sunday 10 June 2012 12:25:51 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/video/v4l2-dev.c |   41 ++---
  1 file changed, 29 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
 index 5c0bb18..54f387d 100644
 --- a/drivers/media/video/v4l2-dev.c
 +++ b/drivers/media/video/v4l2-dev.c
 @@ -305,6 +305,9 @@ static ssize_t v4l2_read(struct file *filp, char __user
 *buf, ret = vdev-fops-read(filp, buf, sz, off);
   if (test_bit(V4L2_FL_LOCK_ALL_FOPS, vdev-flags))
   mutex_unlock(vdev-lock);
 + if (vdev-debug)

As vdev-debug is a bitmask, shouldn't we add an fops debug bit ?

 + pr_info(%s: read: %zd (%d)\n,
 + video_device_node_name(vdev), sz, ret);

Shouldn't we use KERN_DEBUG instead of KERN_INFO ? BTW, what about replacing 
the pr_* calls with dev_* calls ?

   return ret;
  }
 
 @@ -323,6 +326,9 @@ static ssize_t v4l2_write(struct file *filp, const char
 __user *buf, ret = vdev-fops-write(filp, buf, sz, off);
   if (test_bit(V4L2_FL_LOCK_ALL_FOPS, vdev-flags))
   mutex_unlock(vdev-lock);
 + if (vdev-debug)
 + pr_info(%s: write: %zd (%d)\n,
 + video_device_node_name(vdev), sz, ret);
   return ret;
  }
 
 @@ -339,6 +345,9 @@ static unsigned int v4l2_poll(struct file *filp, struct
 poll_table_struct *poll) ret = vdev-fops-poll(filp, poll);
   if (test_bit(V4L2_FL_LOCK_ALL_FOPS, vdev-flags))
   mutex_unlock(vdev-lock);
 + if (vdev-debug)
 + pr_info(%s: poll: %08x\n,
 + video_device_node_name(vdev), ret);
   return ret;
  }
 
 @@ -348,20 +357,14 @@ static long v4l2_ioctl(struct file *filp, unsigned int
 cmd, unsigned long arg) int ret = -ENODEV;
 
   if (vdev-fops-unlocked_ioctl) {
 - bool locked = false;
 + struct mutex *lock = v4l2_ioctl_get_lock(vdev, cmd);
 
 - if (vdev-lock) {
 - /* always lock unless the cmd is marked as don't use 
 lock */
 - locked = !v4l2_is_known_ioctl(cmd) ||
 -  !test_bit(_IOC_NR(cmd), vdev-disable_locking);
 -
 - if (locked  mutex_lock_interruptible(vdev-lock))
 - return -ERESTARTSYS;
 - }
 + if (lock  mutex_lock_interruptible(lock))
 + return -ERESTARTSYS;
   if (video_is_registered(vdev))
   ret = vdev-fops-unlocked_ioctl(filp, cmd, arg);
 - if (locked)
 - mutex_unlock(vdev-lock);
 + if (lock)
 + mutex_unlock(lock);
   } else if (vdev-fops-ioctl) {
   /* This code path is a replacement for the BKL. It is a major
* hack but it will have to do for those drivers that are not
 @@ -409,12 +412,17 @@ static unsigned long v4l2_get_unmapped_area(struct
 file *filp, unsigned long flags)
  {
   struct video_device *vdev = video_devdata(filp);
 + int ret;
 
   if (!vdev-fops-get_unmapped_area)
   return -ENOSYS;
   if (!video_is_registered(vdev))
   return -ENODEV;
 - return vdev-fops-get_unmapped_area(filp, addr, len, pgoff, flags);
 + ret = vdev-fops-get_unmapped_area(filp, addr, len, pgoff, flags);
 + if (vdev-debug)
 + pr_info(%s: get_unmapped_area (%d)\n,
 + video_device_node_name(vdev), ret);
 + return ret;
  }
  #endif
 
 @@ -432,6 +440,9 @@ static int v4l2_mmap(struct file *filp, struct
 vm_area_struct *vm) ret = vdev-fops-mmap(filp, vm);
   if (test_bit(V4L2_FL_LOCK_ALL_FOPS, vdev-flags))
   mutex_unlock(vdev-lock);
 + if (vdev-debug)
 + pr_info(%s: mmap (%d)\n,
 + video_device_node_name(vdev), ret);
   return ret;
  }
 
 @@ -470,6 +481,9 @@ err:
   /* decrease the refcount in case of an error */
   if (ret)
   video_put(vdev);
 + if (vdev-debug)
 + pr_info(%s: open (%d)\n,
 + video_device_node_name(vdev), ret);
   return ret;
  }
 
 @@ -489,6 +503,9 @@ static int v4l2_release(struct inode *inode, struct file
 *filp) /* decrease the refcount unconditionally since the release()
  return value is ignored. */
   video_put(vdev);
 + if (vdev-debug)
 + pr_info(%s: release\n,
 + video_device_node_name(vdev));
   return ret;
  }
-- 
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: [RFCv1 PATCH 28/32] vivi: use vb2 helper functions.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

Thanks for the patch.

On Sunday 10 June 2012 12:25:50 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/video/vivi.c |  160 ++---
  1 file changed, 21 insertions(+), 139 deletions(-)
 
 diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
 index 1e4da5e..1e8c4f3 100644
 --- a/drivers/media/video/vivi.c
 +++ b/drivers/media/video/vivi.c
 @@ -767,7 +767,13 @@ static int queue_setup(struct vb2_queue *vq, const
 struct v4l2_format *fmt, struct vivi_dev *dev = vb2_get_drv_priv(vq);
   unsigned long size;
 
 - size = dev-width * dev-height * dev-pixelsize;
 + if (fmt)
 + size = fmt-fmt.pix.sizeimage;
 + else
 + size = dev-width * dev-height * dev-pixelsize;
 +
 + if (size == 0)
 + return -EINVAL;

If I'm not mistaken, this is a bug fix to properly support CREATE_BUF, right ? 
If so it should be split to its own patch.

   if (0 == *nbuffers)
   *nbuffers = 32;

-- 
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: [RFCv1 PATCH 25/32] create_bufs: handle count == 0.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

Thanks for the patch.

On Sunday 10 June 2012 12:25:47 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  Documentation/DocBook/media/v4l/vidioc-create-bufs.xml |8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
 b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml index
 765549f..afdba4d 100644
 --- a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
 +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
 @@ -97,7 +97,13 @@ information./para
 row
   entry__u32/entry
   entrystructfieldcount/structfield/entry
 - entryThe number of buffers requested or granted./entry
 + entryThe number of buffers requested or granted. If count == 0,
 then
 + constantVIDIOC_CREATE_BUFS/constant will set
 structfieldindex/structfield
 + to the starting buffer index,

I find starting buffer index a bit unclear in this context, as we don't 
create any buffer.

 and it will check the validity of
 + structfieldmemory/structfield and
 structfieldformat.type/structfield.
 + If those are invalid -1 is returned and errno is set to EINVAL;,
 + otherwise constantVIDIOC_CREATE_BUFS/constant returns 0. It will
 + never set errno to EBUSY; in this particular case./entry
 /row
 row
   entry__u32/entry
-- 
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: [RFCv1 PATCH 27/32] vivi: embed struct video_device instead of allocating it.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

Thanks for the patch.

On Sunday 10 June 2012 12:25:49 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
  drivers/media/video/vivi.c |   25 +++--
  1 file changed, 7 insertions(+), 18 deletions(-)
 
 diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
 index 8dd5ae6..1e4da5e 100644
 --- a/drivers/media/video/vivi.c
 +++ b/drivers/media/video/vivi.c

[snip]

 @@ -1080,7 +1078,6 @@ static int vidioc_enum_input(struct file *file, void
 *priv, return -EINVAL;
 
   inp-type = V4L2_INPUT_TYPE_CAMERA;
 - inp-std = V4L2_STD_525_60;

Doesn't this belong to the previous patch ?

   sprintf(inp-name, Camera %u, inp-index);
   return 0;
  }

-- 
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 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

2012-06-18 Thread Bhupesh SHARMA
Hi Laurent,

Can you please review this patch and let me know if any modifications
are required..

 -Original Message-
 From: Bhupesh SHARMA
 Sent: Friday, June 01, 2012 3:09 PM
 To: laurent.pinch...@ideasonboard.com; linux-...@vger.kernel.org
 Cc: ba...@ti.com; linux-media@vger.kernel.org;
 gre...@linuxfoundation.org; Bhupesh SHARMA
 Subject: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use
 videobuf2 framework

 This patch reworks the videobuffer management logic present in the UVC
 webcam gadget and ports it to use the more apt videobuf2 framework
 for
 video buffer management.

 To support routing video data captured from a real V4L2 video capture
 device with a zero copy operation on videobuffers (as they pass from
 the V4L2
 domain to UVC domain via a user-space application), we need to support
 USER_PTR
 IO method at the UVC gadget side.

 So the V4L2 capture device driver can still continue to use MMAO IO
 method
 and now the user-space application can just pass a pointer to the video
 buffers
 being DeQueued from the V4L2 device side while Queueing them at the UVC
 gadget
 end. This ensures that we have a zero-copy design as the videobuffers
 pass
 from the V4L2 capture device to the UVC gadget.

 Note that there will still be a need to apply UVC specific payload
 headers
 on top of each UVC payload data, which will still require a copy
 operation
 to be performed in the 'encode' routines of the UVC gadget.

 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 ---
  drivers/usb/gadget/Kconfig |1 +
  drivers/usb/gadget/uvc_queue.c |  524 +++-
 
  drivers/usb/gadget/uvc_queue.h |   25 +--
  drivers/usb/gadget/uvc_v4l2.c  |   35 ++--
  drivers/usb/gadget/uvc_video.c |   17 +-
  5 files changed, 184 insertions(+), 418 deletions(-)

 diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
 index 1f93861..5a351f8 100644
 --- a/drivers/usb/gadget/Kconfig
 +++ b/drivers/usb/gadget/Kconfig
 @@ -967,6 +967,7 @@ endif
  config USB_G_WEBCAM
   tristate USB Webcam Gadget
   depends on VIDEO_DEV
 + select VIDEOBUF2_VMALLOC
   help
 The Webcam Gadget acts as a composite USB Audio and Video Class
 device. It provides a userspace API to process UVC control
 requests
 diff --git a/drivers/usb/gadget/uvc_queue.c
 b/drivers/usb/gadget/uvc_queue.c
 index 0cdf89d..907ece8 100644
 --- a/drivers/usb/gadget/uvc_queue.c
 +++ b/drivers/usb/gadget/uvc_queue.c
 @@ -10,6 +10,7 @@
   *   (at your option) any later version.
   */

 +#include linux/atomic.h
  #include linux/kernel.h
  #include linux/mm.h
  #include linux/list.h
 @@ -18,7 +19,8 @@
  #include linux/videodev2.h
  #include linux/vmalloc.h
  #include linux/wait.h
 -#include linux/atomic.h
 +
 +#include media/videobuf2-vmalloc.h

  #include uvc.h

 @@ -28,271 +30,156 @@
   * Video queues is initialized by uvc_queue_init(). The function
 performs
   * basic initialization of the uvc_video_queue struct and never fails.
   *
 - * Video buffer allocation and freeing are performed by
 uvc_alloc_buffers and
 - * uvc_free_buffers respectively. The former acquires the video queue
 lock,
 - * while the later must be called with the lock held (so that
 allocation can
 - * free previously allocated buffers). Trying to free buffers that are
 mapped
 - * to user space will return -EBUSY.
 - *
 - * Video buffers are managed using two queues. However, unlike most
 USB video
 - * drivers that use an in queue and an out queue, we use a main queue
 to hold
 - * all queued buffers (both 'empty' and 'done' buffers), and an irq
 queue to
 - * hold empty buffers. This design (copied from video-buf) minimizes
 locking
 - * in interrupt, as only one queue is shared between interrupt and
 user
 - * contexts.
 - *
 - * Use cases
 - * -
 - *
 - * Unless stated otherwise, all operations that modify the irq buffers
 queue
 - * are protected by the irq spinlock.
 - *
 - * 1. The user queues the buffers, starts streaming and dequeues a
 buffer.
 - *
 - *The buffers are added to the main and irq queues. Both
 operations are
 - *protected by the queue lock, and the later is protected by the
 irq
 - *spinlock as well.
 - *
 - *The completion handler fetches a buffer from the irq queue and
 fills it
 - *with video data. If no buffer is available (irq queue empty),
 the handler
 - *returns immediately.
 - *
 - *When the buffer is full, the completion handler removes it from
 the irq
 - *queue, marks it as ready (UVC_BUF_STATE_DONE) and wakes its wait
 queue.
 - *At that point, any process waiting on the buffer will be woken
 up. If a
 - *process tries to dequeue a buffer after it has been marked
 ready, the
 - *dequeing will succeed immediately.
 - *
 - * 2. Buffers are queued, user is waiting on a buffer and the device
 gets
 - *disconnected.
 - *
 - *When the device is disconnected, the kernel calls the completion
 handler
 - *with 

RE: [PATCH 5/5] usb: gadget/uvc: Add support for 'USB_GADGET_DELAYED_STATUS' response for a set_intf(alt-set 1) command

2012-06-18 Thread Bhupesh SHARMA
Hi Laurent,

Can you please review this patch and let me know if any modifications are 
required..

 -Original Message-
 From: Bhupesh SHARMA
 Sent: Friday, June 01, 2012 3:09 PM
 To: laurent.pinch...@ideasonboard.com; linux-...@vger.kernel.org
 Cc: ba...@ti.com; linux-media@vger.kernel.org;
 gre...@linuxfoundation.org; Bhupesh SHARMA
 Subject: [PATCH 5/5] usb: gadget/uvc: Add support for
 'USB_GADGET_DELAYED_STATUS' response for a set_intf(alt-set 1) command
 
 This patch adds the support in UVC webcam gadget design for providing
 USB_GADGET_DELAYED_STATUS in response to a set_interface(alt setting 1)
 command
 issue by the Host.
 
 The current UVC webcam gadget design generates a STREAMON event
 corresponding
 to a set_interface(alt setting 1) command from the Host. This STREAMON
 event
 will eventually be routed to a real V4L2 device.
 
 To start video streaming, it may be required to perform some register
 writes to
 a camera sensor device over slow external busses like I2C or SPI. So,
 it makes
 sense to ensure that we delay the STATUS stage of the
 set_interface(alt setting 1) command.
 
 Otherwise, a lot of ISOC IN tokens sent by the Host will be replied to
 by
 zero-length packets by the webcam device. On certain Hosts this may
 even lead
 to ISOC URBs been cancelled from the Host side.
 
 So, as soon as we finish doing all the streaming related stuff on the
 real
 V4L2 device, we call a STREAMON ioctl on the UVC side and from here we
 call the
 'usb_composite_setup_continue' function to complete the status stage of
 the
 set_interface(alt setting 1) command.
 
 Further, we need to ensure that we queue no video buffers on the UVC
 webcam
 gadget, until we de-queue a video buffer from the V4L2 device. Also, we
 need to
 enable UVC video related stuff at the first QBUF ioctl call itself,
 as the application will call the STREAMON on UVC side only when it has
 dequeued sufficient buffers from the V4L2 side and queued them to the
 UVC
 gadget. So, the UVC video enable stuff cannot be done in STREAMON ioctl
 call.
 
 For the same we add two more UVC states:
   - PRE_STREAMING : not even a single buffer has been queued to UVC
   - BUF_QUEUED_STREAMING_OFF : one video buffer has been queued to
 UVC
   but we have not yet enabled STREAMING on UVC side.
 
 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 ---
  drivers/usb/gadget/f_uvc.c|   17 -
  drivers/usb/gadget/uvc.h  |3 +++
  drivers/usb/gadget/uvc_v4l2.c |   38
 --
  3 files changed, 51 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
 index 2a8bf06..3589ed0 100644
 --- a/drivers/usb/gadget/f_uvc.c
 +++ b/drivers/usb/gadget/f_uvc.c
 @@ -272,6 +272,13 @@ uvc_function_setup(struct usb_function *f, const
 struct usb_ctrlrequest *ctrl)
   return 0;
  }
 
 +void uvc_function_setup_continue(struct uvc_device *uvc)
 +{
 + struct usb_composite_dev *cdev = uvc-func.config-cdev;
 +
 + usb_composite_setup_continue(cdev);
 +}
 +
  static int
  uvc_function_get_alt(struct usb_function *f, unsigned interface)
  {
 @@ -334,7 +341,8 @@ uvc_function_set_alt(struct usb_function *f,
 unsigned interface, unsigned alt)
   v4l2_event_queue(uvc-vdev, v4l2_event);
 
   uvc-state = UVC_STATE_CONNECTED;
 - break;
 +
 + return 0;
 
   case 1:
   if (uvc-state != UVC_STATE_CONNECTED)
 @@ -352,14 +360,13 @@ uvc_function_set_alt(struct usb_function *f,
 unsigned interface, unsigned alt)
   v4l2_event.type = UVC_EVENT_STREAMON;
   v4l2_event_queue(uvc-vdev, v4l2_event);
 
 - uvc-state = UVC_STATE_STREAMING;
 - break;
 + uvc-state = UVC_STATE_PRE_STREAMING;
 +
 + return USB_GADGET_DELAYED_STATUS;
 
   default:
   return -EINVAL;
   }
 -
 - return 0;
  }
 
  static void
 diff --git a/drivers/usb/gadget/uvc.h b/drivers/usb/gadget/uvc.h
 index d78ea25..6cd1435 100644
 --- a/drivers/usb/gadget/uvc.h
 +++ b/drivers/usb/gadget/uvc.h
 @@ -141,6 +141,8 @@ enum uvc_state
  {
   UVC_STATE_DISCONNECTED,
   UVC_STATE_CONNECTED,
 + UVC_STATE_PRE_STREAMING,
 + UVC_STATE_BUF_QUEUED_STREAMING_OFF,
   UVC_STATE_STREAMING,
  };
 
 @@ -190,6 +192,7 @@ struct uvc_file_handle
   * Functions
   */
 
 +extern void uvc_function_setup_continue(struct uvc_device *uvc);
  extern void uvc_endpoint_stream(struct uvc_device *dev);
 
  extern void uvc_function_connect(struct uvc_device *uvc);
 diff --git a/drivers/usb/gadget/uvc_v4l2.c
 b/drivers/usb/gadget/uvc_v4l2.c
 index 9c2b45b..5f23571 100644
 --- a/drivers/usb/gadget/uvc_v4l2.c
 +++ b/drivers/usb/gadget/uvc_v4l2.c
 @@ -235,10 +235,36 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int
 cmd, void *arg)
   }
 
   case VIDIOC_QBUF:
 + /*
 +  * Theory of operation:
 +  * - 

Re: [RFCv1 PATCH 24/32] videobuf2-core: add helper functions.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

Thanks for the patch.

On Sunday 10 June 2012 12:25:46 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Add helper functions to make it easier to adapt drivers to vb2.

What about moving those functions to videobuf2-ioctl.c ? The helper functions 
are based on top of an existing vb2 core that isn't aware of queue ownership. 
It's not clear (to me at least) how the helpers will evolve and whether they 
will be used by all drivers or not, or whether part of what they do will get 
merged into the vb2 core. Splitting the helpers in a separate file would help 
not mixing code too much without really thinking about it.

 These helpers take care of core locking and check if the filehandle is the
 owner of the queue.
 
 This patch also adds support for count == 0 in create_bufs.

Could you please split that to its own patch ? The addition of 
__verify_memory_type() should be split as well.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com

-- 
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: SoC i.mx35 userptr method failure while running capture-example utility

2012-06-18 Thread Laurent Pinchart
Hi,

On Monday 18 June 2012 12:28:44 Prabhakar Lad wrote:
 On Wed, May 2, 2012 at 4:20 AM, Guennadi Liakhovetski wrote:
  On Tue, 1 May 2012, Alex Gershgorin wrote:
  Hi everyone,
  
  I use user-space utility from
   http://git.linuxtv.org/v4l-utils.git/blob/HEAD:/contrib/test/capture-ex
  ample.c I made two small changes in this application and this is running
  on i.MX35 SoC
  
  fmt.fmt.pix.pixelformat = V4L2_PIX_FMT_RGB565;
  fmt.fmt.pix.field   = V4L2_FIELD_ANY;
  
  When MMAP method is used everything works fine, problem occurs when using
  USERPTR method this can see bellow :
  
  ./capture-example -u -f -d /dev/video0
  mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
  Failed acquiring VMA for vaddr 0x76cd9008
  VIDIOC_QBUF error 22, Invalid arg
  
  It doesn't surprise me, that this doesn't work. capture-example allocates
  absolutely normal user-space buffers, when called with USERPTR, and those
  buffers are very likely discontiguous. Whereas mx3-camera needs physically
  contiguous buffers, so, this can only fail. This means, you either have to
  use MMAP or you need to allocate USERPTR buffers in a special way to
  guarantee their contiguity.
 
   Even I am facing a similar issue when ported to VB2 for USERPTR.
  How do we ensure the buffers not discontiguous. Would cmem assure it?

CMEM is definitely not the way to go, it's an out-of-tree hack that should 
disappear once we get proper CMA and DMABUF support in the kernel.

How to allocate memory depends on your use case(s). If you just need to 
capture to anonymous memory that will then be read by userspace you shouldn't 
use USERPTR, but MMAP. If you need to capture to device memory (for instance 
capturing directly to a frame buffer), you should export a DMABUF object on 
from the frame buffer driver (this isn't available in mainline yet, I'll try 
to send a patch soon) and then import it on the V4L2 side (not available in 
mainline yet either I'm afraid :-)). As an interim solution, mmap() your frame 
buffer to userspace and then use USERPTR.

  Unable to handle kernel NULL pointer dereference at virtual address
   
  This, however, is bad and is a bug in the driver. The capture-example
  should just fail nicely with no trouble. I'll add it to my TODO and will
  try to find some time to debug and fix this, however, I'd be more than
  happy if someone else would beat me on this ;-)
  
  Thanks
  Guennadi
  
  pgd = 80004000
  [] *pgd=
  Internal error: Oops: 817 [#1] ARM
  CPU: 0Not tainted  (3.4.0-rc5+ #2283
  PC is at mx3_videobuf_release+0x9c/0x10c
  LR is at mx3_videobuf_release+0x20/0x10c
  pc : [802cd92c]lr : [802cd8b0]psr: 0093
  sp : 86db3e00  ip : 86db3e00  fp : 86db3e2c
  r10: 86ff6b20  r9 : 86817200  r8 : 
  r7 : 86ff568c  r6 :   r5 : 8801a000  r4 : 86da3000
  r3 : 6013  r2 : 86da3264  r1 :   r0 : 
  Flags: nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
  Control: 00c5387d  Table: 86dcc008  DAC: 0015
  Process capture-example (pid: 52, stack limit = 0x86db2268)
  Stack: (0x86db3e00 to 0x86db4000)
  3e00:  6013  86ff568c  0002 86ff56ac
   3e20: 86db3e64 86db3e30 802c8978 802cd89c  80099414
  86db3e84 86ff568c 3e40: 86dc9a80 8801a03c 80491828  86817200
  86ff6b20 86db3e7c 86db3e68 3e60: 802c9a1c 802c8930 802ce048 86ff5600
  86db3e9c 86db3e80 802cca14 802c9a00 3e80: 86ff5800 86dc9a80 0008
  86dc9a88 86db3eb4 86db3ea0 802b936c 802cc9d0 3ea0: 86dc9a80 86ff6b20
  86db3ef4 86db3eb8 80082f00 802b932c   3ec0: 
  86d35010 86d7f000 86dc9a80  86d59000 86d90120 000c 3ee0:
  86db2000  86db3f14 86db3ef8 8007ff58 80082df0  86d59000
  3f00:  0001 86db3f3c 86db3f18 8001c72c 8007fee4 86d59000
  86d82000 3f20: 0100 76ef1770 00f8 8000e564 86db3f4c 86db3f40
  8001c7a8 8001c6b4 3f40: 86db3f7c 86db3f50 8001dab4 8001c78c 7eb002b8
  0001 0004  3f60: 86db3fa4 86db3f70 800824fc 00f8
  86db3f94 86db3f80 8001dfc0 8001d8c4 3f80:  000a3d78 86db3fa4
  86db3f98 8001e004 8001df4c  86db3fa8 3fa0: 8000e3e0 8001dff8
  000a3d78 76ef1770 0001 000a3d64 0008 0001 3fc0: 000a3d78
  76ef1770 76ef1770 00f8 76e1d248  9ecc 7eb02954 3fe0:
  76f2e000 7eb02908 76de14dc 76e4f3d4 6010 0001  
  Backtrace:
  [802cd890] (mx3_videobuf_release+0x0/0x10c) from [802c8978]
  (__vb2_queue_free+0x54/0x15c) r8: r7:86ff56ac r6:0002
  r5: r4:86ff568c
  [802c8924] (__vb2_queue_free+0x0/0x15c) from [802c9a1c]
  (vb2_queue_release+0x28/0x2c) [802c99f4] (vb2_queue_release+0x0/0x2c)
  from [802cca14] (soc_camera_close+0x50/0xac) r4:86ff5600 r3:802ce048
  [802cc9c4] (soc_camera_close+0x0/0xac) from [802b936c]
  (v4l2_release+0x4c/0x6c) r7:86dc9a88 r6:0008 r5:86dc9a80 r4:86ff5800
  [802b9320] (v4l2_release+0x0/0x6c) from [80082f00] 

Re: uvcvideo issue with kernel 3.5-rc2 and 3

2012-06-18 Thread Laurent Pinchart
Hi Philipp,

On Sunday 17 June 2012 13:35:07 Philipp Dreimann wrote:
 Hello,
 
 my external webcam from Logitech (I guess it's a c910) stopped working
 using kernel 3.5-rc3.( 3.4 worked fine.)
 
 uvcvideo: Found UVC 1.00 device unnamed (046d:0821)
 input: UVC Camera (046d:0821) as
 /devices/pci:00/:00:1a.0/usb1/1-1/1-1.2/1-1.2:1.2/input/input14
 usbcore: registered new interface driver uvcvideo
 USB Video Class driver (1.1.1)
 uvcvideo: Failed to query (GET_DEF) UVC control 2 on unit 2: -71 (exp. 2).
 uvcvideo: Failed to query (GET_DEF) UVC control 2 on unit 2: -71 (exp. 2).
 uvcvideo: Failed to query (GET_DEF) UVC control 3 on unit 2: -71 (exp. 2).
 uvcvideo: Failed to query (GET_DEF) UVC control 7 on unit 2: -71 (exp. 2).
 uvcvideo: Failed to query (GET_DEF) UVC control 11 on unit 2: -71 (exp. 1).
 uvcvideo: Failed to query (GET_DEF) UVC control 4 on unit 2: -71 (exp. 2).
 uvcvideo: Failed to query (GET_DEF) UVC control 5 on unit 2: -71 (exp. 1).
 uvcvideo: Failed to query (GET_CUR) UVC control 11 on unit 2: -71 (exp. 1).
 uvcvideo: Failed to query (GET_DEF) UVC control 8 on unit 2: -71 (exp. 2).
 uvcvideo: Failed to query (GET_DEF) UVC control 1 on unit 2: -71 (exp. 2).
 uvcvideo: Failed to set UVC probe control : -71 (exp. 26).
 uvcvideo: Failed to set UVC probe control : -71 (exp. 26).
 uvcvideo: Failed to set UVC probe control : -71 (exp. 26).
 uvcvideo: Failed to set UVC probe control : -71 (exp. 26).
 (the last line is being repeated...)

This might be cause by a bug in the USB core or in the UVC driver. Would you 
be able to bisect the regression ? Or, alternatively, test the v3.4 uvcvideo 
driver on v3.5-rc3 ? Or the other way around, test the latest v4l tree on v3.4 
(instructions regarding how to compile the v4l tree with a different kernel 
are available at 
http://linuxtv.org/wiki/index.php/How_to_Obtain,_Build_and_Install_V4L-
DVB_Device_Drivers).

 I used cheese to test the webcam. My other webcam is working fine:
 uvcvideo: Found UVC 1.00 device Integrated Camera (04f2:b217)
 input: Integrated Camera as
 /devices/pci:00/:00:1a.0/usb1/1-1/1-1.6/1-1.6:1.0/input/input13

-- 
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: [RFCv1 PATCH 18/32] v4l2-ioctl.c: finalize table conversion.

2012-06-18 Thread Mauro Carvalho Chehab
Em 18-06-2012 06:46, Laurent Pinchart escreveu:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:40 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
   drivers/media/video/v4l2-ioctl.c |   35 +--
 1 file changed, 13 insertions(+), 22 deletions(-)

 diff --git a/drivers/media/video/v4l2-ioctl.c
 b/drivers/media/video/v4l2-ioctl.c index 0de31c4..6c91674 100644
 --- a/drivers/media/video/v4l2-ioctl.c
 +++ b/drivers/media/video/v4l2-ioctl.c
 @@ -870,6 +870,11 @@ static void v4l_print_newline(const void *arg)
  pr_cont(\n);
   }

 +static void v4l_print_default(const void *arg)
 +{
 +pr_cont(non-standard ioctl\n);
 
 I'd say driver-specific ioctl instead. non-standard may sound like an
 error to users.

This message is useless as-is, as it provides no glue about what ioctl was
called. You should either remove it or print the ioctl number, in hexa.

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: [GIT PULL FOR 3.6] V4L2 API cleanups

2012-06-18 Thread Laurent Pinchart
Hi Sakari,

On Sunday 17 June 2012 10:54:32 Sakari Ailus wrote:
 On Sun, Jun 17, 2012 at 12:03:06AM +0200, Laurent Pinchart wrote:
  On Monday 11 June 2012 12:39:44 Sakari Ailus wrote:
   On Mon, Jun 11, 2012 at 09:50:54AM +0200, Laurent Pinchart wrote:
On Sunday 10 June 2012 23:22:59 Sakari Ailus wrote:
 Hi Mauro,
 
 Here are two V4L2 API cleanup patches; the first removes __user from
 videodev2.h from a few places, making it possible to use the header
 file
 as such in user space, while the second one changes the
 v4l2_buffer.input field back to reserved.
 
 The following changes since commit
  
  5472d3f17845c4398c6a510b46855820920c2181:
   [media] mt9m032: Implement V4L2_CID_PIXEL_RATE control (2012-05-24
 
 09:27:24 -0300)
 
 are available in the git repository at:
   ssh://linuxtv.org/git/sailus/media_tree.git media-for-3.6
 
 Sakari Ailus (2):
   v4l: Remove __user from interface structure definitions

NAK, sorry.

__user has a purpose, we need to add it where it's missing, not remove
it where it's rightfully present.
   
   It's not quite as simple as adding __user everywhere it might belong to
   ---
   these structs are being used in kernel space, too. The structs that are
   part of the user space interface may at some point contain pointers to
   memory which is in user space. That is being dealt by video_usercopy(),
   so the individual drivers or the rest of the V4L2 framework always gets
   pointers pointing to kernel memory.
  
  Very good point, I haven't thought about that. I'm not sure how to deal
  with this, splitting structures in a __user and a non __user version
  isn't really a good option. Maybe the sparse tool should be somehow
  extended ?
 
 Wouldn't type casting in video_usercopy() just do the job? Albeit I'm far
 from certain it'd make the code better, just make the sparse warnings go
 away...

For pointers that are completely handled in the v4l core (such as the 
v4l2_buffer and v4l2_ext_controls pointer fields), casting casting in 
video_usercopy() is enough as no driver will ever see the user pointers (we 
already cast in those cases). For pointers that are to be handled by drivers, 
I think we need to keep __user (or make the v4l core handle them).

-- 
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: SoC i.mx35 userptr method failure while running capture-example utility

2012-06-18 Thread Prabhakar Lad
Hi Laurent,


On Mon, Jun 18, 2012 at 4:05 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi,

 On Monday 18 June 2012 12:28:44 Prabhakar Lad wrote:
 On Wed, May 2, 2012 at 4:20 AM, Guennadi Liakhovetski wrote:
  On Tue, 1 May 2012, Alex Gershgorin wrote:
  Hi everyone,
 
  I use user-space utility from
   http://git.linuxtv.org/v4l-utils.git/blob/HEAD:/contrib/test/capture-ex
  ample.c I made two small changes in this application and this is running
  on i.MX35 SoC
 
  fmt.fmt.pix.pixelformat = V4L2_PIX_FMT_RGB565;
  fmt.fmt.pix.field       = V4L2_FIELD_ANY;
 
  When MMAP method is used everything works fine, problem occurs when using
  USERPTR method this can see bellow :
 
  ./capture-example -u -f -d /dev/video0
  mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
  Failed acquiring VMA for vaddr 0x76cd9008
  VIDIOC_QBUF error 22, Invalid arg
 
  It doesn't surprise me, that this doesn't work. capture-example allocates
  absolutely normal user-space buffers, when called with USERPTR, and those
  buffers are very likely discontiguous. Whereas mx3-camera needs physically
  contiguous buffers, so, this can only fail. This means, you either have to
  use MMAP or you need to allocate USERPTR buffers in a special way to
  guarantee their contiguity.

   Even I am facing a similar issue when ported to VB2 for USERPTR.
  How do we ensure the buffers not discontiguous. Would cmem assure it?

 CMEM is definitely not the way to go, it's an out-of-tree hack that should
 disappear once we get proper CMA and DMABUF support in the kernel.

 How to allocate memory depends on your use case(s). If you just need to
 capture to anonymous memory that will then be read by userspace you shouldn't
 use USERPTR, but MMAP. If you need to capture to device memory (for instance
 capturing directly to a frame buffer), you should export a DMABUF object on
 from the frame buffer driver (this isn't available in mainline yet, I'll try
 to send a patch soon) and then import it on the V4L2 side (not available in
 mainline yet either I'm afraid :-)). As an interim solution, mmap() your frame
 buffer to userspace and then use USERPTR.


I have got MMAP working with videobuf2, with videobuf there was a support for
userptr which I need to achieve this with videobuf2. Can you point me to your
branch where you are working on it and also an example if you have to
interim solution as you suggested above.

Thx,
--Prabhakar Lad
  Unable to handle kernel NULL pointer dereference at virtual address
  
  This, however, is bad and is a bug in the driver. The capture-example
  should just fail nicely with no trouble. I'll add it to my TODO and will
  try to find some time to debug and fix this, however, I'd be more than
  happy if someone else would beat me on this ;-)
 
  Thanks
  Guennadi
 
  pgd = 80004000
  [] *pgd=
  Internal error: Oops: 817 [#1] ARM
  CPU: 0    Not tainted  (3.4.0-rc5+ #2283
  PC is at mx3_videobuf_release+0x9c/0x10c
  LR is at mx3_videobuf_release+0x20/0x10c
  pc : [802cd92c]    lr : [802cd8b0]    psr: 0093
  sp : 86db3e00  ip : 86db3e00  fp : 86db3e2c
  r10: 86ff6b20  r9 : 86817200  r8 : 
  r7 : 86ff568c  r6 :   r5 : 8801a000  r4 : 86da3000
  r3 : 6013  r2 : 86da3264  r1 :   r0 : 
  Flags: nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
  Control: 00c5387d  Table: 86dcc008  DAC: 0015
  Process capture-example (pid: 52, stack limit = 0x86db2268)
  Stack: (0x86db3e00 to 0x86db4000)
  3e00:  6013  86ff568c  0002 86ff56ac
   3e20: 86db3e64 86db3e30 802c8978 802cd89c  80099414
  86db3e84 86ff568c 3e40: 86dc9a80 8801a03c 80491828  86817200
  86ff6b20 86db3e7c 86db3e68 3e60: 802c9a1c 802c8930 802ce048 86ff5600
  86db3e9c 86db3e80 802cca14 802c9a00 3e80: 86ff5800 86dc9a80 0008
  86dc9a88 86db3eb4 86db3ea0 802b936c 802cc9d0 3ea0: 86dc9a80 86ff6b20
  86db3ef4 86db3eb8 80082f00 802b932c   3ec0: 
  86d35010 86d7f000 86dc9a80  86d59000 86d90120 000c 3ee0:
  86db2000  86db3f14 86db3ef8 8007ff58 80082df0  86d59000
  3f00:  0001 86db3f3c 86db3f18 8001c72c 8007fee4 86d59000
  86d82000 3f20: 0100 76ef1770 00f8 8000e564 86db3f4c 86db3f40
  8001c7a8 8001c6b4 3f40: 86db3f7c 86db3f50 8001dab4 8001c78c 7eb002b8
  0001 0004  3f60: 86db3fa4 86db3f70 800824fc 00f8
  86db3f94 86db3f80 8001dfc0 8001d8c4 3f80:  000a3d78 86db3fa4
  86db3f98 8001e004 8001df4c  86db3fa8 3fa0: 8000e3e0 8001dff8
  000a3d78 76ef1770 0001 000a3d64 0008 0001 3fc0: 000a3d78
  76ef1770 76ef1770 00f8 76e1d248  9ecc 7eb02954 3fe0:
  76f2e000 7eb02908 76de14dc 76e4f3d4 6010 0001  
  Backtrace:
  [802cd890] (mx3_videobuf_release+0x0/0x10c) from [802c8978]
  (__vb2_queue_free+0x54/0x15c) r8: r7:86ff56ac r6:0002
  r5: r4:86ff568c
  [802c8924] 

Re: [RFCv1 PATCH 18/32] v4l2-ioctl.c: finalize table conversion.

2012-06-18 Thread Laurent Pinchart
Hi Mauro,

On Monday 18 June 2012 07:50:35 Mauro Carvalho Chehab wrote:
 Em 18-06-2012 06:46, Laurent Pinchart escreveu:
  On Sunday 10 June 2012 12:25:40 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
  
drivers/media/video/v4l2-ioctl.c |   35
+--
  
  1 file changed, 13 insertions(+), 22 deletions(-)
  
  diff --git a/drivers/media/video/v4l2-ioctl.c
  b/drivers/media/video/v4l2-ioctl.c index 0de31c4..6c91674 100644
  --- a/drivers/media/video/v4l2-ioctl.c
  +++ b/drivers/media/video/v4l2-ioctl.c
  @@ -870,6 +870,11 @@ static void v4l_print_newline(const void *arg)
  
 pr_cont(\n);

}
  
  +static void v4l_print_default(const void *arg)
  +{
  +  pr_cont(non-standard ioctl\n);
  
  I'd say driver-specific ioctl instead. non-standard may sound like an
  error to users.
 
 This message is useless as-is, as it provides no glue about what ioctl was
 called. You should either remove it or print the ioctl number, in hexa.

I think the ioctl number is already printed by the caller of 
v4l_print_default.

-- 
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: [PATCHv7 01/15] v4l: Add DMABUF as a memory type

2012-06-18 Thread Tomasz Stanislawski
Hi everyone,
The automatic build system informed me that
there is a shameful error in this patch.

The declarations of fd fields in v4l2_plane32 and
v4l2_buffer32 are missing. It breaks build process
for 64-bit architectures. I am deeply sorry for
posting a patch without testing it enough.

The build-break will be fixed in the next version of
the patchset.

Regards,
Tomasz Stanislawski


On 06/14/2012 03:37 PM, Tomasz Stanislawski wrote:
 From: Sumit Semwal sumit.sem...@ti.com
 
 Adds DMABUF memory type to v4l framework. Also adds the related file
 descriptor in v4l2_plane and v4l2_buffer.
 
 Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
[original work in the PoC for buffer sharing]
 Signed-off-by: Sumit Semwal sumit.sem...@ti.com
 Signed-off-by: Sumit Semwal sumit.sem...@linaro.org
 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 ---
  drivers/media/video/v4l2-compat-ioctl32.c |   16 
  drivers/media/video/v4l2-ioctl.c  |1 +
  include/linux/videodev2.h |7 +++
  3 files changed, 24 insertions(+)
 
 diff --git a/drivers/media/video/v4l2-compat-ioctl32.c 
 b/drivers/media/video/v4l2-compat-ioctl32.c
 index 5327ad3..d33ab18 100644
 --- a/drivers/media/video/v4l2-compat-ioctl32.c
 +++ b/drivers/media/video/v4l2-compat-ioctl32.c
 @@ -348,6 +348,9 @@ static int get_v4l2_plane32(struct v4l2_plane *up, struct 
 v4l2_plane32 *up32,
   up_pln = compat_ptr(p);
   if (put_user((unsigned long)up_pln, up-m.userptr))
   return -EFAULT;
 + } else if (memory == V4L2_MEMORY_DMABUF) {
 + if (copy_in_user(up-m.fd, up32-m.fd, sizeof(int)))
 + return -EFAULT;
   } else {
   if (copy_in_user(up-m.mem_offset, up32-m.mem_offset,
   sizeof(__u32)))
 @@ -371,6 +374,11 @@ static int put_v4l2_plane32(struct v4l2_plane *up, 
 struct v4l2_plane32 *up32,
   if (copy_in_user(up32-m.mem_offset, up-m.mem_offset,
   sizeof(__u32)))
   return -EFAULT;
 + /* For DMABUF, driver might've set up the fd, so copy it back. */
 + if (memory == V4L2_MEMORY_DMABUF)
 + if (copy_in_user(up32-m.fd, up-m.fd,
 + sizeof(int)))
 + return -EFAULT;
  
   return 0;
  }
 @@ -454,6 +462,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, 
 struct v4l2_buffer32 __user
   if (get_user(kp-m.offset, up-m.offset))
   return -EFAULT;
   break;
 + case V4L2_MEMORY_DMABUF:
 + if (get_user(kp-m.fd, up-m.fd))
 + return -EFAULT;
 + break;
   }
   }
  
 @@ -518,6 +530,10 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, 
 struct v4l2_buffer32 __user
   if (put_user(kp-m.offset, up-m.offset))
   return -EFAULT;
   break;
 + case V4L2_MEMORY_DMABUF:
 + if (put_user(kp-m.fd, up-m.fd))
 + return -EFAULT;
 + break;
   }
   }
  
 diff --git a/drivers/media/video/v4l2-ioctl.c 
 b/drivers/media/video/v4l2-ioctl.c
 index 91be4e8..31fc2ad 100644
 --- a/drivers/media/video/v4l2-ioctl.c
 +++ b/drivers/media/video/v4l2-ioctl.c
 @@ -175,6 +175,7 @@ static const char *v4l2_memory_names[] = {
   [V4L2_MEMORY_MMAP]= mmap,
   [V4L2_MEMORY_USERPTR] = userptr,
   [V4L2_MEMORY_OVERLAY] = overlay,
 + [V4L2_MEMORY_DMABUF] = dmabuf,
  };
  
  #define prt_names(a, arr) a) = 0)  ((a)  ARRAY_SIZE(arr))) ? \
 diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
 index 370d111..51b20f4 100644
 --- a/include/linux/videodev2.h
 +++ b/include/linux/videodev2.h
 @@ -185,6 +185,7 @@ enum v4l2_memory {
   V4L2_MEMORY_MMAP = 1,
   V4L2_MEMORY_USERPTR  = 2,
   V4L2_MEMORY_OVERLAY  = 3,
 + V4L2_MEMORY_DMABUF   = 4,
  };
  
  /* see also http://vektor.theorem.ca/graphics/ycbcr/ */
 @@ -591,6 +592,8 @@ struct v4l2_requestbuffers {
   *   should be passed to mmap() called on the video node)
   * @userptr: when memory is V4L2_MEMORY_USERPTR, a userspace pointer
   *   pointing to this plane
 + * @fd:  when memory is V4L2_MEMORY_DMABUF, a userspace 
 file
 + *   descriptor associated with this plane
   * @data_offset: offset in the plane to the start of data; usually 0,
   *   unless there is a header in front of the data
   *
 @@ -605,6 +608,7 @@ struct v4l2_plane {
   union {
   __u32   mem_offset;
   unsigned long   userptr;
 + int fd;
   } m;
   __u32   

Re: [RFCv1 PATCH 18/32] v4l2-ioctl.c: finalize table conversion.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 11:46:57 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:40 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
   drivers/media/video/v4l2-ioctl.c |   35 +--
  1 file changed, 13 insertions(+), 22 deletions(-)
  
  diff --git a/drivers/media/video/v4l2-ioctl.c
  b/drivers/media/video/v4l2-ioctl.c index 0de31c4..6c91674 100644
  --- a/drivers/media/video/v4l2-ioctl.c
  +++ b/drivers/media/video/v4l2-ioctl.c
  @@ -870,6 +870,11 @@ static void v4l_print_newline(const void *arg)
  pr_cont(\n);
   }
  
  +static void v4l_print_default(const void *arg)
  +{
  +   pr_cont(non-standard ioctl\n);
 
 I'd say driver-specific ioctl instead. non-standard may sound like an 
 error to users.

Much better, thank you!

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: [PATCH 1/2] v4l: added V4L2_BUF_FLAG_EOS flag indicating the last frame in the stream

2012-06-18 Thread Mauro Carvalho Chehab
Em 22-05-2012 12:33, Andrzej Hajda escreveu:
 Some devices requires indicator if the buffer is the last one in the stream.
 Applications and drivers can use this flag in such case.
 
 Signed-off-by: Andrzej Hajda a.ha...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
   Documentation/DocBook/media/v4l/io.xml  |7 +++
   Documentation/DocBook/media/v4l/vidioc-qbuf.xml |2 ++
   include/linux/videodev2.h   |1 +
   3 files changed, 10 insertions(+), 0 deletions(-)
 
 diff --git a/Documentation/DocBook/media/v4l/io.xml 
 b/Documentation/DocBook/media/v4l/io.xml
 index fd6aca2..dcbf1e0 100644
 --- a/Documentation/DocBook/media/v4l/io.xml
 +++ b/Documentation/DocBook/media/v4l/io.xml
 @@ -956,6 +956,13 @@ 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
 +   row
 + entryconstantV4L2_BUF_FLAG_EOS/constant/entry
 + entry0x2000/entry
 + entryApplication should set this flag in the output buffer
 +in order to inform the driver about the last frame of the stream. Some
 +drivers may require it to properly finish processing the stream./entry

This breaks backward compatibility, as applications written before this change
won't set this flag.

 +   /row
   /tbody
 /tgroup
   /table
 diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml 
 b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
 index 9caa49a..ad49f7d 100644
 --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
 +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
 @@ -76,6 +76,8 @@ supports capturing from specific video inputs and you want 
 to specify a video
   input, then structfieldflags/structfield should be set to
   constantV4L2_BUF_FLAG_INPUT/constant and the field
   structfieldinput/structfield must be initialized to the desired input.
 +Some drivers expects applications set constantV4L2_BUF_FLAG_EOS/constant
 +flag on the last buffer of the stream.
   The structfieldreserved/structfield field must be set to 0. When using
   the link linkend=planar-apismulti-planar API/link, the
   structfieldm.planes/structfield field must contain a userspace pointer
 diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
 index 370d111..e44a7cd 100644
 --- a/include/linux/videodev2.h
 +++ b/include/linux/videodev2.h
 @@ -676,6 +676,7 @@ struct v4l2_buffer {
   /* Cache handling flags */
   #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE   0x0800
   #define V4L2_BUF_FLAG_NO_CACHE_CLEAN0x1000
 +#define V4L2_BUF_FLAG_EOS0x2000  /* The last buffer in the stream */
   
   /*
*  O V E R L A Y   P R E V I E W
 


--
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: [RFCv1 PATCH 04/32] v4l2-ioctl.c: v4l2-ioctl: add debug and callback/offset functionality.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 11:47:07 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:26 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Add the necessary plumbing to make it possible to replace the switch by a
  table driven implementation.
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
   drivers/media/video/v4l2-ioctl.c |   91
  -- 1 file changed, 78 insertions(+), 13
  deletions(-)
  
  diff --git a/drivers/media/video/v4l2-ioctl.c
  b/drivers/media/video/v4l2-ioctl.c index a9602db..a4115ce 100644
  --- a/drivers/media/video/v4l2-ioctl.c
  +++ b/drivers/media/video/v4l2-ioctl.c
  @@ -563,23 +595,40 @@ static long __video_do_ioctl(struct file *file,
  }
  
  if (v4l2_is_known_ioctl(cmd)) {
  -   struct v4l2_ioctl_info *info = v4l2_ioctls[_IOC_NR(cmd)];
  +   info = v4l2_ioctls[_IOC_NR(cmd)];
  
  if (!test_bit(_IOC_NR(cmd), vfd-valid_ioctls) 
  !((info-flags  INFO_FL_CTRL)  vfh  vfh-ctrl_handler))
  -   return -ENOTTY;
  +   goto error;
  
  if (use_fh_prio  (info-flags  INFO_FL_PRIO)) {
  ret = v4l2_prio_check(vfd-prio, vfh-prio);
  if (ret)
  -   return ret;
  +   goto error;
  }
  +   } else {
  +   default_info.ioctl = cmd;
  +   default_info.flags = 0;
  +   default_info.debug = NULL;
  +   info = default_info;
  }
  
  -   if ((vfd-debug  V4L2_DEBUG_IOCTL) 
  -   !(vfd-debug  V4L2_DEBUG_IOCTL_ARG)) {
  +   write_only = _IOC_DIR(cmd) == _IOC_WRITE;
  +   if (info-debug  write_only  vfd-debug  V4L2_DEBUG_IOCTL) {
  v4l_print_ioctl(vfd-name, cmd);
  -   printk(KERN_CONT \n);
  +   pr_cont(: );
  +   info-debug(arg);
  +   }
 
 Shouldn't you print the ioctl name and information even if info-debug is 
 NULL 
 ?

The 'info-debug' test is temporary. Once the conversion is finished this test
will be removed since then info-debug will never be NULL.

 
  +   if (info-flags  INFO_FL_STD) {
  +   typedef int (*vidioc_op)(struct file *file, void *fh, void *p);
  +   const void *p = vfd-ioctl_ops;
  +   const vidioc_op *vidioc = p + info-offset;
  +
  +   ret = (*vidioc)(file, fh, arg);
  +   goto error;
  +   } else if (info-flags  INFO_FL_FUNC) {
  +   ret = info-func(ops, file, fh, arg);
  +   goto error;
  }
  
  switch (cmd) {
  @@ -2100,10 +2149,26 @@ static long __video_do_ioctl(struct file *file,
  break;
  } /* switch */
  
  -   if (vfd-debug  V4L2_DEBUG_IOCTL_ARG) {
  -   if (ret  0) {
  -   v4l_print_ioctl(vfd-name, cmd);
  -   printk(KERN_CONT  error %ld\n, ret);
  +error:
 
 This isn't an error, is it ? I'd call it done instead.

True, I'll change this.

 
  +   if (vfd-debug) {
  +   if (write_only  vfd-debug  V4L2_DEBUG_IOCTL) {
 
 vfd-debug is a bitmask (or at least is documented as being a bitmask in 
 include/media/v4l2-ioctl.h).

Yeah, and that makes no sense. Having V4L2_DEBUG_IOCTL_ARG without
V4L2_DEBUG_IOCTL is undefined. It is much more logical to have 0 for no
debugging, 1 for just ioctl name debugging and = 2 for extensive debugging.
It's just weird to have to set the debug sysfs entry to 0, 1 or 3.

In a later patch I want to change the few drivers that use this accordingly.

 
  +   if (ret)
 
 Shouldn't you test for  0 instead ? Driver-specific ioctls might return a  
 0 
 value in case of success.

Good catch. Yes, I need to do that.

 
  +   pr_info(%s: error %ld\n,
  +   video_device_node_name(vfd), ret);
  +   return ret;
  +   }
  +   v4l_print_ioctl(vfd-name, cmd);
  +   if (ret)
  +   pr_cont(: error %ld\n, ret);
  +   else if (vfd-debug == V4L2_DEBUG_IOCTL)
  +   pr_cont(\n);
  +   else if (!info-debug)
  +   return ret;
  +   else if (_IOC_DIR(cmd) == _IOC_NONE)
  +   info-debug(arg);
  +   else {
  +   pr_cont(: );
  +   info-debug(arg);
  }
 
 Ouch. What are you trying to do here ? Can't we simplify debug messages ?

It's simplified a bit in a later patch when the temporary info-debug test
can be dropped.

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: [RFCv1 PATCH 19/32] v4l2-dev.c: add debug sysfs entry.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 11:48:45 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:41 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
   drivers/media/video/v4l2-dev.c |   24 
   1 file changed, 24 insertions(+)
  
  diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
  index 1500208..5c0bb18 100644
  --- a/drivers/media/video/v4l2-dev.c
  +++ b/drivers/media/video/v4l2-dev.c
  @@ -46,6 +46,29 @@ static ssize_t show_index(struct device *cd,
  return sprintf(buf, %i\n, vdev-index);
   }
  
  +static ssize_t show_debug(struct device *cd,
  +struct device_attribute *attr, char *buf)
  +{
  +   struct video_device *vdev = to_video_device(cd);
  +
  +   return sprintf(buf, %i\n, vdev-debug);
  +}
  +
  +static ssize_t set_debug(struct device *cd, struct device_attribute *attr,
  +  const char *buf, size_t len)
  +{
  +   struct video_device *vdev = to_video_device(cd);
  +   int res = 0;
  +   u16 value;
  +
  +   res = kstrtou16(buf, 0, value);
  +   if (res)
  +   return res;
  +
  +   vdev-debug = value;
 
 Can't this race with the various vdev-debug tests we have in the V4L core ?

Not really. You may have a short race where you set it, but some core test is
just reading it and gets the old value. But that's no problem in this case.
The worst that can happen is that you get one more or one less debug message
in the log.

It probably deserves a comment, though.

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: [RFCv1 PATCH 20/32] v4l2-ioctl: remove v4l_(i2c_)print_ioctl

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 11:50:38 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:42 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  v4l_i2c_print_ioctl wasn't used and v4l_print_ioctl could be replaced by
  v4l_i2c_printk_ioctl.
 
 I suppose you meant v4l_printk_ioctl herE.

Yes indeed.

Regards,

Hans

 
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.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


Re: [RFCv1 PATCH 30/32] v4l2-ioctl.c: shorten the lines of the table.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 11:57:24 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:52 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Use some macro magic to reduce the length of the lines in the table. This
  makes it more readable.
 
 It indeed shortens the lines, but to be honest I find the result less 
 readable.

What do you think, should I just keep those long lines?

I've tried splitting them up, but that too made it harder to read.

Regards,

Hans

 
  Signed-off-by: Hans Verkuil hans.verk...@cisco.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


Re: [RFCv1 PATCH 19/32] v4l2-dev.c: add debug sysfs entry.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

On Monday 18 June 2012 13:30:18 Hans Verkuil wrote:
 On Mon June 18 2012 11:48:45 Laurent Pinchart wrote:
  On Sunday 10 June 2012 12:25:41 Hans Verkuil wrote:
   From: Hans Verkuil hans.verk...@cisco.com
   
   Signed-off-by: Hans Verkuil hans.verk...@cisco.com
   ---
   
drivers/media/video/v4l2-dev.c |   24 
1 file changed, 24 insertions(+)
   
   diff --git a/drivers/media/video/v4l2-dev.c
   b/drivers/media/video/v4l2-dev.c index 1500208..5c0bb18 100644
   --- a/drivers/media/video/v4l2-dev.c
   +++ b/drivers/media/video/v4l2-dev.c
   @@ -46,6 +46,29 @@ static ssize_t show_index(struct device *cd,
   
 return sprintf(buf, %i\n, vdev-index);

}
   
   +static ssize_t show_debug(struct device *cd,
   +  struct device_attribute *attr, char *buf)
   +{
   + struct video_device *vdev = to_video_device(cd);
   +
   + return sprintf(buf, %i\n, vdev-debug);
   +}
   +
   +static ssize_t set_debug(struct device *cd, struct device_attribute
   *attr,
   +const char *buf, size_t len)
   +{
   + struct video_device *vdev = to_video_device(cd);
   + int res = 0;
   + u16 value;
   +
   + res = kstrtou16(buf, 0, value);
   + if (res)
   + return res;
   +
   + vdev-debug = value;
  
  Can't this race with the various vdev-debug tests we have in the V4L core
  ?
 Not really. You may have a short race where you set it, but some core test
 is just reading it and gets the old value. But that's no problem in this
 case. The worst that can happen is that you get one more or one less debug
 message in the log.

You test the debug value several times in the __video_do_ioctl() function. I 
haven't checked in details whether changing the value between the two tests 
could for instance lead to a KERN_CONT print without a previous non-KERN_CONT 
message. That won't crash the machine :-) but it should still be avoided.

 It probably deserves a comment, though.

-- 
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: [RFCv1 PATCH 29/32] v4l2-dev.c: also add debug support for the fops.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 12:01:47 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:51 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
   drivers/media/video/v4l2-dev.c |   41 ++---
   1 file changed, 29 insertions(+), 12 deletions(-)
  
  diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
  index 5c0bb18..54f387d 100644
  --- a/drivers/media/video/v4l2-dev.c
  +++ b/drivers/media/video/v4l2-dev.c
  @@ -305,6 +305,9 @@ static ssize_t v4l2_read(struct file *filp, char __user
  *buf, ret = vdev-fops-read(filp, buf, sz, off);
  if (test_bit(V4L2_FL_LOCK_ALL_FOPS, vdev-flags))
  mutex_unlock(vdev-lock);
  +   if (vdev-debug)
 
 As vdev-debug is a bitmask, shouldn't we add an fops debug bit ?

I actually want to move away from the bitmask idea. I've never really liked
it here.

 
  +   pr_info(%s: read: %zd (%d)\n,
  +   video_device_node_name(vdev), sz, ret);
 
 Shouldn't we use KERN_DEBUG instead of KERN_INFO ? BTW, what about replacing 
 the pr_* calls with dev_* calls ?

KERN_DEBUG vs KERN_INFO is actually a good question. My reasoning is that you
explicitly enable logging, and so you really want to see it in the log, so we
use KERN_INFO. With KERN_DEBUG you might have the situation where the debug
level of the logging is disabled, so the messages are ignored.

However, if people disagree with this, then I'm happy to move it back to
KERN_DEBUG.

With regards to dev_ vs pr_: I'd have to test this to see what dev_ prints as
prefix.

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: [RFCv1 PATCH 28/32] vivi: use vb2 helper functions.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 12:08:10 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:50 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
   drivers/media/video/vivi.c |  160 ++---
   1 file changed, 21 insertions(+), 139 deletions(-)
  
  diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
  index 1e4da5e..1e8c4f3 100644
  --- a/drivers/media/video/vivi.c
  +++ b/drivers/media/video/vivi.c
  @@ -767,7 +767,13 @@ static int queue_setup(struct vb2_queue *vq, const
  struct v4l2_format *fmt, struct vivi_dev *dev = vb2_get_drv_priv(vq);
  unsigned long size;
  
  -   size = dev-width * dev-height * dev-pixelsize;
  +   if (fmt)
  +   size = fmt-fmt.pix.sizeimage;
  +   else
  +   size = dev-width * dev-height * dev-pixelsize;
  +
  +   if (size == 0)
  +   return -EINVAL;
 
 If I'm not mistaken, this is a bug fix to properly support CREATE_BUF, right 
 ? 
 If so it should be split to its own patch.

OK.

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: [RFCv1 PATCH 25/32] create_bufs: handle count == 0.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 12:11:27 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:47 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
   Documentation/DocBook/media/v4l/vidioc-create-bufs.xml |8 +++-
   1 file changed, 7 insertions(+), 1 deletion(-)
  
  diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
  b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml index
  765549f..afdba4d 100644
  --- a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
  +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
  @@ -97,7 +97,13 @@ information./para
row
  entry__u32/entry
  entrystructfieldcount/structfield/entry
  -   entryThe number of buffers requested or granted./entry
  +   entryThe number of buffers requested or granted. If count == 0,
  then
  +   constantVIDIOC_CREATE_BUFS/constant will set
  structfieldindex/structfield
  +   to the starting buffer index,
 
 I find starting buffer index a bit unclear in this context, as we don't 
 create any buffer.

How about:

... will set index to the current number of created buffers,

Better alternatives welcome :-)

Hans

 
  and it will check the validity of
  +   structfieldmemory/structfield and
  structfieldformat.type/structfield.
  +   If those are invalid -1 is returned and errno is set to EINVAL;,
  +   otherwise constantVIDIOC_CREATE_BUFS/constant returns 0. It will
  +   never set errno to EBUSY; in this particular case./entry
/row
row
  entry__u32/entry
 
--
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: [RFCv1 PATCH 27/32] vivi: embed struct video_device instead of allocating it.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 12:13:38 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:49 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
   drivers/media/video/vivi.c |   25 +++--
   1 file changed, 7 insertions(+), 18 deletions(-)
  
  diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
  index 8dd5ae6..1e4da5e 100644
  --- a/drivers/media/video/vivi.c
  +++ b/drivers/media/video/vivi.c
 
 [snip]
 
  @@ -1080,7 +1078,6 @@ static int vidioc_enum_input(struct file *file, void
  *priv, return -EINVAL;
  
  inp-type = V4L2_INPUT_TYPE_CAMERA;
  -   inp-std = V4L2_STD_525_60;
 
 Doesn't this belong to the previous patch ?

Indeed.

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: [RFCv1 PATCH 24/32] videobuf2-core: add helper functions.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 12:23:26 Laurent Pinchart wrote:
 Hi Hans,
 
 Thanks for the patch.
 
 On Sunday 10 June 2012 12:25:46 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  Add helper functions to make it easier to adapt drivers to vb2.
 
 What about moving those functions to videobuf2-ioctl.c ? The helper functions 
 are based on top of an existing vb2 core that isn't aware of queue ownership. 
 It's not clear (to me at least) how the helpers will evolve and whether they 
 will be used by all drivers or not, or whether part of what they do will get 
 merged into the vb2 core. Splitting the helpers in a separate file would help 
 not mixing code too much without really thinking about it.

Sounds reasonable. One thing that I want to think about when preparing RFCv2 is
whether the new queue_lock in video_device shouldn't be moved to vb2_queue
instead, since it is a per-vb2_queue lock. I need to check whether that still
makes it possible to split it up like you suggest. It almost certainly will make
sense to do this.

 
  These helpers take care of core locking and check if the filehandle is the
  owner of the queue.
  
  This patch also adds support for count == 0 in create_bufs.
 
 Could you please split that to its own patch ? The addition of 
 __verify_memory_type() should be split as well.

OK.

Regards,

Hans

 
  Signed-off-by: Hans Verkuil hans.verk...@cisco.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


Re: [RFCv1 PATCH 18/32] v4l2-ioctl.c: finalize table conversion.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 12:50:35 Mauro Carvalho Chehab wrote:
 Em 18-06-2012 06:46, Laurent Pinchart escreveu:
  Hi Hans,
  
  Thanks for the patch.
  
  On Sunday 10 June 2012 12:25:40 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
 
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  ---
drivers/media/video/v4l2-ioctl.c |   35 
  +--
  1 file changed, 13 insertions(+), 22 deletions(-)
 
  diff --git a/drivers/media/video/v4l2-ioctl.c
  b/drivers/media/video/v4l2-ioctl.c index 0de31c4..6c91674 100644
  --- a/drivers/media/video/v4l2-ioctl.c
  +++ b/drivers/media/video/v4l2-ioctl.c
  @@ -870,6 +870,11 @@ static void v4l_print_newline(const void *arg)
 pr_cont(\n);
}
 
  +static void v4l_print_default(const void *arg)
  +{
  +  pr_cont(non-standard ioctl\n);
  
  I'd say driver-specific ioctl instead. non-standard may sound like an
  error to users.
 
 This message is useless as-is, as it provides no glue about what ioctl was
 called. You should either remove it or print the ioctl number, in hexa.

That ioctl number is already printed in front of this message.

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: [PATCH 1/2] v4l: added V4L2_BUF_FLAG_EOS flag indicating the last frame in the stream

2012-06-18 Thread Andrzej Hajda
On Mon, 2012-06-18 at 08:24 -0300, Mauro Carvalho Chehab wrote:
 Em 22-05-2012 12:33, Andrzej Hajda escreveu:
  Some devices requires indicator if the buffer is the last one in the stream.
  Applications and drivers can use this flag in such case.
  
  Signed-off-by: Andrzej Hajda a.ha...@samsung.com
  Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
  ---
Documentation/DocBook/media/v4l/io.xml  |7 +++
Documentation/DocBook/media/v4l/vidioc-qbuf.xml |2 ++
include/linux/videodev2.h   |1 +
3 files changed, 10 insertions(+), 0 deletions(-)
  
  diff --git a/Documentation/DocBook/media/v4l/io.xml 
  b/Documentation/DocBook/media/v4l/io.xml
  index fd6aca2..dcbf1e0 100644
  --- a/Documentation/DocBook/media/v4l/io.xml
  +++ b/Documentation/DocBook/media/v4l/io.xml
  @@ -956,6 +956,13 @@ 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
  + row
  +   entryconstantV4L2_BUF_FLAG_EOS/constant/entry
  +   entry0x2000/entry
  +   entryApplication should set this flag in the output buffer
  +in order to inform the driver about the last frame of the stream. Some
  +drivers may require it to properly finish processing the stream./entry
 
 This breaks backward compatibility, as applications written before this change
 won't set this flag.

I am preparing a new patch which will use VIDIOC_ENCODER_CMD with
command V4L2_ENC_CMD_STOP and a new flag
V4L2_ENC_CMD_STOP_AFTER_NEXT_FRAME, according to suggestions by Hans
Verkuil. Discussion is at thread started from the parent email (subject
[PATCH 0/2] s5p-mfc: added encoder support for end of stream
handling).
 
  + /row
  /tbody
  /tgroup
/table
  diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml 
  b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
  index 9caa49a..ad49f7d 100644
  --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
  +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
  @@ -76,6 +76,8 @@ supports capturing from specific video inputs and you 
  want to specify a video
input, then structfieldflags/structfield should be set to
constantV4L2_BUF_FLAG_INPUT/constant and the field
structfieldinput/structfield must be initialized to the desired input.
  +Some drivers expects applications set 
  constantV4L2_BUF_FLAG_EOS/constant
  +flag on the last buffer of the stream.
The structfieldreserved/structfield field must be set to 0. When using
the link linkend=planar-apismulti-planar API/link, the
structfieldm.planes/structfield field must contain a userspace pointer
  diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
  index 370d111..e44a7cd 100644
  --- a/include/linux/videodev2.h
  +++ b/include/linux/videodev2.h
  @@ -676,6 +676,7 @@ struct v4l2_buffer {
/* Cache handling flags */
#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800
#define V4L2_BUF_FLAG_NO_CACHE_CLEAN  0x1000
  +#define V4L2_BUF_FLAG_EOS  0x2000  /* The last buffer in the stream */

/*
 *O V E R L A Y   P R E V I E W
  
 
 
Regards
Andrzej


--
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: [RFCv1 PATCH 18/32] v4l2-ioctl.c: finalize table conversion.

2012-06-18 Thread Mauro Carvalho Chehab
Em 18-06-2012 08:49, Hans Verkuil escreveu:
 On Mon June 18 2012 12:50:35 Mauro Carvalho Chehab wrote:
 Em 18-06-2012 06:46, Laurent Pinchart escreveu:
 Hi Hans,

 Thanks for the patch.

 On Sunday 10 June 2012 12:25:40 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---
drivers/media/video/v4l2-ioctl.c |   35 
 +--
 1 file changed, 13 insertions(+), 22 deletions(-)

 diff --git a/drivers/media/video/v4l2-ioctl.c
 b/drivers/media/video/v4l2-ioctl.c index 0de31c4..6c91674 100644
 --- a/drivers/media/video/v4l2-ioctl.c
 +++ b/drivers/media/video/v4l2-ioctl.c
 @@ -870,6 +870,11 @@ static void v4l_print_newline(const void *arg)
pr_cont(\n);
}

 +static void v4l_print_default(const void *arg)
 +{
 +  pr_cont(non-standard ioctl\n);

 I'd say driver-specific ioctl instead. non-standard may sound like an
 error to users.

 This message is useless as-is, as it provides no glue about what ioctl was
 called. You should either remove it or print the ioctl number, in hexa.
 
 That ioctl number is already printed in front of this message.

Hmm... should we print it when the ioctl is known? IMHO, the behavior should be 
to
either print the ioctl name or its number, as those debug messages are 
generally big.
So, better to not pollute it with duplicated information.
 
 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: [RFCv1 PATCH 29/32] v4l2-dev.c: also add debug support for the fops.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

On Monday 18 June 2012 13:40:24 Hans Verkuil wrote:
 On Mon June 18 2012 12:01:47 Laurent Pinchart wrote:
  On Sunday 10 June 2012 12:25:51 Hans Verkuil wrote:
   From: Hans Verkuil hans.verk...@cisco.com
   
   Signed-off-by: Hans Verkuil hans.verk...@cisco.com
   ---
   
drivers/media/video/v4l2-dev.c |   41 ++---
1 file changed, 29 insertions(+), 12 deletions(-)
   
   diff --git a/drivers/media/video/v4l2-dev.c
   b/drivers/media/video/v4l2-dev.c index 5c0bb18..54f387d 100644
   --- a/drivers/media/video/v4l2-dev.c
   +++ b/drivers/media/video/v4l2-dev.c
   @@ -305,6 +305,9 @@ static ssize_t v4l2_read(struct file *filp, char
   __user
   *buf, ret = vdev-fops-read(filp, buf, sz, off);
   
 if (test_bit(V4L2_FL_LOCK_ALL_FOPS, vdev-flags))
 
 mutex_unlock(vdev-lock);
   
   + if (vdev-debug)
  
  As vdev-debug is a bitmask, shouldn't we add an fops debug bit ?
 
 I actually want to move away from the bitmask idea. I've never really liked
 it here.

Would using dev_dbg with dynamic printk instead of creating our own logging 
system be an option ?

   + pr_info(%s: read: %zd (%d)\n,
   + video_device_node_name(vdev), sz, ret);
  
  Shouldn't we use KERN_DEBUG instead of KERN_INFO ? BTW, what about
  replacing the pr_* calls with dev_* calls ?
 
 KERN_DEBUG vs KERN_INFO is actually a good question. My reasoning is that
 you explicitly enable logging, and so you really want to see it in the log,
 so we use KERN_INFO. With KERN_DEBUG you might have the situation where the
 debug level of the logging is disabled, so the messages are ignored.
 
 However, if people disagree with this, then I'm happy to move it back to
 KERN_DEBUG.

On embedded systems KERN_INFO will be printed to the serial console. 
Interleaving kernel messages with application output during capture result in 
a mess.

If someone enables debugging I expect him/her to know enough to get the kernel 
log debug messages.

 With regards to dev_ vs pr_: I'd have to test this to see what dev_ prints
 as prefix.

-- 
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: [RFCv1 PATCH 25/32] create_bufs: handle count == 0.

2012-06-18 Thread Laurent Pinchart
Hi Hans,

On Monday 18 June 2012 13:43:22 Hans Verkuil wrote:
 On Mon June 18 2012 12:11:27 Laurent Pinchart wrote:
  On Sunday 10 June 2012 12:25:47 Hans Verkuil wrote:
   From: Hans Verkuil hans.verk...@cisco.com
   
   Signed-off-by: Hans Verkuil hans.verk...@cisco.com
   ---
   
Documentation/DocBook/media/v4l/vidioc-create-bufs.xml |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
   
   diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
   b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml index
   765549f..afdba4d 100644
   --- a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
   +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
   @@ -97,7 +97,13 @@ information./para
   
   row
   
 entry__u32/entry
 entrystructfieldcount/structfield/entry
   
   - entryThe number of buffers requested or granted./entry
   + entryThe number of buffers requested or granted. If count == 0,
   then
   + constantVIDIOC_CREATE_BUFS/constant will set
   structfieldindex/structfield
   + to the starting buffer index,
  
  I find starting buffer index a bit unclear in this context, as we don't
  create any buffer.
 
 How about:
 
 ... will set index to the current number of created buffers,

Sounds good to me for now (but it won't be true anymore when we'll support 
deleting buffers individually).

 Better alternatives welcome :-)

-- 
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: [RFCv1 PATCH 29/32] v4l2-dev.c: also add debug support for the fops.

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 14:19:51 Laurent Pinchart wrote:
 Hi Hans,
 
 On Monday 18 June 2012 13:40:24 Hans Verkuil wrote:
  On Mon June 18 2012 12:01:47 Laurent Pinchart wrote:
   On Sunday 10 June 2012 12:25:51 Hans Verkuil wrote:
From: Hans Verkuil hans.verk...@cisco.com

Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---

 drivers/media/video/v4l2-dev.c |   41 ++---
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/media/video/v4l2-dev.c
b/drivers/media/video/v4l2-dev.c index 5c0bb18..54f387d 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -305,6 +305,9 @@ static ssize_t v4l2_read(struct file *filp, char
__user
*buf, ret = vdev-fops-read(filp, buf, sz, off);

if (test_bit(V4L2_FL_LOCK_ALL_FOPS, vdev-flags))

mutex_unlock(vdev-lock);

+   if (vdev-debug)
   
   As vdev-debug is a bitmask, shouldn't we add an fops debug bit ?
  
  I actually want to move away from the bitmask idea. I've never really liked
  it here.
 
 Would using dev_dbg with dynamic printk instead of creating our own logging 
 system be an option ?

I don't think so. There are lots of printk message you'd all have to enable.
You just want to have a standard method of debugging which ioctls are called
that anyone can use and that's consistent for all v4l drivers.

This does just that.

 
+   pr_info(%s: read: %zd (%d)\n,
+   video_device_node_name(vdev), sz, ret);
   
   Shouldn't we use KERN_DEBUG instead of KERN_INFO ? BTW, what about
   replacing the pr_* calls with dev_* calls ?
  
  KERN_DEBUG vs KERN_INFO is actually a good question. My reasoning is that
  you explicitly enable logging, and so you really want to see it in the log,
  so we use KERN_INFO. With KERN_DEBUG you might have the situation where the
  debug level of the logging is disabled, so the messages are ignored.
  
  However, if people disagree with this, then I'm happy to move it back to
  KERN_DEBUG.
 
 On embedded systems KERN_INFO will be printed to the serial console. 
 Interleaving kernel messages with application output during capture result in 
 a mess.
 
 If someone enables debugging I expect him/her to know enough to get the 
 kernel 
 log debug messages.

OK, I'll change this back to KERN_DEBUG. Good argument.

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: [PATCH 1/1] [media] s5p-fimc: Add missing static storage class

2012-06-18 Thread Mauro Carvalho Chehab
Em 11-06-2012 01:27, Sachin Kamat escreveu:
 On 28/05/2012, Sylwester Nawrocki snj...@gmail.com wrote:
 On 05/26/2012 05:11 PM, Sachin Kamat wrote:
 Fixes the following sparse warnings:

 Hi Sachin. Thanks, in case somebody else applies this patch
 before I do:
 Acked-by: Sylwester Nawrocki s.nawro...@samsung.com

 
 Thanks Sylwester.
 
 I would just change the summary line to:
 s5p-fimc: Add missing static storage class specifiers when
 applying this patch.

Applied with the above change.
 
 Ok.
 

 --
 Regards,
 Sylwester

 
 


--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] [media] cx231xx: restore tuner settings on first open

2012-06-18 Thread David Dillow
On Mon, 2012-06-18 at 09:29 +0200, Hans Verkuil wrote:
 On Mon June 18 2012 06:49:58 David Dillow wrote:
  What does the V4L2 API spec say about tuning frequency being persistent
  when there are no users of a video capture device? Is MythTV wrong to
  have that assumption, or is cx231xx wrong to not restore the frequency
  when a user first opens the device?
 
 Tuner standards and frequencies must be persistent. So cx231xx is wrong.
 Actually, all V4L2 settings must in general be persistent (there are
 some per-filehandle settings when dealing with low-level subdev setups or
 mem2mem devices).

Is there a document somewhere I can reference; I need to go through the
cx231xx driver and make sure it is doing the right things and it would
be handy to have a checklist.

  Either way, I think MythTV should keep the device open until it is done
  with it, as that would avoid added latency from putting the tuner to
  sleep and waking it back up. But, I think we should address the issue in
  the driver if it is not living up to the guarantees of the API.
 
 From what I can tell it is a bug in the tda tuner (not restoring the 
 frequency)
 and cx231xx (not setting the initial standard and possibly frequency).

Ok, I'll break this up and have a go at a proper fix. Thanks for the
pointers on the persistence of parameters.

Dave

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] [media] cx231xx: restore tuner settings on first open

2012-06-18 Thread Devin Heitmueller
On Mon, Jun 18, 2012 at 10:15 AM, David Dillow d...@thedillows.org wrote:
 Tuner standards and frequencies must be persistent. So cx231xx is wrong.
 Actually, all V4L2 settings must in general be persistent (there are
 some per-filehandle settings when dealing with low-level subdev setups or
 mem2mem devices).

 Is there a document somewhere I can reference; I need to go through the
 cx231xx driver and make sure it is doing the right things and it would
 be handy to have a checklist.

This isn't just the cx231xx driver.  Almost all USB devices that have
tuners which support power management have this problem
(em28xx/au0828/cx231xx with Xceive or tda18271 tuners).  Really the
tuner_core should be resending the set_params call to the tuner after
waking it up (it shouldn't be the tuner's responsibility to remember
its settings).

The other thing to watch out for is that the API is pretty brittle in
terms of cases where initializing the tuner takes a long time.  In the
case of the Xceive tuners it can take upwards of ten seconds on
devices with slow i2c busses (au0828 in particular due to a hardware
bug in clock stretching), and the 18271 initialization has a
calibration loop which can take 5-10 seconds.  Hence you would have
to be very careful in terms of figuring out where to put the call to
reset the tuner, since you don't want every call to v4l2-ctl taking
ten seconds.

And of course you don't want to leave the tuner running if it's no
longer in use (since it will run down the battery on laptops),
although figuring out when that is can be nearly impossible if the
sequence of events is a series of calls to the v4l2-ctl command line
tool followed by reading from the device node.

I'm certainly in favor of this long-standing problem being fixed, but
it will take considerable investigation to figure out the right place
to make the change.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.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] omap3isp: preview: Add support for non-GRBG Bayer patterns

2012-06-18 Thread Laurent Pinchart
Rearrange the CFA interpolation coefficients table based on the Bayer
pattern. Modifying the table during streaming isn't supported anymore,
but didn't make sense in the first place anyway.

Support for non-Bayer CFA patterns is dropped as they were not correctly
supported, and have never been tested.

Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
 drivers/media/video/omap3isp/isppreview.c |  118 
 1 files changed, 67 insertions(+), 51 deletions(-)

Jean-Philippe,

Could you please test this patch on your hardware ?

diff --git a/drivers/media/video/omap3isp/isppreview.c 
b/drivers/media/video/omap3isp/isppreview.c
index 8a4935e..bfa3107 100644
--- a/drivers/media/video/omap3isp/isppreview.c
+++ b/drivers/media/video/omap3isp/isppreview.c
@@ -309,36 +309,6 @@ preview_config_dcor(struct isp_prev_device *prev, const 
void *prev_dcor)
 }
 
 /*
- * preview_config_cfa - Configures the CFA Interpolation parameters.
- * @prev_cfa: Structure containing the CFA interpolation table, CFA format
- *in the image, vertical and horizontal gradient threshold.
- */
-static void
-preview_config_cfa(struct isp_prev_device *prev, const void *prev_cfa)
-{
-   struct isp_device *isp = to_isp_device(prev);
-   const struct omap3isp_prev_cfa *cfa = prev_cfa;
-   unsigned int i;
-
-   isp_reg_clr_set(isp, OMAP3_ISP_IOMEM_PREV, ISPPRV_PCR,
-   ISPPRV_PCR_CFAFMT_MASK,
-   cfa-format  ISPPRV_PCR_CFAFMT_SHIFT);
-
-   isp_reg_writel(isp,
-   (cfa-gradthrs_vert  ISPPRV_CFA_GRADTH_VER_SHIFT) |
-   (cfa-gradthrs_horz  ISPPRV_CFA_GRADTH_HOR_SHIFT),
-   OMAP3_ISP_IOMEM_PREV, ISPPRV_CFA);
-
-   isp_reg_writel(isp, ISPPRV_CFA_TABLE_ADDR,
-  OMAP3_ISP_IOMEM_PREV, ISPPRV_SET_TBL_ADDR);
-
-   for (i = 0; i  OMAP3ISP_PREV_CFA_TBL_SIZE; i++) {
-   isp_reg_writel(isp, cfa-table[i],
-  OMAP3_ISP_IOMEM_PREV, ISPPRV_SET_TBL_DATA);
-   }
-}
-
-/*
  * preview_config_gammacorrn - Configures the Gamma Correction table values
  * @gtable: Structure containing the table for red, blue, green gamma table.
  */
@@ -813,7 +783,7 @@ static const struct preview_update update_attrs[] = {
FIELD_SIZEOF(struct prev_params, hmed),
offsetof(struct omap3isp_prev_update_config, hmed),
}, /* OMAP3ISP_PREV_CFA */ {
-   preview_config_cfa,
+   NULL,
NULL,
offsetof(struct prev_params, cfa),
FIELD_SIZEOF(struct prev_params, cfa),
@@ -1043,42 +1013,88 @@ preview_config_ycpos(struct isp_prev_device *prev,
 static void preview_config_averager(struct isp_prev_device *prev, u8 average)
 {
struct isp_device *isp = to_isp_device(prev);
-   struct prev_params *params;
-   int reg = 0;
 
-   params = (prev-params.active  OMAP3ISP_PREV_CFA)
-  ? prev-params.params[0] : prev-params.params[1];
-
-   if (params-cfa.format == OMAP3ISP_CFAFMT_BAYER)
-   reg = ISPPRV_AVE_EVENDIST_2  ISPPRV_AVE_EVENDIST_SHIFT |
- ISPPRV_AVE_ODDDIST_2  ISPPRV_AVE_ODDDIST_SHIFT |
- average;
-   else if (params-cfa.format == OMAP3ISP_CFAFMT_RGBFOVEON)
-   reg = ISPPRV_AVE_EVENDIST_3  ISPPRV_AVE_EVENDIST_SHIFT |
- ISPPRV_AVE_ODDDIST_3  ISPPRV_AVE_ODDDIST_SHIFT |
- average;
-   isp_reg_writel(isp, reg, OMAP3_ISP_IOMEM_PREV, ISPPRV_AVE);
+   isp_reg_writel(isp, ISPPRV_AVE_EVENDIST_2  ISPPRV_AVE_EVENDIST_SHIFT |
+  ISPPRV_AVE_ODDDIST_2  ISPPRV_AVE_ODDDIST_SHIFT |
+  average, OMAP3_ISP_IOMEM_PREV, ISPPRV_AVE);
 }
 
+
+#define OMAP3ISP_PREV_CFA_BLK_SIZE (OMAP3ISP_PREV_CFA_TBL_SIZE / 4)
+
 /*
  * preview_config_input_format - Configure the input format
  * @prev: The preview engine
  * @format: Format on the preview engine sink pad
  *
- * Enable CFA interpolation for Bayer formats and disable it for greyscale
- * formats.
+ * Enable and configure CFA interpolation for Bayer formats and disable it for
+ * greyscale formats.
+ *
+ * The CFA table is organised in four blocks, one per Bayer component. The
+ * hardware expects blocks to follow the Bayer order of the input data, while
+ * the driver stores the table in GRBG order in memory. The blocks need to be
+ * reordered to support non-GRBG Bayer patterns.
  */
 static void preview_config_input_format(struct isp_prev_device *prev,
const struct v4l2_mbus_framefmt *format)
 {
+   static const unsigned int cfa_coef_order[4][4] = {
+   { 0, 1, 2, 3 }, /* GRBG */
+   { 1, 0, 3, 2 }, /* RGGB */
+   { 2, 3, 0, 1 }, /* BGGR */
+   { 3, 2, 1, 0 }, /* GBRG */
+   };
struct isp_device *isp = to_isp_device(prev);
+   struct prev_params *params;

Re: [RFC] [media] cx231xx: restore tuner settings on first open

2012-06-18 Thread Hans Verkuil
On Mon June 18 2012 16:15:40 David Dillow wrote:
 On Mon, 2012-06-18 at 09:29 +0200, Hans Verkuil wrote:
  On Mon June 18 2012 06:49:58 David Dillow wrote:
   What does the V4L2 API spec say about tuning frequency being persistent
   when there are no users of a video capture device? Is MythTV wrong to
   have that assumption, or is cx231xx wrong to not restore the frequency
   when a user first opens the device?
  
  Tuner standards and frequencies must be persistent. So cx231xx is wrong.
  Actually, all V4L2 settings must in general be persistent (there are
  some per-filehandle settings when dealing with low-level subdev setups or
  mem2mem devices).
 
 Is there a document somewhere I can reference; I need to go through the
 cx231xx driver and make sure it is doing the right things and it would
 be handy to have a checklist.

By far the easiest method is to run v4l2-compliance from the v4l-utils
repository on the driver. It's not 100%, but it is achieving pretty good
coverage. It's purpose is to verify all the fields are properly filled in,
all the latest frameworks are used, and everything is according to spec.

Make sure you compile the v4l-utils repository yourself to be sure you
use the latest version of this utility.

In principle the V4L2 spec should contain all that information, but it is
sometimes buries in the text and there are some things that are only
available in the collective memory of the developers :-)

   Either way, I think MythTV should keep the device open until it is done
   with it, as that would avoid added latency from putting the tuner to
   sleep and waking it back up. But, I think we should address the issue in
   the driver if it is not living up to the guarantees of the API.
  
  From what I can tell it is a bug in the tda tuner (not restoring the 
  frequency)
  and cx231xx (not setting the initial standard and possibly frequency).
 
 Ok, I'll break this up and have a go at a proper fix. Thanks for the
 pointers on the persistence of parameters.

As Devin mentioned, putting the fix in tuner-core is a better approach
since that fixes it for all such tuners.

And a quite separate problem is when to put a tuner to sleep. That's a very
dark corner in V4L2.

For that someone would have to write an RFC, outlining the various options
and starting a proper discussion on how to solve this.

Regards,

Hans

 
 Dave
 
 --
 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: [RFC] [media] cx231xx: restore tuner settings on first open

2012-06-18 Thread David Dillow
On Mon, 2012-06-18 at 10:23 -0400, Devin Heitmueller wrote:
 This isn't just the cx231xx driver.  Almost all USB devices that have
 tuners which support power management have this problem
 (em28xx/au0828/cx231xx with Xceive or tda18271 tuners).  Really the
 tuner_core should be resending the set_params call to the tuner after
 waking it up (it shouldn't be the tuner's responsibility to remember
 its settings).
 
 The other thing to watch out for is that the API is pretty brittle in
 terms of cases where initializing the tuner takes a long time.  In the
 case of the Xceive tuners it can take upwards of ten seconds on
 devices with slow i2c busses (au0828 in particular due to a hardware
 bug in clock stretching), and the 18271 initialization has a
 calibration loop which can take 5-10 seconds.  Hence you would have
 to be very careful in terms of figuring out where to put the call to
 reset the tuner, since you don't want every call to v4l2-ctl taking
 ten seconds.
 
 And of course you don't want to leave the tuner running if it's no
 longer in use (since it will run down the battery on laptops),
 although figuring out when that is can be nearly impossible if the
 sequence of events is a series of calls to the v4l2-ctl command line
 tool followed by reading from the device node.
 
 I'm certainly in favor of this long-standing problem being fixed, but
 it will take considerable investigation to figure out the right place
 to make the change.

Hmm, it sounds like perhaps changing the standby call in the tuner core
to asynchronously power down the tuner may be the way to go -- ie, when
we tell it to standby, it will do a schedule_work for some 10 seconds
later to really pull it down. If we get a resume call prior to then,
we'll just cancel the work, otherwise we wait for the work to finish and
then issue the resume.

Does that sound reasonable?

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] [media] cx231xx: restore tuner settings on first open

2012-06-18 Thread Devin Heitmueller
On Mon, Jun 18, 2012 at 10:32 AM, David Dillow d...@thedillows.org wrote:
 Hmm, it sounds like perhaps changing the standby call in the tuner core
 to asynchronously power down the tuner may be the way to go -- ie, when
 we tell it to standby, it will do a schedule_work for some 10 seconds
 later to really pull it down. If we get a resume call prior to then,
 we'll just cancel the work, otherwise we wait for the work to finish and
 then issue the resume.

 Does that sound reasonable?

At face value it sounds reasonable, except the approach breaks down as
soon as you have hybrid tuners which support both analog and digital.
Because the digital side of the tuner isn't tied into tuner-core,
you'll break in the following situation:

Start using analog
Stop using analog [schedule_work() call]
Start using digital
Timer pops and powers down the tuner even though it's in use for ATSC
or ClearQAM

Again, I'm not proposing a solution, but just poking a fatal hole in
your proposal (believe me, I had considered the same approach when
first looking at the problem).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.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


cron job: media_tree daily build: ERRORS

2012-06-18 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:Mon Jun 18 19:00:12 CEST 2012
git hash:5472d3f17845c4398c6a510b46855820920c2181
gcc version:  i686-linux-gcc (GCC) 4.7.1
host hardware:x86_64
host os:  3.4.07-marune

linux-git-arm-eabi-davinci: WARNINGS
linux-git-arm-eabi-exynos: WARNINGS
linux-git-arm-eabi-omap: WARNINGS
linux-git-i686: WARNINGS
linux-git-m32r: WARNINGS
linux-git-mips: WARNINGS
linux-git-powerpc64: WARNINGS
linux-git-x86_64: WARNINGS
linux-2.6.31.12-x86_64: WARNINGS
linux-2.6.32.6-x86_64: WARNINGS
linux-2.6.33-x86_64: WARNINGS
linux-2.6.34-x86_64: WARNINGS
linux-2.6.35.3-x86_64: WARNINGS
linux-2.6.36-x86_64: WARNINGS
linux-2.6.37-x86_64: WARNINGS
linux-2.6.38.2-x86_64: WARNINGS
linux-2.6.39.1-x86_64: ERRORS
linux-3.0-x86_64: ERRORS
linux-3.1-x86_64: ERRORS
linux-3.2.1-x86_64: ERRORS
linux-3.3-x86_64: ERRORS
linux-3.4-x86_64: WARNINGS
linux-2.6.31.12-i686: WARNINGS
linux-2.6.32.6-i686: WARNINGS
linux-2.6.33-i686: WARNINGS
linux-2.6.34-i686: WARNINGS
linux-2.6.35.3-i686: WARNINGS
linux-2.6.36-i686: WARNINGS
linux-2.6.37-i686: WARNINGS
linux-2.6.38.2-i686: WARNINGS
linux-2.6.39.1-i686: ERRORS
linux-3.0-i686: ERRORS
linux-3.1-i686: ERRORS
linux-3.2.1-i686: ERRORS
linux-3.3-i686: ERRORS
linux-3.4-i686: WARNINGS
apps: WARNINGS
spec-git: WARNINGS
sparse: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.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


[PATCH 1 of 3] media: gpio-ir-recv: fix missing udev by-path entry

2012-06-18 Thread Benoît Thébaudeau
Add missing information so that udev can create an entry for gpio-ir-recv under
/dev/input/by-path/ .

Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Ravi Kumar V kumar...@codeaurora.org
Cc: linux-media@vger.kernel.org
Signed-off-by: Benoît Thébaudeau benoit.thebaud...@advansee.com
---
 .../drivers/media/rc/gpio-ir-recv.c|6 ++
 1 file changed, 6 insertions(+)

diff --git linux-next-HEAD-6c86b58.orig/drivers/media/rc/gpio-ir-recv.c 
linux-next-HEAD-6c86b58/drivers/media/rc/gpio-ir-recv.c
index 0d87545..b41e13c 100644
--- linux-next-HEAD-6c86b58.orig/drivers/media/rc/gpio-ir-recv.c
+++ linux-next-HEAD-6c86b58/drivers/media/rc/gpio-ir-recv.c
@@ -82,10 +82,16 @@ static int __devinit gpio_ir_recv_probe(struct 
platform_device *pdev)
goto err_allocate_device;
}
 
+   rcdev-priv = gpio_dev;
rcdev-driver_type = RC_DRIVER_IR_RAW;
rcdev-allowed_protos = RC_TYPE_ALL;
rcdev-input_name = GPIO_IR_DEVICE_NAME;
+   rcdev-input_phys = GPIO_IR_DEVICE_NAME /input0;
rcdev-input_id.bustype = BUS_HOST;
+   rcdev-input_id.vendor = 0x0001;
+   rcdev-input_id.product = 0x0001;
+   rcdev-input_id.version = 0x0100;
+   rcdev-dev.parent = pdev-dev;
rcdev-driver_name = GPIO_IR_DRIVER_NAME;
rcdev-map_name = RC_MAP_EMPTY;
 
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0 of 3] media: gpio-ir-recv: several improvements

2012-06-18 Thread Benoît Thébaudeau
Hi all,

This is a set of 3 small improvements for gpio-ir-recv.

Regards,
Benoît Thébaudeau
--
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 2 of 3] media: gpio-ir-recv: add map name

2012-06-18 Thread Benoît Thébaudeau
Make it possible for gpio-ir-recv users to choose a map name.

Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Ravi Kumar V kumar...@codeaurora.org
Cc: linux-media@vger.kernel.org
Signed-off-by: Benoît Thébaudeau benoit.thebaud...@advansee.com
---
 .../drivers/media/rc/gpio-ir-recv.c|2 +-
 .../include/media/gpio-ir-recv.h   |1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git linux-next-HEAD-6c86b58.orig/drivers/media/rc/gpio-ir-recv.c 
linux-next-HEAD-6c86b58/drivers/media/rc/gpio-ir-recv.c
index b41e13c..15e346e 100644
--- linux-next-HEAD-6c86b58.orig/drivers/media/rc/gpio-ir-recv.c
+++ linux-next-HEAD-6c86b58/drivers/media/rc/gpio-ir-recv.c
@@ -93,7 +93,7 @@ static int __devinit gpio_ir_recv_probe(struct 
platform_device *pdev)
rcdev-input_id.version = 0x0100;
rcdev-dev.parent = pdev-dev;
rcdev-driver_name = GPIO_IR_DRIVER_NAME;
-   rcdev-map_name = RC_MAP_EMPTY;
+   rcdev-map_name = pdata-map_name ?: RC_MAP_EMPTY;
 
gpio_dev-rcdev = rcdev;
gpio_dev-gpio_nr = pdata-gpio_nr;
diff --git linux-next-HEAD-6c86b58.orig/include/media/gpio-ir-recv.h 
linux-next-HEAD-6c86b58/include/media/gpio-ir-recv.h
index 67797bf..91546f3 100644
--- linux-next-HEAD-6c86b58.orig/include/media/gpio-ir-recv.h
+++ linux-next-HEAD-6c86b58/include/media/gpio-ir-recv.h
@@ -16,6 +16,7 @@
 struct gpio_ir_recv_platform_data {
int gpio_nr;
bool active_low;
+   const char *map_name;
 };
 
 #endif /* __GPIO_IR_RECV_H__ */
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3 of 3] media: gpio-ir-recv: switch to module_platform_driver

2012-06-18 Thread Benoît Thébaudeau
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Ravi Kumar V kumar...@codeaurora.org
Cc: linux-media@vger.kernel.org
Signed-off-by: Benoît Thébaudeau benoit.thebaud...@advansee.com
---
 .../drivers/media/rc/gpio-ir-recv.c|   13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git linux-next-HEAD-6c86b58.orig/drivers/media/rc/gpio-ir-recv.c 
linux-next-HEAD-6c86b58/drivers/media/rc/gpio-ir-recv.c
index 15e346e..59fe60c 100644
--- linux-next-HEAD-6c86b58.orig/drivers/media/rc/gpio-ir-recv.c
+++ linux-next-HEAD-6c86b58/drivers/media/rc/gpio-ir-recv.c
@@ -194,18 +194,7 @@ static struct platform_driver gpio_ir_recv_driver = {
 #endif
},
 };
-
-static int __init gpio_ir_recv_init(void)
-{
-   return platform_driver_register(gpio_ir_recv_driver);
-}
-module_init(gpio_ir_recv_init);
-
-static void __exit gpio_ir_recv_exit(void)
-{
-   platform_driver_unregister(gpio_ir_recv_driver);
-}
-module_exit(gpio_ir_recv_exit);
+module_platform_driver(gpio_ir_recv_driver);
 
 MODULE_DESCRIPTION(GPIO IR Receiver driver);
 MODULE_LICENSE(GPL v2);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

2012-06-18 Thread Ezequiel Garcia
Hi Mauro,

This patchset cleans the i2c part of some drivers.
This issue was recently reported by Dan Carpenter [1],
and revealed wrong (and harmless) usage of struct i2c_algo_bit.

Also, I properly assigned bus-i2c_rc (return code variable) and
replaced struct memcpy with struct assignment.
The latter is, in my opinion, a much safer way for struct filling
and I'm not aware of any drawbacks.

The patches are based on today's linux-next; I hope this is okey.
As I don't own any of these devices, I can't test the changes beyond
compilation.

Ezequiel Garcia (12):
  cx25821: Replace struct memcpy with struct assignment
  cx25821: Remove useless struct i2c_algo_bit_data usage
  cx25821: Use i2c_rc properly to store i2c register status
  cx231xx: Replace struct memcpy with struct assignment
  cx231xx: Remove useless struct i2c_algo_bit_data usage
  cx231xx: Use i2c_rc properly to store i2c register status
  cx23885: Replace struct memcpy with struct assignment
  cx23885: Remove useless struct i2c_algo_bit_data
  cx23885: Use i2c_rc properly to store i2c register status
  saa7164: Replace struct memcpy with struct assignment
  saa7164: Remove useless struct i2c_algo_bit_data
  saa7164: Use i2c_rc properly to store i2c register status

 drivers/media/video/cx231xx/cx231xx-i2c.c |   10 +++---
 drivers/media/video/cx231xx/cx231xx.h |2 --
 drivers/media/video/cx23885/cx23885-i2c.c |   12 +++-
 drivers/media/video/cx23885/cx23885.h |2 --
 drivers/media/video/cx25821/cx25821-i2c.c |   12 +++-
 drivers/media/video/cx25821/cx25821.h |2 --
 drivers/media/video/saa7164/saa7164-i2c.c |   13 +++--
 drivers/media/video/saa7164/saa7164.h |2 --
 8 files changed, 12 insertions(+), 43 deletions(-)

Thanks,
Ezequiel.

[1] 
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/49553
--
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 01/12] saa7164: Use i2c_rc properly to store i2c register status

2012-06-18 Thread Ezequiel Garcia
Signed-off-by: Ezequiel Garcia elezegar...@gmail.com
---
 drivers/media/video/saa7164/saa7164-i2c.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/saa7164/saa7164-i2c.c 
b/drivers/media/video/saa7164/saa7164-i2c.c
index 26148f7..536f7dc 100644
--- a/drivers/media/video/saa7164/saa7164-i2c.c
+++ b/drivers/media/video/saa7164/saa7164-i2c.c
@@ -123,7 +123,7 @@ int saa7164_i2c_register(struct saa7164_i2c *bus)
bus-i2c_algo.data = bus;
bus-i2c_adap.algo_data = bus;
i2c_set_adapdata(bus-i2c_adap, bus);
-   i2c_add_adapter(bus-i2c_adap);
+   bus-i2c_rc = i2c_add_adapter(bus-i2c_adap);
 
bus-i2c_client.adapter = bus-i2c_adap;
 
-- 
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


[PATCH 02/12] saa7164: Remove useless struct i2c_algo_bit_data

2012-06-18 Thread Ezequiel Garcia
Signed-off-by: Ezequiel Garcia elezegar...@gmail.com
---
 drivers/media/video/saa7164/saa7164-i2c.c |4 
 drivers/media/video/saa7164/saa7164.h |1 -
 2 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/media/video/saa7164/saa7164-i2c.c 
b/drivers/media/video/saa7164/saa7164-i2c.c
index 536f7dc..8df15ca 100644
--- a/drivers/media/video/saa7164/saa7164-i2c.c
+++ b/drivers/media/video/saa7164/saa7164-i2c.c
@@ -109,9 +109,6 @@ int saa7164_i2c_register(struct saa7164_i2c *bus)
memcpy(bus-i2c_adap, saa7164_i2c_adap_template,
   sizeof(bus-i2c_adap));
 
-   memcpy(bus-i2c_algo, saa7164_i2c_algo_template,
-  sizeof(bus-i2c_algo));
-
memcpy(bus-i2c_client, saa7164_i2c_client_template,
   sizeof(bus-i2c_client));
 
@@ -120,7 +117,6 @@ int saa7164_i2c_register(struct saa7164_i2c *bus)
strlcpy(bus-i2c_adap.name, bus-dev-name,
sizeof(bus-i2c_adap.name));
 
-   bus-i2c_algo.data = bus;
bus-i2c_adap.algo_data = bus;
i2c_set_adapdata(bus-i2c_adap, bus);
bus-i2c_rc = i2c_add_adapter(bus-i2c_adap);
diff --git a/drivers/media/video/saa7164/saa7164.h 
b/drivers/media/video/saa7164/saa7164.h
index 8d120e3..fc1f854 100644
--- a/drivers/media/video/saa7164/saa7164.h
+++ b/drivers/media/video/saa7164/saa7164.h
@@ -251,7 +251,6 @@ struct saa7164_i2c {
 
/* I2C I/O */
struct i2c_adapter  i2c_adap;
-   struct i2c_algo_bit_datai2c_algo;
struct i2c_client   i2c_client;
u32 i2c_rc;
 };
-- 
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


[PATCH 09/12] cx231xx: Replace struct memcpy with struct assignment

2012-06-18 Thread Ezequiel Garcia
Signed-off-by: Ezequiel Garcia elezegar...@gmail.com
---
 drivers/media/video/cx231xx/cx231xx-i2c.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/cx231xx/cx231xx-i2c.c 
b/drivers/media/video/cx231xx/cx231xx-i2c.c
index f5f4844..2b2e91e 100644
--- a/drivers/media/video/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/video/cx231xx/cx231xx-i2c.c
@@ -499,10 +499,8 @@ int cx231xx_i2c_register(struct cx231xx_i2c *bus)
 
BUG_ON(!dev-cx231xx_send_usb_command);
 
-   memcpy(bus-i2c_adap, cx231xx_adap_template, sizeof(bus-i2c_adap));
-   memcpy(bus-i2c_client, cx231xx_client_template,
-  sizeof(bus-i2c_client));
-
+   bus-i2c_adap = cx231xx_adap_template;
+   bus-i2c_client = cx231xx_client_template;
bus-i2c_adap.dev.parent = dev-udev-dev;
 
strlcpy(bus-i2c_adap.name, bus-dev-name, sizeof(bus-i2c_adap.name));
-- 
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


[PATCH 08/12] cx231xx: Remove useless struct i2c_algo_bit_data usage

2012-06-18 Thread Ezequiel Garcia
Signed-off-by: Ezequiel Garcia elezegar...@gmail.com
---
 drivers/media/video/cx231xx/cx231xx-i2c.c |2 --
 drivers/media/video/cx231xx/cx231xx.h |2 --
 2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/cx231xx/cx231xx-i2c.c 
b/drivers/media/video/cx231xx/cx231xx-i2c.c
index 8064119..f5f4844 100644
--- a/drivers/media/video/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/video/cx231xx/cx231xx-i2c.c
@@ -500,7 +500,6 @@ int cx231xx_i2c_register(struct cx231xx_i2c *bus)
BUG_ON(!dev-cx231xx_send_usb_command);
 
memcpy(bus-i2c_adap, cx231xx_adap_template, sizeof(bus-i2c_adap));
-   memcpy(bus-i2c_algo, cx231xx_algo, sizeof(bus-i2c_algo));
memcpy(bus-i2c_client, cx231xx_client_template,
   sizeof(bus-i2c_client));
 
@@ -508,7 +507,6 @@ int cx231xx_i2c_register(struct cx231xx_i2c *bus)
 
strlcpy(bus-i2c_adap.name, bus-dev-name, sizeof(bus-i2c_adap.name));
 
-   bus-i2c_algo.data = bus;
bus-i2c_adap.algo_data = bus;
i2c_set_adapdata(bus-i2c_adap, dev-v4l2_dev);
bus-i2c_rc = i2c_add_adapter(bus-i2c_adap);
diff --git a/drivers/media/video/cx231xx/cx231xx.h 
b/drivers/media/video/cx231xx/cx231xx.h
index e174475..a89d020 100644
--- a/drivers/media/video/cx231xx/cx231xx.h
+++ b/drivers/media/video/cx231xx/cx231xx.h
@@ -26,7 +26,6 @@
 #include linux/types.h
 #include linux/ioctl.h
 #include linux/i2c.h
-#include linux/i2c-algo-bit.h
 #include linux/workqueue.h
 #include linux/mutex.h
 
@@ -481,7 +480,6 @@ struct cx231xx_i2c {
 
/* i2c i/o */
struct i2c_adapter i2c_adap;
-   struct i2c_algo_bit_data i2c_algo;
struct i2c_client i2c_client;
u32 i2c_rc;
 
-- 
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


[PATCH 10/12] cx25821: Use i2c_rc properly to store i2c register status

2012-06-18 Thread Ezequiel Garcia
Signed-off-by: Ezequiel Garcia elezegar...@gmail.com
---
 drivers/media/video/cx25821/cx25821-i2c.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/cx25821/cx25821-i2c.c 
b/drivers/media/video/cx25821/cx25821-i2c.c
index 6311180..398e0e6 100644
--- a/drivers/media/video/cx25821/cx25821-i2c.c
+++ b/drivers/media/video/cx25821/cx25821-i2c.c
@@ -319,7 +319,7 @@ int cx25821_i2c_register(struct cx25821_i2c *bus)
bus-i2c_algo.data = bus;
bus-i2c_adap.algo_data = bus;
i2c_set_adapdata(bus-i2c_adap, dev-v4l2_dev);
-   i2c_add_adapter(bus-i2c_adap);
+   bus-i2c_rc = i2c_add_adapter(bus-i2c_adap);
 
bus-i2c_client.adapter = bus-i2c_adap;
 
-- 
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


[PATCH 05/12] cx23885: Remove useless struct i2c_algo_bit_data

2012-06-18 Thread Ezequiel Garcia
Signed-off-by: Ezequiel Garcia elezegar...@gmail.com
---
 drivers/media/video/cx23885/cx23885-i2c.c |3 ---
 drivers/media/video/cx23885/cx23885.h |2 --
 drivers/media/video/saa7164/saa7164.h |1 -
 3 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/cx23885/cx23885-i2c.c 
b/drivers/media/video/cx23885/cx23885-i2c.c
index 7a93fc6..1404050 100644
--- a/drivers/media/video/cx23885/cx23885-i2c.c
+++ b/drivers/media/video/cx23885/cx23885-i2c.c
@@ -318,8 +318,6 @@ int cx23885_i2c_register(struct cx23885_i2c *bus)
 
memcpy(bus-i2c_adap, cx23885_i2c_adap_template,
   sizeof(bus-i2c_adap));
-   memcpy(bus-i2c_algo, cx23885_i2c_algo_template,
-  sizeof(bus-i2c_algo));
memcpy(bus-i2c_client, cx23885_i2c_client_template,
   sizeof(bus-i2c_client));
 
@@ -328,7 +326,6 @@ int cx23885_i2c_register(struct cx23885_i2c *bus)
strlcpy(bus-i2c_adap.name, bus-dev-name,
sizeof(bus-i2c_adap.name));
 
-   bus-i2c_algo.data = bus;
bus-i2c_adap.algo_data = bus;
i2c_set_adapdata(bus-i2c_adap, dev-v4l2_dev);
bus-i2c_rc = i2c_add_adapter(bus-i2c_adap);
diff --git a/drivers/media/video/cx23885/cx23885.h 
b/drivers/media/video/cx23885/cx23885.h
index d884784..3cf397f 100644
--- a/drivers/media/video/cx23885/cx23885.h
+++ b/drivers/media/video/cx23885/cx23885.h
@@ -21,7 +21,6 @@
 
 #include linux/pci.h
 #include linux/i2c.h
-#include linux/i2c-algo-bit.h
 #include linux/kdev_t.h
 #include linux/slab.h
 
@@ -246,7 +245,6 @@ struct cx23885_i2c {
 
/* i2c i/o */
struct i2c_adapter i2c_adap;
-   struct i2c_algo_bit_data   i2c_algo;
struct i2c_client  i2c_client;
u32i2c_rc;
 
diff --git a/drivers/media/video/saa7164/saa7164.h 
b/drivers/media/video/saa7164/saa7164.h
index fc1f854..35219b9 100644
--- a/drivers/media/video/saa7164/saa7164.h
+++ b/drivers/media/video/saa7164/saa7164.h
@@ -46,7 +46,6 @@
 
 #include linux/pci.h
 #include linux/i2c.h
-#include linux/i2c-algo-bit.h
 #include linux/kdev_t.h
 #include linux/mutex.h
 #include linux/crc32.h
-- 
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


[PATCH 12/12] cx25821: Replace struct memcpy with struct assignment

2012-06-18 Thread Ezequiel Garcia
Signed-off-by: Ezequiel Garcia elezegar...@gmail.com
---
 drivers/media/video/cx25821/cx25821-i2c.c |7 ++-
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/video/cx25821/cx25821-i2c.c 
b/drivers/media/video/cx25821/cx25821-i2c.c
index 431fa7f..8a823b8 100644
--- a/drivers/media/video/cx25821/cx25821-i2c.c
+++ b/drivers/media/video/cx25821/cx25821-i2c.c
@@ -305,11 +305,8 @@ int cx25821_i2c_register(struct cx25821_i2c *bus)
 
dprintk(1, %s(bus = %d)\n, __func__, bus-nr);
 
-   memcpy(bus-i2c_adap, cx25821_i2c_adap_template,
-  sizeof(bus-i2c_adap));
-   memcpy(bus-i2c_client, cx25821_i2c_client_template,
-  sizeof(bus-i2c_client));
-
+   bus-i2c_adap = cx25821_i2c_adap_template;
+   bus-i2c_client = cx25821_i2c_client_template;
bus-i2c_adap.dev.parent = dev-pci-dev;
 
strlcpy(bus-i2c_adap.name, bus-dev-name, sizeof(bus-i2c_adap.name));
-- 
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


[PATCH] media: add Analog Devices ADV7393 video encoder driver

2012-06-18 Thread Benoît Thébaudeau
Add ADV7393 I²C-based video encoder driver. This driver has been tested on
custom hardware. It has been tested for composite output. It is derived from the
ADV7343 driver.

Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: linux-media@vger.kernel.org
Signed-off-by: Benoît Thébaudeau benoit.thebaud...@advansee.com
---
 .../drivers/media/video/Kconfig|9 +
 .../drivers/media/video/Makefile   |1 +
 .../drivers/media/video/adv7393.c  |  487 
 .../drivers/media/video/adv7393_regs.h |  188 
 .../include/media/adv7393.h|   28 ++
 .../include/media/v4l2-chip-ident.h|3 +
 6 files changed, 716 insertions(+)
 create mode 100644 linux-next-HEAD-6c86b58/drivers/media/video/adv7393.c
 create mode 100644 linux-next-HEAD-6c86b58/drivers/media/video/adv7393_regs.h
 create mode 100644 linux-next-HEAD-6c86b58/include/media/adv7393.h

diff --git linux-next-HEAD-6c86b58.orig/drivers/media/video/Kconfig 
linux-next-HEAD-6c86b58/drivers/media/video/Kconfig
index 99937c9..d00dee9 100644
--- linux-next-HEAD-6c86b58.orig/drivers/media/video/Kconfig
+++ linux-next-HEAD-6c86b58/drivers/media/video/Kconfig
@@ -461,6 +461,15 @@ config VIDEO_ADV7343
  To compile this driver as a module, choose M here: the
  module will be called adv7343.
 
+config VIDEO_ADV7393
+   tristate ADV7393 video encoder
+   depends on I2C
+   help
+ Support for Analog Devices I2C bus based ADV7393 encoder.
+
+ To compile this driver as a module, choose M here: the
+ module will be called adv7393.
+
 config VIDEO_AK881X
tristate AK8813/AK8814 video encoders
depends on I2C
diff --git linux-next-HEAD-6c86b58.orig/drivers/media/video/Makefile 
linux-next-HEAD-6c86b58/drivers/media/video/Makefile
index d209de0..b7da9fa 100644
--- linux-next-HEAD-6c86b58.orig/drivers/media/video/Makefile
+++ linux-next-HEAD-6c86b58/drivers/media/video/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
 obj-$(CONFIG_VIDEO_ADV7180) += adv7180.o
 obj-$(CONFIG_VIDEO_ADV7183) += adv7183.o
 obj-$(CONFIG_VIDEO_ADV7343) += adv7343.o
+obj-$(CONFIG_VIDEO_ADV7393) += adv7393.o
 obj-$(CONFIG_VIDEO_VPX3220) += vpx3220.o
 obj-$(CONFIG_VIDEO_VS6624)  += vs6624.o
 obj-$(CONFIG_VIDEO_BT819) += bt819.o
diff --git linux-next-HEAD-6c86b58/drivers/media/video/adv7393.c 
linux-next-HEAD-6c86b58/drivers/media/video/adv7393.c
new file mode 100644
index 000..ca72486
--- /dev/null
+++ linux-next-HEAD-6c86b58/drivers/media/video/adv7393.c
@@ -0,0 +1,487 @@
+/*
+ * adv7393 - ADV7393 Video Encoder Driver
+ *
+ * The encoder hardware does not support SECAM.
+ *
+ * Copyright (C) 2010-2012 ADVANSEE - http://www.advansee.com/
+ * Benoît Thébaudeau benoit.thebaud...@advansee.com
+ *
+ * Based on ADV7343 driver,
+ *
+ * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed .as is. WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/init.h
+#include linux/ctype.h
+#include linux/slab.h
+#include linux/i2c.h
+#include linux/device.h
+#include linux/delay.h
+#include linux/module.h
+#include linux/videodev2.h
+#include linux/uaccess.h
+
+#include media/adv7393.h
+#include media/v4l2-device.h
+#include media/v4l2-chip-ident.h
+#include media/v4l2-ctrls.h
+
+#include adv7393_regs.h
+
+MODULE_DESCRIPTION(ADV7393 video encoder driver);
+MODULE_LICENSE(GPL);
+
+static bool debug;
+module_param(debug, bool, 0644);
+MODULE_PARM_DESC(debug, Debug level 0-1);
+
+struct adv7393_state {
+   struct v4l2_subdev sd;
+   struct v4l2_ctrl_handler hdl;
+   u8 reg00;
+   u8 reg01;
+   u8 reg02;
+   u8 reg35;
+   u8 reg80;
+   u8 reg82;
+   u32 output;
+   v4l2_std_id std;
+};
+
+static inline struct adv7393_state *to_state(struct v4l2_subdev *sd)
+{
+   return container_of(sd, struct adv7393_state, sd);
+}
+
+static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl)
+{
+   return container_of(ctrl-handler, struct adv7393_state, hdl)-sd;
+}
+
+static inline int adv7393_write(struct v4l2_subdev *sd, u8 reg, u8 value)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+   return i2c_smbus_write_byte_data(client, reg, value);
+}
+
+static const u8 adv7393_init_reg_val[] = {
+   ADV7393_SOFT_RESET, ADV7393_SOFT_RESET_DEFAULT,
+   ADV7393_POWER_MODE_REG, ADV7393_POWER_MODE_REG_DEFAULT,
+
+   ADV7393_HD_MODE_REG1, ADV7393_HD_MODE_REG1_DEFAULT,
+   ADV7393_HD_MODE_REG2, 

RE: [PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

2012-06-18 Thread Palash Bandyopadhyay
Thanks Ezequiel and Dan. The changes look ok. I'll have someone check out the 
changes on the CX devices.

Rgds,
Palash

-Original Message-
From: Ezequiel Garcia [mailto:elezegar...@gmail.com] 
Sent: Monday, June 18, 2012 12:23 PM
To: Mauro Carvalho Chehab
Cc: linux-media; Dan Carpenter; Palash Bandyopadhyay; st...@kernellabs.com
Subject: [PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

Hi Mauro,

This patchset cleans the i2c part of some drivers.
This issue was recently reported by Dan Carpenter [1], and revealed wrong (and 
harmless) usage of struct i2c_algo_bit.

Also, I properly assigned bus-i2c_rc (return code variable) and replaced 
struct memcpy with struct assignment.
The latter is, in my opinion, a much safer way for struct filling and I'm not 
aware of any drawbacks.

The patches are based on today's linux-next; I hope this is okey.
As I don't own any of these devices, I can't test the changes beyond 
compilation.

Ezequiel Garcia (12):
  cx25821: Replace struct memcpy with struct assignment
  cx25821: Remove useless struct i2c_algo_bit_data usage
  cx25821: Use i2c_rc properly to store i2c register status
  cx231xx: Replace struct memcpy with struct assignment
  cx231xx: Remove useless struct i2c_algo_bit_data usage
  cx231xx: Use i2c_rc properly to store i2c register status
  cx23885: Replace struct memcpy with struct assignment
  cx23885: Remove useless struct i2c_algo_bit_data
  cx23885: Use i2c_rc properly to store i2c register status
  saa7164: Replace struct memcpy with struct assignment
  saa7164: Remove useless struct i2c_algo_bit_data
  saa7164: Use i2c_rc properly to store i2c register status

 drivers/media/video/cx231xx/cx231xx-i2c.c |   10 +++---
 drivers/media/video/cx231xx/cx231xx.h |2 --
 drivers/media/video/cx23885/cx23885-i2c.c |   12 +++-
 drivers/media/video/cx23885/cx23885.h |2 --
 drivers/media/video/cx25821/cx25821-i2c.c |   12 +++-
 drivers/media/video/cx25821/cx25821.h |2 --
 drivers/media/video/saa7164/saa7164-i2c.c |   13 +++--
 drivers/media/video/saa7164/saa7164.h |2 --
 8 files changed, 12 insertions(+), 43 deletions(-)

Thanks,
Ezequiel.

[1] 
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/49553

Conexant E-mail Firewall (Conexant.Com) made the following annotations
-
** Legal Disclaimer  

This email may contain confidential and privileged material for the sole use 
of the intended recipient. Any unauthorized review, use or distribution by 
others is strictly prohibited. If you have received the message in error, 
please advise the sender by reply email and delete the message. Thank you. 

** 

-

--
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 0/12] struct i2c_algo_bit_data cleanup on several drivers

2012-06-18 Thread Dan Carpenter
On Mon, Jun 18, 2012 at 04:23:14PM -0300, Ezequiel Garcia wrote:
 Hi Mauro,
 
 This patchset cleans the i2c part of some drivers.
 This issue was recently reported by Dan Carpenter [1],
 and revealed wrong (and harmless) usage of struct i2c_algo_bit.
 

How is this harmless?  We are setting the function pointers to
something completely bogus.  It seems like a bad thing.

regards,
dan carpenter


--
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 0/12] struct i2c_algo_bit_data cleanup on several drivers

2012-06-18 Thread Ezequiel Garcia
On Mon, Jun 18, 2012 at 5:56 PM, Dan Carpenter dan.carpen...@oracle.com wrote:
 On Mon, Jun 18, 2012 at 04:23:14PM -0300, Ezequiel Garcia wrote:
 Hi Mauro,

 This patchset cleans the i2c part of some drivers.
 This issue was recently reported by Dan Carpenter [1],
 and revealed wrong (and harmless) usage of struct i2c_algo_bit.


 How is this harmless?  We are setting the function pointers to
 something completely bogus.  It seems like a bad thing.


You're right, but that wrongly assigned struct  algo_bit_data is never
*ever* used,
since it is not registered.

So, I meant it was harmless in that way, perhaps it wasn't the right term.

Of course, I can be wrong.
Ezequiel.
--
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 0/12] struct i2c_algo_bit_data cleanup on several drivers

2012-06-18 Thread Dan Carpenter
On Mon, Jun 18, 2012 at 06:00:52PM -0300, Ezequiel Garcia wrote:
 On Mon, Jun 18, 2012 at 5:56 PM, Dan Carpenter dan.carpen...@oracle.com 
 wrote:
  On Mon, Jun 18, 2012 at 04:23:14PM -0300, Ezequiel Garcia wrote:
  Hi Mauro,
 
  This patchset cleans the i2c part of some drivers.
  This issue was recently reported by Dan Carpenter [1],
  and revealed wrong (and harmless) usage of struct i2c_algo_bit.
 
 
  How is this harmless?  We are setting the function pointers to
  something completely bogus.  It seems like a bad thing.
 
 
 You're right, but that wrongly assigned struct  algo_bit_data is never
 *ever* used,
 since it is not registered.
 
 So, I meant it was harmless in that way, perhaps it wasn't the right term.

No no.  I didn't realize these files didn't call i2c_add_bus().
Harmless is the right term.

regards,
dan carpenter

--
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 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

2012-06-18 Thread Laurent Pinchart
Hi Bhupesh,

Thanks for the patch. It looks quite good, please see below for various small 
comments.

On Friday 01 June 2012 15:08:57 Bhupesh Sharma wrote:
 This patch reworks the videobuffer management logic present in the UVC
 webcam gadget and ports it to use the more apt videobuf2 framework for
 video buffer management.
 
 To support routing video data captured from a real V4L2 video capture
 device with a zero copy operation on videobuffers (as they pass from the
 V4L2 domain to UVC domain via a user-space application), we need to support
 USER_PTR IO method at the UVC gadget side.
 
 So the V4L2 capture device driver can still continue to use MMAO IO method

s/MMAO/MMAP/

 and now the user-space application can just pass a pointer to the video
 buffers being DeQueued from the V4L2 device side while Queueing them at the

I don't think dequeued and queueing need capitals :-)

 UVC gadget end. This ensures that we have a zero-copy design as the
 videobuffers pass from the V4L2 capture device to the UVC gadget.
 
 Note that there will still be a need to apply UVC specific payload headers
 on top of each UVC payload data, which will still require a copy operation
 to be performed in the 'encode' routines of the UVC gadget.
 
 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 ---
  drivers/usb/gadget/Kconfig |1 +
  drivers/usb/gadget/uvc_queue.c |  524 ++---
  drivers/usb/gadget/uvc_queue.h |   25 +--
  drivers/usb/gadget/uvc_v4l2.c  |   35 ++--
  drivers/usb/gadget/uvc_video.c |   17 +-
  5 files changed, 184 insertions(+), 418 deletions(-)

[snip]

 diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c
 index 0cdf89d..907ece8 100644
 --- a/drivers/usb/gadget/uvc_queue.c
 +++ b/drivers/usb/gadget/uvc_queue.c

[snip]

This part is a bit difficult to review, as git tried too hard to create a 
universal diff where your patch really replaces the code. I'll remove the - 
lines to make the comments as readable as possible.

 +/*
 ---
 --
 + * videobuf2 queue operations
   */
 +
 +static int uvc_queue_setup(struct vb2_queue *vq, const struct v4l2_format
 *fmt,
 + unsigned int *nbuffers, unsigned int *nplanes,
 + unsigned int sizes[], void *alloc_ctxs[])
  {
 + struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
 + struct uvc_video *video =
 + container_of(queue, struct uvc_video, queue);

No need for a line split.

 
 + if (*nbuffers  UVC_MAX_VIDEO_BUFFERS)
 + *nbuffers = UVC_MAX_VIDEO_BUFFERS;
 
 + *nplanes = 1;
 +
 + sizes[0] = video-imagesize;
 
   return 0;
  }
 
 +static int uvc_buffer_prepare(struct vb2_buffer *vb)
  {
 + struct uvc_video_queue *queue = vb2_get_drv_priv(vb-vb2_queue);
 + struct uvc_buffer *buf = container_of(vb, struct uvc_buffer, buf);
 
 + if (vb-v4l2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT 
 + vb2_get_plane_payload(vb, 0)  vb2_plane_size(vb, 0)) {

Please align vb2 with the vb- on the previous line.

Have you by any chance found some inspiration in 
drivers/media/video/uvc/uvc_queue.c ? :-) It would probably make sense to move 
this check to vb2 core, but that's outside of the scope of this patch.

 + uvc_trace(UVC_TRACE_CAPTURE, [E] Bytes used out of bounds.\n);
 + return -EINVAL;
 + }
 
 + if (unlikely(queue-flags  UVC_QUEUE_DISCONNECTED))
 + return -ENODEV;
 
 + buf-state = UVC_BUF_STATE_QUEUED;
 + buf-mem = vb2_plane_vaddr(vb, 0);
 + buf-length = vb2_plane_size(vb, 0);
 + if (vb-v4l2_buf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 + buf-bytesused = 0;
 + else
 + buf-bytesused = vb2_get_plane_payload(vb, 0);

The driver doesn't support the capture type at the moment so this might be a 
bit overkill, but I think it's a good idea to support capture in the queue 
imeplementation. I plan to try and merge the uvcvideo and uvcgadget queue 
implementations at some point.

 
 + return 0;
 +}
 
 +static void uvc_buffer_queue(struct vb2_buffer *vb)
 +{
 + struct uvc_video_queue *queue = vb2_get_drv_priv(vb-vb2_queue);
 + struct uvc_buffer *buf = container_of(vb, struct uvc_buffer, buf);
 + unsigned long flags;
 
 + spin_lock_irqsave(queue-irqlock, flags);
 
 + if (likely(!(queue-flags  UVC_QUEUE_DISCONNECTED))) {
 + list_add_tail(buf-queue, queue-irqqueue);
 + } else {
 + /* If the device is disconnected return the buffer to userspace
 +  * directly. The next QBUF call will fail with -ENODEV.
 +  */
 + buf-state = UVC_BUF_STATE_ERROR;
 + vb2_buffer_done(buf-buf, VB2_BUF_STATE_ERROR);
   }
 
 + spin_unlock_irqrestore(queue-irqlock, flags);
  }
 
 +static struct vb2_ops uvc_queue_qops = {
 + .queue_setup = uvc_queue_setup,
 + 

Re: [PATCH] make VIDEO_MEDIA depends on DVB_CORE only (removing depends VIDEO_DEV)

2012-06-18 Thread Mauro Carvalho Chehab
Em 07-06-2012 22:55, cheng renquan escreveu:
 I think the root cause is VIDEO_MEDIA depending on VIDEO_DEV or DVB_CORE;
 since MEDIA_TUNER is depending on VIDEO_MEDIA;
 I have VIDEO_DEV but not DVB_CORE, hence should be no VIDEO_MEDIA,
 
 config MEDIA_TUNER
  tristate
  default VIDEO_MEDIA  I2C
  depends on VIDEO_MEDIA  I2C
 
 
 diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
 index 9575db4..1b35dae 100644
 --- a/drivers/media/Kconfig
 +++ b/drivers/media/Kconfig
 @@ -99,7 +99,7 @@ config DVB_NET
 
   config VIDEO_MEDIA
   tristate
 - default (DVB_CORE  (VIDEO_DEV = n)) || (VIDEO_DEV  (DVB_CORE =
 n)) || (DVB_CORE  VIDEO_DEV)
 + default DVB_CORE

Sorry, but this is wrong and will break compilation for analog drivers.

I think my RFC patches should fix this issue[1] and [2]. If not, they will at 
the final
version, after removing drivers/media/video and drivers/media/dvb, and
re-writing the dependency chain (planned as part 2 and 3 of my patch series).

[1] http://www.spinics.net/lists/linux-media/msg48451.html

[2] http://www.spinics.net/lists/linux-media/msg48974.html

Regards,
Mauro

 
   comment Multimedia drivers
 
 
 On Thu, Jun 7, 2012 at 4:09 PM, VDR User user@gmail.com wrote:
 On Thu, Jun 7, 2012 at 2:53 PM, cheng renquan crq...@gmail.com wrote:
 till recently I found that also chosen those media tuner modules,

 $ grep MEDIA_TUNER /boot/config
 CONFIG_MEDIA_TUNER=m
 # CONFIG_MEDIA_TUNER_CUSTOMISE is not set
 CONFIG_MEDIA_TUNER_SIMPLE=m
 CONFIG_MEDIA_TUNER_TDA8290=m
 CONFIG_MEDIA_TUNER_TDA827X=m
 CONFIG_MEDIA_TUNER_TDA18271=m
 CONFIG_MEDIA_TUNER_TDA9887=m
 CONFIG_MEDIA_TUNER_TEA5761=m
 CONFIG_MEDIA_TUNER_TEA5767=m
 CONFIG_MEDIA_TUNER_MT20XX=m
 CONFIG_MEDIA_TUNER_XC2028=m
 CONFIG_MEDIA_TUNER_XC5000=m
 CONFIG_MEDIA_TUNER_XC4000=m
 CONFIG_MEDIA_TUNER_MC44S803=m

 as I understand, MEDIA_TUNER is for some tv adapters but I don't have
 such hardware,
 to disable them I need to enable MEDIA_TUNER_CUSTOMISE, then
 a menu Customize TV tuners becomes visible then I need to enter that
 menu and disable all the tuners one-by-one;
 this looks not convenient,

 I hate that too so you're not alone. I've just gotten into the habit
 of having to manually disabling everything I don't need as opposed to
 only needing to enable what I do need. :\



 --
 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: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.

2012-06-18 Thread Mauro Carvalho Chehab
Em 28-05-2012 07:46, Hans Verkuil escreveu:
 From: Hans Verkuil hans.verk...@cisco.com
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Acked-by: Hans de Goede hdego...@redhat.com
 ---
   include/linux/videodev2.h |   19 +--
   1 file changed, 17 insertions(+), 2 deletions(-)
 
 diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
 index 2339678..013ee46 100644
 --- a/include/linux/videodev2.h
 +++ b/include/linux/videodev2.h
 @@ -2023,7 +2023,8 @@ struct v4l2_tuner {
   __u32   audmode;
   __s32   signal;
   __s32   afc;
 - __u32   reserved[4];
 + __u32   band;
 + __u32   reserved[3];
   };
   
   struct v4l2_modulator {
 @@ -2033,7 +2034,8 @@ struct v4l2_modulator {
   __u32   rangelow;
   __u32   rangehigh;
   __u32   txsubchans;
 - __u32   reserved[4];
 + __u32   band;
 + __u32   reserved[3];
   };
   
   /*  Flags for the 'capability' field */
 @@ -2048,6 +2050,11 @@ struct v4l2_modulator {
   #define V4L2_TUNER_CAP_RDS  0x0080
   #define V4L2_TUNER_CAP_RDS_BLOCK_IO 0x0100
   #define V4L2_TUNER_CAP_RDS_CONTROLS 0x0200
 +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US 0x0001
 +#define V4L2_TUNER_CAP_BAND_FM_JAPAN 0x0002
 +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN   0x0004
 +#define V4L2_TUNER_CAP_BAND_FM_WEATHER   0x0008
 +#define V4L2_TUNER_CAP_BAND_AM_MW0x0010

Frequency band is already specified by rangelow/rangehigh.

Why do you need to duplicate this information?


   
   /*  Flags for the 'rxsubchans' field */
   #define V4L2_TUNER_SUB_MONO 0x0001
 @@ -2065,6 +2072,14 @@ struct v4l2_modulator {
   #define V4L2_TUNER_MODE_LANG1   0x0003
   #define V4L2_TUNER_MODE_LANG1_LANG2 0x0004
   
 +/*  Values for the 'band' field */
 +#define V4L2_TUNER_BAND_DEFAULT   0

What does default mean?

 +#define V4L2_TUNER_BAND_FM_EUROPE_US  1   /* 87.5 Mhz - 108 MHz */

EUROPE_US is a bad name for this range. According with Wikipedia, this
range is used at ITU region 1 (Europe/Africa), while America uses 
ITU region 2 (88-108).

In Brazil, the range from 87.5-88 were added several years ago, so it is
currently at the ITU region 1 range, just like in US.

I don't doubt that there are still some places at the 88-108 MHz range.

 +#define V4L2_TUNER_BAND_FM_JAPAN  2   /* 76 MHz - 90 MHz */

This is currently true, but wikipedia points that they may increase it 
(from 76MHz to 108MHz?) after the end of NTSC broadcast.

The DTV range there starts at channel 14 (473 MHz and upper). Maybe they
may reserve the channel 7-13 range (VHF High - starting at 177 MHz) like
Brazil for DTV. 

Anyway, what I mean is that calling a frequency range with a Country name
is dangerous, as frequency ranges can vary from time to time.

 +#define V4L2_TUNER_BAND_FM_RUSSIAN3   /* 65.8 MHz - 74 MHz */

AFAIKT, this is wrong. The range used there is 65.8-104MHz.

It used to be 65.8 to 100 MHz.

Also, other ex-soviet countries are still using such range.

 +#define V4L2_TUNER_BAND_FM_WEATHER4   /* 162.4 MHz - 162.55 MHz */
 +#define V4L2_TUNER_BAND_AM_MW 5
 +
   struct v4l2_frequency {
   __u32 tuner;
   __u32 type; /* enum v4l2_tuner_type */
 

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: SoC i.mx35 userptr method failure while running capture-example utility

2012-06-18 Thread Laurent Pinchart
Hi,

On Monday 18 June 2012 16:26:01 Prabhakar Lad wrote:
 On Mon, Jun 18, 2012 at 4:05 PM, Laurent Pinchart wrote:
  On Monday 18 June 2012 12:28:44 Prabhakar Lad wrote:
  On Wed, May 2, 2012 at 4:20 AM, Guennadi Liakhovetski wrote:
   On Tue, 1 May 2012, Alex Gershgorin wrote:
   Hi everyone,
   
   I use user-space utility from
   http://git.linuxtv.org/v4l-utils.git/blob/HEAD:/contrib/test/capture-
   example.c I made two small changes in this application and this is
   running on i.MX35 SoC
   
   fmt.fmt.pix.pixelformat = V4L2_PIX_FMT_RGB565;
   fmt.fmt.pix.field   = V4L2_FIELD_ANY;
   
   When MMAP method is used everything works fine, problem occurs when
   using USERPTR method this can see bellow :
   
   ./capture-example -u -f -d /dev/video0
   mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
   Failed acquiring VMA for vaddr 0x76cd9008
   VIDIOC_QBUF error 22, Invalid arg
   
   It doesn't surprise me, that this doesn't work. capture-example
   allocates absolutely normal user-space buffers, when called with
   USERPTR, and those buffers are very likely discontiguous. Whereas mx3-
   camera needs physically contiguous buffers, so, this can only fail.
   This means, you either have to use MMAP or you need to allocate USERPTR
   buffers in a special way to guarantee their contiguity.
  
  Even I am facing a similar issue when ported to VB2 for USERPTR.
  How do we ensure the buffers not discontiguous. Would cmem assure it?
  
  CMEM is definitely not the way to go, it's an out-of-tree hack that should
  disappear once we get proper CMA and DMABUF support in the kernel.
  
  How to allocate memory depends on your use case(s). If you just need to
  capture to anonymous memory that will then be read by userspace you
  shouldn't use USERPTR, but MMAP. If you need to capture to device memory
  (for instance capturing directly to a frame buffer), you should export a
  DMABUF object on from the frame buffer driver (this isn't available in
  mainline yet, I'll try to send a patch soon) and then import it on the
  V4L2 side (not available in mainline yet either I'm afraid :-)). As an
  interim solution, mmap() your frame buffer to userspace and then use
  USERPTR.
 
 I have got MMAP working with videobuf2, with videobuf there was a support
 for userptr which I need to achieve this with videobuf2.

videobuf2 supports USERPTR as well. If the memory pointed to by your user 
pointer matches the device requirements, USERPTR should work out of the box.

 Can you point me to your branch where you are working on it and also an
 example if you have to interim solution as you suggested above.

I currently have no public branch with the FBDEV DMABUF patches. I need to 
clean them up and submit them upstream, I hope to do that in the next couple 
of days.

Regarding the interim solution, just mmap() your frame buffer to userspace and 
pass that to your V4L driver using USERPTR. That's what embedded systems 
usually do nowadays.

-- 
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] Revert [media] media: mx2_camera: Fix mbus format handling

2012-06-18 Thread Fabio Estevam
Hi Mauro,

On Fri, May 25, 2012 at 7:06 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 This reverts commit d509835e32bd761a2b7b446034a273da568e5573. That commit
 breaks support for the generic pass-through mode in the driver for formats,
 not natively supported by it. Besides due to a merge conflict it also breaks
 driver compilation:

 drivers/media/video/mx2_camera.c: In function 'mx2_camera_set_bus_param':
 drivers/media/video/mx2_camera.c:937: error: 'pixfmt' undeclared (first use 
 in this function)
 drivers/media/video/mx2_camera.c:937: error: (Each undeclared identifier is 
 reported only once
 drivers/media/video/mx2_camera.c:937: error: for each function it appears in.)

Can this be applied?

It is breaking mxs_defconfig for one month now.

Thanks,

Fabio Estevam
--
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] Revert [media] media: mx2_camera: Fix mbus format handling

2012-06-18 Thread Mauro Carvalho Chehab
Em 18-06-2012 21:55, Fabio Estevam escreveu:
 Hi Mauro,
 
 On Fri, May 25, 2012 at 7:06 PM, Guennadi Liakhovetski
 g.liakhovet...@gmx.de wrote:
 This reverts commit d509835e32bd761a2b7b446034a273da568e5573. That commit
 breaks support for the generic pass-through mode in the driver for formats,
 not natively supported by it. Besides due to a merge conflict it also breaks
 driver compilation:

 drivers/media/video/mx2_camera.c: In function 'mx2_camera_set_bus_param':
 drivers/media/video/mx2_camera.c:937: error: 'pixfmt' undeclared (first use 
 in this function)
 drivers/media/video/mx2_camera.c:937: error: (Each undeclared identifier is 
 reported only once
 drivers/media/video/mx2_camera.c:937: error: for each function it appears 
 in.)
 
 Can this be applied?
 
 It is breaking mxs_defconfig for one month now.

It were applied today at my -next tree.

 
 Thanks,
 
 Fabio Estevam
 


--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] [media] cx231xx: restore tuner settings on first open

2012-06-18 Thread David Dillow
On Mon, 2012-06-18 at 10:36 -0400, Devin Heitmueller wrote:
 On Mon, Jun 18, 2012 at 10:32 AM, David Dillow d...@thedillows.org wrote:
  Hmm, it sounds like perhaps changing the standby call in the tuner core
  to asynchronously power down the tuner may be the way to go -- ie, when
  we tell it to standby, it will do a schedule_work for some 10 seconds
  later to really pull it down. If we get a resume call prior to then,
  we'll just cancel the work, otherwise we wait for the work to finish and
  then issue the resume.
 
  Does that sound reasonable?
 
 At face value it sounds reasonable, except the approach breaks down as
 soon as you have hybrid tuners which support both analog and digital.
 Because the digital side of the tuner isn't tied into tuner-core,
 you'll break in the following situation:
 
 Start using analog
 Stop using analog [schedule_work() call]
 Start using digital
 Timer pops and powers down the tuner even though it's in use for ATSC
 or ClearQAM
 
 Again, I'm not proposing a solution, but just poking a fatal hole in
 your proposal (believe me, I had considered the same approach when
 first looking at the problem).

No worries, I just started looking at V4L stuff on Friday, so I expect
to make some mistakes...

It sounds like we want a solution that
  * lives in core code
  * doesn't require tuner drivers to save state
  * manages hybrid tuners appropriately
  * allows for gradual API change-over (no flag day for tuners or
for capture devices)
  * has a reasonable grace period before putting tuner to standby

Aside from the entering standby issue, fixing the loss of mode/frequency
looks like it may be fixable by just having the capture cards explicitly
request the tuner become active -- the tuner core will already restore
those for us. It's just that few cards do that today.

Is it safe to say that the tuner core will know about all hybrid tuners,
even if it doesn't know if the digital side is still in use?

Can a single tuner be used for both digital and analog tuning at the
same time?

I'm wondering if just having a simple counter would work, with the
digital side calling for power just as the capture side should already
be doing. If the count is non-zero on a standby call, don't do anything.
If it goes to zero, then schedule the HW standby. The challenge would
seem to be getting the DVB system to call into the tuner-core (or other
proper place).

So much to learn... thank you for your patience,
Dave



--
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