date:20110913

Hi Scott

Thanks for the patch. I am not doing a full review, just a couple of notes:

On Tue, 13 Sep 2011, Scott Jiang wrote:

> this is a v4l2 sensor-level driver for the ST VS6624 camera
> 
> Signed-off-by: Scott Jiang 
> ---

[snip]

> diff --git a/drivers/media/video/vs6624.c b/drivers/media/video/vs6624.c
> new file mode 100644
> index 000..b0030ab
> --- /dev/null
> +++ b/drivers/media/video/vs6624.c
> @@ -0,0 +1,941 @@

[snip]

> +#define VGA_WIDTH   640
> +#define VGA_HEIGHT  480
> +#define QVGA_WIDTH  320
> +#define QVGA_HEIGHT 240
> +#define QQVGA_WIDTH 160
> +#define QQVGA_HEIGHT120
> +#define CIF_WIDTH   352
> +#define CIF_HEIGHT  288
> +#define QCIF_WIDTH  176
> +#define QCIF_HEIGHT 144
> +#define QQCIF_WIDTH 88
> +#define QQCIF_HEIGHT72

...Can anyone put these in a central header, please? really, please?;-)

[snip]

> +static const u16 vs6624_p1[] = {
> + 0x8104, 0x03,
> + 0x8105, 0x01,
> + 0xc900, 0x03,
> + 0xc904, 0x47,
> + 0xc905, 0x10,
> + 0xc906, 0x80,
> + 0xc907, 0x3a,
> + 0x903a, 0x02,
> + 0x903b, 0x47,
> + 0x903c, 0x15,
> + 0xc908, 0x31,
> + 0xc909, 0xdc,
> + 0xc90a, 0x80,
> + 0xc90b, 0x44,
> + 0x9044, 0x02,
> + 0x9045, 0x31,
> + 0x9046, 0xe2,
> + 0xc90c, 0x07,
> + 0xc90d, 0xe0,
> + 0xc90e, 0x80,
> + 0xc90f, 0x47,
> + 0x9047, 0x90,
> + 0x9048, 0x83,
> + 0x9049, 0x81,
> + 0x904a, 0xe0,
> + 0x904b, 0x60,
> + 0x904c, 0x08,
> + 0x904d, 0x90,
> + 0x904e, 0xc0,
> + 0x904f, 0x43,
> + 0x9050, 0x74,
> + 0x9051, 0x01,
> + 0x9052, 0xf0,
> + 0x9053, 0x80,
> + 0x9054, 0x05,
> + 0x9055, 0xE4,
> + 0x9056, 0x90,
> + 0x9057, 0xc0,
> + 0x9058, 0x43,
> + 0x9059, 0xf0,
> + 0x905a, 0x02,
> + 0x905b, 0x07,
> + 0x905c, 0xec,
> + 0xc910, 0x5d,
> + 0xc911, 0xca,
> + 0xc912, 0x80,
> + 0xc913, 0x5d,
> + 0x905d, 0xa3,
> + 0x905e, 0x04,
> + 0x905f, 0xf0,
> + 0x9060, 0xa3,
> + 0x9061, 0x04,
> + 0x9062, 0xf0,
> + 0x9063, 0x22,
> + 0xc914, 0x72,
> + 0xc915, 0x92,
> + 0xc916, 0x80,
> + 0xc917, 0x64,
> + 0x9064, 0x74,
> + 0x9065, 0x01,
> + 0x9066, 0x02,
> + 0x9067, 0x72,
> + 0x9068, 0x95,
> + 0xc918, 0x47,
> + 0xc919, 0xf2,
> + 0xc91a, 0x81,
> + 0xc91b, 0x69,
> + 0x9169, 0x74,
> + 0x916a, 0x02,
> + 0x916b, 0xf0,
> + 0x916c, 0xec,
> + 0x916d, 0xb4,
> + 0x916e, 0x10,
> + 0x916f, 0x0a,
> + 0x9170, 0x90,
> + 0x9171, 0x80,
> + 0x9172, 0x16,
> + 0x9173, 0xe0,
> + 0x9174, 0x70,
> + 0x9175, 0x04,
> + 0x9176, 0x90,
> + 0x9177, 0xd3,
> + 0x9178, 0xc4,
> + 0x9179, 0xf0,
> + 0x917a, 0x22,
> + 0xc91c, 0x0a,
> + 0xc91d, 0xbe,
> + 0xc91e, 0x80,
> + 0xc91f, 0x73,
> + 0x9073, 0xfc,
> + 0x9074, 0xa3,
> + 0x9075, 0xe0,
> + 0x9076, 0xf5,
> + 0x9077, 0x82,
> + 0x9078, 0x8c,
> + 0x9079, 0x83,
> + 0x907a, 0xa3,
> + 0x907b, 0xa3,
> + 0x907c, 0xe0,
> + 0x907d, 0xfc,
> + 0x907e, 0xa3,
> + 0x907f, 0xe0,
> + 0x9080, 0xc3,
> + 0x9081, 0x9f,
> + 0x9082, 0xff,
> + 0x9083, 0xec,
> + 0x9084, 0x9e,
> + 0x9085, 0xfe,
> + 0x9086, 0x02,
> + 0x9087, 0x0a,
> + 0x9088, 0xea,
> + 0xc920, 0x47,
> + 0xc921, 0x38,
> + 0xc922, 0x80,
> + 0xc923, 0x89,
> + 0x9089, 0xec,
> + 0x908a, 0xd3,
> + 0x908b, 0x94,
> + 0x908c, 0x20,
> + 0x908d, 0x40,
> + 0x908e, 0x01,
> + 0x908f, 0x1c,
> + 0x9090, 0x90,
> + 0x9091, 0xd3,
> + 0x9092, 0xd4,
> + 0x9093, 0xec,
> + 0x9094, 0xf0,
> + 0x9095, 0x02,
> + 0x9096, 0x47,
> + 0x9097, 0x3d,
> + 0xc924, 0x45,
> + 0xc925, 0xca,
> + 0xc926, 0x80,
> + 0xc927, 0x98,
> + 0x9098, 0x12,
> + 0x9099, 0x77,
> + 0x909a, 0xd6,
> + 0x909b, 0x02,
> + 0x909c, 0x45,
> + 0x909d, 0xcd,
> + 0xc928, 0x20,
> + 0xc929, 0xd5,
> + 0xc92a, 0x80,
> + 0xc92b, 0x9e,
> + 0x909e, 0x90,
> + 0x909f, 0x82,
> + 0x90a0, 0x18,
> + 0x90a1, 0xe0,
> + 0x90a2, 0xb4,
> + 0x90a3, 0x03,
> + 0x90a4, 0x0e,
> + 0x90a5, 0x90,
> + 0x90a6, 0x83,
> + 0x90a7, 0xbf,
> + 0x90a8, 0xe0,
> + 0x90a9, 0x60,
> + 0x90aa, 0x08,
> + 0x90ab, 0x90,
> + 0x90ac, 0x81,
> + 0x90ad, 0xfc,
> + 0x90ae, 0xe0,
> + 0x90af, 0xff,
> + 0x90b0, 0xc3,
> + 0x90b1, 0x13,
> + 0x90b2, 0xf0,
> + 0x90b3, 0x90,
> + 0x90b4, 0x81,
> + 0x90b5, 0xfc,
> + 0x90b6, 0xe0,
> + 0x90b7, 0xff,
> + 0x90b8, 0x02,
> + 0x90b9, 0x20,
> + 0x90ba, 0xda,
> + 0xc92c, 0x70,
> + 0xc92d, 0xbc,
> + 0xc92e, 0x80,
> + 0xc92f, 0xbb,
> + 0x90bb, 0x90,
> + 0x90bc, 0x82,
> + 0x90bd, 0x18,
> + 0x90be, 0xe0,
> + 0x90bf, 0xb4,
> + 0x90c0, 0x03,
> + 0x90c1, 0x06,
> + 0x90c2, 0x90

Re: [PATCH 4/4] v4l2: add blackfin capture bridge driver

Again, no complete review, just a couple of remarks

On Tue, 13 Sep 2011, Scott Jiang wrote:

> this is a v4l2 bridge driver for Blackfin video capture device,
> support ppi interface
> 
> Signed-off-by: Scott Jiang 
> ---

[snip]

> diff --git a/drivers/media/video/blackfin/bfin_capture.c 
> b/drivers/media/video/blackfin/bfin_capture.c
> new file mode 100644
> index 000..24f89f2
> --- /dev/null
> +++ b/drivers/media/video/blackfin/bfin_capture.c
> @@ -0,0 +1,1099 @@
> +/*
> + * Analog Devices video capture driver
> + *
> + * Copyright (c) 2011 Analog Devices Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include 

Alphabetic order of headers is preferred. Also holds for other your patches.

> +
> +#define CAPTURE_DRV_NAME"bfin_capture"
> +#define BCAP_MIN_NUM_BUF2
> +
> +struct bcap_format {
> + u8 *desc;
> + u32 pixelformat;
> + enum v4l2_mbus_pixelcode mbus_code;
> + int bpp; /* bytes per pixel */

Don't you think you might have to process 12 bpp formats at some point, 
like YUV 4:2:0, or NV12? Maybe better calculate in bits from the beginning?

> +};
> +
> +struct bcap_buffer {
> + struct vb2_buffer vb;
> + struct list_head list;
> +};
> +
> +struct bcap_device {
> + /* capture device instance */
> + struct v4l2_device v4l2_dev;
> + /* device node data */
> + struct video_device *video_dev;
> + /* sub device instance */
> + struct v4l2_subdev *sd;
> + /* caputre config */
> + struct bfin_capture_config *cfg;
> + /* ppi interface */
> + struct ppi_if *ppi;
> + /* current input */
> + unsigned int cur_input;
> + /* current selected standard */
> + v4l2_std_id std;
> + /* used to store pixel format */
> + struct v4l2_pix_format fmt;
> + /* bytes per pixel*/
> + int bpp;
> + /* pointing to current video buffer */
> + struct bcap_buffer *cur_frm;
> + /* pointing to next video buffer */
> + struct bcap_buffer *next_frm;
> + /* buffer queue used in videobuf2 */
> + struct vb2_queue buffer_queue;
> + /* allocator-specific contexts for each plane */
> + struct vb2_alloc_ctx *alloc_ctx;
> + /* queue of filled frames */
> + struct list_head dma_queue;
> + /* used in videobuf2 callback */
> + spinlock_t lock;
> + /* used to access capture device */
> + struct mutex mutex;
> + /* used to wait ppi to complete one transfer */
> + struct completion comp;
> + /* number of users performing IO */
> + u32 io_usrs;

Does it really have to be fixed 32 bits? Seems a plane simple int would do 
just fine.

> + /* number of open instances of the device */
> + u32 usrs;

ditto

> + /* indicate whether streaming has started */
> + u8 started;

bool?

> +};
> +
> +struct bcap_fh {
> + struct v4l2_fh fh;
> + struct bcap_device *bcap_dev;
> + /* indicates whether this file handle is doing IO */
> + u8 io_allowed;

bool

[snip]

> +static irqreturn_t bcap_isr(int irq, void *dev_id)
> +{
> + struct ppi_if *ppi = dev_id;
> + struct bcap_device *bcap_dev = ppi->priv;
> + struct timeval timevalue;
> + struct vb2_buffer *vb = &bcap_dev->cur_frm->vb;
> + dma_addr_t *addr;
> +
> + spin_lock(&bcap_dev->lock);
> +
> + if (bcap_dev->cur_frm != bcap_dev->next_frm) {
> + do_gettimeofday(&timevalue);
> + vb->v4l2_buf.timestamp = timevalue;
> + vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
> + bcap_dev->cur_frm = bcap_dev->next_frm;
> + }
> +
> + ppi->ops->stop(ppi);
> +
> + if (!bcap_dev->started)
> + complete(&bcap_dev->comp);
> + else {
> + if (!list_empty(&bcap_dev->dma_queue)) {
> + bcap_dev->next_frm = 
> list_entry(bcap_dev->dma_queue.next,
> + struct bcap_buffer, list);
> + list_del(&bcap_dev->next_frm->list);
> + addr = vb2_plane_cookie(&bcap_dev->next_fr

Re: [PATCH 1/4] v4l2: add vb2_get_unmapped_area in vb2 core

On Tue, 13 Sep 2011, Scott Jiang wrote:

> no mmu system needs get_unmapped_area file operations to do mmap
> 
> Signed-off-by: Scott Jiang 
> ---
>  drivers/media/video/videobuf2-core.c |   31 +++
>  include/media/videobuf2-core.h   |7 +++
>  2 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf2-core.c 
> b/drivers/media/video/videobuf2-core.c
> index 3015e60..02a0ec6 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -1344,6 +1344,37 @@ int vb2_mmap(struct vb2_queue *q, struct 
> vm_area_struct *vma)
>  }
>  EXPORT_SYMBOL_GPL(vb2_mmap);
>  
> +#ifndef CONFIG_MMU
> +unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
> + unsigned long addr,
> + unsigned long len,
> + unsigned long pgoff,
> + unsigned long flags)
> +{
> + unsigned long off = pgoff << PAGE_SHIFT;
> + struct vb2_buffer *vb;
> + unsigned int buffer, plane;
> + int ret;
> +
> + if (q->memory != V4L2_MEMORY_MMAP) {
> + dprintk(1, "Queue is not currently set up for mmap\n");
> + return -EINVAL;
> + }
> +
> + /*
> +  * Find the plane corresponding to the offset passed by userspace.
> +  */
> + ret = __find_plane_by_offset(q, off, &buffer, &plane);
> + if (ret)
> + return ret;
> +
> + vb = q->bufs[buffer];
> +
> + return (unsigned long)vb2_plane_vaddr(vb, plane);
> +}
> +EXPORT_SYMBOL_GPL(vb2_get_unmapped_area);

Hm, wouldn't it be better to use vb2_mmap() and provide a dummy .mmap() 
method in videobuf2-dma-contig.c for the NOMMU case?

Thanks
Guennadi

> +#endif
> +
>  static int __vb2_init_fileio(struct vb2_queue *q, int read);
>  static int __vb2_cleanup_fileio(struct vb2_queue *q);
>  
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index f87472a..5c7b5b4 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -302,6 +302,13 @@ int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type 
> type);
>  int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type);
>  
>  int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma);
> +#ifndef CONFIG_MMU
> +unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
> + unsigned long addr,
> + unsigned long len,
> + unsigned long pgoff,
> + unsigned long flags);
> +#endif
>  unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table 
> *wait);
>  size_t vb2_read(struct vb2_queue *q, char __user *data, size_t count,
>   loff_t *ppos, int nonblock);
> -- 
> 1.7.0.4
> 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] v4l2: add vs6624 sensor driver

>> +#define VGA_WIDTH       640
>> +#define VGA_HEIGHT      480
>> +#define QVGA_WIDTH      320
>> +#define QVGA_HEIGHT     240
>> +#define QQVGA_WIDTH     160
>> +#define QQVGA_HEIGHT    120
>> +#define CIF_WIDTH       352
>> +#define CIF_HEIGHT      288
>> +#define QCIF_WIDTH      176
>> +#define QCIF_HEIGHT     144
>> +#define QQCIF_WIDTH     88
>> +#define QQCIF_HEIGHT    72
>
> ...Can anyone put these in a central header, please? really, please?;-)
>
if already exists in some common file, please tell me

>
> I'm sure many other reviewers will also ask you to replace numerical
> register addresses with symbolic names, since it looks like a sufficiently
> detailed documentation is available to you.
>
sorry, I can't find these names in the datasheet I downloaded from st website.

>
>> +     0x200d, 0x3c,           /* Damper PeakGain Output MSB */
>
> Actually, some of these registers are already defined in your header:
>
> +#define VS6624_PEAK_MIN_OUT_G_MSB     0x200D /* minimum damper output for 
> gain MSB */
>
> so, please, just use those names here and add defines for missing registers
>
I can't find many register names in datasheet. so I treat it as a
binary firmware patch.

>> +     ret = gpio_request(*ce, "VS6624 Chip Enable");
>> +     if (ret) {
>> +             v4l_err(client, "failed to request GPIO %d\n", *ce);
>> +             return ret;
>> +     }
>> +     gpio_direction_output(*ce, 1);
>> +     /* wait 100ms before any further i2c writes are performed */
>> +     mdelay(100);
>
> Logically, it could be a good idea to toggle chip-enable in your
> v4l2_subdev_core_ops::s_power() method, but if you really have to wait for
> 100ms before accessing the chip...
yes, I found if I don't wait a long time, the i2c operation will fail

>
>> +
>> +     vs6624_writeregs(sd, vs6624_p1);
>> +     vs6624_write(sd, VS6624_MICRO_EN, 0x2);
>> +     vs6624_write(sd, VS6624_DIO_EN, 0x1);
>> +     mdelay(10);
>> +     vs6624_writeregs(sd, vs6624_p2);
>> +
>> +     /* make sure the sensor is vs6624 */
>> +     device_id = vs6624_read(sd, VS6624_DEV_ID_MSB) << 8
>> +                     | vs6624_read(sd, VS6624_DEV_ID_LSB);
>
> Wow... this is like saying - sorry, guys, the chip, we just killed by
> writing random rubbish to it wasn't a vs6624;-) I mean, are ID registers
> really unreadable before writing defaults to registers?
>
I remember I put this before writing registers at the first version
but it failed...
Perhaps I should remove the id check, it looks strange

Scott
--
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 3/4] v4l2: add vs6624 sensor driver

On Tue, 13 Sep 2011, Scott Jiang wrote:

> >> +#define VGA_WIDTH       640
> >> +#define VGA_HEIGHT      480
> >> +#define QVGA_WIDTH      320
> >> +#define QVGA_HEIGHT     240
> >> +#define QQVGA_WIDTH     160
> >> +#define QQVGA_HEIGHT    120
> >> +#define CIF_WIDTH       352
> >> +#define CIF_HEIGHT      288
> >> +#define QCIF_WIDTH      176
> >> +#define QCIF_HEIGHT     144
> >> +#define QQCIF_WIDTH     88
> >> +#define QQCIF_HEIGHT    72
> >
> > ...Can anyone put these in a central header, please? really, please?;-)
> >
> if already exists in some common file, please tell me
> 
> >
> > I'm sure many other reviewers will also ask you to replace numerical
> > register addresses with symbolic names, since it looks like a sufficiently
> > detailed documentation is available to you.
> >
> sorry, I can't find these names in the datasheet I downloaded from st website.
> 
> >
> >> +     0x200d, 0x3c,           /* Damper PeakGain Output MSB */
> >
> > Actually, some of these registers are already defined in your header:
> >
> > +#define VS6624_PEAK_MIN_OUT_G_MSB     0x200D /* minimum damper output for 
> > gain MSB */
> >
> > so, please, just use those names here and add defines for missing registers
> >
> I can't find many register names in datasheet. so I treat it as a
> binary firmware patch.

Please, at least use those, that you already define in your header.

Thanks
Guennadi

> 
> >> +     ret = gpio_request(*ce, "VS6624 Chip Enable");
> >> +     if (ret) {
> >> +             v4l_err(client, "failed to request GPIO %d\n", *ce);
> >> +             return ret;
> >> +     }
> >> +     gpio_direction_output(*ce, 1);
> >> +     /* wait 100ms before any further i2c writes are performed */
> >> +     mdelay(100);
> >
> > Logically, it could be a good idea to toggle chip-enable in your
> > v4l2_subdev_core_ops::s_power() method, but if you really have to wait for
> > 100ms before accessing the chip...
> yes, I found if I don't wait a long time, the i2c operation will fail
> 
> >
> >> +
> >> +     vs6624_writeregs(sd, vs6624_p1);
> >> +     vs6624_write(sd, VS6624_MICRO_EN, 0x2);
> >> +     vs6624_write(sd, VS6624_DIO_EN, 0x1);
> >> +     mdelay(10);
> >> +     vs6624_writeregs(sd, vs6624_p2);
> >> +
> >> +     /* make sure the sensor is vs6624 */
> >> +     device_id = vs6624_read(sd, VS6624_DEV_ID_MSB) << 8
> >> +                     | vs6624_read(sd, VS6624_DEV_ID_LSB);
> >
> > Wow... this is like saying - sorry, guys, the chip, we just killed by
> > writing random rubbish to it wasn't a vs6624;-) I mean, are ID registers
> > really unreadable before writing defaults to registers?
> >
> I remember I put this before writing registers at the first version
> but it failed...
> Perhaps I should remove the id check, it looks strange
> 
> Scott
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] v4l2: add vb2_get_unmapped_area in vb2 core

>
> Hm, wouldn't it be better to use vb2_mmap() and provide a dummy .mmap()
> method in videobuf2-dma-contig.c for the NOMMU case?
>
these are two different file operations called by mm system.
--
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] uvcvideo: Fix crash when linking entities

Hi Jonathan,

On Monday 12 September 2011 19:22:33 Jonathan Nieder wrote:
> Laurent Pinchart wrote:
> > I've just sent a pull request to Mauro.
> 
> Thanks!  Looks good to me, for what little that's worth.  My only nits
> are that in the future it might be nice to "Cc: stable" and credit
> testers so they can grep through commit logs to find out if the kernel
> is fixed.

I agree. Sorry for having forgotten about that.

Mauro, if it's not too late, can you add "Cc: sta...@kernel.org" to this patch 
? Or should I send you a new pull request ?

-- 
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: Questions regarding Devices/Subdevices/MediaController usage in case of a SoC

2011-09-13 Thread Alain VOLMAT

Hi Sakari, Hi Laurent,

Thanks for your replies. Sorry for taking so much time.

I don't have perfect graphs to explain our device but the following links helps 
a little.
Device as this one are targeted:
http://www.st.com/internet/imag_video/product/251021.jsp
Corresponding circuit diagram:  
http://www.st.com/internet/com/TECHNICAL_RESOURCES/TECHNICAL_DIAGRAM/CIRCUIT_DIAGRAM/circuit_diagram_17848.pdf

Although the audio part will have to be addressed also at some point, I'm now 
focusing on the video part so it is the area above the ST-Bus INTERCONNECT.
Basically we have several kind of inputs (memory, HDMI, analog, frontends) and 
several kind of outputs (memory, graphic plane, video plane, dual ..)

Currently those kind of devices are already supported at some level via 
LinuxDVB/V4L2 drivers (those drivers are actually already available on the web) 
but they do not offer enough flexibility.
As you know those kind of devices can have several data path which were not 
easily configurable via LinuxDVB/V4L2 and that's the reason why we are now 
trying to move to a Subdev/Media Controller based implementation.
I actually discovered recently the presentation about the OMAP2+ Display 
Subsystem (DSS) (http://elinux.org/images/8/83/Elc2011_semwal.pdf). It is quite 
similar to what we have to do except that in case of the DSS, as the name says, 
it is about the display part only.
One difference with the DSS is that in our case, we do not feed directly the 
GFX/OVLs from the userspace (as framebuffer or video device) but they can ALSO 
be feed via data decoded by the hardware, coming from data pushed via LinuxDVB. 
To give you an example, we can pushed streams to be decoded via LinuxDVB, they 
are decoded, will receive all the necessary processing before "going out" as 
V4L2 capture devices (all this is done within the kernel and in some cases 
might never even come back to user space before being displayed on the display 
panel).
So going back to the graph of the DSS, in our case, in front of the GFX/OVLs, 
we'll have another set of subdevices that correspond to our decoders "capture 
device". And even before that (but not available as a subdevice/media 
controller entity), we have LinuxDVB inputs.

I will post you a graph to explain that more easily but need to have a bit more 
of internal paper work for that.

> In general, V4L2 device nodes should represent memory input / output for the
> device, or a DMA engine. The devices you are referring to above offer
> possibilities to write the data to memory in several points in the pipeline.
> Based on what you're writing above, it sounds like to me that your device
> should likely expose several V4L2 device nodes.

Ok, yes, since we can output / input data at various part of the device, we 
will have several device nodes.

Concerning the media controller, since we have 1 entity for each resource we 
can use, we should be able to have a whole bunch of entities(sub devices), 
attached to several video devices and to a single media device.

Talking now a bit more about legacy applications (application that are using 
V4L2 and thus need to have some "default" data path but do not know anything 
about the media controller), what is the intended way to handle them ? Should 
we have a "platform configuration" application that configure data path via the 
media controller in order to make those application happy ?
I kind of understood that there were some idea of plugin for libv4l in order to 
configure the media controller. Are there any useful document about this plugin 
thing are should I just dig into libv4l source code to have a better 
understanding of that ?

Regards,

Alain Volmat

-Original Message-
From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] 
Sent: Monday, September 05, 2011 12:10 PM
To: Sakari Ailus
Cc: Alain VOLMAT; linux-media@vger.kernel.org
Subject: Re: Questions regarding Devices/Subdevices/MediaController usage in 
case of a SoC

Hi,

On Friday 02 September 2011 23:30:11 Sakari Ailus wrote:
> On Fri, Sep 02, 2011 at 11:38:06AM +0200, Alain VOLMAT wrote:
> > I'm writing you in order to have some advices in the design of the V4L2
> > driver for a rather complex device. It is mainly related to
> > device/subdev/media controller.
> > 
> > This driver would target SoCs which usually handle inputs (capture
> > devices, for ex LinuxDVB, HDMI capture), several layers of graphical or
> > video planes and outputs such as HDMI/analog. Basically we have 3 levels,
> > capture devices data being pushed onto planes and planes being mixed on
> > outputs. Moreover it is also possible to input or output datas from
> > several points of the device.
> 
> Do you have a graphical representation of the flow of data inside the
> device and memory inputs / outputs?

I second Sakari's request here. It would be much easier to advice you with a 
block diagram of your hardware.

> > The idea is to take advantage of the new MediaController in ord

Re: omap3isp as a wakeup source

Hi Enrico,

On Monday 12 September 2011 16:50:42 Enrico wrote:
> 
> While testing omap3isp+tvp5150 with latest Deepthy bt656 patches
> (kernel 3.1rc4) i noticed that yavta hangs very often when grabbing
> or, if not hanged, it grabs at max ~10fps.
> 
> Then i noticed that tapping on the (serial) console made it "unblock"
> for some frames, so i thought it doesn't prevent the cpu to go
> idle/sleep. Using the boot arg "nohlt" the problem disappear and it
> grabs at a steady 25fps.
> 
> In the code i found a comment that says the camera can't be a wakeup
> source but the camera powerdomain is instead used to decide to not go
> idle, so at this point i think the camera powerdomain is not enabled
> but i don't know how/where to enable it. Any ideas?

Could that be related to the OMAP3 ISP driver not implementing the runtime PM 
API ?

-- 
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 v3] V4L: dynamically allocate video_device nodes in subdevices

Hi Guennadi,

On Monday 12 September 2011 12:55:46 Guennadi Liakhovetski wrote:
> Currently only very few drivers actually use video_device nodes, embedded
> in struct v4l2_subdev. Allocate these nodes dynamically for those drivers
> to save memory for the rest.
> 
> Signed-off-by: Guennadi Liakhovetski 
> ---
> 
> v3: addressed comments from Laurent Pinchart - thanks

Thanks for the patch. Just one small comment below.

> 1. switch to using a device-release method, instead of freeing directly in
> v4l2_device_unregister_subdev()
> 
> 2. switch to using drvdata instead of a wrapper struct
> 
>  drivers/media/video/v4l2-device.c |   41
>  include/media/v4l2-subdev.h   |  
>  4 +-
>  2 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-device.c
> b/drivers/media/video/v4l2-device.c index c72856c..9bf3d70 100644
> --- a/drivers/media/video/v4l2-device.c
> +++ b/drivers/media/video/v4l2-device.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #if defined(CONFIG_SPI)
>  #include 
>  #endif
> @@ -191,6 +192,13 @@ int v4l2_device_register_subdev(struct v4l2_device
> *v4l2_dev, }
>  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
> 
> +void v4l2_device_release_subdev_node(struct video_device *vdev)
> +{
> + struct v4l2_subdev *sd = video_get_drvdata(vdev);
> + sd->devnode = NULL;
> + kfree(vdev);
> +}
> +
>  int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
>  {
>   struct video_device *vdev;
> @@ -204,22 +212,42 @@ int v4l2_device_register_subdev_nodes(struct
> v4l2_device *v4l2_dev) if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
>   continue;
> 
> - vdev = &sd->devnode;
> + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> + if (!vdev) {
> + err = -ENOMEM;
> + goto clean_up;
> + }
> +
> + video_set_drvdata(vdev, sd);
>   strlcpy(vdev->name, sd->name, sizeof(vdev->name));
>   vdev->v4l2_dev = v4l2_dev;
>   vdev->fops = &v4l2_subdev_fops;
> - vdev->release = video_device_release_empty;
> + vdev->release = v4l2_device_release_subdev_node;
>   vdev->ctrl_handler = sd->ctrl_handler;
>   err = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
> sd->owner);
> - if (err < 0)
> - return err;
> + if (err < 0) {
> + kfree(vdev);
> + goto clean_up;
> + }
> + get_device(&vdev->dev);

Is get_device() (and the corresponding put_device() calls below) required ? I 
thought device_register() initialized the reference count to 1 (don't take my 
word for it though).

>  #if defined(CONFIG_MEDIA_CONTROLLER)
>   sd->entity.v4l.major = VIDEO_MAJOR;
>   sd->entity.v4l.minor = vdev->minor;
>  #endif
> + sd->devnode = vdev;
>   }
>   return 0;
> +
> +clean_up:
> + list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> + if (!sd->devnode)
> + break;
> + video_unregister_device(sd->devnode);
> + put_device(&sd->devnode->dev);
> + }
> +
> + return err;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev_nodes);
> 
> @@ -245,7 +273,10 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev
> *sd) if (v4l2_dev->mdev)
>   media_device_unregister_entity(&sd->entity);
>  #endif
> - video_unregister_device(&sd->devnode);
> + if (sd->devnode) {
> + video_unregister_device(sd->devnode);
> + put_device(&sd->devnode->dev);
> + }
>   module_put(sd->owner);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 257da1a..5dd049a 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -534,13 +534,13 @@ struct v4l2_subdev {
>   void *dev_priv;
>   void *host_priv;
>   /* subdev device node */
> - struct video_device devnode;
> + struct video_device *devnode;
>  };
> 
>  #define media_entity_to_v4l2_subdev(ent) \
>   container_of(ent, struct v4l2_subdev, entity)
>  #define vdev_to_v4l2_subdev(vdev) \
> - container_of(vdev, struct v4l2_subdev, devnode)
> + video_get_drvdata(vdev)
> 
>  /*
>   * Used for storing subdev information per file handle

-- 
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 v3] V4L: dynamically allocate video_device nodes in subdevices

On Tue, 13 Sep 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Monday 12 September 2011 12:55:46 Guennadi Liakhovetski wrote:
> > Currently only very few drivers actually use video_device nodes, embedded
> > in struct v4l2_subdev. Allocate these nodes dynamically for those drivers
> > to save memory for the rest.
> > 
> > Signed-off-by: Guennadi Liakhovetski 
> > ---
> > 
> > v3: addressed comments from Laurent Pinchart - thanks
> 
> Thanks for the patch. Just one small comment below.
> 
> > 1. switch to using a device-release method, instead of freeing directly in
> > v4l2_device_unregister_subdev()
> > 
> > 2. switch to using drvdata instead of a wrapper struct
> > 
> >  drivers/media/video/v4l2-device.c |   41
> >  include/media/v4l2-subdev.h   |  
> >  4 +-
> >  2 files changed, 38 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/media/video/v4l2-device.c
> > b/drivers/media/video/v4l2-device.c index c72856c..9bf3d70 100644
> > --- a/drivers/media/video/v4l2-device.c
> > +++ b/drivers/media/video/v4l2-device.c
> > @@ -21,6 +21,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #if defined(CONFIG_SPI)
> >  #include 
> >  #endif
> > @@ -191,6 +192,13 @@ int v4l2_device_register_subdev(struct v4l2_device
> > *v4l2_dev, }
> >  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
> > 
> > +void v4l2_device_release_subdev_node(struct video_device *vdev)
> > +{
> > +   struct v4l2_subdev *sd = video_get_drvdata(vdev);
> > +   sd->devnode = NULL;
> > +   kfree(vdev);
> > +}
> > +
> >  int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
> >  {
> > struct video_device *vdev;
> > @@ -204,22 +212,42 @@ int v4l2_device_register_subdev_nodes(struct
> > v4l2_device *v4l2_dev) if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
> > continue;
> > 
> > -   vdev = &sd->devnode;
> > +   vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> > +   if (!vdev) {
> > +   err = -ENOMEM;
> > +   goto clean_up;
> > +   }
> > +
> > +   video_set_drvdata(vdev, sd);
> > strlcpy(vdev->name, sd->name, sizeof(vdev->name));
> > vdev->v4l2_dev = v4l2_dev;
> > vdev->fops = &v4l2_subdev_fops;
> > -   vdev->release = video_device_release_empty;
> > +   vdev->release = v4l2_device_release_subdev_node;
> > vdev->ctrl_handler = sd->ctrl_handler;
> > err = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
> >   sd->owner);
> > -   if (err < 0)
> > -   return err;
> > +   if (err < 0) {
> > +   kfree(vdev);
> > +   goto clean_up;
> > +   }
> > +   get_device(&vdev->dev);
> 
> Is get_device() (and the corresponding put_device() calls below) required ? I 
> thought device_register() initialized the reference count to 1 (don't take my 
> word for it though).

Indeed, I think, you're right. Will update.

Thanks
Guennadi

> 
> >  #if defined(CONFIG_MEDIA_CONTROLLER)
> > sd->entity.v4l.major = VIDEO_MAJOR;
> > sd->entity.v4l.minor = vdev->minor;
> >  #endif
> > +   sd->devnode = vdev;
> > }
> > return 0;
> > +
> > +clean_up:
> > +   list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> > +   if (!sd->devnode)
> > +   break;
> > +   video_unregister_device(sd->devnode);
> > +   put_device(&sd->devnode->dev);
> > +   }
> > +
> > +   return err;
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev_nodes);
> > 
> > @@ -245,7 +273,10 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev
> > *sd) if (v4l2_dev->mdev)
> > media_device_unregister_entity(&sd->entity);
> >  #endif
> > -   video_unregister_device(&sd->devnode);
> > +   if (sd->devnode) {
> > +   video_unregister_device(sd->devnode);
> > +   put_device(&sd->devnode->dev);
> > +   }
> > module_put(sd->owner);
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index 257da1a..5dd049a 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -534,13 +534,13 @@ struct v4l2_subdev {
> > void *dev_priv;
> > void *host_priv;
> > /* subdev device node */
> > -   struct video_device devnode;
> > +   struct video_device *devnode;
> >  };
> > 
> >  #define media_entity_to_v4l2_subdev(ent) \
> > container_of(ent, struct v4l2_subdev, entity)
> >  #define vdev_to_v4l2_subdev(vdev) \
> > -   container_of(vdev, struct v4l2_subdev, devnode)
> > +   video_get_drvdata(vdev)
> > 
> >  /*
> >   * Used for storing subdev information per file handle
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To un

Re: [PATCH v3] V4L: dynamically allocate video_device nodes in subdevices

Hi Guennadi,

On Tuesday 13 September 2011 11:26:23 Guennadi Liakhovetski wrote:
> On Tue, 13 Sep 2011, Laurent Pinchart wrote:
> > On Monday 12 September 2011 12:55:46 Guennadi Liakhovetski wrote:
> > > Currently only very few drivers actually use video_device nodes,
> > > embedded in struct v4l2_subdev. Allocate these nodes dynamically for
> > > those drivers to save memory for the rest.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski 
> > > ---
> > > 
> > > v3: addressed comments from Laurent Pinchart - thanks
> > 
> > Thanks for the patch. Just one small comment below.
> > 
> > > 1. switch to using a device-release method, instead of freeing directly
> > > in v4l2_device_unregister_subdev()
> > > 
> > > 2. switch to using drvdata instead of a wrapper struct
> > > 
> > >  drivers/media/video/v4l2-device.c |   41
> > > 
> > >  include/media/v4l2-subdev.h  
> > > |
> > > 
> > >  4 +-
> > >  2 files changed, 38 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/media/video/v4l2-device.c
> > > b/drivers/media/video/v4l2-device.c index c72856c..9bf3d70 100644
> > > --- a/drivers/media/video/v4l2-device.c
> > > +++ b/drivers/media/video/v4l2-device.c
> > > @@ -21,6 +21,7 @@
> > > 
> > >  #include 
> > >  #include 
> > >  #include 
> > > 
> > > +#include 
> > > 
> > >  #if defined(CONFIG_SPI)
> > >  #include 
> > >  #endif
> > > 
> > > @@ -191,6 +192,13 @@ int v4l2_device_register_subdev(struct v4l2_device
> > > *v4l2_dev, }
> > > 
> > >  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
> > > 
> > > +void v4l2_device_release_subdev_node(struct video_device *vdev)
> > > +{
> > > + struct v4l2_subdev *sd = video_get_drvdata(vdev);
> > > + sd->devnode = NULL;
> > > + kfree(vdev);
> > > +}
> > > +
> > > 
> > >  int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
> > >  {
> > >  
> > >   struct video_device *vdev;
> > > 
> > > @@ -204,22 +212,42 @@ int v4l2_device_register_subdev_nodes(struct
> > > v4l2_device *v4l2_dev) if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
> > > 
> > >   continue;
> > > 
> > > - vdev = &sd->devnode;
> > > + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> > > + if (!vdev) {
> > > + err = -ENOMEM;
> > > + goto clean_up;
> > > + }
> > > +
> > > + video_set_drvdata(vdev, sd);
> > > 
> > >   strlcpy(vdev->name, sd->name, sizeof(vdev->name));
> > >   vdev->v4l2_dev = v4l2_dev;
> > >   vdev->fops = &v4l2_subdev_fops;
> > > 
> > > - vdev->release = video_device_release_empty;
> > > + vdev->release = v4l2_device_release_subdev_node;
> > > 
> > >   vdev->ctrl_handler = sd->ctrl_handler;
> > >   err = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
> > >   
> > > sd->owner);
> > > 
> > > - if (err < 0)
> > > - return err;
> > > + if (err < 0) {
> > > + kfree(vdev);
> > > + goto clean_up;
> > > + }
> > > + get_device(&vdev->dev);
> > 
> > Is get_device() (and the corresponding put_device() calls below) required
> > ? I thought device_register() initialized the reference count to 1
> > (don't take my word for it though).
> 
> Indeed, I think, you're right. Will update.

Please test it as well :-)

-- 
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: omap3isp as a wakeup source

2011-09-13 Thread Tero Kristo

On Tue, 2011-09-13 at 08:49 +0200, Sakari Ailus wrote:
> anish singh wrote:
> > On Tue, Sep 13, 2011 at 1:58 AM, Sakari Ailus  wrote:
> >> On Mon, Sep 12, 2011 at 04:50:42PM +0200, Enrico wrote:
> >>> Hi,
> >>
> >> Hi Enrico,
> >>
> >>> While testing omap3isp+tvp5150 with latest Deepthy bt656 patches
> >>> (kernel 3.1rc4) i noticed that yavta hangs very often when grabbing
> >>> or, if not hanged, it grabs at max ~10fps.
> >>>
> >>> Then i noticed that tapping on the (serial) console made it "unblock"
> >>> for some frames, so i thought it doesn't prevent the cpu to go
> >>> idle/sleep. Using the boot arg "nohlt" the problem disappear and it
> >>> grabs at a steady 25fps.
> >>>
> >>> In the code i found a comment that says the camera can't be a wakeup
> >>> source but the camera powerdomain is instead used to decide to not go
> >>> idle, so at this point i think the camera powerdomain is not enabled
> >>> but i don't know how/where to enable it. Any ideas?
> >>
> >> I can confirm this indeed is the case --- ISP can't wake up the system ---
> >> but don't know how to prevent the system from going to sleep when using the
> >> ISP.
> > Had it been on android i think wakelock would have been very useful.
> 
> I believe there are proper means to do this using more standard methods
> as well. Not being a PM expert, I don't know how.
> 
> Cc Tero.
> 

Hi,

I don't think there are proper means yet to do this, as camera is
somewhat a special case in omap3, it is apparently the only module that
is causing this kind of problem. However, you can prevent idle when
camera is active with something like this:

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 2789e0a..7fdf6e2 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -358,6 +358,9 @@ void omap_sram_idle(void)
omap3_per_save_context();
}
 
+   if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
+   clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
+
/* CORE */
if (core_next_state < PWRDM_POWER_ON) {
omap_uart_prepare_idle(0);



-Tero


Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. 
Kotipaikka: Helsinki
 

--
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/4] v4l2: add blackfin capture bridge driver

