Re: [PATCH v2 2/6] OMAP1: Add support for SoC camera interface

2010-09-21 Thread Guennadi Liakhovetski
That's up to the platform maintainer to review / apply this one, but if 
you like, a couple of nit-picks from me:

On Sat, 11 Sep 2010, Janusz Krzysztofik wrote:

> This patch adds support for SoC camera interface to OMAP1 devices.
> 
> Created and tested against linux-2.6.36-rc3 on Amstrad Delta.
> 
> For successfull compilation, requires a header file provided by PATCH 1/6 
> from 
> this series, "SoC Camera: add driver for OMAP1 camera interface".
> 
> Signed-off-by: Janusz Krzysztofik 
> ---
> v1->v2 changes:
> - no functional changes,
> - refreshed against linux-2.6.36-rc3
> 
> 
>  arch/arm/mach-omap1/devices.c |   43 
> ++
>  arch/arm/mach-omap1/include/mach/camera.h |8 +
>  2 files changed, 51 insertions(+)
> 
> 
> diff -upr linux-2.6.36-rc3.orig/arch/arm/mach-omap1/devices.c 
> linux-2.6.36-rc3/arch/arm/mach-omap1/devices.c
> --- linux-2.6.36-rc3.orig/arch/arm/mach-omap1/devices.c   2010-09-03 
> 22:29:00.0 +0200
> +++ linux-2.6.36-rc3/arch/arm/mach-omap1/devices.c2010-09-09 
> 18:42:30.0 +0200
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -25,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

You might want to try to group headers according to their location, i.e., 
put  higher - together with , also it helps to have 
headers alphabetically ordered.

>  
>  /*-*/
>  
> @@ -191,6 +193,47 @@ static inline void omap_init_spi100k(voi
>  }
>  #endif
>  
> +
> +#define OMAP1_CAMERA_BASE0xfffb6800
> +
> +static struct resource omap1_camera_resources[] = {
> + [0] = {
> + .start  = OMAP1_CAMERA_BASE,
> + .end= OMAP1_CAMERA_BASE + OMAP1_CAMERA_IOSIZE - 1,
> + .flags  = IORESOURCE_MEM,
> + },
> + [1] = {
> + .start  = INT_CAMERA,
> + .flags  = IORESOURCE_IRQ,
> + },
> +};
> +
> +static u64 omap1_camera_dma_mask = DMA_BIT_MASK(32);
> +
> +static struct platform_device omap1_camera_device = {
> + .name   = "omap1-camera",
> + .id = 0, /* This is used to put cameras on this interface */
> + .dev= {
> + .dma_mask   = &omap1_camera_dma_mask,
> + .coherent_dma_mask  = DMA_BIT_MASK(32),
> + },
> + .num_resources  = ARRAY_SIZE(omap1_camera_resources),
> + .resource   = omap1_camera_resources,
> +};
> +
> +void __init omap1_set_camera_info(struct omap1_cam_platform_data *info)
> +{
> + struct platform_device *dev = &omap1_camera_device;
> + int ret;
> +
> + dev->dev.platform_data = info;
> +
> + ret = platform_device_register(dev);
> + if (ret)
> + dev_err(&dev->dev, "unable to register device: %d\n", ret);
> +}
> +
> +
>  /*-*/
>  
>  static inline void omap_init_sti(void) {}
> diff -upr linux-2.6.36-rc3.orig/arch/arm/mach-omap1/include/mach/camera.h 
> linux-2.6.36-rc3/arch/arm/mach-omap1/include/mach/camera.h
> --- linux-2.6.36-rc3.orig/arch/arm/mach-omap1/include/mach/camera.h   
> 2010-09-03 
> 22:34:03.0 +0200
> +++ linux-2.6.36-rc3/arch/arm/mach-omap1/include/mach/camera.h
> 2010-09-09 
> 18:42:30.0 +0200
> @@ -0,0 +1,8 @@
> +#ifndef __ASM_ARCH_CAMERA_H_
> +#define __ASM_ARCH_CAMERA_H_
> +
> +#include 
> +
> +extern void omap1_set_camera_info(struct omap1_cam_platform_data *);

function declarations don't need an "extern" - something I've also been 
doing needlessly for a few years...

> +
> +#endif /* __ASM_ARCH_CAMERA_H_ */

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: [PATCH v2 1/6] SoC Camera: add driver for OMAP1 camera interface

2010-09-21 Thread Guennadi Liakhovetski
On Wed, 22 Sep 2010, hermann pitton wrote:

> Am Mittwoch, den 22.09.2010, 01:23 +0200 schrieb Guennadi Liakhovetski:
> > On Sat, 11 Sep 2010, Janusz Krzysztofik wrote:
> > 
> > > This is a V4L2 driver for TI OMAP1 SoC camera interface.
> 
> [snip]
> 
> > > +
> > > + } else {
> > > + dev_warn(dev, "%s: unhandled camera interrupt, status == "
> > > + "0x%0x\n", __func__, it_status);
> > 
> > Please, don't split strings
> 
> sorry for any OT interference.
> 
> But, are there any new rules out?
> 
> Maybe I missed them.
> 
> Either way, the above was forced during the last three years.
> 
> Not at all ?

No. Splitting print strings has always been discouraged, because it makes 
tracking back kernel logs difficult. The reason for this has been the 80 
character line length limit, which has now been effectively lifted. I'm 
sure you can find enough links on the internet for any of these topics.

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: [PATCH v2 1/6] SoC Camera: add driver for OMAP1 camera interface

2010-09-21 Thread hermann pitton
Hi,

Am Mittwoch, den 22.09.2010, 01:23 +0200 schrieb Guennadi Liakhovetski:
> On Sat, 11 Sep 2010, Janusz Krzysztofik wrote:
> 
> > This is a V4L2 driver for TI OMAP1 SoC camera interface.

[snip]

> > +
> > +   } else {
> > +   dev_warn(dev, "%s: unhandled camera interrupt, status == "
> > +   "0x%0x\n", __func__, it_status);
> 
> Please, don't split strings

sorry for any OT interference.

But, are there any new rules out?

Maybe I missed them.

Either way, the above was forced during the last three years.

Not at all ?

Cheers,
Hermann



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


Re: [linux-dvb] Asus MyCinema P7131 Dual support

2010-09-21 Thread hermann pitton
Hi Dejan,

Am Dienstag, den 21.09.2010, 10:07 +0200 schrieb Dejan Rodiger:
> Hi,
> 
> I am using Ubuntu linux 10.10 with the latest kernel 2.6.35-22-generic
> on x86_64. I have installed nonfree firmware which should support this
> card, but to be sure, can somebody confirm that my TV card is
> supported in Analog or DVB mode?
> 
> sudo lspci -vnn
> 01:06.0 Multimedia controller [0480]: Philips Semiconductors
> SAA7131/SAA7133/SAA7135 Video Broadcast Decoder [1131:7133] (rev d1)
> Subsystem: ASUSTeK Computer Inc. My Cinema-P7131 Hybrid
> [1043:4876]
> Flags: bus master, medium devsel, latency 32, IRQ 18
> Memory at fdeff000 (32-bit, non-prefetchable) [size=2K]
> Capabilities: [40] Power Management version 2
> Kernel driver in use: saa7134
> Kernel modules: saa7134
> 
> It says Hybrid, but I put the following in
> the /etc/modprobe.d/saa7134.conf
> options saa7134 card=78 tuner=54
> 
> 
> Thanks
> -- 
> Dejan Rodiger
> S: callto://drodiger

don't have time to follow this closely anymore.

But forcing it to card=78 is plain wrong. It has an early additional LNA
in confirmed config = 2 status.

Your card should be auto detected and previously always was, based on
what we have in saa7134-cards.c and further for it. (saa7134-dvb and
related tuner/demod stuff)

}, {
.vendor   = PCI_VENDOR_ID_PHILIPS,
.device   = PCI_DEVICE_ID_PHILIPS_SAA7133,
.subvendor= 0x1043,
.subdevice= 0x4876,
.driver_data  = SAA7134_BOARD_ASUSTeK_P7131_HYBRID_LNA,
},{

I remember for sure, that this card was fully functional for all use
cases and it was not easy to get it there. I don't have it.

Please provide the "dmesg" for failing auto detection without forcing
some card = number as a starting point.

I for sure want to see this board fully functional again.

Cheers,
Hermann








--
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 v2] tm6000+audio