>> +
>> +struct bcap_format {
>> +     u8 *desc;
>> +     u32 pixelformat;
>> +     enum v4l2_mbus_pixelcode mbus_code;
>> +     int bpp; /* bytes per pixel */
>
> Don't you think you might have to process 12 bpp formats at some point,
> like YUV 4:2:0, or NV12? Maybe better calculate in bits from the beginning?
>
sounds good, changed in v2

>
> Does it really have to be fixed 32 bits? Seems a plane simple int would do
> just fine.
>
users can't be negative, so I used u32

>> +struct bcap_fh {
>> +     struct v4l2_fh fh;
>> +     struct bcap_device *bcap_dev;
>> +     /* indicates whether this file handle is doing IO */
>> +     u8 io_allowed;
>
> bool
>
does kernel prefer bool now?

>> +     if (!bcap_dev->started)
>> +             complete(&bcap_dev->comp);
>> +     else {
>> +             if (!list_empty(&bcap_dev->dma_queue)) {
>> +                     bcap_dev->next_frm = 
>> list_entry(bcap_dev->dma_queue.next,
>> +                                             struct bcap_buffer, list);
>> +                     list_del(&bcap_dev->next_frm->list);
>> +                     addr = vb2_plane_cookie(&bcap_dev->next_frm->vb, 0);
>
> I think, the direct use of vb2_plane_cookie() is discouraged.
> vb2_dma_contig_plane_dma_addr() should work for you.
>
I guess you mean vb2_dma_contig_plane_paddr

>> +
>> +     for (i = 0; i < BCAP_MAX_FMTS; i++) {
>> +             if (mbus_fmt.code == bcap_formats[i].mbus_code) {
>> +                     bcap_fmt = &bcap_formats[i];
>> +                     v4l2_fill_pix_format(pixfmt, &mbus_fmt);
>> +                     pixfmt->bytesperline = pixfmt->width * bcap_fmt->bpp;
>> +                     pixfmt->sizeimage = pixfmt->bytesperline
>> +                                             * pixfmt->height;
>
> It seems to me, you're forgetting to fill in ->pixelformat?
>
yes, add in 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

Re: omap3isp as a wakeup source

2011-09-13 Thread Enrico

On Tue, Sep 13, 2011 at 11:48 AM, Tero Kristo  wrote:
> On Tue, 2011-09-13 at 08:49 +0200, Sakari Ailus wrote:
>> anish singh wrote:
>> > On Tue, Sep 13, 2011 at 1:58 AM, Sakari Ailus  wrote:
>> >> On Mon, Sep 12, 2011 at 04:50:42PM +0200, Enrico wrote:
>> >>> Hi,
>> >>
>> >> Hi Enrico,
>> >>
>> >>> While testing omap3isp+tvp5150 with latest Deepthy bt656 patches
>> >>> (kernel 3.1rc4) i noticed that yavta hangs very often when grabbing
>> >>> or, if not hanged, it grabs at max ~10fps.
>> >>>
>> >>> Then i noticed that tapping on the (serial) console made it "unblock"
>> >>> for some frames, so i thought it doesn't prevent the cpu to go
>> >>> idle/sleep. Using the boot arg "nohlt" the problem disappear and it
>> >>> grabs at a steady 25fps.
>> >>>
>> >>> In the code i found a comment that says the camera can't be a wakeup
>> >>> source but the camera powerdomain is instead used to decide to not go
>> >>> idle, so at this point i think the camera powerdomain is not enabled
>> >>> but i don't know how/where to enable it. Any ideas?
>> >>
>> >> I can confirm this indeed is the case --- ISP can't wake up the system ---
>> >> but don't know how to prevent the system from going to sleep when using 
>> >> the
>> >> ISP.
>> > Had it been on android i think wakelock would have been very useful.
>>
>> I believe there are proper means to do this using more standard methods
>> as well. Not being a PM expert, I don't know how.
>>
>> Cc Tero.
>>
>
> Hi,
>
> I don't think there are proper means yet to do this, as camera is
> somewhat a special case in omap3, it is apparently the only module that
> is causing this kind of problem. However, you can prevent idle when
> camera is active with something like this:
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 2789e0a..7fdf6e2 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -358,6 +358,9 @@ void omap_sram_idle(void)
>                                omap3_per_save_context();
>        }
>
> +       if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
> +               clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
> +
>        /* CORE */
>        if (core_next_state < PWRDM_POWER_ON) {
>                omap_uart_prepare_idle(0);
>
>
>
> -Tero

i think something related is already in
arch/arm/mach-omap2/cpuidle34xx.c omap3_enter_idle_bm(...):

/*
 * Prevent idle completely if CAM is active.
 * CAM does not have wakeup capability in OMAP3.
 */
cam_state = pwrdm_read_pwrst(cam_pd);
if (cam_state == PWRDM_POWER_ON) {
new_state = dev->safe_state;
goto select_state;
}


But probably the power domain is not set to ON, and i don't know where
it should be set. Maybe, as Laurent suggested, adding runtime PM
support will fix it?

Thanks,

Enrico
--
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: omap3isp as a wakeup source

2011-09-13 Thread Enrico

On Tue, Sep 13, 2011 at 11:48 AM, Tero Kristo  wrote:
> On Tue, 2011-09-13 at 08:49 +0200, Sakari Ailus wrote:
>> anish singh wrote:
>> > On Tue, Sep 13, 2011 at 1:58 AM, Sakari Ailus  wrote:
>> >> On Mon, Sep 12, 2011 at 04:50:42PM +0200, Enrico wrote:
>> >>> Hi,
>> >>
>> >> Hi Enrico,
>> >>
>> >>> While testing omap3isp+tvp5150 with latest Deepthy bt656 patches
>> >>> (kernel 3.1rc4) i noticed that yavta hangs very often when grabbing
>> >>> or, if not hanged, it grabs at max ~10fps.
>> >>>
>> >>> Then i noticed that tapping on the (serial) console made it "unblock"
>> >>> for some frames, so i thought it doesn't prevent the cpu to go
>> >>> idle/sleep. Using the boot arg "nohlt" the problem disappear and it
>> >>> grabs at a steady 25fps.
>> >>>
>> >>> In the code i found a comment that says the camera can't be a wakeup
>> >>> source but the camera powerdomain is instead used to decide to not go
>> >>> idle, so at this point i think the camera powerdomain is not enabled
>> >>> but i don't know how/where to enable it. Any ideas?
>> >>
>> >> I can confirm this indeed is the case --- ISP can't wake up the system ---
>> >> but don't know how to prevent the system from going to sleep when using 
>> >> the
>> >> ISP.
>> > Had it been on android i think wakelock would have been very useful.
>>
>> I believe there are proper means to do this using more standard methods
>> as well. Not being a PM expert, I don't know how.
>>
>> Cc Tero.
>>
>
> Hi,
>
> I don't think there are proper means yet to do this, as camera is
> somewhat a special case in omap3, it is apparently the only module that
> is causing this kind of problem. However, you can prevent idle when
> camera is active with something like this:
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 2789e0a..7fdf6e2 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -358,6 +358,9 @@ void omap_sram_idle(void)
>                                omap3_per_save_context();
>        }
>
> +       if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
> +               clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
> +
>        /* CORE */
>        if (core_next_state < PWRDM_POWER_ON) {
>                omap_uart_prepare_idle(0);
>
>
>
> -Tero


One more thing: i just noticed that in Deepthy bt656 patches there is
one patch [1] to add cam regulators (1v8 and 2v8) to the omap3evm
board file, i'm not using an omap3evm but maybe i should add them to
my board file too?

Enrico

[1]: http://www.spinics.net/lists/linux-omap/msg56922.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: omap3isp as a wakeup source

2011-09-13 Thread Tero Kristo

On Tue, 2011-09-13 at 12:08 +0200, Enrico wrote:
> On Tue, Sep 13, 2011 at 11:48 AM, Tero Kristo  wrote:
> > On Tue, 2011-09-13 at 08:49 +0200, Sakari Ailus wrote:
> >> anish singh wrote:
> >> > On Tue, Sep 13, 2011 at 1:58 AM, Sakari Ailus  
> >> > wrote:
> >> >> On Mon, Sep 12, 2011 at 04:50:42PM +0200, Enrico wrote:
> >> >>> Hi,
> >> >>
> >> >> Hi Enrico,
> >> >>
> >> >>> While testing omap3isp+tvp5150 with latest Deepthy bt656 patches
> >> >>> (kernel 3.1rc4) i noticed that yavta hangs very often when grabbing
> >> >>> or, if not hanged, it grabs at max ~10fps.
> >> >>>
> >> >>> Then i noticed that tapping on the (serial) console made it "unblock"
> >> >>> for some frames, so i thought it doesn't prevent the cpu to go
> >> >>> idle/sleep. Using the boot arg "nohlt" the problem disappear and it
> >> >>> grabs at a steady 25fps.
> >> >>>
> >> >>> In the code i found a comment that says the camera can't be a wakeup
> >> >>> source but the camera powerdomain is instead used to decide to not go
> >> >>> idle, so at this point i think the camera powerdomain is not enabled
> >> >>> but i don't know how/where to enable it. Any ideas?
> >> >>
> >> >> I can confirm this indeed is the case --- ISP can't wake up the system 
> >> >> ---
> >> >> but don't know how to prevent the system from going to sleep when using 
> >> >> the
> >> >> ISP.
> >> > Had it been on android i think wakelock would have been very useful.
> >>
> >> I believe there are proper means to do this using more standard methods
> >> as well. Not being a PM expert, I don't know how.
> >>
> >> Cc Tero.
> >>
> >
> > Hi,
> >
> > I don't think there are proper means yet to do this, as camera is
> > somewhat a special case in omap3, it is apparently the only module that
> > is causing this kind of problem. However, you can prevent idle when
> > camera is active with something like this:
> >
> > diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> > index 2789e0a..7fdf6e2 100644
> > --- a/arch/arm/mach-omap2/pm34xx.c
> > +++ b/arch/arm/mach-omap2/pm34xx.c
> > @@ -358,6 +358,9 @@ void omap_sram_idle(void)
> >omap3_per_save_context();
> >}
> >
> > +   if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
> > +   clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
> > +
> >/* CORE */
> >if (core_next_state < PWRDM_POWER_ON) {
> >omap_uart_prepare_idle(0);
> >
> >
> >
> > -Tero
> 
> i think something related is already in
> arch/arm/mach-omap2/cpuidle34xx.c omap3_enter_idle_bm(...):
> 
> /*
>  * Prevent idle completely if CAM is active.
>  * CAM does not have wakeup capability in OMAP3.
>  */
> cam_state = pwrdm_read_pwrst(cam_pd);
> if (cam_state == PWRDM_POWER_ON) {
> new_state = dev->safe_state;
> goto select_state;
> }
> 
> 

Yea, this should take care of it already.

> But probably the power domain is not set to ON, and i don't know where
> it should be set. Maybe, as Laurent suggested, adding runtime PM
> support will fix it?

Powerdomain is automatically on if there are any clocks enabled on it.
If you make sure that ISP has some activity ongoing, then it should be
on. You can check the state of the camera powerdomain
from /sys/kernel/debug/pm_debug/count file, if you have mounted debugfs.

But yea, there might be some conflict also ongoing with lack of runtime
PM here, I haven't been looking at ISP related issues for a long time.

-Tero



Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. 
Kotipaikka: Helsinki
 

--
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] New class for low level sensors controls?

Hi Guennadi,

On Thursday 08 September 2011 14:35:54 Guennadi Liakhovetski wrote:
> On Tue, 6 Sep 2011, Sakari Ailus wrote:
> > On Tue, Sep 06, 2011 at 01:41:11PM +0200, Laurent Pinchart wrote:
> > > On Tuesday 06 September 2011 13:36:53 Sakari Ailus wrote:

[snip]

> > > > Typically such sensors are not controlled by general purpose
> > > > applications but e.g. require a camera control algorithm framework
> > > > in user space. This needs to be implemented in libv4l for general
> > > > purpose applications to work properly on this kind of hardware.
> > > > 
> > > > These sensors expose controls such as
> > > > 
> > > > - Per-component gain controls. Red, blue, green (blue) and green
> > > > (red) gains.
> > > > 
> > > > - Link frequency. The frequency of the data link from the sensor to
> > > > the bridge.
> > > > 
> > > > - Horizontal and vertical blanking.
> > > 
> > > Other controls often found in bayer sensors are black level
> > > compensation and test pattern.
> 
> May I suggest a couple more:
> 
> (1) snapshot mode (I really badly want this one, please;-))

What do you mean exactly by snapshot mode ? Is that just external trigger, or 
does it cover more features than that ?

> (2) flash controls (yes, I know we already have V4L2_CTRL_CLASS_FLASH, I
> just have the impression, that these controls are mostly meant for
> either pure software implementations, or for external controllers, I
> think it should also be possible to have them exported by a normal
> sensor driver, but wasn't really sure. So, wanted to point out to this
> possibility once again.)

As Sakari told you in his answer, we can add new controls to the flash class.

> (3) AEC / AGC regions

This will get tricky. I'm tempted to propose the idea of v4l2_rect controls 
and control arrays again :-)

> (4) stereo (3D anyone?;)) - no, don't think we need it now...

-- 
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: [RFC] New class for low level sensors controls?

Hi Sakari,

On Thursday 08 September 2011 13:44:28 Sakari Ailus wrote:
> On Thu, Sep 08, 2011 at 04:54:23PM +0530, Subash Patel wrote:
> > On 09/06/2011 05:52 PM, Sakari Ailus wrote:
> > > On Tue, Sep 06, 2011 at 01:41:11PM +0200, Laurent Pinchart wrote:
> > > >
> > > > Other controls often found in bayer sensors are black level
> > > > compensation and test pattern.
> > 
> > Does all BAYER sensor allow the dark level compensation programming?
> 
> I'm not sure. I have always seen ISPs being used for that, not sensors.
> 
> > I thought it must be auto dark level compensation, which is done by
> > the sensor. The sensor detects the optical black value at start of
> > each frame, and analog-to-digital conversion is shifted to
> > compensate the dark level for that frame. Hence I am thinking if
> > this should be a controllable feature.
> 
> This is probably what smart sensors could do. If we have a raw bayer sensor
> the computation of the optimal black level compensation could be done by
> some of the controls algorithms run in the user space. Automatic exposure
> probably?

Many "non-smart" raw bayer sensors implement both manual and automatic black 
level compensation. In the first case the user programs a value to be 
subtracted from the pixels (whether that's done in the analog or digital 
domain might be sensor-specific), and in the second case the sensor computes a 
mean black level value based on black lines (optically unexposed) at the top 
of the image.

-- 
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: [RFC] Reserved fields in v4l2_mbus_framefmt, v4l2_subdev_format alignment

Hi Sylwester,

On Friday 09 September 2011 19:37:55 Sylwester Nawrocki wrote:
> On 09/08/2011 12:21 PM, Laurent Pinchart wrote:
> > On Tuesday 06 September 2011 23:07:43 Sakari Ailus wrote:
> >> On Tue, Sep 06, 2011 at 09:10:17PM +0200, Sylwester Nawrocki wrote:
> >>> On 09/05/2011 05:55 PM, Sakari Ailus wrote:
>  Hi all,
>  
>  I recently came across a few issues in the definitions of
>  v4l2_subdev_format and v4l2_mbus_framefmt when I was working on sensor
>  control that I wanted to bring up here. The appropriate structure
>  right now look like this:
>  
>  include/linux/v4l2-subdev.h:
>  ---8<---
>  /**
>  
> * struct v4l2_subdev_format - Pad-level media bus format
> * @which: format type (from enum v4l2_subdev_format_whence)
> * @pad: pad number, as reported by the media API
> * @format: media bus format (format code and frame size)
> */
>  
>  struct v4l2_subdev_format {
>  
>    __u32 which;
>    __u32 pad;
>    struct v4l2_mbus_framefmt format;
>    __u32 reserved[8];
>  
>  };
>  ---8<---
>  
>  include/linux/v4l2-mediabus.h:
>  ---8<---
>  /**
>  
> * struct v4l2_mbus_framefmt - frame format on the media bus
> * @width:  frame width
> * @height: frame height
> * @code:   data format code (from enum v4l2_mbus_pixelcode)
> * @field:  used interlacing type (from enum v4l2_field)
> * @colorspace: colorspace of the data (from enum v4l2_colorspace)
> */
>  
>  struct v4l2_mbus_framefmt {
>  
>    __u32   width;
>    __u32   height;
>    __u32   code;
>    __u32   field;
>    __u32   colorspace;
>    __u32   reserved[7];
>  
>  };
>  ---8<---
>  
>  Offering a lower level interface for sensors which allows better
>  control of them from the user space involves providing the link
>  frequency to the user space. While the link frequency will be a
>  control, together with the bus type and number of lanes (on serial
>  links), this will define the pixel clock.
>  
>  http://www.spinics.net/lists/linux-media/msg36492.html>
>  
>  After adding pixel clock to v4l2_mbus_framefmt there will be six
>  reserved fields left, one of which will be further possibly consumed
>  by maximum image size:
>  
>  http://www.spinics.net/lists/linux-media/msg35949.html>
> >>> 
> >>> Yes, thanks for remembering about it. I have done some experiments with
> >>> a sensor producing JPEG data and I'd like to add '__u32 framesamples'
> >>> field to struct v4l2_mbus_framefmt, even though it solves only part of
> >>> the problem. I'm not sure when I'll be able to get this finished
> >>> though. I've just attached the initial patch now.
> >>> 
>  Frame blanking (horizontal and vertical) and number of lanes might be
>  needed in the struct as well in the future, bringing the reserved
>  count down to two. I find this alarmingly low for a relatively new
>  structure definition which will potentially have a few different uses
>  in the future.
> >>> 
> >>> Sorry, could you explain why we need to put the blanking information in
> >>> struct v4l2_mbus_framefmt ? I thought it had been initially agreed that
> >>> the control framework will be used for this.
> >> 
> >> Configuration of blanking will be implemented as controls, yes.
> >> 
> >> Bandwidth calculation in the ISP driver may well need to know more
> >> detailed information than just the maximum pixel rate. Averge rate over
> >> certain period may also be important.
> >> 
> >> For example, take a sensor which is able to produce pixel rate of 200
> >> Mp/s. In the OMAP 3 ISP only the CSI2 block will be able to process
> >> pixels at such rate. The ISP driver needs this information to be able
> >> to decide whether it's safe to start streaming or not.
> >> 
> >> Higher momentary pixel rates are still possible as there are buffers
> >> between some of the blocks. When using downscaling on sensors this gets
> >> more tricky. There may be bursts of data which may overflow these
> >> buffers since the sensors do not output data at amortised rate.
> >> Information on the sensor (bursts) and size of the buffers is at least
> >> required to assess this question.
> >> 
> >> I have a vague feeling we may need some of this data as part of the
> >> v4l2_mbus_framefmt before we have a solution.
> > 
> > Do we really need to make all this (including the proposed framesamples
> > field) part of v4l2_mbus_framefmt ? My understanding is that the
> > information needs to be propagated down the pipeline to verify pipeline
> > validity at streamon time and to configure blocks down in the chain.
>

Re: Questions regarding Devices/Subdevices/MediaController usage in case of a SoC

Hi Alain,

On Tuesday 13 September 2011 11:07:02 Alain VOLMAT wrote:
> Hi Sakari, Hi Laurent,
> 
> Thanks for your replies. Sorry for taking so much time.
> 
> I don't have perfect graphs to explain our device but the following links
> helps a little. Device as this one are targeted:   
> http://www.st.com/internet/imag_video/product/251021.jsp Corresponding
> circuit diagram: 
> http://www.st.com/internet/com/TECHNICAL_RESOURCES/TECHNICAL_DIAGRAM/CIRCU
> IT_DIAGRAM/circuit_diagram_17848.pdf
> 
> Although the audio part will have to be addressed also at some point, I'm
> now focusing on the video part so it is the area above the ST-Bus
> INTERCONNECT. Basically we have several kind of inputs (memory, HDMI,
> analog, frontends) and several kind of outputs (memory, graphic plane,
> video plane, dual ..)
> 
> Currently those kind of devices are already supported at some level via
> LinuxDVB/V4L2 drivers (those drivers are actually already available on the
> web) but they do not offer enough flexibility. As you know those kind of
> devices can have several data path which were not easily configurable via
> LinuxDVB/V4L2 and that's the reason why we are now trying to move to a
> Subdev/Media Controller based implementation. I actually discovered
> recently the presentation about the OMAP2+ Display Subsystem (DSS)
> (http://elinux.org/images/8/83/Elc2011_semwal.pdf). It is quite similar to
> what we have to do except that in case of the DSS, as the name says, it is
> about the display part only. One difference with the DSS is that in our
> case, we do not feed directly the GFX/OVLs from the userspace (as
> framebuffer or video device) but they can ALSO be feed via data decoded by
> the hardware, coming from data pushed via LinuxDVB. To give you an
> example, we can pushed streams to be decoded via LinuxDVB, they are
> decoded, will receive all the necessary processing before "going out" as
> V4L2 capture devices (all this is done within the kernel and in some cases
> might never even come back to user space before being displayed on the
> display panel). So going back to the graph of the DSS, in our case, in
> front of the GFX/OVLs, we'll have another set of subdevices that
> correspond to our decoders "capture device". And even before that (but not
> available as a subdevice/media controller entity), we have LinuxDVB
> inputs.
> 
> I will post you a graph to explain that more easily but need to have a bit
> more of internal paper work for that.

Thank you for the information. The hardware looks quite complex indeed, and I 
believe using the media controller would be a good solution.

> > In general, V4L2 device nodes should represent memory input / output for
> > the device, or a DMA engine. The devices you are referring to above
> > offer possibilities to write the data to memory in several points in the
> > pipeline. Based on what you're writing above, it sounds like to me that
> > your device should likely expose several V4L2 device nodes.
> 
> Ok, yes, since we can output / input data at various part of the device, we
> will have several device nodes.
> 
> Concerning the media controller, since we have 1 entity for each resource
> we can use, we should be able to have a whole bunch of entities(sub
> devices), attached to several video devices and to a single media device.

That looks good to me.

> Talking now a bit more about legacy applications (application that are
> using V4L2 and thus need to have some "default" data path but do not know
> anything about the media controller), what is the intended way to handle
> them ? Should we have a "platform configuration" application that
> configure data path via the media controller in order to make those
> application happy ? I kind of understood that there were some idea of
> plugin for libv4l in order to configure the media controller.

If you can configure your hardware with a default pipeline at startup that's 
of course good, but as soon as a media controller-aware application will 
modify the pipeline pure V4L2 applications will be stuck.

For that reason implementing pipeline configuration support in a libv4l plugin 
for pure V4L2 applications has my preference. The idea is that high-level V4L2 
applications (such as a popular closed-source video-conferencing application 
that people seem to like for a reason I can't fathom :-)) should work with 
that plugin, and the high-level features they expect should be provided.

> Are there any useful document about this plugin thing are should I just dig
> into libv4l source code to have a better understanding of that ?

Sakari, do you know if the libv4l plugin API is documented ? Do you have a 
link to the OMAP3 ISP libv4l plugin ?

-- 
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/4] v4l2: add blackfin capture bridge driver

On Tue, 13 Sep 2011, Scott Jiang wrote:

> >> +
> >> +struct bcap_format {
> >> +     u8 *desc;
> >> +     u32 pixelformat;
> >> +     enum v4l2_mbus_pixelcode mbus_code;
> >> +     int bpp; /* bytes per pixel */
> >
> > Don't you think you might have to process 12 bpp formats at some point,
> > like YUV 4:2:0, or NV12? Maybe better calculate in bits from the beginning?
> >
> sounds good, changed in v2
> 
> >
> > Does it really have to be fixed 32 bits? Seems a plane simple int would do
> > just fine.
> >
> users can't be negative, so I used u32

no, then use unsigned int.

> >> +struct bcap_fh {
> >> +     struct v4l2_fh fh;
> >> +     struct bcap_device *bcap_dev;
> >> +     /* indicates whether this file handle is doing IO */
> >> +     u8 io_allowed;
> >
> > bool
> >
> does kernel prefer bool now?

yes

> >> +     if (!bcap_dev->started)
> >> +             complete(&bcap_dev->comp);
> >> +     else {
> >> +             if (!list_empty(&bcap_dev->dma_queue)) {
> >> +                     bcap_dev->next_frm = 
> >> list_entry(bcap_dev->dma_queue.next,
> >> +                                             struct bcap_buffer, list);
> >> +                     list_del(&bcap_dev->next_frm->list);
> >> +                     addr = vb2_plane_cookie(&bcap_dev->next_frm->vb, 0);
> >
> > I think, the direct use of vb2_plane_cookie() is discouraged.
> > vb2_dma_contig_plane_dma_addr() should work for you.
> >
> I guess you mean vb2_dma_contig_plane_paddr

no, in the current kernel it's vb2_dma_contig_plane_dma_addr(). See

http://git.linuxtv.org/media_tree.git/shortlog/refs/heads/staging/for_v3.2

> 
> >> +
> >> +     for (i = 0; i < BCAP_MAX_FMTS; i++) {
> >> +             if (mbus_fmt.code == bcap_formats[i].mbus_code) {
> >> +                     bcap_fmt = &bcap_formats[i];
> >> +                     v4l2_fill_pix_format(pixfmt, &mbus_fmt);
> >> +                     pixfmt->bytesperline = pixfmt->width * bcap_fmt->bpp;
> >> +                     pixfmt->sizeimage = pixfmt->bytesperline
> >> +                                             * pixfmt->height;
> >
> > It seems to me, you're forgetting to fill in ->pixelformat?
> >
> yes, add in v2
> 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] drivers/media/video/stk-webcam.c: webcam LED bug fix

2011-09-13 Thread Arvydas Sidorenko

The probem was on my DC-1125 webcam chip from Syntek. Whenever the webcam turns
on, the LED light on it is turn on also and never turns off again unless system
is shut downed or restarted.

This patch will fix this issue - the LED will be turned off whenever the device
is released.

Signed-off-by: Arvydas Sidorenko 
---
 drivers/media/video/stk-webcam.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/stk-webcam.c b/drivers/media/video/stk-webcam.c
index d1a2cef..859e78f 100644
--- a/drivers/media/video/stk-webcam.c
+++ b/drivers/media/video/stk-webcam.c
@@ -574,6 +574,8 @@ static int v4l_stk_release(struct file *fp)
if (dev->owner == fp) {
stk_stop_stream(dev);
stk_free_buffers(dev);
+   stk_camera_write_reg(dev, 0x0, 0x48); /* turn off the LED */
+   unset_initialised(dev);
dev->owner = NULL;
}
 
-- 
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 2/2] drivers/media/video/stk-webcam.c: coding style issue

2011-09-13 Thread Arvydas Sidorenko

checkpatch.pl gave some coding style errors, so on the way to the first
patch I added the fix to them.

Signed-off-by: Arvydas Sidorenko 
---
 drivers/media/video/stk-webcam.c |   18 --
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/media/video/stk-webcam.c b/drivers/media/video/stk-webcam.c
index 859e78f..5fc6bbc 100644
--- a/drivers/media/video/stk-webcam.c
+++ b/drivers/media/video/stk-webcam.c
@@ -518,7 +518,7 @@ static int stk_prepare_sio_buffers(struct stk_camera *dev, 
unsigned n_sbufs)
return -ENOMEM;
for (i = 0; i < n_sbufs; i++) {
if (stk_setup_siobuf(dev, i))
-   return (dev->n_sbufs > 1)? 0 : -ENOMEM;
+   return (dev->n_sbufs > 1 ? 0 : -ENOMEM);
dev->n_sbufs = i+1;
}
}
@@ -558,9 +558,8 @@ static int v4l_stk_open(struct file *fp)
vdev = video_devdata(fp);
dev = vdev_to_camera(vdev);
 
-   if (dev == NULL || !is_present(dev)) {
+   if (dev == NULL || !is_present(dev))
return -ENXIO;
-   }
fp->private_data = dev;
usb_autopm_get_interface(dev->interface);
 
@@ -579,7 +578,7 @@ static int v4l_stk_release(struct file *fp)
dev->owner = NULL;
}
 
-   if(is_present(dev))
+   if (is_present(dev))
usb_autopm_put_interface(dev->interface);
 
return 0;
@@ -656,7 +655,7 @@ static unsigned int v4l_stk_poll(struct file *fp, 
poll_table *wait)
return POLLERR;
 
if (!list_empty(&dev->sio_full))
-   return (POLLIN | POLLRDNORM);
+   return POLLIN | POLLRDNORM;
 
return 0;
 }
@@ -893,9 +892,9 @@ static int stk_vidioc_g_fmt_vid_cap(struct file *filp,
struct stk_camera *dev = priv;
int i;
 
-   for (i = 0; i < ARRAY_SIZE(stk_sizes)
-   && stk_sizes[i].m != dev->vsettings.mode;
-   i++);
+   for (i = 0; i < ARRAY_SIZE(stk_sizes) &&
+   stk_sizes[i].m != dev->vsettings.mode; i++)
+   ;
if (i == ARRAY_SIZE(stk_sizes)) {
STK_ERROR("ERROR: mode invalid\n");
return -EINVAL;
@@ -1307,9 +1306,8 @@ static int stk_camera_probe(struct usb_interface 
*interface,
usb_set_intfdata(interface, dev);
 
err = stk_register_video_device(dev);
-   if (err) {
+   if (err)
goto error;
-   }
 
return 0;
 
-- 
1.7.4.4

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

Re: omap3isp as a wakeup source

2011-09-13 Thread Sakari Ailus

On Tue, Sep 13, 2011 at 12:28:12PM +0200, Enrico wrote:
> On Tue, Sep 13, 2011 at 11:48 AM, Tero Kristo  wrote:
> > On Tue, 2011-09-13 at 08:49 +0200, Sakari Ailus wrote:
> >> anish singh wrote:
> >> > On Tue, Sep 13, 2011 at 1:58 AM, Sakari Ailus  
> >> > wrote:
> >> >> On Mon, Sep 12, 2011 at 04:50:42PM +0200, Enrico wrote:
> >> >>> Hi,
> >> >>
> >> >> Hi Enrico,
> >> >>
> >> >>> While testing omap3isp+tvp5150 with latest Deepthy bt656 patches
> >> >>> (kernel 3.1rc4) i noticed that yavta hangs very often when grabbing
> >> >>> or, if not hanged, it grabs at max ~10fps.
> >> >>>
> >> >>> Then i noticed that tapping on the (serial) console made it "unblock"
> >> >>> for some frames, so i thought it doesn't prevent the cpu to go
> >> >>> idle/sleep. Using the boot arg "nohlt" the problem disappear and it
> >> >>> grabs at a steady 25fps.
> >> >>>
> >> >>> In the code i found a comment that says the camera can't be a wakeup
> >> >>> source but the camera powerdomain is instead used to decide to not go
> >> >>> idle, so at this point i think the camera powerdomain is not enabled
> >> >>> but i don't know how/where to enable it. Any ideas?
> >> >>
> >> >> I can confirm this indeed is the case --- ISP can't wake up the system 
> >> >> ---
> >> >> but don't know how to prevent the system from going to sleep when using 
> >> >> the
> >> >> ISP.
> >> > Had it been on android i think wakelock would have been very useful.
> >>
> >> I believe there are proper means to do this using more standard methods
> >> as well. Not being a PM expert, I don't know how.
> >>
> >> Cc Tero.
> >>
> >
> > Hi,
> >
> > I don't think there are proper means yet to do this, as camera is
> > somewhat a special case in omap3, it is apparently the only module that
> > is causing this kind of problem. However, you can prevent idle when
> > camera is active with something like this:
> >
> > diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> > index 2789e0a..7fdf6e2 100644
> > --- a/arch/arm/mach-omap2/pm34xx.c
> > +++ b/arch/arm/mach-omap2/pm34xx.c
> > @@ -358,6 +358,9 @@ void omap_sram_idle(void)
> >                                omap3_per_save_context();
> >        }
> >
> > +       if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
> > +               clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
> > +
> >        /* CORE */
> >        if (core_next_state < PWRDM_POWER_ON) {
> >                omap_uart_prepare_idle(0);
> >
> >
> >
> > -Tero
> 
> 
> One more thing: i just noticed that in Deepthy bt656 patches there is
> one patch [1] to add cam regulators (1v8 and 2v8) to the omap3evm
> board file, i'm not using an omap3evm but maybe i should add them to
> my board file too?

In general, if your board doesn't require them (e.g. for sensors' use) you
should't add them.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
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] New class for low level sensors controls?

2011-09-13 Thread Sakari Ailus

On Tue, Sep 13, 2011 at 12:33:58PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Thursday 08 September 2011 13:44:28 Sakari Ailus wrote:
> > On Thu, Sep 08, 2011 at 04:54:23PM +0530, Subash Patel wrote:
> > > On 09/06/2011 05:52 PM, Sakari Ailus wrote:
> > > > On Tue, Sep 06, 2011 at 01:41:11PM +0200, Laurent Pinchart wrote:
> > > > >
> > > > > Other controls often found in bayer sensors are black level
> > > > > compensation and test pattern.
> > > 
> > > Does all BAYER sensor allow the dark level compensation programming?
> > 
> > I'm not sure. I have always seen ISPs being used for that, not sensors.
> > 
> > > I thought it must be auto dark level compensation, which is done by
> > > the sensor. The sensor detects the optical black value at start of
> > > each frame, and analog-to-digital conversion is shifted to
> > > compensate the dark level for that frame. Hence I am thinking if
> > > this should be a controllable feature.
> > 
> > This is probably what smart sensors could do. If we have a raw bayer sensor
> > the computation of the optimal black level compensation could be done by
> > some of the controls algorithms run in the user space. Automatic exposure
> > probably?
> 
> Many "non-smart" raw bayer sensors implement both manual and automatic black 
> level compensation. In the first case the user programs a value to be 
> subtracted from the pixels (whether that's done in the analog or digital 
> domain might be sensor-specific), and in the second case the sensor computes 
> a 
> mean black level value based on black lines (optically unexposed) at the top 
> of the image.

Sounds like two controls to me, right?

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
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] New class for low level sensors controls?

Hi Sakari,

On Tuesday 13 September 2011 14:00:36 Sakari Ailus wrote:
> On Tue, Sep 13, 2011 at 12:33:58PM +0200, Laurent Pinchart wrote:
> > On Thursday 08 September 2011 13:44:28 Sakari Ailus wrote:
> > > On Thu, Sep 08, 2011 at 04:54:23PM +0530, Subash Patel wrote:
> > > > On 09/06/2011 05:52 PM, Sakari Ailus wrote:
> > > > > On Tue, Sep 06, 2011 at 01:41:11PM +0200, Laurent Pinchart wrote:
> > > > > > Other controls often found in bayer sensors are black level
> > > > > > compensation and test pattern.
> > > > 
> > > > Does all BAYER sensor allow the dark level compensation programming?
> > > 
> > > I'm not sure. I have always seen ISPs being used for that, not sensors.
> > > 
> > > > I thought it must be auto dark level compensation, which is done by
> > > > the sensor. The sensor detects the optical black value at start of
> > > > each frame, and analog-to-digital conversion is shifted to
> > > > compensate the dark level for that frame. Hence I am thinking if
> > > > this should be a controllable feature.
> > > 
> > > This is probably what smart sensors could do. If we have a raw bayer
> > > sensor the computation of the optimal black level compensation could
> > > be done by some of the controls algorithms run in the user space.
> > > Automatic exposure probably?
> > 
> > Many "non-smart" raw bayer sensors implement both manual and automatic
> > black level compensation. In the first case the user programs a value to
> > be subtracted from the pixels (whether that's done in the analog or
> > digital domain might be sensor-specific), and in the second case the
> > sensor computes a mean black level value based on black lines (optically
> > unexposed) at the top of the image.
> 
> Sounds like two controls to me, right?

At least two, an auto control and a manual value control. Additional controls 
(such as the number of lines on which to compute the black level average 
automatically for instance, or a read-only control to report the computed 
average) will probably be added later.

-- 
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: omap3isp as a wakeup source

2011-09-13 Thread Enrico

On Tue, Sep 13, 2011 at 12:29 PM, Tero Kristo  wrote:
> Powerdomain is automatically on if there are any clocks enabled on it.
> If you make sure that ISP has some activity ongoing, then it should be
> on. You can check the state of the camera powerdomain
> from /sys/kernel/debug/pm_debug/count file, if you have mounted debugfs.

And in fact something seems wrong (this is on a patched 3.0.4 kernel):

usbhost_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
per_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
dss_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
cam_pwrdm (RET),OFF:0,RET:9,INA:0,ON:9,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
core_pwrdm 
(ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0
neon_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0
mpu_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
iva2_pwrdm 
(RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0
per_clkdm->per_pwrdm (20)
usbhost_clkdm->usbhost_pwrdm (3)
cam_clkdm->cam_pwrdm (0)
dss_clkdm->dss_pwrdm (1)
core_l4_clkdm->core_pwrdm (23)
core_l3_clkdm->core_pwrdm (4)
d2d_clkdm->core_pwrdm (0)
sgx_clkdm->sgx_pwrdm (0)
iva2_clkdm->iva2_pwrdm (0)
neon_clkdm->neon_pwrdm (0)
mpu_clkdm->mpu_pwrdm (0)
prm_clkdm->wkup_pwrdm (0)
cm_clkdm->core_pwrdm (0)


I think the line "cam_clkdm->cam_pwrdm (0)" means that it was never
enabled, but i grabbed some frames with yavta before that.

Anyone more into runtime PM that i can CC for suggestions?

Enrico
--
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 v3] V4L: dynamically allocate video_device nodes in subdevices

On Tue, 13 Sep 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Tuesday 13 September 2011 11:26:23 Guennadi Liakhovetski wrote:
> > On Tue, 13 Sep 2011, Laurent Pinchart wrote:
> > > On Monday 12 September 2011 12:55:46 Guennadi Liakhovetski wrote:
> > > > Currently only very few drivers actually use video_device nodes,
> > > > embedded in struct v4l2_subdev. Allocate these nodes dynamically for
> > > > those drivers to save memory for the rest.
> > > > 
> > > > Signed-off-by: Guennadi Liakhovetski 
> > > > ---
> > > > 
> > > > v3: addressed comments from Laurent Pinchart - thanks
> > > 
> > > Thanks for the patch. Just one small comment below.
> > > 
> > > > 1. switch to using a device-release method, instead of freeing directly
> > > > in v4l2_device_unregister_subdev()
> > > > 
> > > > 2. switch to using drvdata instead of a wrapper struct
> > > > 
> > > >  drivers/media/video/v4l2-device.c |   41
> > > > 
> > > >  include/media/v4l2-subdev.h  
> > > > |
> > > > 
> > > >  4 +-
> > > >  2 files changed, 38 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/video/v4l2-device.c
> > > > b/drivers/media/video/v4l2-device.c index c72856c..9bf3d70 100644
> > > > --- a/drivers/media/video/v4l2-device.c
> > > > +++ b/drivers/media/video/v4l2-device.c
> > > > @@ -21,6 +21,7 @@
> > > > 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > 
> > > > +#include 
> > > > 
> > > >  #if defined(CONFIG_SPI)
> > > >  #include 
> > > >  #endif
> > > > 
> > > > @@ -191,6 +192,13 @@ int v4l2_device_register_subdev(struct v4l2_device
> > > > *v4l2_dev, }
> > > > 
> > > >  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
> > > > 
> > > > +void v4l2_device_release_subdev_node(struct video_device *vdev)
> > > > +{
> > > > +   struct v4l2_subdev *sd = video_get_drvdata(vdev);
> > > > +   sd->devnode = NULL;
> > > > +   kfree(vdev);
> > > > +}
> > > > +
> > > > 
> > > >  int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
> > > >  {
> > > >  
> > > > struct video_device *vdev;
> > > > 
> > > > @@ -204,22 +212,42 @@ int v4l2_device_register_subdev_nodes(struct
> > > > v4l2_device *v4l2_dev) if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
> > > > 
> > > > continue;
> > > > 
> > > > -   vdev = &sd->devnode;
> > > > +   vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> > > > +   if (!vdev) {
> > > > +   err = -ENOMEM;
> > > > +   goto clean_up;
> > > > +   }
> > > > +
> > > > +   video_set_drvdata(vdev, sd);
> > > > 
> > > > strlcpy(vdev->name, sd->name, sizeof(vdev->name));
> > > > vdev->v4l2_dev = v4l2_dev;
> > > > vdev->fops = &v4l2_subdev_fops;
> > > > 
> > > > -   vdev->release = video_device_release_empty;
> > > > +   vdev->release = v4l2_device_release_subdev_node;
> > > > 
> > > > vdev->ctrl_handler = sd->ctrl_handler;
> > > > err = __video_register_device(vdev, VFL_TYPE_SUBDEV, 
> > > > -1, 1,
> > > > 
> > > >   sd->owner);
> > > > 
> > > > -   if (err < 0)
> > > > -   return err;
> > > > +   if (err < 0) {
> > > > +   kfree(vdev);
> > > > +   goto clean_up;
> > > > +   }
> > > > +   get_device(&vdev->dev);
> > > 
> > > Is get_device() (and the corresponding put_device() calls below) required
> > > ? I thought device_register() initialized the reference count to 1
> > > (don't take my word for it though).
> > 
> > Indeed, I think, you're right. Will update.
> 
> Please test it as well :-)

I'm afraid, testing it wouldn't be very easy for me: I only have one 
system here, on which MC is used - the beagle-board. And it is not an easy 
nor a quick exersize to bring it up and run a test on it;-) But if noone 
else finds time to test it and if we're not confident enough in its 
correctness, well, we'll have to wait until I find time to do that...

BTW, there's one more improvement to be made for this patch:

-void v4l2_device_release_subdev_node(struct video_device *vdev)
+static void v4l2_device_release_subdev_node(struct video_device *vdev)

My copy-paste from video_device_release_empty() was too precise:-(

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] New class for low level sensors controls?

On Tue, 13 Sep 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Thursday 08 September 2011 14:35:54 Guennadi Liakhovetski wrote:
> > On Tue, 6 Sep 2011, Sakari Ailus wrote:
> > > On Tue, Sep 06, 2011 at 01:41:11PM +0200, Laurent Pinchart wrote:
> > > > On Tuesday 06 September 2011 13:36:53 Sakari Ailus wrote:
> 
> [snip]
> 
> > > > > Typically such sensors are not controlled by general purpose
> > > > > applications but e.g. require a camera control algorithm framework
> > > > > in user space. This needs to be implemented in libv4l for general
> > > > > purpose applications to work properly on this kind of hardware.
> > > > > 
> > > > > These sensors expose controls such as
> > > > > 
> > > > > - Per-component gain controls. Red, blue, green (blue) and green
> > > > > (red) gains.
> > > > > 
> > > > > - Link frequency. The frequency of the data link from the sensor to
> > > > > the bridge.
> > > > > 
> > > > > - Horizontal and vertical blanking.
> > > > 
> > > > Other controls often found in bayer sensors are black level
> > > > compensation and test pattern.
> > 
> > May I suggest a couple more:
> > 
> > (1) snapshot mode (I really badly want this one, please;-))
> 
> What do you mean exactly by snapshot mode ? Is that just external trigger, or 
> does it cover more features than that ?

I mean flipping the "snapshot" bit in the respective sensor configuration 
register. There are variants, of course. On some sensors there's just one 
such bit and all it does is enable the flash strobe. On others it also 
switches the configuration registers to immediately start recording a 
different frame size. On some other sensors, I think, there are more than 
2 sets of such configurations, then you need more than one control to 
switch between them. On yet other sensors you can program, how many frames 
you want to take in this mode (before switching back or before halting). I 
think, as a minimum we would expect from this control: switch flash strobe 
on & switch to the snapshot frame size, driver implementation details 
depend on the hardware capabilities, of course.

Thanks
Guennadi

> 
> > (2) flash controls (yes, I know we already have V4L2_CTRL_CLASS_FLASH, I
> > just have the impression, that these controls are mostly meant for
> > either pure software implementations, or for external controllers, I
> > think it should also be possible to have them exported by a normal
> > sensor driver, but wasn't really sure. So, wanted to point out to this
> > possibility once again.)
> 
> As Sakari told you in his answer, we can add new controls to the flash class.
> 
> > (3) AEC / AGC regions
> 
> This will get tricky. I'm tempted to propose the idea of v4l2_rect controls 
> and control arrays again :-)
> 
> > (4) stereo (3D anyone?;)) - no, don't think we need it now...
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

soc-camera: future directions

Hi all

(a slightly more complete version of this post with some background 
information is available at 
http://www.open-technology.de/index.php?/archives/80-soc-camera-3.2-and-beyond-roadmap.html
)

As you all are certainly aware, the soc-camera framework in its present 
state, as published at

http://git.linuxtv.org/gliakhovetski/v4l-dvb.git/shortlog/refs/heads/rc1-for-3.2

has converted almost all its sensor and TV-decoder drivers to be usable 
with generic V4L2 bridge drivers (thanks to Hans Verkuil for V4L2 control 
framework conversion). Next on our roadmap is support for the Media 
Controller and pad-level APIs. Below are a couple of ideas, how this could 
be done, without any supporting code yet. The purpose of this post is to 
formalise my ideas a bit and to give you all a chance to point out any 
flaws in my concept. Since I haven't so far worked too closely with MC, 
such flaws are quite possible.

At the moment the soc-camera framework is mostly designed around a model, 
in which the video subsystem consists of a video capture interface on the 
SoC, handled as a single block, and one external capture device, like a 
camera sensor or a TV-decoder, connected to the above interface and 
additionally controlled over I2C or by some other means.

Extending this model to better support multi-entity configurations is also 
on my TODO list, but is a separate task, therefore in this first step of 
the MC conversion I will initially address this simplistic 2-point scheme, 
but try to make design decisions, that would make supporting more complex 
configurations in the future simple enough.

The actual idea for this first step is to add an ability to support client 
(sensor and decoder) drivers, implementing the pad-level API, to 
soc-camera in a native way. This means, without wrapping subdev pad 
operations in standard video and core subdev operations, but by building a 
minimum MC implementation on top of existing soc-camera SoC (host / 
bridge) drivers, ideally, without having to modify them at all. That way, 
if a standard subdev driver is attached to your SoC, you get a standard 
V4L2 user-space interface, if your subdev driver implements pad-level 
operations, you can get a functional MC interface in user space with the 
same SoC driver.

Such a minimal MC implementation would create two entities: one for the 
actual video device (for the DMA engine), and one for the external video 
interface. In this simple case they shall be connected by an immutable 
link. The external pad will then be linked to the external video device, 
at least in the typical simple case of only one source pad on it.

Additionally, it should be possible for SoC drivers to implement advanced 
MC support, while still using parts of the soc-camera infrastructure.

In the most generic case, when both the SoC and the client drivers 
implement their own MC API support, the soc-camera framework should be 
made aware of the fact, that the configuration of all the entities in the 
system can change at any time, bypassing soc-camera interfaces, which 
means no cached values can be used by the soc-camera core.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] V4L: dynamically allocate video_device nodes in subdevices

Currently only very few drivers actually use video_device nodes, embedded
in struct v4l2_subdev. Allocate these nodes dynamically for those drivers
to save memory for the rest.

Signed-off-by: Guennadi Liakhovetski 
---

v4:

1. added "static" in v4l2_device_release_subdev_node() definition
2. removed superfluous get_device() and put_device() (thanks to Laurent 
for pointing out)

 drivers/media/video/v4l2-device.c |   36 +++-
 include/media/v4l2-subdev.h   |4 ++--
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/drivers/media/video/v4l2-device.c 
b/drivers/media/video/v4l2-device.c
index c72856c..8abf830 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #if defined(CONFIG_SPI)
 #include 
 #endif
@@ -191,6 +192,13 @@ int v4l2_device_register_subdev(struct v4l2_device 
*v4l2_dev,
 }
 EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
 
+static void v4l2_device_release_subdev_node(struct video_device *vdev)
+{
+   struct v4l2_subdev *sd = video_get_drvdata(vdev);
+   sd->devnode = NULL;
+   kfree(vdev);
+}
+
 int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
 {
struct video_device *vdev;
@@ -204,22 +212,40 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device 
*v4l2_dev)
if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
continue;
 
-   vdev = &sd->devnode;
+   vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
+   if (!vdev) {
+   err = -ENOMEM;
+   goto clean_up;
+   }
+
+   video_set_drvdata(vdev, sd);
strlcpy(vdev->name, sd->name, sizeof(vdev->name));
vdev->v4l2_dev = v4l2_dev;
vdev->fops = &v4l2_subdev_fops;
-   vdev->release = video_device_release_empty;
+   vdev->release = v4l2_device_release_subdev_node;
vdev->ctrl_handler = sd->ctrl_handler;
err = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
  sd->owner);
-   if (err < 0)
-   return err;
+   if (err < 0) {
+   kfree(vdev);
+   goto clean_up;
+   }
 #if defined(CONFIG_MEDIA_CONTROLLER)
sd->entity.v4l.major = VIDEO_MAJOR;
sd->entity.v4l.minor = vdev->minor;
 #endif
+   sd->devnode = vdev;
}
return 0;
+
+clean_up:
+   list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
+   if (!sd->devnode)
+   break;
+   video_unregister_device(sd->devnode);
+   }
+
+   return err;
 }
 EXPORT_SYMBOL_GPL(v4l2_device_register_subdev_nodes);
 
@@ -245,7 +271,7 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
if (v4l2_dev->mdev)
media_device_unregister_entity(&sd->entity);
 #endif
-   video_unregister_device(&sd->devnode);
+   video_unregister_device(sd->devnode);
module_put(sd->owner);
 }
 EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 257da1a..5dd049a 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -534,13 +534,13 @@ struct v4l2_subdev {
void *dev_priv;
void *host_priv;
/* subdev device node */
-   struct video_device devnode;
+   struct video_device *devnode;
 };
 
 #define media_entity_to_v4l2_subdev(ent) \
container_of(ent, struct v4l2_subdev, entity)
 #define vdev_to_v4l2_subdev(vdev) \