2010-09-21 Thread Mauro Carvalho Chehab
Em 20-09-2010 17:07, Dmitri Belimov escreveu:
> Hi 
> 
> I rework my last patch for audio and now audio works well. This patch can be 
> submited to GIT tree
> Quality of audio now is good for SECAM-DK. For other standard I set some 
> value from datasheet need some tests.
> 
> 1. Fix pcm buffer overflow
> 2. Rework pcm buffer fill method
> 3. Swap bytes in audio stream
> 4. Change some registers value for TM6010
> 5. Change pcm buffer size
> --- a/drivers/staging/tm6000/tm6000-stds.c
> +++ b/drivers/staging/tm6000/tm6000-stds.c
> @@ -96,6 +96,7 @@ static struct tm6000_std_tv_settings tv_stds[] = {
>  
>   {TM6010_REQ07_R04_LUMA_HAGC_CONTROL, 0xdc},
>   {TM6010_REQ07_R0D_CHROMA_KILL_LEVEL, 0x07},
> + {TM6010_REQ08_R05_A_STANDARD_MOD, 0x21}, /* FIXME */

This didn't seem to work for PAL-M. Probably, the right value for it is 0x22, to
follow NTSC/M, since both uses the same audio standard.

On some tests, I was able to receive some audio there, at the proper rate, with 
a
tm6010-based device. It died when I tried to change the channel, so I didn't 
rear
yet the real audio, but I suspect it will work on my next tests.

Yet, is being hard to test, as the driver has a some spinlock logic broken.
I'm enclosing the logs.

I was able to test only when using a monitor on the same machine. All trials of
using vnc and X11 export ended by not receiving any audio and hanging the 
machine.

I suspect that we need to fix the spinlock issue, in order to better test it.

Cheers,
Mauro.

[  564.483502] [drm] nouveau :0f:00.0: Allocating FIFO number 1
[  564.492341] [drm] nouveau :0f:00.0: nouveau_channel_alloc: initialised F1
[  579.380503] BUG: spinlock wrong CPU on CPU#0, pulseaudio/4760
[  579.386244]  lock: 880119bde7e8, .magic: dead4ead, .owner: pulseaudio/471
[  579.394738] Pid: 4760, comm: pulseaudio Tainted: G C  2.6.35+ #4
[  579.401415] Call Trace:
[  579.403856]  [] spin_bug+0x9c/0xa3
[  579.408803]  [] do_raw_spin_unlock+0xe5/0xfc
[  579.414617]  [] _raw_spin_unlock+0x2b/0x30
[  579.420256]  [] snd_tm6000_card_trigger+0xb9/0xc7 [tm6000_]
[  579.427719]  [] snd_pcm_do_start+0x2c/0x2e [snd_pcm]
[  579.434228]  [] snd_pcm_action_single+0x33/0x6a [snd_pcm]
[  579.441166]  [] ? _raw_spin_lock+0x39/0x40
[  579.446808]  [] ? snd_pcm_action_lock_irq+0x7d/0xb1 [snd_p]
[  579.454092]  [] snd_pcm_action_lock_irq+0x8b/0xb1 [snd_pcm]
[  579.461205]  [] snd_pcm_common_ioctl1+0x3ec/0xb08 [snd_pcm]
[  579.468316]  [] ? inode_has_perm+0xab/0xcf
[  579.473958]  [] snd_pcm_capture_ioctl1+0x208/0x225 [snd_pc]
[  579.481160]  [] ? __lock_acquire+0x201/0x424
[  579.486974]  [] snd_pcm_capture_ioctl+0x2f/0x33 [snd_pcm]
[  579.493910]  [] vfs_ioctl+0x32/0xa6
[  579.498945]  [] do_vfs_ioctl+0x497/0x4d0
[  579.504414]  [] sys_ioctl+0x5c/0x9c
[  579.509450]  [] system_call_fastpath+0x16/0x1b

Message from sysl...@nehalem at Sep 21 20:06:31 ...
 kernel:[  579.380503] BUG: spinlock wrong CPU on CPU#0, pulseaudio/4760

Message from sysl...@nehalem at Sep 21 20:06:31 ...
 kernel:[  579.386244]  lock: 880119bde7e8, .magic: dead4ead, .owner: pulse1
[  745.147642] fuse init (API version 7.14)
[ 1170.332614] tm6000: open called (dev=video0)
[ 1171.028670] xc2028 3-0061: Loading firmware for type=BASE (1), id 00.
[ 1233.289714] xc2028 3-0061: Loading firmware for type=(0), id b70.
[ 1234.345782] SCODE (2000), id b700:
[ 1234.350586] xc2028 3-0061: Loading SCODE for type=MONO SCODE HAS_IF_4320 (60.
[ 1235.495700] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.541628] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.581902] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.616388] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.645121] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.668112] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.685352] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.697594] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.720580] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.766553] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.806788] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.841268] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.870005] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.892991] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.910230] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.922474] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.945465] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1235.991441] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1236.031667] tm6000 tm6000_irq_callback :urb resubmit failed (error=-1)
[ 1236.066150] tm6000 tm6000_irq_callback :urb resubmi

Re: Remaining BKL users, what to do

2010-09-21 Thread Arnd Bergmann
On Saturday 18 September 2010 01:21:41 Petr Vandrovec wrote:
> 
> I'll try to come up with something for ncpfs.

Ok, good.

> Trivial lock replacement will open deadlock possibility when
> someone reads to page which is also mmaped from the same 
> filesystem (like grep likes to do). BKL with its automated
> release on sleep helped (or papered over) a lot here.

Right, I was more or less expecting something like this.
So I guess this is some AB-BA deadlock with another mutex
or a call to flush_scheduled_work that is currently done
under the BKL?

There is still the possibility of just working around those
by adding explicit mutex_unlock() calls around those, which
is what I initially did in the tty subsystem. The better
long-term approach would obviously be to understand all of
the data structures that actually need locking and only
lock the actual accesses, but that may be more work than
you are willing to spend on it.

Arnd
--
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: Webcam Driver Bug while using two Multilaser Cameras simultaneously

2010-09-21 Thread Mauro Carvalho Chehab
Hi Daniel,

Em 21-09-2010 16:05, Daniel Moraes escreveu:
> I'm using Ubuntu 10.04 and I need to get images from two Multilaser
> Cameras simultaneously. First I tried to do that using OpenCV, but I
> got an error. So, I entered the OpenCV Mailing List to report that and
> I discovered that's a driver problem. To ensure that, I used mplayer
> to get imagens from the both cameras and I got the following error
> (again):
> 
>> v4l2: ioctl streamon failed: No space left on device

This is not a driver issue, but a limit imposed by USB specs. This
error code is returned by USB core when you try to use more than 100% of
the available bandwidth for an USB isoc stream.

The amount of bandwidth basically depends on what type of compression
is provided by your webcams.