-   container_of(vdev, struct v4l2_subdev, devnode)
+   video_get_drvdata(vdev)
 
 /*
  * Used for storing subdev information per file handle
-- 
1.7.2.5

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

Re: Fwd: V4L2 driver EM28xx

2011-09-13 Thread Mauro Carvalho Chehab

Em 12-09-2011 18:56, Lukas Sukdol escreveu:
> Hello,
> I can't get working USB DVR Box (4 video channels / 2 audio) with the
> EM2860 chip.
> The USB device is recognized, but it doesn't work with any of 77 cards
> in the list...
> I'm using Fedora 14 (2.6.35.14-96.fc14.i686).
> 
> See details in attached dmesg log file.
> 
> Link to card (TE-3104AE): http://www.tungson.cn/en/product_info.asp?InfoID=164
> 
> Is this card (or will be) supported by EM28xx driver?

Only if someone with the hardware adds support for it ;)

It shouldn't be that hard to add support for it: all you need to do is to
capture the USB logs from the original driver and use the existing parsers
for em28xx to discover what it does to select between the 4 video inputs
and the 2 audio inputs. It probably uses some GPIO's to select them.

Linuxtv wiki pages explain how to do it at the developer's section. You should
search there for the USB sniffing pages.

Good luck,
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

ngene/cxd2099 occasional timeouts/errors

2011-09-13 Thread Eric Petit

Hello,

I am trying to run Mystique SaTiX-S2 CI Dual cards to fetch and decrypt a 
number of channels 24/7. I got it to work with the ngene/cxd2099 drivers 
(passing the stream through the sec0 device etc), but one remaining issue is 
that the driver occasionally reports a command timeout followed by more errors 
from the CI:

Sep  6 08:46:48 s102-34 kernel: [1636918.174974] slot_shutdown
Sep  6 08:46:50 s102-34 kernel: [1636920.481706] dvb_ca adapter 2: DVB CAM 
detected and initialised successfully
  (Running fine...)
Sep 10 09:20:41 s102-34 kernel: [1984551.380033] ngene: Command timeout cmd=03 
prev=04
Sep 10 09:20:41 s102-34 kernel: [1984551.384675] host_to_ngene (c000): 03 04 80 
01 01 00 00 00
Sep 10 09:20:41 s102-34 kernel: [1984551.389345] ngene_to_host (c100): 00 00 00 
00 00 00 00 00
Sep 10 09:20:41 s102-34 kernel: [1984551.393900] dev->hosttongene (f3025000): 
03 04 80 01 01 00 00 00
Sep 10 09:20:41 s102-34 kernel: [1984551.398551] dev->ngenetohost (f3025100): 
00 00 00 00 00 00 00 00
Sep 10 09:20:41 s102-34 kernel: [1984551.403176] error in i2c_read
Sep 10 09:20:41 s102-34 kernel: [1984551.407999] Failed to write to I2C!
Sep 10 09:20:41 s102-34 kernel: [1984551.412801] Failed to write to I2C!
Sep 10 09:20:41 s102-34 kernel: [1984551.417474] Failed to write to I2C!
Sep 10 09:20:41 s102-34 kernel: [1984551.422085] Failed to write to I2C!
Sep 10 09:20:41 s102-34 kernel: [1984551.426605] Failed to write to I2C!
Sep 10 09:20:41 s102-34 kernel: [1984551.512361] Failed to write to I2C 
register 00@40!
Sep 10 09:20:41 s102-34 kernel: [1984551.516719] Failed to write to I2C 
register 00@40!
Sep 10 09:20:41 s102-34 kernel: [1984551.520784] DR
Sep 10 09:20:41 s102-34 kernel: [1984551.520789] WC
Sep 10 09:20:41 s102-34 kernel: [1984551.521108] Failed to write to I2C 
register 00@40!
Sep 10 09:20:41 s102-34 kernel: [1984551.525402] Failed to write to I2C 
register 00@40!
Sep 10 09:20:41 s102-34 kernel: [1984551.529326] NO CAM
Sep 10 09:20:41 s102-34 kernel: [1984551.529652] Failed to write to I2C 
register 00@40!
Sep 10 09:20:41 s102-34 kernel: [1984551.533528] slot_shutdown
Sep 10 09:20:41 s102-34 kernel: [1984551.533853] Failed to write to I2C 
register 00@40!
Sep 10 09:20:41 s102-34 kernel: [1984551.538034] Failed to write to I2C 
register 00@40!
Sep 10 09:20:41 s102-34 kernel: [1984551.542166] Failed to write to I2C 
register 00@40!

>From the moment that happens, I can't do anything with the card: all ioctls 
>fail even after re-opening the adapter or after reloading the drivers. Only a 
>reboot can bring it back to a good state.

This condition is not so frequent, I've seen it happen maybe 5 times running a 
dozen cards for a week. Has anyone experienced it? Is this something the driver 
would be able to recover from?

Versions details below,

Thanks,

-- 
Eric




Kernel:
linux-image-3.0.0-1-686-pae Debian package (32-bit)

Modules built from v4l-dvb:
cxd2099
dvb_core
lnbp21
ngene
stv090x
stv6110x

Dmesg output from loading the drivers:
slot_shutdown
ngene :03:00.0: PCI INT A disabled
WARNING: You are using an experimental version of the media stack.
As the driver is backported to an older kernel, it doesn't offer
enough quality for its usage in production.
Use it with care.
Latest git patches (needed if you report a bug to 
linux-media@vger.kernel.org):
3d589db03f09c1ace6f71849085595f1f114cd3c [media] v4l: mt9v032: Fix 
Bayer pattern
fcbd986d61c726d64db940b27d4f3604a6cbecb0 [media] V4L: mt9m111: rewrite 
set_pixfmt
4e817223d7f4cf8b740037be4a1ca1578850e8c9 [media] V4L: mt9m111: fix 
missing return value check mt9m111_reg_clear
nGene PCIE bridge driver, Copyright (C) 2005-2007 Micronas
ngene :03:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
ngene: Found Mystique SaTiX-S2 Dual (v2)
ngene :03:00.0: setting latency timer to 64
ngene: Device version 1
ngene: Loading firmware file ngene_18.fw.
ngene :03:00.0: irq 68 for MSI/MSI-X
Attached CXD2099AR at 40
LNBx2x attached on addr=a
stv6110x_attach: Attaching STV6110x
DVB: registering new adapter (nGene)
DVB: registering adapter 0 frontend 0 (STV090x Multistandard)...
LNBx2x attached on addr=8
stv6110x_attach: Attaching STV6110x
DVB: registering new adapter (nGene)
DVB: registering adapter 1 frontend 0 (STV090x Multistandard)...
No demod found on chan 2
No demod found on chan 3
DVB: registering new adapter (nGene)

>From lspci:
03:00.0 Multimedia video controller [0400]: Micronas Semiconductor Holding 
AG Device [18c3:0720] (rev 01)
Subsystem: Micronas Semiconductor Holding AG Device [18c3:db02]
Flags: bus master, fast devsel, latency 0, IRQ 68
Memory at b891 (32-bit, non-prefetchable) [size=64K]
Memory at b890 (64-bit, non-prefetchable) [size=64K]
Capabilities: [40] Power Management version 2
Capabilities: [48] MSI

Re: [PATCH v4] V4L: dynamically allocate video_device nodes in subdevices

2011-09-13 Thread Sylwester Nawrocki

Hi Guennadi,

On 09/13/2011 04:48 PM, Guennadi Liakhovetski wrote:
> Currently only very few drivers actually use video_device nodes, embedded
> in struct v4l2_subdev. Allocate these nodes dynamically for those drivers
> to save memory for the rest.
> 
> Signed-off-by: Guennadi Liakhovetski 

I have tested this patch with Samsung FIMC driver and with MC enabled
sensor driver.
After some hundreds of module load/unload I didn't observe anything unusual.
The patch seem to be safe for device node enabled subdevs. You can stick my:

Tested-by: Sylwester Nawrocki 

if you feel so.


Thanks,
Sylwester Nawrocki
Samsung Poland R&D Center

> ---
> 
> v4:
> 
> 1. added "static" in v4l2_device_release_subdev_node() definition
> 2. removed superfluous get_device() and put_device() (thanks to Laurent 
> for pointing out)
> 
>  drivers/media/video/v4l2-device.c |   36 +++-
>  include/media/v4l2-subdev.h   |4 ++--
>  2 files changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-device.c 
> b/drivers/media/video/v4l2-device.c
> index c72856c..8abf830 100644
> --- a/drivers/media/video/v4l2-device.c
> +++ b/drivers/media/video/v4l2-device.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #if defined(CONFIG_SPI)
>  #include 
>  #endif
> @@ -191,6 +192,13 @@ int v4l2_device_register_subdev(struct v4l2_device 
> *v4l2_dev,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
>  
> +static void v4l2_device_release_subdev_node(struct video_device *vdev)
> +{
> + struct v4l2_subdev *sd = video_get_drvdata(vdev);
> + sd->devnode = NULL;
> + kfree(vdev);
> +}
> +
>  int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
>  {
>   struct video_device *vdev;
> @@ -204,22 +212,40 @@ int v4l2_device_register_subdev_nodes(struct 
> v4l2_device *v4l2_dev)
>   if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
>   continue;
>  
> - vdev = &sd->devnode;
> + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> + if (!vdev) {
> + err = -ENOMEM;
> + goto clean_up;
> + }
> +
> + video_set_drvdata(vdev, sd);
>   strlcpy(vdev->name, sd->name, sizeof(vdev->name));
>   vdev->v4l2_dev = v4l2_dev;
>   vdev->fops = &v4l2_subdev_fops;
> - vdev->release = video_device_release_empty;
> + vdev->release = v4l2_device_release_subdev_node;
>   vdev->ctrl_handler = sd->ctrl_handler;
>   err = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
> sd->owner);
> - if (err < 0)
> - return err;
> + if (err < 0) {
> + kfree(vdev);
> + goto clean_up;
> + }
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>   sd->entity.v4l.major = VIDEO_MAJOR;
>   sd->entity.v4l.minor = vdev->minor;
>  #endif
> + sd->devnode = vdev;
>   }
>   return 0;
> +
> +clean_up:
> + list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> + if (!sd->devnode)
> + break;
> + video_unregister_device(sd->devnode);
> + }
> +
> + return err;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev_nodes);
>  
> @@ -245,7 +271,7 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
>   if (v4l2_dev->mdev)
>   media_device_unregister_entity(&sd->entity);
>  #endif
> - video_unregister_device(&sd->devnode);
> + video_unregister_device(sd->devnode);
>   module_put(sd->owner);
>  }
>  EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 257da1a..5dd049a 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -534,13 +534,13 @@ struct v4l2_subdev {
>   void *dev_priv;
>   void *host_priv;
>   /* subdev device node */
> - struct video_device devnode;
> + struct video_device *devnode;
>  };
>  
>  #define media_entity_to_v4l2_subdev(ent) \
>   container_of(ent, struct v4l2_subdev, entity)
>  #define vdev_to_v4l2_subdev(vdev) \
> - container_of(vdev, struct v4l2_subdev, devnode)
> + video_get_drvdata(vdev)
>  
>  /*
>   * Used for storing subdev information per file handle


-- 
Sylwester Nawrocki
Samsung Poland R&D Center
--
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 v4] V4L: dynamically allocate video_device nodes in subdevices

Hi Guennadi,

On Tuesday 13 September 2011 16:48:10 Guennadi Liakhovetski wrote:
> Currently only very few drivers actually use video_device nodes, embedded
> in struct v4l2_subdev. Allocate these nodes dynamically for those drivers
> to save memory for the rest.
> 
> Signed-off-by: Guennadi Liakhovetski 

Acked-by: Laurent Pinchart 

I will try to test the patch tomorrow on an OMAP3 system.

-- 
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: Cinergy T USB XXS (HD): wrong keymap loaded

2011-09-13 Thread Sander Devrieze

Hello,

Watching television works out of the box, but the remote control does
not work by default. The remote uses the NEC protocol as is also noted
on
http://www.linuxtv.org/wiki/index.php/TerraTec_Cinergy_T_USB_XXS#Cinergy_T_USB_XXS_.28HD.29_.280ccd:00ab.29
However the RC5 protocol is loaded with a wrong keymap. This is the
command that made the remote work:

ir-keytab --sysdev rc1 -w /lib/udev/rc_keymaps/dib0700_nec

lsusb output:
Bus 001 Device 006: ID 0ccd:00ab TerraTec Electronic GmbH

Related discussion:
http://linuxtv.org/irc/v4l/index.php?date=2011-09-08 (user: fgfhdfh)

Note to Terratec support: last month I bought hardware from LiteOn,
Logitech, Spire, Epson, Lexmark, Intel, AMD (ATI), Crucial, MSI,
Seagate and Terratec. All hardware works flawlessly out of the box or
after installation of software from the vendor, except for Terratec.
Inexperienced people will think their Terratec hardware is broken in a
case like this. Can you guys regularly test your hardware and send
reports like this to the linux-media mailing list (if something is
broken)? With the expected growth of Linux-based Android tablets, the
number of inexperienced people that will think Terratec hardware is
broken could only increase (if they want to use a Terratec USB device
to watch television on their tablet).

--
Mvg, Sander Devrieze.
--
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

2011-09-13 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:Tue Sep 13 19:00:30 CEST 2011
git hash:2d04c13a507f5a01daa7422cd52250809573cfdb
gcc version:  i686-linux-gcc (GCC) 4.6.1
host hardware:x86_64
host os:  2.6.32.5

linux-git-armv5: WARNINGS
linux-git-armv5-davinci: WARNINGS
linux-git-armv5-ixp: WARNINGS
linux-git-armv5-omap2: WARNINGS
linux-git-i686: WARNINGS
linux-git-m32r: OK
linux-git-mips: WARNINGS
linux-git-powerpc64: WARNINGS
linux-git-x86_64: WARNINGS
linux-2.6.31.12-i686: ERRORS
linux-2.6.32.6-i686: ERRORS
linux-2.6.33-i686: ERRORS
linux-2.6.34-i686: ERRORS
linux-2.6.35.3-i686: ERRORS
linux-2.6.36-i686: WARNINGS
linux-2.6.37-i686: WARNINGS
linux-2.6.38.2-i686: WARNINGS
linux-2.6.39.1-i686: WARNINGS
linux-3.0-i686: WARNINGS
linux-3.1-rc1-i686: WARNINGS
linux-2.6.31.12-x86_64: ERRORS
linux-2.6.32.6-x86_64: ERRORS
linux-2.6.33-x86_64: ERRORS
linux-2.6.34-x86_64: ERRORS
linux-2.6.35.3-x86_64: ERRORS
linux-2.6.36-x86_64: WARNINGS
linux-2.6.37-x86_64: WARNINGS
linux-2.6.38.2-x86_64: WARNINGS
linux-2.6.39.1-x86_64: WARNINGS
linux-3.0-x86_64: WARNINGS
linux-3.1-rc1-x86_64: WARNINGS
spec-git: WARNINGS
sparse: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

Question about USB interface index restriction in gspca

2011-09-13 Thread Frank Schäfer


Hi,

I have a question about the following code in gspca.c:

in function gspca_dev_probe(...):
...
/* the USB video interface must be the first one */
if (dev->config->desc.bNumInterfaces != 1
&& intf->cur_altsetting->desc.bInterfaceNumber != 0)
return -ENODEV;
...


Is there a special reason for not allowing devices with USB interface 
index > 0 for video ?
I'm experimenting with a device that has the video interface at index 3 
and two audio interfaces at index 0 and 1 (index two is missing !).
And the follow-up question: can we assume that all device handled by the 
gspca-driver have vendor specific video interfaces ?

Then we could change the code to

...
/* the USB video interface must be of class vendor */
if (intf->cur_altsetting->desc.bInterfaceClass != 
USB_CLASS_VENDOR_SPEC)

return -ENODEV;
 ...

Regards,
Frank


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

[GIT PULL FOR 3.2] PCTV DVB-S2 Stick 460e


Morjens Mauro

This patch series adds support for PCTV DVB-S2 Stick 460e, that is 
DVB-S2 USB stick.


Is is based of;
* Empia EM28174
* NXP TDA10071 & Conexant CX24118A combo
* Allegro A8293

It introduces two new chipset drivers namely tda10071 and a8293 - former 
is DVB-S/S2 demod + tuner combo and later is LNB controller.


Also Chris Rankin em28xx bug fix is included since it wasn't committed 
to 3.2 tree as today.


regards
Antti


The following changes since commit 2d04c13a507f5a01daa7422cd52250809573cfdb:

  [media] dvb-usb: improve sanity check of adap->active_fe in 