You'll need to plug the other webcam on a separate USB bus.
> 
> The cameras model is Multilaser WC0440.
> 
> This problem only happens when I try to capture images from two
> IDENTICAL cameras simultaneously. I have three cameras here, two
> Multilaser Cameras and one HP Camera, from my laptop. I have no
> problem to capture images from my HP Camera and one of the Multilaser
> Cameras simultaneously, but when I try to capture from the both
> Multilaser Cameras simultaneously, i got that error.
> 
> I think that the problem may be something related to the generic
> driver. When I use the Multilaser Cameras, they use the same driver.
> That's not happen with the HP Camera, which uses another driver.

Probably, the HP Camera is connected internally into a different USB bus,
or provide a more compressed stream.

> Someone knows a solution for that?
> 
> Att,
>  Daniel Bastos Moraes
>  Graduando em Ciência da Computação - Universidade Tiradentes
>  +55 79 88455531
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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


[cron job] v4l-dvb daily build 2.6.26 and up: ERRORS

2010-09-21 Thread Hans Verkuil
This message is generated daily by a cron job that builds v4l-dvb for
the kernels and architectures in the list below.

Results of the daily build of v4l-dvb:

date:Tue Sep 21 19:00:11 CEST 2010
path:http://www.linuxtv.org/hg/v4l-dvb
changeset:   15164:1da5fed5c8b2
git master:   3e6dce76d99b328716b43929b9195adfee1de00c
git media-master: 991403c594f666a2ed46297c592c60c3b9f4e1e2
gcc version:  i686-linux-gcc (GCC) 4.4.3
host hardware:x86_64
host os:  2.6.32.5

linux-2.6.32.6-armv5: WARNINGS
linux-2.6.33-armv5: OK
linux-2.6.34-armv5: WARNINGS
linux-2.6.35.3-armv5: WARNINGS
linux-2.6.36-rc2-armv5: ERRORS
linux-2.6.32.6-armv5-davinci: WARNINGS
linux-2.6.33-armv5-davinci: WARNINGS
linux-2.6.34-armv5-davinci: WARNINGS
linux-2.6.35.3-armv5-davinci: WARNINGS
linux-2.6.36-rc2-armv5-davinci: ERRORS
linux-2.6.32.6-armv5-ixp: WARNINGS
linux-2.6.33-armv5-ixp: WARNINGS
linux-2.6.34-armv5-ixp: WARNINGS
linux-2.6.35.3-armv5-ixp: WARNINGS
linux-2.6.36-rc2-armv5-ixp: ERRORS
linux-2.6.32.6-armv5-omap2: WARNINGS
linux-2.6.33-armv5-omap2: WARNINGS
linux-2.6.34-armv5-omap2: WARNINGS
linux-2.6.35.3-armv5-omap2: WARNINGS
linux-2.6.36-rc2-armv5-omap2: ERRORS
linux-2.6.26.8-i686: WARNINGS
linux-2.6.27.44-i686: WARNINGS
linux-2.6.28.10-i686: WARNINGS
linux-2.6.29.1-i686: WARNINGS
linux-2.6.30.10-i686: WARNINGS
linux-2.6.31.12-i686: WARNINGS
linux-2.6.32.6-i686: WARNINGS
linux-2.6.33-i686: WARNINGS
linux-2.6.34-i686: WARNINGS
linux-2.6.35.3-i686: WARNINGS
linux-2.6.36-rc2-i686: ERRORS
linux-2.6.32.6-m32r: WARNINGS
linux-2.6.33-m32r: OK
linux-2.6.34-m32r: WARNINGS
linux-2.6.35.3-m32r: WARNINGS
linux-2.6.36-rc2-m32r: ERRORS
linux-2.6.32.6-mips: WARNINGS
linux-2.6.33-mips: WARNINGS
linux-2.6.34-mips: WARNINGS
linux-2.6.35.3-mips: WARNINGS
linux-2.6.36-rc2-mips: ERRORS
linux-2.6.32.6-powerpc64: WARNINGS
linux-2.6.33-powerpc64: WARNINGS
linux-2.6.34-powerpc64: WARNINGS
linux-2.6.35.3-powerpc64: WARNINGS
linux-2.6.36-rc2-powerpc64: ERRORS
linux-2.6.26.8-x86_64: WARNINGS
linux-2.6.27.44-x86_64: WARNINGS
linux-2.6.28.10-x86_64: WARNINGS
linux-2.6.29.1-x86_64: WARNINGS
linux-2.6.30.10-x86_64: WARNINGS
linux-2.6.31.12-x86_64: WARNINGS
linux-2.6.32.6-x86_64: WARNINGS
linux-2.6.33-x86_64: WARNINGS
linux-2.6.34-x86_64: WARNINGS
linux-2.6.35.3-x86_64: WARNINGS
linux-2.6.36-rc2-x86_64: ERRORS
linux-git-Module.symvers: ERRORS
linux-git-armv5: ERRORS
linux-git-armv5-davinci: ERRORS
linux-git-armv5-ixp: ERRORS
linux-git-armv5-omap2: ERRORS
linux-git-i686: ERRORS
linux-git-m32r: ERRORS
linux-git-mips: ERRORS
linux-git-powerpc64: ERRORS
linux-git-x86_64: ERRORS
spec: ERRORS
spec-git: ERRORS
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 failed to build, but the last compiled spec is here:

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


Re: [RFC PATCHES] First version of the V4L2 core locking patches

2010-09-21 Thread Hans Verkuil
On Tuesday, September 21, 2010 19:14:07 Mauro Carvalho Chehab wrote:
> Em Tue, 21 Sep 2010 15:50:13 +0200
> "Hans Verkuil"  escreveu:
> 
> > Hi Mauro,
> > 
> > > Em 20-09-2010 18:37, Hans Verkuil escreveu:
> ...
> > >   /*
> > >* Need to sleep in order to wait for videobufs to complete.
> > >* It is not a good idea to sleep while waiting for an event with the 
> > > dev
> > > lock hold,
> > >* as it will block any other access to the device. Just unlock it while
> > > waiting,
> > >* locking it again at the end.
> > >*/
> > >
> > >   is_vdev_locked = (q->vdev_lock && mutex_is_locked(q->vdev_lock)) ? true
> > > : false;
> > >   if (is_vdev_locked)
> > >   mutex_unlock(q->vdev_lock);
> > >   if (intr)
> > >   return wait_event_interruptible(vb->done, 
> > > is_state_active_or_queued(vb,
> > > q));
> > 
> > This obviously needs to save the return value and continue to make sure
> > the lock is taken again.
> 
> Yeah, it should be:
>  rc = wait_event_interruptible(vb->done, is_state_active_or_queued(vb, q));
> 
> and return rc at the end.
>  
> > >   else
> > >   wait_event(vb->done, is_state_active_or_queued(vb, q));
> > >   if (is_vdev_locked)
> > >   mutex_lock(q->vdev_lock);
> > >
> > >   return 0;
> > > }
> > 
> > Agreed. Thanks for reviewing this, it was the one patch that I knew I had
> > to look into more closely. I'll incorporate your changes.
> 
> Ok, thanks.

I added a patch with basically this code to my test tree. I will try to convert
and test a few more drivers, but that will probably be Friday. If the conversion
goes well then I plan to post a pull request by Friday or Saturday with somewhat
cleaned up patches.

I strongly suspect that for the 2-3 weeks after that I will not be able to
continue with this, so I hope others will take over from me.

Regards,

Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco
--
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


Webcam Driver Bug while using two Multilaser Cameras simultaneously

2010-09-21 Thread Daniel Moraes
I'm using Ubuntu 10.04 and I need to get images from two Multilaser
Cameras simultaneously. First I tried to do that using OpenCV, but I
got an error. So, I entered the OpenCV Mailing List to report that and
I discovered that's a driver problem. To ensure that, I used mplayer
to get imagens from the both cameras and I got the following error
(again):

> v4l2: ioctl streamon failed: No space left on device

The cameras model is Multilaser WC0440.

This problem only happens when I try to capture images from two
IDENTICAL cameras simultaneously. I have three cameras here, two
Multilaser Cameras and one HP Camera, from my laptop. I have no
problem to capture images from my HP Camera and one of the Multilaser
Cameras simultaneously, but when I try to capture from the both
Multilaser Cameras simultaneously, i got that error.

I think that the problem may be something related to the generic
driver. When I use the Multilaser Cameras, they use the same driver.
That's not happen with the HP Camera, which uses another driver.
Someone knows a solution for that?

Att,
 Daniel Bastos Moraes
 Graduando em Ciência da Computação - Universidade Tiradentes
 +55 79 88455531
--
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 v2] tm6000+audio

2010-09-21 Thread Mauro Carvalho Chehab
Em 21-09-2010 16:56, Dmitri Belimov escreveu:
> Hi Mauro
> 
>> Hi Dmitri,
>> Em 20-09-2010 17:07, Dmitri Belimov escreveu:
>>> Hi 
>>>
>>> I rework my last patch for audio and now audio works well. This
>>> patch can be submited to GIT tree Quality of audio now is good for
>>> SECAM-DK. For other standard I set some value from datasheet need
>>> some tests.
>>>
>>> 1. Fix pcm buffer overflow
>>> 2. Rework pcm buffer fill method
>>> 3. Swap bytes in audio stream
>>> 4. Change some registers value for TM6010
>>> 5. Change pcm buffer size
>>
>> One small compilation fix for your patch:
>>
>> diff --git a/drivers/staging/tm6000/tm6000-stds.c
>> b/drivers/staging/tm6000/tm6000-stds.c index 6bf4a73..fe22f42 100644
>> --- a/drivers/staging/tm6000/tm6000-stds.c
>> +++ b/drivers/staging/tm6000/tm6000-stds.c
>> @@ -32,7 +32,7 @@ struct tm6000_std_tv_settings {
>>  v4l2_std_id id;
>>  struct tm6000_reg_settings sif[12];
>>  struct tm6000_reg_settings nosif[12];
>> -struct tm6000_reg_settings common[25];
>> +struct tm6000_reg_settings common[26];
>>  };
>>  
> 
> Ooops :)
> 
>> I'll do some tests on it.
> 
> Ok
> 
> With my best regards, Dmitry.

By startingt audio capture before video, using mmap() for audio, I got this 
OOPS:

[ 3154.916559] BUG: unable to handle kernel paging request at eae380217f38
[ 3154.923520] IP: [] get_page+0xe/0x3d [snd_pcm]
[ 3154.929518] PGD 0 
[ 3154.931534] Oops:  [#1] SMP 
[ 3154.934772] last sysfs file: 
/sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map
[ 3154.942571] CPU 1 
[ 3154.944400] Modules linked in: tm6000_alsa(C) tuner_xc2028 tuner 
ir_lirc_codec lirc_dev ir_sony_decoder ir_jvc_decoder ir_rc6_decoder tm6000(C) 
ir_rc5_decoder v4l2_common ir_nec_decoder videodev v4l2_compat_ioctl32 
videobuf_vmalloc videobuf_core ir_common ir_core fuse ebtable_nat ebtables 
xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 
nf_defrag_ipv4 xt_state nf_conntrack ipt_REJECT bridge stp llc cpufreq_ondemand 
xt_physdev iptable_filter ip6t_REJECT ip6table_filter ip6_tables ipv6 
binfmt_misc parport kvm_intel kvm uinput tpm_infineon rtc_cmos rtc_core rtc_lib 
hp_wmi wmi psmouse serio_raw iTCO_wdt iTCO_vendor_support tg3 
snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq 
snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc i7core_edac 
edac_core nouveau ttm drm_kms_helper video output firewire_ohci firewire_core 
crc_itu_t ahci libahci ehci_hcd uhci_hcd floppy [last unloaded: tuner_xc2028]
[ 3155.028977] 
[ 3155.030464] Pid: 23437, comm: arecord Tainted: G C  2.6.35+ #4 
0AE4h/HP Z400 Workstation
[ 3155.039261] RIP: 0010:[]  [] 
get_page+0xe/0x3d [snd_pcm]
[ 3155.047725] RSP: :8800aed1bd48  EFLAGS: 00010246
[ 3155.053014] RAX: 000410009921 RBX: 8800aed1bdd8 RCX: 8800aec07c58
[ 3155.060120] RDX:  RSI:  RDI: eae380217f38
[ 3155.067226] RBP: 8800aed1bd58 R08:  R09: 
[ 3155.074334] R10:  R11: 8800c8eeac68 R12: eae380217f38
[ 3155.081441] R13:  R14: 8800c8eeabc0 R15: 3000
[ 3155.088548] FS:  7ff1f16d1700() GS:880002e2() 
knlGS:
[ 3155.096605] CS:  0010 DS:  ES:  CR0: 8005003b
[ 3155.102326] CR2: eae380217f38 CR3: aed0 CR4: 06e0
[ 3155.109432] DR0:  DR1:  DR2: 
[ 3155.116538] DR3:  DR6: 0ff0 DR7: 0400
[ 3155.123645] Process arecord (pid: 23437, threadinfo 8800aed1a000, task 
880118c4a620)
[ 3155.132050] Stack:
[ 3155.134052]  8800aed1bdd8 eae380217f38 8800aed1bd78 
a0165c03
[ 3155.141279] <0> 8800c0715a20  8800aed1be28 
820f276d
[ 3155.148962] <0> 8301b620 8800 8801 
8800c8eeac68
[ 3155.156827] Call Trace:
[ 3155.159268]  [] snd_pcm_mmap_data_fault+0x90/0xa2 [snd_pcm]
[ 3155.166378]  [] __do_fault+0x58/0x4ac
[ 3155.171582]  [] handle_mm_fault+0x3c5/0x8c2
[ 3155.177307]  [] ? do_page_fault+0x200/0x491
[ 3155.183031]  [] ? vfs_ioctl+0x32/0xa6
[ 3155.188239]  [] ? need_resched+0x35/0x3b
[ 3155.193702]  [] do_page_fault+0x408/0x491
[ 3155.199253]  [] page_fault+0x25/0x30
[ 3155.204371] Code: 66 31 c0 eb 16 48 89 c6 e8 67 fe ff ff eb 0c b8 fa ff ff 
ff eb 05 b8 ea ff ff ff c9 c3 55 48 89 e5 41 54 53 0f 1f 44 00 00 31 d2 <4c> 8b 
27 48 89 fb 49 c1 ec 0f 48 c7 c7 08 f7 16 a0 41 83 e4 01 
[ 3155.223920] RIP  [] get_page+0xe/0x3d [snd_pcm]
[ 3155.230003]  RSP 
[ 3155.233476] CR2: eae380217f38
[ 3155.238030] tm6000 tm6000_irq_callback :urb resubmit failed (error=-27)
[ 3155.244650] tm6000 tm6000_irq_callback :urb resubmit failed (error=-27)
[ 3155.251269] tm6000 tm6000_irq_callback :urb resubmit failed (error=-27)
[ 3155.257885] tm6000 tm6000_irq_callback :urb resubmit failed (error=-27)
[ 3155.264513] tm6000 t

Re: Remaining BKL users, what to do

2010-09-21 Thread Petr Vandrovec
I'll try to come up with something for ncpfs.

Trivial lock replacement will open deadlock possibility when someone reads to 
page which is also mmaped from the same filesystem (like grep likes to do). BKL 
with its automated release on sleep helped (or papered over) a lot here.

Petr

"Arnd Bergmann"  wrote:

>On Thursday 16 September 2010, Anton Altaparmakov wrote:
>> On 16 Sep 2010, at 16:04, Jan Kara wrote:
>> > On Thu 16-09-10 16:32:59, Arnd Bergmann wrote:
>> >> The big kernel lock is gone from almost all code in linux-next, this is
>> >> the status of what I think will happen to the remaining users:
>> > ...
>> >> fs/ncpfs:
>> >>  Should be fixable if Petr still cares about it. Otherwise suggest
>> >>  moving to drivers/staging if there are no users left.
>> >  I think some people still use this...
>> 
>> Yes, indeed.  Netware is still alive (unfortunately!) and ncpfs is used in a 
>> lot of 
>> Universities here in the UK at least (we use it about a thousand 
>> workstations and
>> servers here at Cambridge University!).
>
>Ok, that means at least when someone gets around to fix it, there will be
>people that can test the patches.
>
>If you know someone who would like to help on this, it would be nice to try
>out the patch below, unless someone can come up with a better solution.
>My naïve understanding of the code tells me that simply using the super block
>lock there may work. In fact it makes locking stricter, so if it still works
>with that patch, there are probably no subtle regressions.
>The patch applies to current linux-next of my bkl/vfs series.
>
>   Arnd
>
>---
>ncpfs: replace BKL with lock_super
>
>This mindlessly changes every instance of lock_kernel in ncpfs to
>lock_super. I haven't tested this, it may work or may break horribly.
>Please test with CONFIG_LOCKDEP enabled.
>
>Signed-off-by: Arnd Bergmann 
>
>diff --git a/fs/ncpfs/dir.c b/fs/ncpfs/dir.c
>index 9578cbe..303338d 100644
>--- a/fs/ncpfs/dir.c
>+++ b/fs/ncpfs/dir.c
>@@ -19,7 +19,6 @@
> #include 
> #include 
> #include 
>-#include 
> 
> #include 
> 
>@@ -339,9 +338,10 @@ static int
> ncp_lookup_validate(struct dentry * dentry, struct nameidata *nd)
> {
>   int res;
>-  lock_kernel();
>+  struct super_block *sb = dentry->d_inode->i_sb;
>+  lock_super(sb);
>   res = __ncp_lookup_validate(dentry);
>-  unlock_kernel();
>+  unlock_super(sb);
>   return res;
> }
> 
>@@ -404,6 +404,7 @@ static int ncp_readdir(struct file *filp, void *dirent, 
>filldir_t filldir)
> {
>   struct dentry *dentry = filp->f_path.dentry;
>   struct inode *inode = dentry->d_inode;
>+  struct super_block *sb = inode->i_sb;
>   struct page *page = NULL;
>   struct ncp_server *server = NCP_SERVER(inode);
>   union  ncp_dir_cache *cache = NULL;
>@@ -411,7 +412,7 @@ static int ncp_readdir(struct file *filp, void *dirent, 
>filldir_t filldir)
>   int result, mtime_valid = 0;
>   time_t mtime = 0;
> 
>-  lock_kernel();
>+  lock_super(sb);
> 
>   ctl.page  = NULL;
>   ctl.cache = NULL;
>@@ -546,7 +547,7 @@ finished:
>   page_cache_release(ctl.page);
>   }
> out:
>-  unlock_kernel();
>+  unlock_super(sb);
>   return result;
> }
> 
>@@ -794,12 +795,13 @@ out:
> static struct dentry *ncp_lookup(struct inode *dir, struct dentry *dentry, 
> struct nameidata *nd)
> {
>   struct ncp_server *server = NCP_SERVER(dir);
>+  struct super_block *sb = dir->i_sb;
>   struct inode *inode = NULL;
>   struct ncp_entry_info finfo;
>   int error, res, len;
>   __u8 __name[NCP_MAXPATHLEN + 1];
> 
>-  lock_kernel();
>+  lock_super(sb);
>   error = -EIO;
>   if (!ncp_conn_valid(server))
>   goto finished;
>@@ -846,7 +848,7 @@ add_entry:
> 
> finished:
>   PPRINTK("ncp_lookup: result=%d\n", error);
>-  unlock_kernel();
>+  unlock_super(sb);
>   return ERR_PTR(error);
> }
> 
>@@ -880,6 +882,7 @@ int ncp_create_new(struct inode *dir, struct dentry 
>*dentry, int mode,
> {
>   struct ncp_server *server = NCP_SERVER(dir);
>   struct ncp_entry_info finfo;
>+  struct super_block *sb = dir->i_sb;
>   int error, result, len;
>   int opmode;
>   __u8 __name[NCP_MAXPATHLEN + 1];
>@@ -888,7 +891,7 @@ int ncp_create_new(struct inode *dir, struct dentry 
>*dentry, int mode,
>   dentry->d_parent->d_name.name, dentry->d_name.name, mode);
> 
>   error = -EIO;
>-  lock_kernel();
>+  lock_super(sb);
>   if (!ncp_conn_valid(server))
>   goto out;
> 
>@@ -935,7 +938,7 @@ int ncp_create_new(struct inode *dir, struct dentry 
>*dentry, int mode,
> 
>   error = ncp_instantiate(dir, dentry, &finfo);
> out:
>-  unlock_kernel();
>+  unlock_super(sb);
>   return error;
> }
> 
>@@ -949,6 +952,7 @@ static int ncp_mkdir(struct inode *dir, struct dentry 
>*dentry, int mode)
> {
>   struct ncp_entry_info finfo;
>   struct ncp_server *server = NCP_SER

Re: [RFC PATCHES] First version of the V4L2 core locking patches

2010-09-21 Thread Mauro Carvalho Chehab
Em Tue, 21 Sep 2010 15:50:13 +0200
"Hans Verkuil"  escreveu:

> Hi Mauro,
> 
> > Em 20-09-2010 18:37, Hans Verkuil escreveu:
...
> > /*
> >  * Need to sleep in order to wait for videobufs to complete.
> >  * It is not a good idea to sleep while waiting for an event with the 
> > dev
> > lock hold,
> >  * as it will block any other access to the device. Just unlock it while
> > waiting,
> >  * locking it again at the end.
> >  */
> >
> > is_vdev_locked = (q->vdev_lock && mutex_is_locked(q->vdev_lock)) ? true
> > : false;
> > if (is_vdev_locked)
> > mutex_unlock(q->vdev_lock);
> > if (intr)
> > return wait_event_interruptible(vb->done, 
> > is_state_active_or_queued(vb,
> > q));
> 
> This obviously needs to save the return value and continue to make sure
> the lock is taken again.

Yeah, it should be:
 rc = wait_event_interruptible(vb->done, is_state_active_or_queued(vb, q));

and return rc at the end.
 
> > else
> > wait_event(vb->done, is_state_active_or_queued(vb, q));
> > if (is_vdev_locked)
> > mutex_lock(q->vdev_lock);
> >
> > return 0;
> > }
> 
> Agreed. Thanks for reviewing this, it was the one patch that I knew I had
> to look into more closely. I'll incorporate your changes.

Ok, thanks.


-- 

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


Re: [RFCv5 7/9] mm: vcm: Virtual Contiguous Memory framework added

2010-09-21 Thread Konrad Rzeszutek Wilk
> +* The Virtual Contiguous Memory Manager
> +
> +The VCMM was built to solve the system-wide memory mapping issues that
> +occur when many bus-masters have IOMMUs.
> +
> +An IOMMU maps device addresses to physical addresses.  It also
> +insulates the system from spurious or malicious device bus
> +transactions and allows fine-grained mapping attribute control.  The
> +Linux kernel core does not contain a generic API to handle IOMMU
> +mapped memory; device driver writers must implement device specific
> +code to interoperate with the Linux kernel core.  As the number of
> +IOMMUs increases, coordinating the many address spaces mapped by all
> +discrete IOMMUs becomes difficult without in-kernel support.

Looking at the set of calls and the examples it struck me as similar
to the agp.h API (drivers/char/agp/). It has allocate, bind, de-allocate.
Naturally it has no bus device mapping, but the DRM code that utilizes
the AGP API bridge has that: drivers/gpu/drm/ati_pcigart.c (DRM API).

Then there are the radeon and nouveau drivers that program the GPU GART
bypassing the AGP API but still utilize the DMA API.

The nice ASCII art you included in your writeup looks to cover those
use cases.

What I am ineptly trying to say is that is that we have a bunch
of APIs that do this, and in case where they are inadequate (or just
look to be a one-off solution) we have functions that are similar in API
view but differ in implementation (check out how the Nouveau programs
its VMM compared to how the Radeon does it).

Your API offers a way to unify all of that, but it looks to be
an API on top of the other ones. You would still have to implement
different mechanisms for the utilizing this say on the radeon driver:
AGP API, or the home-grown GPU GART programming, and then the DMA API
wrapped around them all. Oh, and the DMA API sits on top of the IOMMU API.

I am not sure how this would solve the proliferation of different APIs
- it sounds like it just adds another piece where you still have to
shoe-in the other APIs in.

But I do understand the problem you are facing. You want to switch
to different IOMMUs for different drivers using only one API. Folks have
been asking about whether it makes sense to include your algorithms in
expanding the memory allocator to have huge chunks of memory reserved
for specific drivers. But that does not solve the IOMMU problem.

So my question is, would it perhaps make sense to concentrate on the DMA API?
Could we expand it so that for specific devices it sets the DMA API
to use a different IOMMU? If you look at the Calgary IOMMU - that is a perfect
example of your problem  - it is only used if the specific devices which fall
within its control - all other DMA operations are utilized by the SWIOTLB
(the default IOMMU).

Would it possible to do something similar to that so when CMA is activated
it scans the region list, finds which devices can share the same memory region
and sets the struct device DMA API to point to a CMA IOMMU which is happy
to utilize the memory allocator reserved chunks of memory (that would
be the code lifted from your CMA and stuck in the memory allocate)
and allowing different drivers (those on the whitelist) to share the same 
region?

This has the extra benefit that it would inclusive allow all drivers
that utilize the DMA API to use this without any extra VCMA/CMA API calls?

P.S.
I am quite X86 specific here - I don't know much about ARM (is there some
desktop box I can buy with it?), so I am probably missing some details.
--
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 PATCHES] First version of the V4L2 core locking patches