dvb_usb_ctrl_feed (2011-09-09 15:28:04 -0300)


are available in the git repository at:
  git://linuxtv.org/anttip/media_tree.git pctv_460e

Antti Palosaari (8):
  a8293: Allegro A8293 SEC driver
  tda10071: NXP TDA10071 DVB-S/S2 driver
  em28xx: add support for PCTV DVB-S2 Stick 460e [2013:024f]
  get_dvb_firmware: add dvb-fe-tda10071.fw
  get_dvb_firmware: update tda10071 file url
  tda10071: do not download last byte of fw
  tda10071: change sleeps to more suitable ones
  get_dvb_firmware: whitespace fix

Chris Rankin (1):
  em28xx: ERROR: "em28xx_add_into_devlist" 
[drivers/media/video/em28xx/em28xx.ko] undefined!


 Documentation/dvb/get_dvb_firmware  |   19 +-
 drivers/media/dvb/frontends/Kconfig |   12 +
 drivers/media/dvb/frontends/Makefile|2 +
 drivers/media/dvb/frontends/a8293.c |  184 
 drivers/media/dvb/frontends/a8293.h |   41 +
 drivers/media/dvb/frontends/tda10071.c  | 1269 
+++

 drivers/media/dvb/frontends/tda10071.h  |   81 ++
 drivers/media/dvb/frontends/tda10071_priv.h |  122 +++
 drivers/media/video/em28xx/Kconfig  |2 +
 drivers/media/video/em28xx/em28xx-cards.c   |   33 +-
 drivers/media/video/em28xx/em28xx-dvb.c |   25 +
 drivers/media/video/em28xx/em28xx.h |1 +
 12 files changed, 1788 insertions(+), 3 deletions(-)
 create mode 100644 drivers/media/dvb/frontends/a8293.c
 create mode 100644 drivers/media/dvb/frontends/a8293.h
 create mode 100644 drivers/media/dvb/frontends/tda10071.c
 create mode 100644 drivers/media/dvb/frontends/tda10071.h
 create mode 100644 drivers/media/dvb/frontends/tda10071_priv.h

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

Re: [PATCH 1/1] EM28xx - fix deadlock when unplugging and replugging a DVB adapter


On 08/21/2011 03:32 PM, Chris Rankin wrote:

It occurred to me this morning that since we're no longer supposed to be
holding the device lock when taking the device list lock, then the
em28xx_usb_disconnect() function needs changing too.

Signed-off-by: Chris Rankin 


I ran that also when re-plugging both PCTV 290e and 460e as today 
LinuxTV 3.2 tree. Seems like this patch is still missing and maybe some 
more.


git log drivers/media/video/em28xx/ |grep -A3 "Author: Chris Rankin"
Author: Chris Rankin 
Date:   Tue Sep 13 12:23:39 2011 +0300

em28xx: ERROR: "em28xx_add_into_devlist" 
[drivers/media/video/em28xx/em28xx.ko] undefined!

--
Author: Chris Rankin 
Date:   Sat Aug 20 16:01:26 2011 -0300

[media] em28xx: don't sleep on disconnect
--
Author: Chris Rankin 
Date:   Sat Aug 20 08:31:05 2011 -0300

[media] em28xx: move printk lines outside mutex lock
--
Author: Chris Rankin 
Date:   Sat Aug 20 08:28:17 2011 -0300

[media] em28xx: clean up resources should init fail
--
Author: Chris Rankin 
Date:   Sat Aug 20 08:21:03 2011 -0300

[media] em28xx: use atomic bit operations for devices-in-use mask
--
Author: Chris Rankin 
Date:   Sat Aug 20 08:08:34 2011 -0300

[media] em28xx: pass correct buffer size to snprintf


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

Re: [PATCH 1/1] EM28xx - fix deadlock when unplugging and replugging a DVB adapter

2011-09-13 Thread Chris Rankin


On 13/09/11 21:04, Antti Palosaari wrote:

On 08/21/2011 03:32 PM, Chris Rankin wrote:

It occurred to me this morning that since we're no longer supposed to be
holding the device lock when taking the device list lock, then the
em28xx_usb_disconnect() function needs changing too.

Signed-off-by: Chris Rankin 


I ran that also when re-plugging both PCTV 290e and 460e as today LinuxTV 3.2
tree. Seems like this patch is still missing and maybe some more.


There was also this patch, which fixed a couple of memory leaks:

http://www.mail-archive.com/linux-media@vger.kernel.org/msg36432.html

IIRC, the main purpose of the other patch was to delete the 
em28xx_remove_from_devlist() function as well by adding the


list_del(&dev->devlist);

to the em28xx_close_extension() function instead.

http://www.mail-archive.com/linux-media@vger.kernel.org/msg35783.html

Cheers,
Chris
--
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: recursive locking problem


On 09/09/2011 01:45 PM, David Waring wrote:

On 08/09/11 17:34, Antti Palosaari wrote:

[snip]

Is there any lock can do recursive locking but unlock frees all locks?

Like that:
gate_open
+gate_open
+gate_close
== lock is free

AFAIK mutex can do only simple lock() + unlock(). Semaphore can do
recursive locking, like lock() + lock() + unlock() + unlock(). But how I
can do lock() + lock() + unlock() == free.


Antti,

It's a very bad idea to try and use a mutex like that. The number of
locks and unlocks must be balanced otherwise you risk accessing
variables without a lock.

Consider:

static struct mutex foo_mutex;
static int foo=3;

void a() {
   mutex_lock(&foo_mutex);
   if (foo<5) foo++;
   b();
   foo--; /*<<<  still need lock here */
   mutex_unlock(&foo_mutex);
}

void b() {
   mutex_lock(&foo_mutex);
   if (foo>6) foo=(foo>>1);
   mutex_unlock(&foo_mutex);
}

Note: this assumes mutex_lock will allow the same thread get multiple
locks as you would like (which it doesn't).

As pointed out in the code, when a() is called, you still need the lock
for accesses to foo after the call to b() that also requires the lock.
If we used the locks in the way you propose then foo would be accessed
without a lock.

To code properly for cases like these I usually use a wrapper functions
to acquire the lock and call a thread unsafe version (i.e. doesn't use
locks) of the function that only uses other thread unsafe functions. e.g.

void a() {
   mutex_lock(&foo_mutex);
   __a_thr_unsafe();
   mutex_unlock(&foo_mutex);
}

void b() {
   mutex_lock(&foo_mutex);
   __b_thr_unsafe();
   mutex_unlock(&foo_mutex);
}

static void __a_thr_unsafe() {
   if (foo<5) foo++;
   __b_thr_unsafe();
   foo--;
}

static void __b_thr_unsafe() {
   if (foo>6) foo=(foo>>1);
}

This way a call to a() or b() will acquire the lock once for that
thread, perform all actions and then release the lock. The mutex is
handled properly.

Can you restructure the code so that you don't need multiple locks?


Thank you for very long and detailed reply with examples :)

I need lock for hardware access. Single I2C-adapter have two I2C-clients 
that have same I2C-address in same bus. There is gate (demod I2C-gate) 
logic that is used to select desired tuner. See that:

http://palosaari.fi/linux/v4l-dvb/controlling_tuner_af9015_dual_demod.txt

You can never know surely how tuner drivers calls to open or close gate, 
very commonly there is situations where multiple close or open happens. 
That's why lock/unlock is problematic.


.i2c_gate_ctrl() is demod driver callback (struct dvb_frontend_ops) 
which controls gate that gate. That callback is always called from tuner 
driver when gate is needed to open or close.


regards
Antti

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

Re: [PATCH v4] V4L: dynamically allocate video_device nodes in subdevices

Hi Sylwester

On Tue, 13 Sep 2011, Sylwester Nawrocki wrote:

> Hi Guennadi,
> 
> On 09/13/2011 04:48 PM, Guennadi Liakhovetski wrote:
> > Currently only very few drivers actually use video_device nodes, embedded
> > in struct v4l2_subdev. Allocate these nodes dynamically for those drivers
> > to save memory for the rest.
> > 
> > Signed-off-by: Guennadi Liakhovetski 
> 
> I have tested this patch with Samsung FIMC driver and with MC enabled
> sensor driver.
> After some hundreds of module load/unload I didn't observe anything unusual.
> The patch seem to be safe for device node enabled subdevs. You can stick my:
> 
> Tested-by: Sylwester Nawrocki 
> 
> if you feel so.

Thanks very much for testing! However, depending on your test scenario, 
you might still not notice a problem by just loading and unloading of 
modules. It would, however, be useful to execute just one test:

1. add one line v4l2-device.c:

diff --git a/drivers/media/video/v4l2-device.c 
b/drivers/media/video/v4l2-device.c
index a3b89f4..33226857 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -195,6 +195,7 @@ EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
 static void v4l2_device_release_subdev_node(struct video_device *vdev)
 {
struct v4l2_subdev *sd = video_get_drvdata(vdev);
+   dev_info(&vdev->dev, "%s()\n", __func__);
sd->devnode = NULL;
kfree(vdev);
 }

2. with this patch start and stop capture

3. check dmesg - v4l2_device_release_subdev_node() output should not be 
there yet

4. rmmod modules, then the output should be there

If you could test that - that would be great!

Thanks
Guennadi

> Thanks,
> Sylwester Nawrocki
> Samsung Poland R&D Center
> 
> > ---
> > 
> > v4:
> > 
> > 1. added "static" in v4l2_device_release_subdev_node() definition
> > 2. removed superfluous get_device() and put_device() (thanks to Laurent 
> > for pointing out)
> > 
> >  drivers/media/video/v4l2-device.c |   36 
> > +++-
> >  include/media/v4l2-subdev.h   |4 ++--
> >  2 files changed, 33 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/media/video/v4l2-device.c 
> > b/drivers/media/video/v4l2-device.c
> > index c72856c..8abf830 100644
> > --- a/drivers/media/video/v4l2-device.c
> > +++ b/drivers/media/video/v4l2-device.c
> > @@ -21,6 +21,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #if defined(CONFIG_SPI)
> >  #include 
> >  #endif
> > @@ -191,6 +192,13 @@ int v4l2_device_register_subdev(struct v4l2_device 
> > *v4l2_dev,
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
> >  
> > +static void v4l2_device_release_subdev_node(struct video_device *vdev)
> > +{
> > +   struct v4l2_subdev *sd = video_get_drvdata(vdev);
> > +   sd->devnode = NULL;
> > +   kfree(vdev);
> > +}
> > +
> >  int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
> >  {
> > struct video_device *vdev;
> > @@ -204,22 +212,40 @@ int v4l2_device_register_subdev_nodes(struct 
> > v4l2_device *v4l2_dev)
> > if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
> > continue;
> >  
> > -   vdev = &sd->devnode;
> > +   vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> > +   if (!vdev) {
> > +   err = -ENOMEM;
> > +   goto clean_up;
> > +   }
> > +
> > +   video_set_drvdata(vdev, sd);
> > strlcpy(vdev->name, sd->name, sizeof(vdev->name));
> > vdev->v4l2_dev = v4l2_dev;
> > vdev->fops = &v4l2_subdev_fops;
> > -   vdev->release = video_device_release_empty;
> > +   vdev->release = v4l2_device_release_subdev_node;
> > vdev->ctrl_handler = sd->ctrl_handler;
> > err = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
> >   sd->owner);
> > -   if (err < 0)
> > -   return err;
> > +   if (err < 0) {
> > +   kfree(vdev);
> > +   goto clean_up;
> > +   }
> >  #if defined(CONFIG_MEDIA_CONTROLLER)
> > sd->entity.v4l.major = VIDEO_MAJOR;
> > sd->entity.v4l.minor = vdev->minor;
> >  #endif
> > +   sd->devnode = vdev;
> > }
> > return 0;
> > +
> > +clean_up:
> > +   list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
> > +   if (!sd->devnode)
> > +   break;
> > +   video_unregister_device(sd->devnode);
> > +   }
> > +
> > +   return err;
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_device_register_subdev_nodes);
> >  
> > @@ -245,7 +271,7 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev 
> > *sd)
> > if (v4l2_dev->mdev)
> > media_device_unregister_entity(&sd->entity);
> >  #endif
> > -   video_unregister_device(&sd->devnode);
> > +   video_unregister_device(sd->devnode);
> > module_put(sd->owner);
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
> > diff

Re: [PATCH 4/4] v4l2: add blackfin capture bridge driver

2011-09-13 Thread Sylwester Nawrocki

Hi Scott,


On 09/13/2011 08:34 PM, Scott Jiang wrote:
> this is a v4l2 bridge driver for Blackfin video capture device,
> support ppi interface
> 
> Signed-off-by: Scott Jiang
> ---
>   drivers/media/video/Makefile|2 +
>   drivers/media/video/blackfin/Kconfig|   10 +
>   drivers/media/video/blackfin/Makefile   |2 +
>   drivers/media/video/blackfin/bfin_capture.c | 1099 
> +++
>   drivers/media/video/blackfin/ppi.c  |  204 +
>   5 files changed, 1317 insertions(+), 0 deletions(-)
>   create mode 100644 drivers/media/video/blackfin/Kconfig
>   create mode 100644 drivers/media/video/blackfin/Makefile
>   create mode 100644 drivers/media/video/blackfin/bfin_capture.c
>   create mode 100644 drivers/media/video/blackfin/ppi.c
> 
...
> diff --git a/drivers/media/video/blackfin/bfin_capture.c 
> b/drivers/media/video/blackfin/bfin_capture.c
> new file mode 100644
> index 000..24f89f2
> --- /dev/null
> +++ b/drivers/media/video/blackfin/bfin_capture.c
> @@ -0,0 +1,1099 @@
> +/*
> + * Analog Devices video capture driver
> + *
> + * Copyright (c) 2011 Analog Devices Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include
> +
> +#define CAPTURE_DRV_NAME"bfin_capture"
> +#define BCAP_MIN_NUM_BUF2
> +
> +struct bcap_format {
> + u8 *desc;
> + u32 pixelformat;
> + enum v4l2_mbus_pixelcode mbus_code;
> + int bpp; /* bytes per pixel */
> +};
> +
> +struct bcap_buffer {
> + struct vb2_buffer vb;
> + struct list_head list;
> +};
> +
> +struct bcap_device {
> + /* capture device instance */
> + struct v4l2_device v4l2_dev;
> + /* device node data */
> + struct video_device *video_dev;
> + /* sub device instance */
> + struct v4l2_subdev *sd;
> + /* caputre config */

typo: caputre -> capture

> + struct bfin_capture_config *cfg;
> + /* ppi interface */
> + struct ppi_if *ppi;
> + /* current input */
> + unsigned int cur_input;
> + /* current selected standard */
> + v4l2_std_id std;
> + /* used to store pixel format */
> + struct v4l2_pix_format fmt;
> + /* bytes per pixel*/
> + int bpp;
> + /* pointing to current video buffer */
> + struct bcap_buffer *cur_frm;
> + /* pointing to next video buffer */
> + struct bcap_buffer *next_frm;
> + /* buffer queue used in videobuf2 */
> + struct vb2_queue buffer_queue;
> + /* allocator-specific contexts for each plane */
> + struct vb2_alloc_ctx *alloc_ctx;
> + /* queue of filled frames */
> + struct list_head dma_queue;
> + /* used in videobuf2 callback */
> + spinlock_t lock;
> + /* used to access capture device */
> + struct mutex mutex;
> + /* used to wait ppi to complete one transfer */
> + struct completion comp;
> + /* number of users performing IO */
> + u32 io_usrs;
> + /* number of open instances of the device */
> + u32 usrs;
> + /* indicate whether streaming has started */
> + u8 started;
> +};
> +
> +struct bcap_fh {
> + struct v4l2_fh fh;
> + struct bcap_device *bcap_dev;
> + /* indicates whether this file handle is doing IO */
> + u8 io_allowed;
> +};
> +
> +static const struct bcap_format bcap_formats[] = {
> + {
> + .desc= "YCbCr 4:2:2 Interleaved UYVY",
> + .pixelformat = V4L2_PIX_FMT_UYVY,
> + .mbus_code   = V4L2_MBUS_FMT_UYVY8_2X8,
> + .bpp = 2,
> + },
> + {
> + .desc= "YCbCr 4:2:2 Interleaved YUYV",
> + .pixelformat = V4L2_PIX_FMT_YUYV,
> + .mbus_code   = V4L2_MBUS_FMT_YUYV8_2X8,
> + .bpp = 2,
> + },
> + {
> + .desc= "RGB 565",
> + .pixelformat = V4L2_PIX_FMT_RGB565,
> + .mbus_code   = V4L2_MBUS_FMT_RGB565_2X8_LE,
> + .bpp = 2,
> + },
> + {
> + .desc= "RGB 444",
> + .pixelformat = V4L2_PIX_FMT_RGB44

Re: recursive locking problem

2011-09-13 Thread Steve Kerrison

At the risk of sounding silly, why do we rely on i2c gating so much? The
whole point of i2c is that you can sit a bunch of devices on the same
pair of wires and talk to one at a time.

Why not just open up the gates and be done with it, except for
situations where the i2c chain foolishly has two devices that have the
same address because somebody didn't net the address configuration pins
correctly, or it's a really big system with more devices on it that a
particular chip family has sub-addresses?

>From a signal-driving point of view, the gate circuitry is surely a
sufficient buffer. I guess the only two things I can think of as real
reasons are to 1) reduce the chance of misbehaving devices from causing
problems and 2) to save power by not sending clocks and data where we
know they aren't needed.