2010-09-21 Thread Hans Verkuil
Hi Mauro,

> Em 20-09-2010 18:37, Hans Verkuil escreveu:
>> Hi all,
>>
>> I've made a first version of the core locking patches available here:
>>
>> http://git.linuxtv.org/hverkuil/v4l-dvb.git?a=shortlog;h=refs/heads/test
>>
>> I'm actually surprised how trivial the patches are. Which makes me
>> wonder if
>> I am overlooking something, it feels too easy.
>>
>> One thing I did not yet have time to analyze fully is if it is really OK
>> to
>> unlock/relock the vdev_lock in videobuf_waiton. I hope it is, because
>> without
>> this another thread will find it impossible to access the video node
>> while it
>> is in waiton.
>>
>> Currently I've only tested with vivi. I hope to be able to spend more
>> time
>> this week for a more thorough analysis and converting a few more drivers
>> to
>> this.
>>
>> In the meantime, please feel free to shoot at this code!
>
> Hi Hans,
>
> This patch will likely break most drivers:
>   
> http://git.linuxtv.org/hverkuil/v4l-dvb.git?a=commitdiff;h=d1ca35f3e69d909a958eb1cf8c75dd1c0bb2a98c

This was indeed something I wanted to review more closely.

> In the case of events and videobuf_waiton, it doesn't seem to be safe to
> just
> unlock when waiting for an event.
>
> For example, in the case of videobuf_waiton, the code for it is:
>
> #define WAITON_CONDITION (vb->state != VIDEOBUF_ACTIVE &&\
>   vb->state != VIDEOBUF_QUEUED)
> int videobuf_waiton(struct videobuf_buffer *vb, int non_blocking, int
> intr)
> {
>   MAGIC_CHECK(vb->magic, MAGIC_BUFFER);
>
>   if (non_blocking) {
>   if (WAITON_CONDITION)
>   return 0;
>   else
>   return -EAGAIN;
>   }
>
>   if (intr)
>   return wait_event_interruptible(vb->done, WAITON_CONDITION);
>   else
>   wait_event(vb->done, WAITON_CONDITION);
>
>   return 0;
> }
>
> When called internally, it have the vb mutex_locked, while, when called
> externally, it
> doesn't.
>
> By looking on other parts where vb->done is protected, like on
> videobuf_queue_cancel:
>
>   spin_lock_irqsave(q->irqlock, flags);
>   for (i = 0; i < VIDEO_MAX_FRAME; i++) {
>   if (NULL == q->bufs[i])
>   continue;
>   if (q->bufs[i]->state == VIDEOBUF_QUEUED) {
>   list_del(&q->bufs[i]->queue);
>   q->bufs[i]->state = VIDEOBUF_ERROR;
>   wake_up_all(&q->bufs[i]->done);
>   }
>   }
>   spin_unlock_irqrestore(q->irqlock, flags);
>
> It is clear that vb state is protected by a spinlock, and not by a mutex.
> Using a mutex
> there makes no sense at all. Instead of touching a mutex, callers of this
> function should
> be reviewed to not call a mutex.
>
> So, the better approach for videobuf_waiton would be to protect it with a
> spinlock.

Sounds reasonable.

> Also, your patches assume that no driver will touch at vdev lock before
> calling videobuf_waiton().
> This seems to be a risky assumption. So, the better would be to define it
> as:
>
> static int is_state_active_or_queued(struct videobuf_buffer *vb, struct
> videobuf_queue *q, )
> {
>   bool rc;
>
>   spin_lock_irqsave(q->irqlock, flags);
>   rc = (vb->state != VIDEOBUF_ACTIVE) && (vb->state != VIDEOBUF_QUEUED));
>   spin_unlock_irqrestore(q->irqlock, flags);
>
>   return rc;
> };
>
> int videobuf_waiton(struct videobuf_queue *q, struct videobuf_buffer *vb,
> int non_blocking, int intr)
> {
>   rc = 0;
>   bool is_vdev_locked;
>   MAGIC_CHECK(vb->magic, MAGIC_BUFFER);
>
>   /*
>* If there's nothing to wait, just return
>*/
>   if (is_state_active_or_queued(vb, q))
>   return 0;
>
>   if (non_blocking)
>   return -EAGAIN;
>
>   /*
>* Need to sleep in order to wait for videobufs to complete.
>* It is not a good idea to sleep while waiting for an event with the 
> dev
> lock hold,
>* as it will block any other access to the device. Just unlock it while
> waiting,
>* locking it again at the end.
>*/
>
>   is_vdev_locked = (q->vdev_lock && mutex_is_locked(q->vdev_lock)) ? true
> : false;
>   if (is_vdev_locked)
>   mutex_unlock(q->vdev_lock);
>   if (intr)
>   return wait_event_interruptible(vb->done, 
> is_state_active_or_queued(vb,
> q));

This obviously needs to save the return value and continue to make sure
the lock is taken again.

>   else
>   wait_event(vb->done, is_state_active_or_queued(vb, q));
>   if (is_vdev_locked)
>   mutex_lock(q->vdev_lock);
>
>   return 0;
> }