Can any one shed some light on this? I appreciate it's not a linux or
indeed linux-media specific issue as the hardware itself is designed
this way.

Getting back to the subject, Antti's af9015 example demonstrates
precisely when gating is needed. But I have to ask, does the MXL5003
really only support one address; can it not be reconfigured? it's a bit
late to ask that question once the PCB is fabbed, I admit.

Would it make any sense to haul the gate locking (or some wrapper at
least) up into the uC for that particular configuration?

E.g:

Tuner driver wants to i2c write tuner 0
Calls i2c_gate_ctrl(open)
uC checks if a gate is already open, if it is, and is the same gate as
is already open, just carry on. If not, wait on a lock, then set which
gate is open and proceed to the 'real' gate_ctrl op.
Similar path for i2c_gate_ctrl(close), as relaxed as it needs to be to
cope with how different tuner drivers work.

You could wrap the checking of which gate is open in a lock for
atomicity, and only wait on a device lock where necessary. This way you
can also check whether you're trying to change the state of a gate or
not and early-out (unless calling gate_ctrl(open) more than once does
something /different/ to calling it just once, in which case we're
doomed and this was nothing more than naive rambling).

Cheers,
-- 
Steve Kerrison MEng Hons.
http://www.stevekerrison.com/ 

On Tue, 2011-09-13 at 23:59 +0300, Antti Palosaari wrote:
> On 09/09/2011 01:45 PM, David Waring wrote:
> > On 08/09/11 17:34, Antti Palosaari wrote:
> >> [snip]
> >>
> >> Is there any lock can do recursive locking but unlock frees all locks?
> >>
> >> Like that:
> >> gate_open
> >> +gate_open
> >> +gate_close
> >> == lock is free
> >>
> >> AFAIK mutex can do only simple lock() + unlock(). Semaphore can do
> >> recursive locking, like lock() + lock() + unlock() + unlock(). But how I
> >> can do lock() + lock() + unlock() == free.
> >>
> > Antti,
> >
> > It's a very bad idea to try and use a mutex like that. The number of
> > locks and unlocks must be balanced otherwise you risk accessing
> > variables without a lock.
> >
> > Consider:
> >
> > static struct mutex foo_mutex;
> > static int foo=3;
> >
> > void a() {
> >mutex_lock(&foo_mutex);
> >if (foo<5) foo++;
> >b();
> >foo--; /*<<<  still need lock here */
> >mutex_unlock(&foo_mutex);
> > }
> >
> > void b() {
> >mutex_lock(&foo_mutex);
> >if (foo>6) foo=(foo>>1);
> >mutex_unlock(&foo_mutex);
> > }
> >
> > Note: this assumes mutex_lock will allow the same thread get multiple
> > locks as you would like (which it doesn't).
> >
> > As pointed out in the code, when a() is called, you still need the lock
> > for accesses to foo after the call to b() that also requires the lock.
> > If we used the locks in the way you propose then foo would be accessed
> > without a lock.
> >
> > To code properly for cases like these I usually use a wrapper functions
> > to acquire the lock and call a thread unsafe version (i.e. doesn't use
> > locks) of the function that only uses other thread unsafe functions. e.g.
> >
> > void a() {
> >mutex_lock(&foo_mutex);
> >__a_thr_unsafe();
> >mutex_unlock(&foo_mutex);
> > }
> >
> > void b() {
> >mutex_lock(&foo_mutex);
> >__b_thr_unsafe();
> >mutex_unlock(&foo_mutex);
> > }
> >
> > static void __a_thr_unsafe() {
> >if (foo<5) foo++;
> >__b_thr_unsafe();
> >foo--;
> > }
> >
> > static void __b_thr_unsafe() {
> >if (foo>6) foo=(foo>>1);
> > }
> >
> > This way a call to a() or b() will acquire the lock once for that
> > thread, perform all actions and then release the lock. The mutex is
> > handled properly.
> >
> > Can you restructure the code so that you don't need multiple locks?
> 
> Thank you for very long and detailed reply with examples :)
> 
> I need lock for hardware access. Single I2C-adapter have two I2C-clients 
> that have same I2C-address in same bus. There is gate (demod I2C-gate) 
> logic that is used to select desired tuner. See that:
> http://palosaari.fi/linux/v4l-dvb/controlling_tuner_af9015_dual_demod.txt
> 
> You can never know surely how tuner drivers calls to open or close gate,

Re: recursive locking problem

2011-09-13 Thread Steven Toth

> Can any one shed some light on this? I appreciate it's not a linux or
> indeed linux-media specific issue as the hardware itself is designed
> this way.

i2c gates exist to isolate the downstream components from any spurious
RF noise generated by noisy components on the i2c bus. You don't want
to couple demod noise into the tuner by default.

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

Re: recursive locking problem

2011-09-13 Thread Devin Heitmueller

On Tue, Sep 13, 2011 at 5:34 PM, Steve Kerrison  wrote:
> At the risk of sounding silly, why do we rely on i2c gating so much? The
> whole point of i2c is that you can sit a bunch of devices on the same
> pair of wires and talk to one at a time.

Steve,

There are essentially two issues here.  To address the general
question, many tuner chips require an i2c gate because their onboard
i2c controller is implemented using interrupts, and servicing the
interrupts to even check if the traffic is intended for the tuner can
interfere with the core tuning function.  In other words, the cost of
the chip "watching for traffic" can adversely effect tuning quality.
As a result, most hardware designs are such that the demodulator gates
the i2c traffic such that the tuner only *ever* sees traffic intended
for it.

The second issue is that within the LinuxTV drivers there is
inconsistency regarding whether the i2c gate is opened/closed by the
tuner driver or whether it's done by the demod.  Some drivers have the
demod driver open the gate, issue the tuning request, and then close
the gate, while in other drivers the tuner driver opens/closes the
gate whenever there are register reads/writes to the tuner.  It's all
about the granularity of implementation (the demod approach only
involves one open/close but it's for potentially a longer period of
time, versus the tuner approach which opens/closes the gate repeatedly
as needed, which means more open/closes but the gate is open for the
bare minimum of time required).

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

Re: recursive locking problem

2011-09-13 Thread Devin Heitmueller

On Tue, Sep 13, 2011 at 5:58 PM, Steven Toth  wrote:
>> Can any one shed some light on this? I appreciate it's not a linux or
>> indeed linux-media specific issue as the hardware itself is designed
>> this way.
>
> i2c gates exist to isolate the downstream components from any spurious
> RF noise generated by noisy components on the i2c bus. You don't want
> to couple demod noise into the tuner by default.

Steve (Kerrison),

I would take Steven Toth's explanation as more authoritative than mine
any day of the week.  It's possible that I may have been misinformed
regarding the rationale for why the gate is required.

Regards,

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

Re: recursive locking problem

2011-09-13 Thread Steven Toth

> I would take Steven Toth's explanation as more authoritative than mine
> any day of the week.  It's possible that I may have been misinformed
> regarding the rationale for why the gate is required.

In general, complete bus isolation lowers noise levels, whether
through RF coupling or needless interrupt servicing related. Either
rationale is valid and varies between designs.

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

Compro U680F USB stick receiver for FM/DAB/DAB+/DVB-T.

2011-09-13 Thread Erik Dam

I recently came across this interesting little USB device which seems 
the so far best bet on covering all the relevant standards for these 
parts of the woods (although it still misses out on DVB-T2 support). 
However, I have so far been unable to determine whether any level of 
linux support exists (and therefore whether it's useful or not). Anybody 
know if drivers exist?


BR

Erik.
--
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: Compro U680F USB stick receiver for FM/DAB/DAB+/DVB-T.


On 09/14/2011 01:31 AM, Erik Dam wrote:

I recently came across this interesting little USB device which seems
the so far best bet on covering all the relevant standards for these
parts of the woods (although it still misses out on DVB-T2 support).
However, I have so far been unable to determine whether any level of
linux support exists (and therefore whether it's useful or not). Anybody
know if drivers exist?


According to properties it must be Realtek RTL2832U based. It is only 
chip can do FM/DAB+/DVB-T.


There is no community supported drivers but vendor drivers exits. Try those.

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

Re: recursive locking problem


On 09/09/2011 02:46 PM, Daniel Glöckner wrote:

On Thu, Sep 08, 2011 at 07:34:32PM +0300, Antti Palosaari wrote:

I am working with AF9015 I2C-adapter lock. I need lock I2C-bus since
there is two tuners having same I2C address on same bus, demod I2C
gate is used to select correct tuner.


Would it be possible to use the i2c-mux framework to handle this?
Each tuner will then have its own i2c bus.


Interesting idea, but it didn't worked. It deadlocks. I think it locks 
since I2C-mux is controlled by I2C "switch" in same I2C bus, not GPIO or 
some other HW.


* tuner does I2C xfer on I2C-mux adapter
* I2C-mux adapter calls demod .i2c_gate_ctrl()
* demod does register access using I2C
* DEADLOCK

Maybe since tuner I2C xfer have already locked I2C-adater. Then demod 
access same adapter and it is locked.


But nice I2C mux anyhow.

@David Daney, see that pic in order to get understanding what kind of 
problem I am working;

http://palosaari.fi/linux/v4l-dv/controlling_tuner_af9015_dual_demod.txt


regards
Antti


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

tm6010 based tv tuner uses the correct firmware? or the audio part problem

2011-09-13 Thread hendry zhang

hi,
My tm6010 based tv tuner is WINTV-HVR-900H, Model number is 66009, and
USB id is 2040:6600. According to the guide, I use the firmware
xc3028L-v36.fw from http://www.stefanringel.de/pub/xc3028L-v36.fw.
I'm sure this firmware is for Xceive products and is also suitable for
this product?
I'm able to get video but without audio. Is it the problem of the
firmware or the audio part of the module tm6000-alsa code ?

BTW, I have hcw66xxx.sys and hcwCP.ax from tuner installation cd-rom,
and how can I extract the firmware from the .sys file ?

--
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: [uclinux-dist-devel] [PATCH 2/4] v4l2: add adv7183 decoder driver

2011-09-13 Thread Mike Frysinger

On Tue, Sep 13, 2011 at 14:34, Scott Jiang wrote:
> --- /dev/null
> +++ b/drivers/media/video/adv7183_regs.h
>
> +#define        ADV7183_IN_CTRL            0x00 /* Input control */

should be a space after the #define, not a tab

> --- /dev/null
> +++ b/include/media/adv7183.h
>
> +#define        ADV7183_16BIT_OUT   1

same here
-mike
--
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: [uclinux-dist-devel] [PATCH 4/4] v4l2: add blackfin capture bridge driver

2011-09-13 Thread Mike Frysinger

On Tue, Sep 13, 2011 at 14:34, Scott Jiang wrote:
> --- /dev/null
> +++ b/drivers/media/video/blackfin/Kconfig
> @@ -0,0 +1,10 @@
> +config VIDEO_BLACKFIN_CAPTURE
> +       tristate "Blackfin Video Capture Driver"
> +       depends on VIDEO_DEV && BLACKFIN
> +       select VIDEOBUF2_DMA_CONTIG

since the code needs i2c, this needs to list I2C under depends

> --- /dev/null
> +++ b/drivers/media/video/blackfin/bfin_capture.c
>
> +#include 
> +#include 
> +#include 

i think at least these three are unused and should get punted

> +static int __devinit bcap_probe(struct platform_device *pdev)
> +{
> +       struct bcap_device *bcap_dev;
> +       struct video_device *vfd;
> +       struct i2c_adapter *i2c_adap;

you need to include linux/i2c.h for this

> +static struct platform_driver bcap_driver = {
> +       .driver = {
> +               .name   = CAPTURE_DRV_NAME,
> +               .owner = THIS_MODULE,
> +       },
> +       .probe = bcap_probe,
> +       .remove = __devexit_p(bcap_remove),
> +};

no suspend/resume ? :)

> +MODULE_DESCRIPTION("Analog Devices video capture driver");

should mention the device part name in the desc

> --- /dev/null
> +++ b/drivers/media/video/blackfin/ppi.c
>
> +struct ppi_if *create_ppi_instance(const struct ppi_info *info)
> +void delete_ppi_instance(struct ppi_if *ppi)

should be ppi_{create,delete}_instance to match existing ppi_xxx style
-mike
--
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: [uclinux-dist-devel] [PATCH 3/4] v4l2: add vs6624 sensor driver

2011-09-13 Thread Mike Frysinger

On Tue, Sep 13, 2011 at 14:34, Scott Jiang wrote:
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
>
> +obj-$(CONFIG_VIDEO_VS6624)  += vs6624.o
>  obj-$(CONFIG_VIDEO_VPX3220) += vpx3220.o

should be after vpx, not before ?

> --- /dev/null
> +++ b/drivers/media/video/vs6624.c
>
> +#include 

run these patches through checkpatch.pl ?  this should be linux/gpio.h ...

> +static const u16 vs6624_p1[] = {
> +static const u16 vs6624_p2[] = {

add comments as to what these are for ?

> +static inline int vs6624_read(struct v4l2_subdev *sd, u16 index)
> +static inline int vs6624_write(struct v4l2_subdev *sd, u16 index,
> +                               u8 value)

should these be inline ?  they're a little "fat" ... better to let the
compiler choose

> +static int vs6624_writeregs(struct v4l2_subdev *sd, const u16 *regs)
> +{
> +       u16 reg, data;
> +
> +       while (*regs != 0x00) {
> +               reg = *regs++;
> +               data = *regs++;
> +
> +               vs6624_write(sd, reg, (u8)data);

what's the point of declaring data as u16 if the top 8 bits are never used ?

> +static int vs6624_g_chip_ident(struct v4l2_subdev *sd,
> +               struct v4l2_dbg_chip_ident *chip)
> +{
> +       int rev;
> +       struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +       rev = vs6624_read(sd, VS6624_FW_VSN_MAJOR) << 8
> +               | vs6624_read(sd, VS6624_FW_VSN_MINOR);

i'm a little surprised the compiler didnt warn about this.  usually
bit shifts + bitwise operators want paren to keep things clear.

> +#ifdef CONFIG_VIDEO_ADV_DEBUG

just use DEBUG ?

> +       v4l_info(client, "chip found @ 0x%02x (%s)\n",
> +                       client->addr << 1, client->adapter->name);

is that "<< 1" correct ?  i dont think so ...
-mike
--
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

Asking advice for Camera/ISP driver framework design

2011-09-13 Thread Cliff Cai

Dear guys,

I'm currently working on a camera/ISP Linux driver project.Of course,I
want it to be a V4L2 driver,but I got a problem about how to design
the driver framework.
let me introduce the background of this ISP(Image signal processor) a
little bit.
1.The ISP has two output paths,first one called main path which is
used to transfer image data for taking picture and recording,the other
one called preview path which is used to transfer image data for
previewing.
2.the two paths have the same image data input from sensor,but their
outputs are different,the output of main path is high quality and
larger image,while the output of preview path is smaller image.
3.the two output paths have independent DMA engines used to move image
data to system memory.

The problem is currently, the V4L2 framework seems only support one
buffer queue,and in my case,obviously,two buffer queues are required.
Any idea/advice for implementing such kind of V4L2 driver? or any
other better solutions?

Thanks a lot!
Cliff
--
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/4] v4l2: add blackfin capture bridge driver