Agreed. Thanks for reviewing this, it was the one patch that I knew I had
to look into more closely. I'll incorporate your changes.

Regards,

 Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

--
To unsubscribe from this list: send the

Re: [RFC PATCHES] First version of the V4L2 core locking patches

2010-09-21 Thread Mauro Carvalho Chehab
Em 20-09-2010 18:37, Hans Verkuil escreveu:
> Hi all,
> 
> I've made a first version of the core locking patches available here:
> 
> http://git.linuxtv.org/hverkuil/v4l-dvb.git?a=shortlog;h=refs/heads/test
> 
> I'm actually surprised how trivial the patches are. Which makes me wonder if
> I am overlooking something, it feels too easy.
> 
> One thing I did not yet have time to analyze fully is if it is really OK to
> unlock/relock the vdev_lock in videobuf_waiton. I hope it is, because without
> this another thread will find it impossible to access the video node while it
> is in waiton.
> 
> Currently I've only tested with vivi. I hope to be able to spend more time
> this week for a more thorough analysis and converting a few more drivers to
> this.
> 
> In the meantime, please feel free to shoot at this code!

Hi Hans,

This patch will likely break most drivers:

http://git.linuxtv.org/hverkuil/v4l-dvb.git?a=commitdiff;h=d1ca35f3e69d909a958eb1cf8c75dd1c0bb2a98c

In the case of events and videobuf_waiton, it doesn't seem to be safe to just
unlock when waiting for an event.

For example, in the case of videobuf_waiton, the code for it is:

#define WAITON_CONDITION (vb->state != VIDEOBUF_ACTIVE &&\
vb->state != VIDEOBUF_QUEUED)
int videobuf_waiton(struct videobuf_buffer *vb, int non_blocking, int intr)
{
MAGIC_CHECK(vb->magic, MAGIC_BUFFER);

if (non_blocking) {
if (WAITON_CONDITION)
return 0;
else
return -EAGAIN;
}

if (intr)
return wait_event_interruptible(vb->done, WAITON_CONDITION);
else
wait_event(vb->done, WAITON_CONDITION);

return 0;
}

When called internally, it have the vb mutex_locked, while, when called 
externally, it
doesn't.

By looking on other parts where vb->done is protected, like on 
videobuf_queue_cancel:

spin_lock_irqsave(q->irqlock, flags);
for (i = 0; i < VIDEO_MAX_FRAME; i++) {
if (NULL == q->bufs[i])
continue;
if (q->bufs[i]->state == VIDEOBUF_QUEUED) {
list_del(&q->bufs[i]->queue);
q->bufs[i]->state = VIDEOBUF_ERROR;
wake_up_all(&q->bufs[i]->done);
}
}
spin_unlock_irqrestore(q->irqlock, flags);

It is clear that vb state is protected by a spinlock, and not by a mutex. Using 
a mutex
there makes no sense at all. Instead of touching a mutex, callers of this 
function should
be reviewed to not call a mutex.

So, the better approach for videobuf_waiton would be to protect it with a
spinlock.

Also, your patches assume that no driver will touch at vdev lock before calling 
videobuf_waiton().
This seems to be a risky assumption. So, the better would be to define it as:

static int is_state_active_or_queued(struct videobuf_buffer *vb, struct 
videobuf_queue *q, )
{
bool rc;

spin_lock_irqsave(q->irqlock, flags);
rc = (vb->state != VIDEOBUF_ACTIVE) && (vb->state != VIDEOBUF_QUEUED));
spin_unlock_irqrestore(q->irqlock, flags);

return rc;
};

int videobuf_waiton(struct videobuf_queue *q, struct videobuf_buffer *vb, int 
non_blocking, int intr)
{
rc = 0;
bool is_vdev_locked;
MAGIC_CHECK(vb->magic, MAGIC_BUFFER);

/*
 * If there's nothing to wait, just return  
 */
if (is_state_active_or_queued(vb, q))
return 0;

if (non_blocking)
return -EAGAIN;

/*
 * Need to sleep in order to wait for videobufs to complete.
 * It is not a good idea to sleep while waiting for an event with the 
dev lock hold,
 * as it will block any other access to the device. Just unlock it 
while waiting,
 * locking it again at the end.
 */

is_vdev_locked = (q->vdev_lock && mutex_is_locked(q->vdev_lock)) ? true 
: false;
if (is_vdev_locked)
mutex_unlock(q->vdev_lock);
if (intr)
return wait_event_interruptible(vb->done, 
is_state_active_or_queued(vb, q));
else
wait_event(vb->done, is_state_active_or_queued(vb, q));
if (is_vdev_locked)
mutex_lock(q->vdev_lock);

return 0;
}

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


[PATCHv2 2/2] V4L/DVB: radio-si4713: Add regulator framework support

2010-09-21 Thread Jarkko Nikula
Convert the driver to use regulator framework instead of set_power callback.
This with gpio_reset platform data provide cleaner way to manage chip VIO,
VDD and reset signal inside the driver.

Signed-off-by: Jarkko Nikula 
Cc: Eduardo Valentin 
---
v2:
I moved all the regulator management to i2c driver part of si4713 as suggested
by Eduardo. Then the si4713-i2c.c can still be used separately from
radio-si4713.c and it's easier to follow what supply voltages are needed.

v1:
I don't have specifications for this chip so I don't know how long the
reset signal must be active after power-up. I used 50 us from Maemo
kernel sources for Nokia N900 and I can successfully enable-disable
transmitter on N900 with vdd power cycling.
---
 drivers/media/radio/si4713-i2c.c |   74 --
 drivers/media/radio/si4713-i2c.h |5 ++-
 2 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/drivers/media/radio/si4713-i2c.c b/drivers/media/radio/si4713-i2c.c
index fc7f4b7..f1502c6 100644
--- a/drivers/media/radio/si4713-i2c.c
+++ b/drivers/media/radio/si4713-i2c.c
@@ -27,6 +27,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -43,6 +45,11 @@ MODULE_AUTHOR("Eduardo Valentin 
");
 MODULE_DESCRIPTION("I2C driver for Si4713 FM Radio Transmitter");
 MODULE_VERSION("0.0.1");
 
+static const char *si4713_supply_names[SI4713_NUM_SUPPLIES] = {
+   "vio",
+   "vdd",
+};
+
 #define DEFAULT_RDS_PI 0x00
 #define DEFAULT_RDS_PTY0x00
 #define DEFAULT_RDS_PS_NAME""
@@ -369,7 +376,17 @@ static int si4713_powerup(struct si4713_device *sdev)
if (sdev->power_state)
return 0;
 
-   sdev->platform_data->set_power(1);
+   err = regulator_bulk_enable(ARRAY_SIZE(sdev->supplies),
+   sdev->supplies);
+   if (err) {
+   v4l2_err(&sdev->sd, "Failed to enable supplies: %d\n", err);
+   return err;
+   }
+   if (gpio_is_valid(sdev->gpio_reset)) {
+   udelay(50);
+   gpio_set_value(sdev->gpio_reset, 1);
+   }
+
err = si4713_send_command(sdev, SI4713_CMD_POWER_UP,
args, ARRAY_SIZE(args),
resp, ARRAY_SIZE(resp),
@@ -384,7 +401,13 @@ static int si4713_powerup(struct si4713_device *sdev)
err = si4713_write_property(sdev, SI4713_GPO_IEN,
SI4713_STC_INT | SI4713_CTS);
} else {
-   sdev->platform_data->set_power(0);
+   if (gpio_is_valid(sdev->gpio_reset))
+   gpio_set_value(sdev->gpio_reset, 0);
+   err = regulator_bulk_disable(ARRAY_SIZE(sdev->supplies),
+sdev->supplies);
+   if (err)
+   v4l2_err(&sdev->sd,
+"Failed to disable supplies: %d\n", err);
}
 
return err;
@@ -411,7 +434,13 @@ static int si4713_powerdown(struct si4713_device *sdev)
v4l2_dbg(1, debug, &sdev->sd, "Power down response: 0x%02x\n",
resp[0]);
v4l2_dbg(1, debug, &sdev->sd, "Device in reset mode\n");
-   sdev->platform_data->set_power(0);
+   if (gpio_is_valid(sdev->gpio_reset))
+   gpio_set_value(sdev->gpio_reset, 0);
+   err = regulator_bulk_disable(ARRAY_SIZE(sdev->supplies),
+sdev->supplies);
+   if (err)
+   v4l2_err(&sdev->sd,
+"Failed to disable supplies: %d\n", err);
sdev->power_state = POWER_OFF;
}
 
@@ -1967,7 +1996,8 @@ static int si4713_probe(struct i2c_client *client,
const struct i2c_device_id *id)
 {
struct si4713_device *sdev;
-   int rval;
+   struct si4713_platform_data *pdata = client->dev.platform_data;
+   int rval, i;
 
sdev = kzalloc(sizeof *sdev, GFP_KERNEL);
if (!sdev) {
@@ -1976,11 +2006,26 @@ static int si4713_probe(struct i2c_client *client,
goto exit;
}
 
-   sdev->platform_data = client->dev.platform_data;
-   if (!sdev->platform_data) {
-   v4l2_err(&sdev->sd, "No platform data registered.\n");
-   rval = -ENODEV;
-   goto free_sdev;
+   sdev->gpio_reset = -1;
+   if (pdata && gpio_is_valid(pdata->gpio_reset)) {
+   rval = gpio_request(pdata->gpio_reset, "si4713 reset");
+   if (rval) {
+   dev_err(&client->dev,
+   "Failed to request gpio: %d\n", rval);
+   goto free_sdev;
+   }
+   sdev->gpio_reset = pdata->gpio_reset;
+   gpio_directio

[PATCHv2 1/2] V4L/DVB: radio-si4713: Release i2c adapter in driver cleanup paths

2010-09-21 Thread Jarkko Nikula
Call to i2c_put_adapter was missing in radio_si4713_pdriver_probe and
radio_si4713_pdriver_remove.

Signed-off-by: Jarkko Nikula 
Acked-by: Eduardo Valentin 
---
No changes, Eduardo's ack added.
---
 drivers/media/radio/radio-si4713.c |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/radio/radio-si4713.c 
b/drivers/media/radio/radio-si4713.c
index 13554ab..0a9fc4d 100644
--- a/drivers/media/radio/radio-si4713.c
+++ b/drivers/media/radio/radio-si4713.c
@@ -296,14 +296,14 @@ static int radio_si4713_pdriver_probe(struct 
platform_device *pdev)
if (!sd) {
dev_err(&pdev->dev, "Cannot get v4l2 subdevice\n");
rval = -ENODEV;
-   goto unregister_v4l2_dev;
+   goto put_adapter;
}
 
rsdev->radio_dev = video_device_alloc();
if (!rsdev->radio_dev) {
dev_err(&pdev->dev, "Failed to alloc video device.\n");
rval = -ENOMEM;
-   goto unregister_v4l2_dev;
+   goto put_adapter;
}
 
memcpy(rsdev->radio_dev, &radio_si4713_vdev_template,
@@ -320,6 +320,8 @@ static int radio_si4713_pdriver_probe(struct 
platform_device *pdev)
 
 free_vdev:
video_device_release(rsdev->radio_dev);
+put_adapter:
+   i2c_put_adapter(adapter);
 unregister_v4l2_dev:
v4l2_device_unregister(&rsdev->v4l2_dev);
 free_rsdev:
@@ -335,8 +337,12 @@ static int __exit radio_si4713_pdriver_remove(struct 
platform_device *pdev)
struct radio_si4713_device *rsdev = container_of(v4l2_dev,
struct radio_si4713_device,
v4l2_dev);
+   struct v4l2_subdev *sd = list_entry(v4l2_dev->subdevs.next,
+   struct v4l2_subdev, list);
+   struct i2c_client *client = v4l2_get_subdevdata(sd);
 
video_unregister_device(rsdev->radio_dev);
+   i2c_put_adapter(client->adapter);
v4l2_device_unregister(&rsdev->v4l2_dev);
kfree(rsdev);
 
-- 
1.7.1

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


[PATCHv2 0/2] Si4713 fix and regulator fw support

2010-09-21 Thread Jarkko Nikula
Hi

Repost of these two patches. I added Eduardo's ack to 1st and moved regulator
management in 2nd to single place only as suggested by Eduardo.

My first version was separating them to radio-si4713.c and si4713-i2c.c and
indeed then there is a risk that vio is missed if si4713-i2c.c is reused in
another driver. Anyway vio issue (if not enabled before si4713_probe) must be
solved in system level or in i2c core, not in radio-si4713.c.

Earlier version and thread can be found from here:
http://www.spinics.net/lists/linux-media/msg20199.html


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


Asus MyCinema P7131 Dual support

2010-09-21 Thread Dejan Rodiger
Hi,

I am using Ubuntu linux 10.10 with the latest kernel 2.6.35-22-generic
on x86_64. I have installed nonfree firmware which should support this
card, but to be sure, can somebody confirm that my TV card is
supported in Analog or DVB mode?

sudo lspci -vnn
01:06.0 Multimedia controller [0480]: Philips Semiconductors
SAA7131/SAA7133/SAA7135 Video Broadcast Decoder [1131:7133] (rev d1)
Subsystem: ASUSTeK Computer Inc. My Cinema-P7131 Hybrid [1043:4876]
Flags: bus master, medium devsel, latency 32, IRQ 18
Memory at fdeff000 (32-bit, non-prefetchable) [size=2K]
Capabilities: [40] Power Management version 2
Kernel driver in use: saa7134
Kernel modules: saa7134

It says Hybrid, but I put the following in the /etc/modprobe.d/saa7134.conf
options saa7134 card=78 tuner=54


Thanks
--
Dejan Rodiger
S: callto://drodiger
--
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: Hauppauge WinTV-NOVA-T-500 support

2010-09-21 Thread Thomas Kernen


So I've tested a WinTV-NOVA-T-500 model 283 (SL-283-V2.0-GER) which 
according to the wiki isn't suppose to work:

http://www.linuxtv.org/wiki/index.php/Hauppauge_WinTV-NOVA-T-500

Anyhow, the revision I own, (WinTV-NOVA-T-500, 99101 LF, Rev D8B5) I 
seem to have no issues with support, no errors on loading the modules or 
tuning to the different DVB-T tuners. I'll go ahead and update the Wiki 
page to add a note that this revision actually does work and that the 
blanket statement claiming all model 283 are not supported.


Regards,
Thomas

On 9/7/10 10:22 AM, Thomas Kernen wrote:


Hello,

According to the wiki entry for the Hauppauge WinTV-NOVA-T-500 hardware:
http://www.linuxtv.org/wiki/index.php/Hauppauge_WinTV-NOVA-T-500

Models 289 and 287 are supported (ie: the UK sold cards), but model 283
sold in Germany, Switzerland and maybe some other countries isn't.

Is this still an accurate statement or has this situation evolved but
hasn't been updated in the wiki entry?

Regards,
Thomas
--

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