[PATCH] [media] em28xx: enable DMABUF

2013-01-05 Thread Mauro Carvalho Chehab
Now that it uses videobuf2, em28xx can support DMABUF.

Tested with an HVR-950 on analog mode and a 2gen i5core machine
with an i915 graphics adapter.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c 
b/drivers/media/usb/em28xx/em28xx-video.c
index 75027e3..2eabf2a 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -699,7 +699,7 @@ int em28xx_vb2_setup(struct em28xx *dev)
/* Setup Videobuf2 for Video capture */
q = &dev->vb_vidq;
q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-   q->io_modes = VB2_READ | VB2_MMAP | VB2_USERPTR;
+   q->io_modes = VB2_READ | VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
q->drv_priv = dev;
q->buf_struct_size = sizeof(struct em28xx_buffer);
q->ops = &em28xx_video_qops;
-- 
1.7.11.7

--
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: How to configure resizer in ISP pipeline?

2013-01-05 Thread Andreas Nagel

Hi Sakari,

thanks for helping.


My "sensor" (TVP5146) already provides YUV data, so I can skip the
previewer. I tried setting the input and output pad of the resizer
subdevice to incoming resolution (input pad) and desired resolution
(output pad). For example: 720x576 --> 352x288. But it didn't work
out quite well.

How did it not work quite well? :)


Not sure, if I recall all the details. I haven't done much in this area 
for a few weeks now.
Currently, I actually can configure the resizer, but then 
VIDIOC_STREAMON fails with EINVAL when I configure the devnode. Don't 
know why.
I do connect the resizer source to the resizer output (devnode) and then 
capture from there. I think it is /dev/video6.
If I leave the resizer out and connect the ccdc source to the ccdc 
output (/dev/video2), capturing works just fine.


One reason could be, that the resizer isn't supported right now. (You 
remember, I have to use Technexion's TI kernel 2.6.37 with its exotic 
ISP driver. ;-) )

That's, what one could interpret from this TI wiki page.
http://processors.wiki.ti.com/index.php/UserGuideOmap35xCaptureDriver_PSP_04.02.00.07#Architecture
Under the block diagram, there's a note saying, that only the path with 
the continuous line has been validated. So, the dotted lines are ISP 
paths that might not have been validated ("supported"?) yet.



(I might add, that all this is part of my master thesis and resizing 
would be a nice-to-have goal, but not a must-have. So I can live with it 
if it won't work.)



Can someone explain how one properly configures the resizer in an
ISP pipeline (with Media Controller API) ? I spend some hours
researching, but this topic seems to be a well guarded secret...

There's nothing special about it. Really.

The resizing factor is chosen by setting the media bus format on the source
pad to the desired size.


Yes, that is what I figured out eventually.
--
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: How to configure resizer in ISP pipeline?

2013-01-05 Thread Sakari Ailus
Hi Andreas,

On Tue, Dec 11, 2012 at 03:48:23PM +0100, Andreas Nagel wrote:
> Hello,
> 
> using Media Controller API, I can successfully configure a simple
> ISP pipeline on an OMAP3530 and capture video data. Now I want to
> include the resizer. So the pipeline would look like this (where MEM
> would be the devnode corresponding to "resizer output"):
> 
> Sensor --> CCDC --> Resizer --> MEM
> 
> My "sensor" (TVP5146) already provides YUV data, so I can skip the
> previewer. I tried setting the input and output pad of the resizer
> subdevice to incoming resolution (input pad) and desired resolution
> (output pad). For example: 720x576 --> 352x288. But it didn't work
> out quite well.

How did it not work quite well? :)

> Can someone explain how one properly configures the resizer in an
> ISP pipeline (with Media Controller API) ? I spend some hours
> researching, but this topic seems to be a well guarded secret...

There's nothing special about it. Really.

The resizing factor is chosen by setting the media bus format on the source
pad to the desired size.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: 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: [PATCH 1/2] omap3isp: Remove unneeded memset after kzalloc

2013-01-05 Thread Sakari Ailus
Sakari Ailus wrote:
> Acked-by: Sakari Ailus 

I forgot to mention this applies to both of the patches.

-- 
Cheers,

Sakari Ailus
sakari.ai...@iki.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


[PATCH 1/1] v4l: Don't compile v4l2-int-device unless really needed

2013-01-05 Thread Sakari Ailus
Add a configuration option for v4l2-int-device so it is only compiled when
necessary, which is only by omap24xxcam and tcm825x drivers.

Signed-off-by: Sakari Ailus 
---
The side effect of this patch is that there's a new config option in the
main media menu to select V4L2 int device. The drivers using it are hidden
unless the option is selected.

 drivers/media/i2c/Kconfig|2 +-
 drivers/media/platform/Kconfig   |2 +-
 drivers/media/v4l2-core/Kconfig  |   11 +++
 drivers/media/v4l2-core/Makefile |3 ++-
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 24d78e2..1e4b2d0 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -477,7 +477,7 @@ config VIDEO_MT9V032
 
 config VIDEO_TCM825X
tristate "TCM825x camera sensor support"
-   depends on I2C && VIDEO_V4L2
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_INT_DEVICE
depends on MEDIA_CAMERA_SUPPORT
---help---
  This is a driver for the Toshiba TCM825x VGA camera sensor.
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 3dcfea6..0641ade 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -92,7 +92,7 @@ config VIDEO_M32R_AR_M64278
 
 config VIDEO_OMAP2
tristate "OMAP2 Camera Capture Interface driver"
-   depends on VIDEO_DEV && ARCH_OMAP2
+   depends on VIDEO_DEV && ARCH_OMAP2 && VIDEO_V4L2_INT_DEVICE
select VIDEOBUF_DMA_SG
---help---
  This is a v4l2 driver for the TI OMAP2 camera capture interface
diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 65875c3..1f16110 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -82,3 +82,14 @@ config VIDEOBUF2_DMA_SG
#depends on HAS_DMA
select VIDEOBUF2_CORE
select VIDEOBUF2_MEMOPS
+
+config VIDEO_V4L2_INT_DEVICE
+   tristate "V4L2 int device (DEPRECATED)"
+   depends on VIDEO_V4L2
+   ---help---
+ An early framework for a hardware-independent interface for
+ image sensors and bridges etc. Currently used by omap24xxcam and
+ tcm825x drivers that should be converted to V4L2 subdev.
+
+ Do not use for new developments.
+
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..a9d3552 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -10,7 +10,8 @@ ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
 
-obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
+obj-$(CONFIG_VIDEO_DEV) += videodev.o
+obj-$(CONFIG_VIDEO_V4L2_INT_DEVICE) += v4l2-int-device.o
 obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
 
 obj-$(CONFIG_VIDEO_TUNER) += tuner.o
-- 
1.7.10.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


cron job: media_tree daily build: ERRORS

2013-01-05 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:Sat Jan  5 19:00:22 CET 2013
git hash:2c46bb119f13a3ebc62461dac498a7057f5b4a94
gcc version:  i686-linux-gcc (GCC) 4.7.1
host hardware:x86_64
host os:  3.4.07-marune

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

Detailed results are available here:

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

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2

The V4L-DVB specification from this daily build is here:

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


Re: [PATCH 4/4] v4l: Tell user space we're using monotonic timestamps

2013-01-05 Thread Sakari Ailus
Hi Kamil,

On Mon, Dec 17, 2012 at 12:48:25PM +0100, Kamil Debski wrote:
> > From: 'Sakari Ailus' [mailto:sakari.ai...@iki.fi]
> > Sent: Monday, December 17, 2012 12:35 PM
> > Subject: Re: [PATCH 4/4] v4l: Tell user space we're using monotonic
> > timestamps
> > 
> > Hi Kamil,
> > 
> > On Mon, Dec 17, 2012 at 12:19:51PM +0100, Kamil Debski wrote:
> > ...
> > > > > @@ -367,7 +368,8 @@ static void __fill_v4l2_buffer(struct
> > > > > vb2_buffer
> > > > *vb, struct v4l2_buffer *b)
> > > > >   /*
> > > > >* Clear any buffer state related flags.
> > > > >*/
> > > > > - b->flags &= ~V4L2_BUFFER_STATE_FLAGS;
> > > > > + b->flags &= ~V4L2_BUFFER_MASK_FLAGS;
> > > > > + b->flags |= V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > >
> > > As far as I know, after __fill_v4l2_buffer is run driver has no means
> > > to change flags. Right?
> > 
> > Correct. Querybuf, for example, is implemented in vb2 and no driver
> > involvement is required. And we sure don't want to add it. ;)
> 
> I did not suggest that it should be added.

Good. :-)

> > 
> > > So how should a driver, which is not using the MONOTONIC timestamps
> > > inform the user space about it?
> > 
> > We currently support only monotonic timestamps. Support for different
> > kind of timestamps should be added to videobuf2 when they are needed.
> > The drivers would then be using a videobuf2 equivalent of
> > v4l2_get_timestamp().
> 
> Just as I though.
> Mind you - v4l2_get_timestamp() does not apply if the timestamp are simply
> copied.
> This is the case of some of the mem2mem devices, remember?

Yeah. Makes sense.

I guess memory-to-memory devices (apart from possibly those dealing with
compressed data which contains the timestamps) should just copy the
timestamps from output to capture buffers.

-- 
Cheers,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: 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: [git:v4l-dvb/for_v3.9] [media] rc/keymaps: add RC keytable for MyGica X8507

2013-01-05 Thread Alfredo Jesús Delaiti

Hi all

El 23/12/12 18:42, Mauro Carvalho Chehab escribió:

This is an automatic generated email to let you know that the following patch 
were queued at the
http://git.linuxtv.org/media_tree.git tree:

Subject: [media] rc/keymaps: add RC keytable for MyGica X8507
Author:  Alfredo Jesús Delaiti 
Date:Thu Nov 8 16:14:50 2012 -0300

Add RC-5 remote keytable definition for MyGica X8507.

[mche...@redhat.com: fixed whitespacing - it seems that Alfredo's emailer 
mangled
  it]
Signed-off-by: Alfredo J. Delaiti 
Signed-off-by: Mauro Carvalho Chehab 



Thank you very much for the help and corrections.


Alfredo
--
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 2/3] [media] winbond-cir: increase IR receiver resolution

2013-01-05 Thread Sean Young
On Thu, Jan 03, 2013 at 01:16:57AM +0100, David Härdeman wrote:
> On Wed, Oct 24, 2012 at 10:22:41PM +0100, Sean Young wrote:
> >This is needed for carrier reporting.
> >
> >Signed-off-by: Sean Young 
> >---
> > drivers/media/rc/winbond-cir.c | 14 +-
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> 
> Using a resolution of 2us rather than 10us means that the resolution
> (and amount of work necessary for decoding a given signal) is about 25x
> higher than in the windows driver (which uses a 50us resolution IIRC)...
> 
> Most of it is mitigated by using RLE (which I don't think the windows
> driver usesagain...IIRC), but it still seems unnecessary for the
> general case.

You're right, the hardware will generate more data for 2us rather than 
10us. For one key press on a nec remote, I get 69 interrupts before 
this patch and 302 after. That's almost 5 times as much, but not a 
ridiculous amount of work.

> Wouldn't it be possible to only use the high-res mode when carrier
> reports are actually enabled?

That is possible, although is it really worth the effort? I'll have a
look at implementing it and see what the code will look like.


Sean
--
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: BUG: bttv does not load module ir-kbd-i2c for Hauppauge model 37284, rev B421

2013-01-05 Thread Mauro Carvalho Chehab
Em Sat, 05 Jan 2013 15:00:18 +0100
Frank Schäfer  escreveu:

> While we are at it ;) :
> 
> [   15.280772] bttv: Bt8xx card found (0)
> [   15.281349] bttv: 0: Bt878 (rev 17) at :01:08.0, irq: 18,
> latency: 32, mmio: 0xfdfff000
> [   15.281386] bttv: 0: detected: Hauppauge WinTV [card=10], PCI
> subsystem ID is 0070:13eb
> [   15.281391] bttv: 0: using: Hauppauge (bt878) [card=10,insmod option]
> [   15.283964] bttv: 0: Hauppauge/Voodoo msp34xx: reset line init [5]
> [   15.316043] tveeprom 4-0050: Hauppauge model 37284, rev B421, serial#
> 5111944
> [   15.316049] tveeprom 4-0050: tuner model is Philips FM1216 (idx 21,
> type 5)
> [   15.316054] tveeprom 4-0050: TV standards PAL(B/G) (eeprom 0x04)
> [   15.316059] tveeprom 4-0050: audio processor is MSP3410D (idx 5)
> [   15.316063] tveeprom 4-0050: has radio
> [   15.316066] bttv: 0: Hauppauge eeprom indicates model#37284
> [   15.316071] bttv: 0: tuner type=5
> [   16.178816] bttv: 0: registered device video0
> [   16.179071] bttv: 0: registered device vbi0
> [   16.180587] bttv: 0: registered device radio0
> 
> When I load module ir-kbd-i2c manually:
> 
> Registered IR keymap rc-hauppauge
> input: i2c IR (Hauppauge) as /devices/virtual/rc/rc0/input6
> rc0: i2c IR (Hauppauge) as /devices/virtual/rc/rc0
> ir-kbd-i2c: i2c IR (Hauppauge) detected at i2c-4/4-0018/ir0 [bt878 #0 [sw]]
> 
> Remote control works fine then.

Yeah, this is a known misfeature of bttv, and very likely documented on
several wiki pages like those:
http://linuxtv.org/wiki/index.php/Remote_controllers-V4L
(btw, this wiki page is very likely outdated)
http://www.mythtv.org/wiki/Ir-kbd-i2c
http://www.linuxtv.org/wiki/index.php/Prolink_Pixelview_PlayTV_Pro

I can't remember if this were always this way, or if this was
caused by the I2C core redesign (2006?). I think it was always like that,
and, as I2C comes with a cost (polling can affect video processing with
old machines), so, we decided to not do the auto-load on the older
drivers that weren't doing it already.

Btw, the fact that there's no clear indication about what bttv boards
require I2C made hard to remove the get_key codes from ir-kbd-i2c.

It probably makes sense to add a "has_ir_i2c" field at bttv, add a flag
there at modprobe to prevent the autoload, and start tagging the boards
with I2C IR with such tag.

If you care enough, feel free to send us such patch.

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: [PATCH 10/15] em28xx: fix broken TRY_FMT.

2013-01-05 Thread Mauro Carvalho Chehab
Em Sat, 5 Jan 2013 14:34:04 +0100
Hans Verkuil  escreveu:

> On Sat January 5 2013 03:54:44 Mauro Carvalho Chehab wrote:
> > Hans/Devin,
> > 
> > Em Fri,  4 Jan 2013 15:59:40 -0500
> > Devin Heitmueller  escreveu:
> > 
> > > TRY_FMT should not return an error if a pixelformat is unsupported. 
> > > Instead just
> > > pick a common pixelformat.
> > > 
> > > Also the bytesperline calculation was incorrect: it used the old width 
> > > instead of
> > > the provided with, and it miscalculated the bytesperline value for the 
> > > depth == 12
> > > case.
> > > 
> > > Signed-off-by: Hans Verkuil 
> > > Signed-off-by: Devin Heitmueller 
> > > ---
> > >  drivers/media/usb/em28xx/em28xx-video.c |4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/media/usb/em28xx/em28xx-video.c 
> > > b/drivers/media/usb/em28xx/em28xx-video.c
> > > index a91a248..7c09b55 100644
> > > --- a/drivers/media/usb/em28xx/em28xx-video.c
> > > +++ b/drivers/media/usb/em28xx/em28xx-video.c
> > > @@ -821,7 +821,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, 
> > > void *priv,
> > >   if (!fmt) {
> > >   em28xx_videodbg("Fourcc format (%08x) invalid.\n",
> > >   f->fmt.pix.pixelformat);
> > > - return -EINVAL;
> > > + fmt = format_by_fourcc(V4L2_PIX_FMT_YUYV);
> > 
> > This change has the potential of causing userspace regressions, so,
> > for now, I won't apply such change.
> 
> Good!
> 
> > We need to discuss it better, before risk breaking things, and likely fix
> > applications first.
> 
> Absolutely. I also want to change this test in v4l2-compliance from 'failure'
> to 'warning' for the time being.

Sounds reasonable for me.

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


Re: [PATCH 0/4] Some IR fixes for I2C devices on em28xx

2013-01-05 Thread Mauro Carvalho Chehab
Em Sat, 05 Jan 2013 14:42:10 +0100
Frank Schäfer  escreveu:

> Am 05.01.2013 14:22, schrieb Frank Schäfer:
> > Am 04.01.2013 22:15, schrieb Mauro Carvalho Chehab:
> >> Frank pointed that IR was not working with I2C devices. So, I took some
> >> time to fix them.
> >>
> >> Tested with Hauppauge WinTV USB2.
> >>
> >> Mauro Carvalho Chehab (4):
> >>   [media] em28xx: initialize button/I2C IR earlier
> >>   [media] em28xx: autoload em28xx-rc if the device has an I2C IR
> >>   [media] em28xx: simplify IR names on I2C devices
> >>   [media] em28xx: tell ir-kbd-i2c that WinTV uses an RC5 protocol
> >>
> >>  drivers/media/usb/em28xx/em28xx-cards.c |  2 +-
> >>  drivers/media/usb/em28xx/em28xx-input.c | 29 -
> >>  2 files changed, 17 insertions(+), 14 deletions(-)
> >>
> > While these patches make I2C IR remote controls working again, they
> > leave several issues unaddressed which should really be fixed:
> > 1) the i2c client isn't unregistered on module unload. This was the
> > reason for patch 2 in my series. There is also a FIXME comment about
> > this in em28xx_release_resources() (although this is the wrong place to
> > do it).
> > 2) there is no error checking in em28xx_register_i2c_ir().
> > em28xx_ir_init should really bail out if no i2c device is found.
> > 3) All RC maps should be assigned at the same place, no matter if the
> > receiver/demodulator is built in or external. Spreading them over the
> > code is inconsistent and makes the code bug prone.
> > 4) the list of known i2c devices in em28xx-i2c.c misses client address
> > 0x3e >> 1 = 0x1f. See client list in em28xx_register_i2c_ir().
> > 5) there should be a warning message for the case that we call
> > ir-kbd-i2c with an unknown rc device.
> > 6) because we use our own key polling functions with ir-kbd-i2c, we
> > should also select the polling interval value manually. That makes
> > things consistent and avoids confusion.
> >
> > The rest is a matter of taste / prefered code layout. I'm fine with it.
> >
> > Regards,
> > Frank
> 
> It seems like already applied them... :(
> 
> While I certainly appreciate patches beeing applied as soon as possible,
> I think there should really be a chance to review them before this happens.
> Especially when the changes are non-trivial and someone else has posted
> patches addressing the same issues before (other contributers might feel
> offended ;) ).

All the 4 applied patches are really trivial:
- patch 1: just reorder existing code;
- patch 2: one-line patch adding another condition to an existing if;
- patch 3: pure string rename;
- patch 4: one line patch properly reporting the RC5 protocol on WinTV.

Also, my time is very limited, especially when I need to test a driver, as
I need to allocate a bigger time window. On such cases, I just reorder the
patches to to apply all of them at the same time, to optimize my time.

Also, both Devin and you are working right now at the same driver, and you
both have pending work. Merging the patches quicker helps to avoid merge
conflicts.

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


Re: [PATCH 6/6] ir-kbd-i2c: fix get_key_knc1()

2013-01-05 Thread Mauro Carvalho Chehab
Em Sat, 05 Jan 2013 14:32:30 +0100
Frank Schäfer  escreveu:

> Am 05.01.2013 03:39, schrieb Mauro Carvalho Chehab:
> > Em Fri, 28 Dec 2012 00:02:48 +0100
> > Frank Schäfer  escreveu:
> >
> >> - return valid key code when button is hold
> >> - debug: print key code only when a button is pressed
> >>
> >> Tested with device "Terratec Cinergy 200 USB" (em28xx).
> >>
> >> Signed-off-by: Frank Schäfer 
> >> ---
> >>  drivers/media/i2c/ir-kbd-i2c.c |   15 +--
> >>  1 Datei geändert, 5 Zeilen hinzugefügt(+), 10 Zeilen entfernt(-)
> >>
> >> diff --git a/drivers/media/i2c/ir-kbd-i2c.c 
> >> b/drivers/media/i2c/ir-kbd-i2c.c
> >> index 08ae067..2984b7d 100644
> >> --- a/drivers/media/i2c/ir-kbd-i2c.c
> >> +++ b/drivers/media/i2c/ir-kbd-i2c.c
> >> @@ -184,18 +184,13 @@ static int get_key_knc1(struct IR_i2c *ir, u32 
> >> *ir_key, u32 *ir_raw)
> >>return -EIO;
> >>}
> >>  
> >> -  /* it seems that 0xFE indicates that a button is still hold
> >> - down, while 0xff indicates that no button is hold
> >> - down. 0xfe sequences are sometimes interrupted by 0xFF */
> >> -
> >> -  dprintk(2,"key %02x\n", b);
> >> -
> >> -  if (b == 0xff)
> >> +  if (b == 0xff) /* no button */
> >>return 0;
> >>  
> >> -  if (b == 0xfe)
> >> -  /* keep old data */
> >> -  return 1;
> >> +  if (b == 0xfe) /* button is still hold */
> >> +  b = ir->rc->last_scancode; /* keep old data */
> >> +
> >> +  dprintk(2,"key %02x\n", b);
> >>  
> >>*ir_key = b;
> >>*ir_raw = b;
> > Don't do that. This piece of code is old, and it was added there 
> > before the em28xx driver. Originally, the ir-i2c-kbd were used by
> > bttv and saa7134 drivers and the code there were auto-detecting the
> > I2C IR hardware decoding chips that used to be very common on media
> > devices. I'm almost sure that the original device that started using
> > this code is this model:
> >
> > drivers/media/pci/bt8xx/bttv-cards.c: .name   = 
> > "Typhoon TView RDS + FM Stereo / KNC1 TV Station RDS",
> >
> > That's why it is called as KNC1, but there are other cards that use
> > it as well. I think I have one bttv using it. Not sure.
> >
> > The routine on em28xx is a fork of the original one, that was changed
> > to work with the devices there.
> 
> Indeed, it's a fork, 100% identical:
> 
> 
> static int em28xx_get_key_terratec(struct IR_i2c *ir, u32 *ir_key, u32
> *ir_raw)
> {
> unsigned char b;
> 
> /* poll IR chip */
> if (1 != i2c_master_recv(ir->c, &b, 1)) {
> i2cdprintk("read error\n");
> return -EIO;
> }
> 
> /* it seems that 0xFE indicates that a button is still hold
>down, while 0xff indicates that no button is hold
>down. 0xfe sequences are sometimes interrupted by 0xFF */
> 
> i2cdprintk("key %02x\n", b);
> 
> if (b == 0xff)
> return 0;
> 
> if (b == 0xfe)
> /* keep old data */
> return 1;
> 
> *ir_key = b;
> *ir_raw = b;
> return 1;
> }
> 
> 
> 
> 
> static int get_key_knc1(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
> {
> unsigned char b;
> 
> /* poll IR chip */
> if (1 != i2c_master_recv(ir->c, &b, 1)) {
> dprintk(1,"read error\n");
> return -EIO;
> }
> 
> /* it seems that 0xFE indicates that a button is still hold
>down, while 0xff indicates that no button is hold
>down. 0xfe sequences are sometimes interrupted by 0xFF */
> 
> dprintk(2,"key %02x\n", b);
> 
> if (b == 0xff)
> return 0;
> 
> if (b == 0xfe)
> /* keep old data */
> return 1;
> 
> *ir_key = b;
> *ir_raw = b;
> return 1;
> }
> 
> 
> 
> Why should we keep two 100% identical functions ? See patch 4/6.
> I'm 99% sure that both devices are absolutely identical.

99% sure is not enough. A simple firmware difference at the microprocessor
is enough to make the devices different.

Also, this was widely discussed several years ago, when we decided to cleanup
the I2C code. We then invested lot of efforts to move those get_keys away
from ir-i2c-kbd. The only things left there are the ones we identified that
were needed by auto-detection mode on old devices that we don't have.

What was decided is to move everything that we know to the *-input driver,
keeping there only the legacy stuff.

> Concerning the fix I'm suggesting here:
> First of all, I have to say that the Terratec RC works even without this
> patch.
> Nevertheless, I think the function should really return valid values for
> ir_key and ir_raw when 0xfe=button hold is received. Especially because
> the function succeeds.
> This also allows us to make u32 ir_key, ir_raw in ir_key_poll() in
> ir-kbd-i2c.c non-static.
> While I agree that we should be careful, I can't see how this can cause
> any trouble.

Ok, the net effect is the same, except that the current way is faster, as it
will skip some code that would simply put the value that it is already at
ir_key/ir_

Re: [PATCH 0/4] Some IR fixes for I2C devices on em28xx

2013-01-05 Thread Mauro Carvalho Chehab
Em Sat, 05 Jan 2013 14:22:08 +0100
Frank Schäfer  escreveu:

> Am 04.01.2013 22:15, schrieb Mauro Carvalho Chehab:
> > Frank pointed that IR was not working with I2C devices. So, I took some
> > time to fix them.
> >
> > Tested with Hauppauge WinTV USB2.
> >
> > Mauro Carvalho Chehab (4):
> >   [media] em28xx: initialize button/I2C IR earlier
> >   [media] em28xx: autoload em28xx-rc if the device has an I2C IR
> >   [media] em28xx: simplify IR names on I2C devices
> >   [media] em28xx: tell ir-kbd-i2c that WinTV uses an RC5 protocol
> >
> >  drivers/media/usb/em28xx/em28xx-cards.c |  2 +-
> >  drivers/media/usb/em28xx/em28xx-input.c | 29 -
> >  2 files changed, 17 insertions(+), 14 deletions(-)
> >
> 
> While these patches make I2C IR remote controls working again, they
> leave several issues unaddressed which should really be fixed:
> 1) the i2c client isn't unregistered on module unload. This was the
> reason for patch 2 in my series. There is also a FIXME comment about
> this in em28xx_release_resources() (although this is the wrong place to
> do it).

AFAIKT, this is not really needed, as the I2C clients are unregistered
when the I2C bus is unregistered.

So, a device disconnect will release it. Also, an em28xx driver unload.

The only difference might be if just ir-kbd-i2c and em28xx-rc are
unloaded, but em28xx is still loaded, but I think that, even on this
case, calling the .release code for an I2C bus will release it.

So, I don't see any need for such patch. I might be wrong, of course, but,
in order to proof that a release code is needed, you'll need to check if
some memory are lost after module load/unload.

> 2) there is no error checking in em28xx_register_i2c_ir().
> em28xx_ir_init should really bail out if no i2c device is found.

A failure to initialize IR should not be fatal for the driver, as the
rest of the hardware still works.

Also, there's no way to warrant that the I2C code is actually running,
as ir-i2c-kbd may not even be compiled.

So, returning 0 there doesn't mean that IR is working.

> 3) All RC maps should be assigned at the same place, no matter if the
> receiver/demodulator is built in or external. Spreading them over the
> code is inconsistent and makes the code bug prone.

I don't agree. It is better to keep RC maps for those devices together
with the RC protocol setting, get_key config, etc. At boards config,
it is very easy to identify I2C IR's, as there's an special field there
to mark those devices (has_ir_i2c). So, if the board has_ir_i2c, the
IR config is inside em28xx-input. That's the same logic that it is
there for has_dvb: if this field is true, the DVB specifics is inside
em28xx-dvb.

> 4) the list of known i2c devices in em28xx-i2c.c misses client address
> 0x3e >> 1 = 0x1f. See client list in em28xx_register_i2c_ir().

Ok. Separate patch, please.

> 5) there should be a warning message for the case that we call
> ir-kbd-i2c with an unknown rc device.

Why? All boards with has_ir_i2c have entries there. I double-checked.
Adding will just bloat the code with no reason. We just need to take
care if we get a patch adding I2C IR support for an old card, to be
sure that data is filled on both places.

Considering that we don't receive any IR I2C code for several years,
and that newer devices won't use that part of the code, it seems highly
unlikely that such code would be ever used.

> 6) because we use our own key polling functions with ir-kbd-i2c, we
> should also select the polling interval value manually. That makes
> things consistent and avoids confusion.

I disagree. The polling interval is mainly dictated by the RC protocol
used (e. g. the minimal time for a repeat code) and by the speed that 
users can type things. It is typically ~100 ms everywhere, except when
there are some exceptional cases, like GPIO polling.

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


Re: [BUG] Problem with LV5TDLX DVB-T USB and the 3.7.1 kernel

2013-01-05 Thread Antti Palosaari

On 01/05/2013 04:05 PM, Jacek Konieczny wrote:

Hi,

I have a 'NOT Only TV DVB-T USB Deluxe' tuner device:

Model name: LV5TDLX DVB-T USB
P/N: STLV5TDLXT702
S/N: LV5TDLX120700116
USB ID: 1f4d:c803

This is based on the RTL2838UHIDIR chip with e4000 tuner (at least, that
is detected by various drivers).

I had some minor success with it with some old 3.x kernel and the
drivers from:

https://github.com/tmair/DVB-Realtek-RTL2832U-2.2.2-10tuner-mod_kernel-3.0.0

This stopped working with kernel 3.5 and would not even build with newer
kernels.

Then I tried drivers from linuxtv.org, with little success. The RTL2838u
driver has been recently included in the upstream kernel (3.7), so I
have tried that (3.7.1). The hardware is detected, but I am not able to
tune in.

The signal is good - tested with my TV set. The USB tuner device is also
OK, I have tried it with Windows and the software provided with the
device and the same channels are available as on the TV.

So the driver must be broken. Any ideas how can I debug or fix that?

dmesg:

[ 3336.916384] usb 2-4: new high-speed USB device number 7 using ehci_hcd
[ 3337.051822] usb 2-4: New USB device found, idVendor=1f4d, idProduct=c803
[ 3337.051829] usb 2-4: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 3337.051835] usb 2-4: Product: RTL2838UHIDIR
[ 3337.051839] usb 2-4: Manufacturer: Realtek
[ 3337.051843] usb 2-4: SerialNumber: 0001
[ 3337.072145] usb 2-4: dvb_usb_v2: found a 'Trekstor DVB-T Stick Terres 2.0' 
in warm state
[ 3337.072194] usbcore: registered new interface driver dvb_usb_rtl28xxu
[ 3337.136867] usb 2-4: dvb_usb_v2: will pass the complete MPEG2 transport 
stream to the software demuxer
[ 3337.136886] DVB: registering new adapter (Trekstor DVB-T Stick Terres 2.0)
[ 3337.147449] usb 2-4: DVB: registering adapter 0 frontend 0 (Realtek RTL2832 
(DVB-T))...
[ 3337.163939] i2c i2c-7: e4000: Elonics E4000 successfully identified
[ 3337.174823] Registered IR keymap rc-empty
[ 3337.174928] input: Trekstor DVB-T Stick Terres 2.0 as 
/devices/pci:00/:00:1d.7/usb2/2-4/rc/rc0/input15
[ 3337.174989] rc0: Trekstor DVB-T Stick Terres 2.0 as 
/devices/pci:00/:00:1d.7/usb2/2-4/rc/rc0
[ 3337.174994] usb 2-4: dvb_usb_v2: schedule remote query interval to 400 msecs
[ 3337.187693] usb 2-4: dvb_usb_v2: 'Trekstor DVB-T Stick Terres 2.0' 
successfully initialized and connected


Scanning on one of the available channels:

# tzap -r "TVP2"
using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0'
reading channels from file '/root/.tzap/channels.conf'
tuning to 74600 Hz
video pid 0x00ca, audio pid 0x00cb
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |
status 00 | signal bfe1 | snr 008c | ber 4ca0 | unc bfe14648 |
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |
status 00 | signal bfe1 | snr 008d | ber 4ca0 | unc bfe14648 |
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |
status 00 | signal bfe1 | snr 0072 | ber 4ca0 | unc bfe14648 |
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |
status 00 | signal bfe1 | snr 008b | ber 4ca0 | unc bfe14648 |
status 00 | signal bfe1 | snr  | ber  | unc bfe14648 |


And on the other one:

# tzap -r "Polsat"
using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0'
reading channels from file '/root/.tzap/channels.conf'
tuning to 69800 Hz
video pid 0x0066, audio pid 0x0067
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |
status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 |


Greets,
Jacek



It is likely e4000 driver bug. It is not optimized nor tested very well 
- just few live multiplexes I have here. You are the first one reporting 
(performance?) issues like that, I am quite sure it works somehow well 
for the most.


Take USB sniffs, make scripts to generate e4000 register write code from

[BUG] Problem with LV5TDLX DVB-T USB and the 3.7.1 kernel

2013-01-05 Thread Jacek Konieczny
Hi,

I have a 'NOT Only TV DVB-T USB Deluxe' tuner device:

Model name: LV5TDLX DVB-T USB
P/N: STLV5TDLXT702
S/N: LV5TDLX120700116
USB ID: 1f4d:c803

This is based on the RTL2838UHIDIR chip with e4000 tuner (at least, that
is detected by various drivers).

I had some minor success with it with some old 3.x kernel and the
drivers from:

https://github.com/tmair/DVB-Realtek-RTL2832U-2.2.2-10tuner-mod_kernel-3.0.0

This stopped working with kernel 3.5 and would not even build with newer
kernels.

Then I tried drivers from linuxtv.org, with little success. The RTL2838u
driver has been recently included in the upstream kernel (3.7), so I
have tried that (3.7.1). The hardware is detected, but I am not able to
tune in. 

The signal is good - tested with my TV set. The USB tuner device is also
OK, I have tried it with Windows and the software provided with the
device and the same channels are available as on the TV.

So the driver must be broken. Any ideas how can I debug or fix that?

dmesg:
> [ 3336.916384] usb 2-4: new high-speed USB device number 7 using ehci_hcd
> [ 3337.051822] usb 2-4: New USB device found, idVendor=1f4d, idProduct=c803
> [ 3337.051829] usb 2-4: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=3
> [ 3337.051835] usb 2-4: Product: RTL2838UHIDIR
> [ 3337.051839] usb 2-4: Manufacturer: Realtek
> [ 3337.051843] usb 2-4: SerialNumber: 0001
> [ 3337.072145] usb 2-4: dvb_usb_v2: found a 'Trekstor DVB-T Stick Terres 2.0' 
> in warm state
> [ 3337.072194] usbcore: registered new interface driver dvb_usb_rtl28xxu
> [ 3337.136867] usb 2-4: dvb_usb_v2: will pass the complete MPEG2 transport 
> stream to the software demuxer
> [ 3337.136886] DVB: registering new adapter (Trekstor DVB-T Stick Terres 2.0)
> [ 3337.147449] usb 2-4: DVB: registering adapter 0 frontend 0 (Realtek 
> RTL2832 (DVB-T))...
> [ 3337.163939] i2c i2c-7: e4000: Elonics E4000 successfully identified
> [ 3337.174823] Registered IR keymap rc-empty
> [ 3337.174928] input: Trekstor DVB-T Stick Terres 2.0 as 
> /devices/pci:00/:00:1d.7/usb2/2-4/rc/rc0/input15
> [ 3337.174989] rc0: Trekstor DVB-T Stick Terres 2.0 as 
> /devices/pci:00/:00:1d.7/usb2/2-4/rc/rc0
> [ 3337.174994] usb 2-4: dvb_usb_v2: schedule remote query interval to 400 
> msecs
> [ 3337.187693] usb 2-4: dvb_usb_v2: 'Trekstor DVB-T Stick Terres 2.0' 
> successfully initialized and connected

Scanning on one of the available channels:
> # tzap -r "TVP2" 
> using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0'
> reading channels from file '/root/.tzap/channels.conf'
> tuning to 74600 Hz
> video pid 0x00ca, audio pid 0x00cb
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 
> status 00 | signal bfe1 | snr 008c | ber 4ca0 | unc bfe14648 | 
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 
> status 00 | signal bfe1 | snr 008d | ber 4ca0 | unc bfe14648 | 
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 
> status 00 | signal bfe1 | snr 0072 | ber 4ca0 | unc bfe14648 | 
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 
> status 00 | signal bfe1 | snr 008b | ber 4ca0 | unc bfe14648 | 
> status 00 | signal bfe1 | snr  | ber  | unc bfe14648 | 

And on the other one:
> # tzap -r "Polsat"
> using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0'
> reading channels from file '/root/.tzap/channels.conf'
> tuning to 69800 Hz
> video pid 0x0066, audio pid 0x0067
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 
> status 00 | signal bfb5 | snr  | ber  | unc bfb5f4d8 | 

Greets,
Jacek
--
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


BUG: bttv does not load module ir-kbd-i2c for Hauppauge model 37284, rev B421

2013-01-05 Thread Frank Schäfer
While we are at it ;) :

[   15.280772] bttv: Bt8xx card found (0)
[   15.281349] bttv: 0: Bt878 (rev 17) at :01:08.0, irq: 18,
latency: 32, mmio: 0xfdfff000
[   15.281386] bttv: 0: detected: Hauppauge WinTV [card=10], PCI
subsystem ID is 0070:13eb
[   15.281391] bttv: 0: using: Hauppauge (bt878) [card=10,insmod option]
[   15.283964] bttv: 0: Hauppauge/Voodoo msp34xx: reset line init [5]
[   15.316043] tveeprom 4-0050: Hauppauge model 37284, rev B421, serial#
5111944
[   15.316049] tveeprom 4-0050: tuner model is Philips FM1216 (idx 21,
type 5)
[   15.316054] tveeprom 4-0050: TV standards PAL(B/G) (eeprom 0x04)
[   15.316059] tveeprom 4-0050: audio processor is MSP3410D (idx 5)
[   15.316063] tveeprom 4-0050: has radio
[   15.316066] bttv: 0: Hauppauge eeprom indicates model#37284
[   15.316071] bttv: 0: tuner type=5
[   16.178816] bttv: 0: registered device video0
[   16.179071] bttv: 0: registered device vbi0
[   16.180587] bttv: 0: registered device radio0

When I load module ir-kbd-i2c manually:

Registered IR keymap rc-hauppauge
input: i2c IR (Hauppauge) as /devices/virtual/rc/rc0/input6
rc0: i2c IR (Hauppauge) as /devices/virtual/rc/rc0
ir-kbd-i2c: i2c IR (Hauppauge) detected at i2c-4/4-0018/ir0 [bt878 #0 [sw]]

Remote control works fine then.

Regards,
rank


--
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/6] em28xx: make remote controls of devices with external IR IC working again

2013-01-05 Thread Frank Schäfer
Am 05.01.2013 14:26, schrieb Mauro Carvalho Chehab:
> Em Sat, 05 Jan 2013 13:58:20 +0100
> Frank Schäfer  escreveu:
>
>> Am 04.01.2013 22:12, schrieb Mauro Carvalho Chehab:
>>> Em Fri, 28 Dec 2012 00:02:45 +0100
>>> Frank Schäfer  escreveu:
>>>
 Tested with device "Terratec Cinergy 200 USB".
>>> Sorry, but this patch is completely wrong ;)
>> Completely wrong ? That's not helpful...
>> Please elaborate a bit more on this so that I can do things right next
>> time. ;)
> Sorry, I was too busy yesterday with the tests to elaborate it more.
>
> In general, big patches like that to fix bug fixes are generally wrong:
> they touch on a lot of the code and it is hard to be sure that it doesn't
> come with regressions on it.

Ok, I think you are right.
The patch description was definitely insufficient. I should have
explained the issues I tried to address in more details.
Maybe I should have split this into several smaller patches, too.

> In this particular case, it was:
>   - mixing bug fixes with some other random stuff;
>   - moving only one part of the IR needed data elsewhere (it were
> moving the IR tables, to the board descriptions, keeping them on
> a separate part of the code, obfuscating the code);
>   - putting a large amount of the code inside an if, increasing the
> driver's complexity with no need;
>   - initializing some data for IR that are never used, at em28xx_ir_init;
>   - not fixing the snapshot button.
>
> The bug fix was as simple as:
>   1) move snapshot button register to happen before IR;
>   2) move I2C init to happen before the em2860/2874 IR init.

See the mail I've sent a few minutes ago.

> ...
>
> Btw, I really prefer to have the RC tables for the I2C devices inside
> em28xx-input, as:
>
>   1) there are other board-specific platform_data that needed to
>  be filled for the IR to work there;

Sure.

>   2) we want to keep all those platform_data initialization together,
>  to make the code simpler to maintain;

platform_data initialization is kept together, no changes here.
To me it seems to be important to keep all _board_ specific stuff
together as much as possible.

>   3) moving all those data to em28xx cards struct is a bad idea, as
>  newer em28xx won't use I2C IR's, as the new chipsets have already
>  its own IR decoder. Moving those 4-5 fields to the board struct
>  would increase its size for every board. So, it would be a waste
>  of space.

Im my opinion, having board specifc code spread all over the code is a
desease. It makes the code bug prone.
Actually, this was one of the reasons why the i2c rc got broken...
Sure, it's hard to avoid, especially for the DVB stuff. But we should at
least reduce it to a minimum.

For the RC map, it's easy to do this, as the corresponding field is
already there.

Regards,
Frank

>
> Regards,
> Mauro

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


Re: [PATCH 0/4] Some IR fixes for I2C devices on em28xx

2013-01-05 Thread Frank Schäfer
Am 05.01.2013 14:22, schrieb Frank Schäfer:
> Am 04.01.2013 22:15, schrieb Mauro Carvalho Chehab:
>> Frank pointed that IR was not working with I2C devices. So, I took some
>> time to fix them.
>>
>> Tested with Hauppauge WinTV USB2.
>>
>> Mauro Carvalho Chehab (4):
>>   [media] em28xx: initialize button/I2C IR earlier
>>   [media] em28xx: autoload em28xx-rc if the device has an I2C IR
>>   [media] em28xx: simplify IR names on I2C devices
>>   [media] em28xx: tell ir-kbd-i2c that WinTV uses an RC5 protocol
>>
>>  drivers/media/usb/em28xx/em28xx-cards.c |  2 +-
>>  drivers/media/usb/em28xx/em28xx-input.c | 29 -
>>  2 files changed, 17 insertions(+), 14 deletions(-)
>>
> While these patches make I2C IR remote controls working again, they
> leave several issues unaddressed which should really be fixed:
> 1) the i2c client isn't unregistered on module unload. This was the
> reason for patch 2 in my series. There is also a FIXME comment about
> this in em28xx_release_resources() (although this is the wrong place to
> do it).
> 2) there is no error checking in em28xx_register_i2c_ir().
> em28xx_ir_init should really bail out if no i2c device is found.
> 3) All RC maps should be assigned at the same place, no matter if the
> receiver/demodulator is built in or external. Spreading them over the
> code is inconsistent and makes the code bug prone.
> 4) the list of known i2c devices in em28xx-i2c.c misses client address
> 0x3e >> 1 = 0x1f. See client list in em28xx_register_i2c_ir().
> 5) there should be a warning message for the case that we call
> ir-kbd-i2c with an unknown rc device.
> 6) because we use our own key polling functions with ir-kbd-i2c, we
> should also select the polling interval value manually. That makes
> things consistent and avoids confusion.
>
> The rest is a matter of taste / prefered code layout. I'm fine with it.
>
> Regards,
> Frank

It seems like already applied them... :(

While I certainly appreciate patches beeing applied as soon as possible,
I think there should really be a chance to review them before this happens.
Especially when the changes are non-trivial and someone else has posted
patches addressing the same issues before (other contributers might feel
offended ;) ).

Care to fix these issues ?

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


Re: [PATCH 10/15] em28xx: fix broken TRY_FMT.

2013-01-05 Thread Hans Verkuil
On Sat January 5 2013 03:54:44 Mauro Carvalho Chehab wrote:
> Hans/Devin,
> 
> Em Fri,  4 Jan 2013 15:59:40 -0500
> Devin Heitmueller  escreveu:
> 
> > TRY_FMT should not return an error if a pixelformat is unsupported. Instead 
> > just
> > pick a common pixelformat.
> > 
> > Also the bytesperline calculation was incorrect: it used the old width 
> > instead of
> > the provided with, and it miscalculated the bytesperline value for the 
> > depth == 12
> > case.
> > 
> > Signed-off-by: Hans Verkuil 
> > Signed-off-by: Devin Heitmueller 
> > ---
> >  drivers/media/usb/em28xx/em28xx-video.c |4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/usb/em28xx/em28xx-video.c 
> > b/drivers/media/usb/em28xx/em28xx-video.c
> > index a91a248..7c09b55 100644
> > --- a/drivers/media/usb/em28xx/em28xx-video.c
> > +++ b/drivers/media/usb/em28xx/em28xx-video.c
> > @@ -821,7 +821,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, 
> > void *priv,
> > if (!fmt) {
> > em28xx_videodbg("Fourcc format (%08x) invalid.\n",
> > f->fmt.pix.pixelformat);
> > -   return -EINVAL;
> > +   fmt = format_by_fourcc(V4L2_PIX_FMT_YUYV);
> 
> This change has the potential of causing userspace regressions, so,
> for now, I won't apply such change.

Good!

> We need to discuss it better, before risk breaking things, and likely fix
> applications first.

Absolutely. I also want to change this test in v4l2-compliance from 'failure'
to 'warning' for the time being.

Regards,

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


Re: [PATCH 6/6] ir-kbd-i2c: fix get_key_knc1()

2013-01-05 Thread Frank Schäfer
Am 05.01.2013 03:39, schrieb Mauro Carvalho Chehab:
> Em Fri, 28 Dec 2012 00:02:48 +0100
> Frank Schäfer  escreveu:
>
>> - return valid key code when button is hold
>> - debug: print key code only when a button is pressed
>>
>> Tested with device "Terratec Cinergy 200 USB" (em28xx).
>>
>> Signed-off-by: Frank Schäfer 
>> ---
>>  drivers/media/i2c/ir-kbd-i2c.c |   15 +--
>>  1 Datei geändert, 5 Zeilen hinzugefügt(+), 10 Zeilen entfernt(-)
>>
>> diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
>> index 08ae067..2984b7d 100644
>> --- a/drivers/media/i2c/ir-kbd-i2c.c
>> +++ b/drivers/media/i2c/ir-kbd-i2c.c
>> @@ -184,18 +184,13 @@ static int get_key_knc1(struct IR_i2c *ir, u32 
>> *ir_key, u32 *ir_raw)
>>  return -EIO;
>>  }
>>  
>> -/* it seems that 0xFE indicates that a button is still hold
>> -   down, while 0xff indicates that no button is hold
>> -   down. 0xfe sequences are sometimes interrupted by 0xFF */
>> -
>> -dprintk(2,"key %02x\n", b);
>> -
>> -if (b == 0xff)
>> +if (b == 0xff) /* no button */
>>  return 0;
>>  
>> -if (b == 0xfe)
>> -/* keep old data */
>> -return 1;
>> +if (b == 0xfe) /* button is still hold */
>> +b = ir->rc->last_scancode; /* keep old data */
>> +
>> +dprintk(2,"key %02x\n", b);
>>  
>>  *ir_key = b;
>>  *ir_raw = b;
> Don't do that. This piece of code is old, and it was added there 
> before the em28xx driver. Originally, the ir-i2c-kbd were used by
> bttv and saa7134 drivers and the code there were auto-detecting the
> I2C IR hardware decoding chips that used to be very common on media
> devices. I'm almost sure that the original device that started using
> this code is this model:
>
> drivers/media/pci/bt8xx/bttv-cards.c: .name   = "Typhoon 
> TView RDS + FM Stereo / KNC1 TV Station RDS",
>
> That's why it is called as KNC1, but there are other cards that use
> it as well. I think I have one bttv using it. Not sure.
>
> The routine on em28xx is a fork of the original one, that was changed
> to work with the devices there.

Indeed, it's a fork, 100% identical:


static int em28xx_get_key_terratec(struct IR_i2c *ir, u32 *ir_key, u32
*ir_raw)
{
unsigned char b;

/* poll IR chip */
if (1 != i2c_master_recv(ir->c, &b, 1)) {
i2cdprintk("read error\n");
return -EIO;
}

/* it seems that 0xFE indicates that a button is still hold
   down, while 0xff indicates that no button is hold
   down. 0xfe sequences are sometimes interrupted by 0xFF */

i2cdprintk("key %02x\n", b);

if (b == 0xff)
return 0;

if (b == 0xfe)
/* keep old data */
return 1;

*ir_key = b;
*ir_raw = b;
return 1;
}




static int get_key_knc1(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
{
unsigned char b;

/* poll IR chip */
if (1 != i2c_master_recv(ir->c, &b, 1)) {
dprintk(1,"read error\n");
return -EIO;
}

/* it seems that 0xFE indicates that a button is still hold
   down, while 0xff indicates that no button is hold
   down. 0xfe sequences are sometimes interrupted by 0xFF */

dprintk(2,"key %02x\n", b);

if (b == 0xff)
return 0;

if (b == 0xfe)
/* keep old data */
return 1;

*ir_key = b;
*ir_raw = b;
return 1;
}



Why should we keep two 100% identical functions ? See patch 4/6.
I'm 99% sure that both devices are absolutely identical.

Concerning the fix I'm suggesting here:
First of all, I have to say that the Terratec RC works even without this
patch.
Nevertheless, I think the function should really return valid values for
ir_key and ir_raw when 0xfe=button hold is received. Especially because
the function succeeds.
This also allows us to make u32 ir_key, ir_raw in ir_key_poll() in
ir-kbd-i2c.c non-static.
While I agree that we should be careful, I can't see how this can cause
any trouble.

The second thing is the small fix for the key code debug output. Don't
you think it makes sense ?

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


Re: [PATCH] ngene: fix commit 36a495a336c3fbbb2f4eeed2a94ab6d5be19d186

2013-01-05 Thread Mauro Carvalho Chehab
Em Sat, 05 Jan 2013 10:39:21 +0100
Patrice Chotard  escreveu:

> Hi Mauro,
> 
> Yes, i confirm that without this patch, tuner_attach_dtt7520x() callback
> was never called, so no tuning was possible.

Thanks for double-checking. Not sure why the original patch got truncated.
Maybe due to some bad conflict solving, or maybe patchwork mangled the
original patch. That sometimes happen when one hunk description has more
than 80-cols and the email got word-wrapped.

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


Re: [PATCH 3/6] em28xx: make remote controls of devices with external IR IC working again

2013-01-05 Thread Mauro Carvalho Chehab
Em Sat, 05 Jan 2013 13:58:20 +0100
Frank Schäfer  escreveu:

> Am 04.01.2013 22:12, schrieb Mauro Carvalho Chehab:
> > Em Fri, 28 Dec 2012 00:02:45 +0100
> > Frank Schäfer  escreveu:
> >
> >> Tested with device "Terratec Cinergy 200 USB".
> > Sorry, but this patch is completely wrong ;)
> 
> Completely wrong ? That's not helpful...
> Please elaborate a bit more on this so that I can do things right next
> time. ;)

Sorry, I was too busy yesterday with the tests to elaborate it more.

In general, big patches like that to fix bug fixes are generally wrong:
they touch on a lot of the code and it is hard to be sure that it doesn't
come with regressions on it.

In this particular case, it was:
- mixing bug fixes with some other random stuff;
- moving only one part of the IR needed data elsewhere (it were
  moving the IR tables, to the board descriptions, keeping them on
  a separate part of the code, obfuscating the code);
- putting a large amount of the code inside an if, increasing the
  driver's complexity with no need;
- initializing some data for IR that are never used, at em28xx_ir_init;
- not fixing the snapshot button.

The bug fix was as simple as:
1) move snapshot button register to happen before IR;
2) move I2C init to happen before the em2860/2874 IR init.

See:

http://git.linuxtv.org/media_tree.git/commitdiff/8303dc9952758ab3060a3ee9a19ecb6fec83c600

That's simple to review, and the produced patch can be accepted later at
stable, because it is not a code rewrite.

Of course, if we backport this to -stable, this one is also recommended:

http://git.linuxtv.org/media_tree.git/commitdiff/728f9778e273a11a65926ac21574e6ca8d911ebf

in order to autoload the RC extension automatically for I2C IR's, but this
is a separate bug.

Btw, I really prefer to have the RC tables for the I2C devices inside
em28xx-input, as:

1) there are other board-specific platform_data that needed to
   be filled for the IR to work there;

2) we want to keep all those platform_data initialization together,
   to make the code simpler to maintain;

3) moving all those data to em28xx cards struct is a bad idea, as
   newer em28xx won't use I2C IR's, as the new chipsets have already
   its own IR decoder. Moving those 4-5 fields to the board struct
   would increase its size for every board. So, it would be a waste
   of space.

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


Re: [PATCH 0/4] Some IR fixes for I2C devices on em28xx

2013-01-05 Thread Frank Schäfer
Am 04.01.2013 22:15, schrieb Mauro Carvalho Chehab:
> Frank pointed that IR was not working with I2C devices. So, I took some
> time to fix them.
>
> Tested with Hauppauge WinTV USB2.
>
> Mauro Carvalho Chehab (4):
>   [media] em28xx: initialize button/I2C IR earlier
>   [media] em28xx: autoload em28xx-rc if the device has an I2C IR
>   [media] em28xx: simplify IR names on I2C devices
>   [media] em28xx: tell ir-kbd-i2c that WinTV uses an RC5 protocol
>
>  drivers/media/usb/em28xx/em28xx-cards.c |  2 +-
>  drivers/media/usb/em28xx/em28xx-input.c | 29 -
>  2 files changed, 17 insertions(+), 14 deletions(-)
>

While these patches make I2C IR remote controls working again, they
leave several issues unaddressed which should really be fixed:
1) the i2c client isn't unregistered on module unload. This was the
reason for patch 2 in my series. There is also a FIXME comment about
this in em28xx_release_resources() (although this is the wrong place to
do it).
2) there is no error checking in em28xx_register_i2c_ir().
em28xx_ir_init should really bail out if no i2c device is found.
3) All RC maps should be assigned at the same place, no matter if the
receiver/demodulator is built in or external. Spreading them over the
code is inconsistent and makes the code bug prone.
4) the list of known i2c devices in em28xx-i2c.c misses client address
0x3e >> 1 = 0x1f. See client list in em28xx_register_i2c_ir().
5) there should be a warning message for the case that we call
ir-kbd-i2c with an unknown rc device.
6) because we use our own key polling functions with ir-kbd-i2c, we
should also select the polling interval value manually. That makes
things consistent and avoids confusion.

The rest is a matter of taste / prefered code layout. I'm fine with it.

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


Re: [PATCH 3/6] em28xx: make remote controls of devices with external IR IC working again

2013-01-05 Thread Frank Schäfer
Am 04.01.2013 22:12, schrieb Mauro Carvalho Chehab:
> Em Fri, 28 Dec 2012 00:02:45 +0100
> Frank Schäfer  escreveu:
>
>> Tested with device "Terratec Cinergy 200 USB".
> Sorry, but this patch is completely wrong ;)

Completely wrong ? That's not helpful...
Please elaborate a bit more on this so that I can do things right next
time. ;)

Regards,
Frank

> The fix here is simple: just move the initialization to happen
> earlier.
>
> I'm posting it right now, together with a bunch of other fixes for
> I2C-based IR devices.
>
> Regards,
> Mauro
>> Signed-off-by: Frank Schäfer 
>> ---
>>  drivers/media/usb/em28xx/em28xx-cards.c |9 +-
>>  drivers/media/usb/em28xx/em28xx-i2c.c   |1 +
>>  drivers/media/usb/em28xx/em28xx-input.c |  142 
>> +--
>>  3 Dateien geändert, 83 Zeilen hinzugefügt(+), 69 Zeilen entfernt(-)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
>> b/drivers/media/usb/em28xx/em28xx-cards.c
>> index 40c3e45..3b226b1 100644
>> --- a/drivers/media/usb/em28xx/em28xx-cards.c
>> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
>> @@ -488,6 +488,7 @@ struct em28xx_board em28xx_boards[] = {
>>  .name = "Terratec Cinergy 250 USB",
>>  .tuner_type   = TUNER_LG_PAL_NEW_TAPC,
>>  .has_ir_i2c   = 1,
>> +.ir_codes = RC_MAP_EM_TERRATEC,
>>  .tda9887_conf = TDA9887_PRESENT,
>>  .decoder  = EM28XX_SAA711X,
>>  .input= { {
>> @@ -508,6 +509,7 @@ struct em28xx_board em28xx_boards[] = {
>>  .name = "Pinnacle PCTV USB 2",
>>  .tuner_type   = TUNER_LG_PAL_NEW_TAPC,
>>  .has_ir_i2c   = 1,
>> +.ir_codes = RC_MAP_PINNACLE_GREY,
>>  .tda9887_conf = TDA9887_PRESENT,
>>  .decoder  = EM28XX_SAA711X,
>>  .input= { {
>> @@ -533,6 +535,7 @@ struct em28xx_board em28xx_boards[] = {
>>  .decoder  = EM28XX_TVP5150,
>>  .has_msp34xx  = 1,
>>  .has_ir_i2c   = 1,
>> +.ir_codes = RC_MAP_HAUPPAUGE,
>>  .input= { {
>>  .type = EM28XX_VMUX_TELEVISION,
>>  .vmux = TVP5150_COMPOSITE0,
>> @@ -629,6 +632,7 @@ struct em28xx_board em28xx_boards[] = {
>>  .valid= EM28XX_BOARD_NOT_VALIDATED,
>>  .tuner_type   = TUNER_PHILIPS_FM1216ME_MK3,
>>  .has_ir_i2c   = 1,
>> +.ir_codes = RC_MAP_WINFAST_USBII_DELUXE,
>>  .tvaudio_addr = 0x58,
>>  .tda9887_conf = TDA9887_PRESENT |
>>  TDA9887_PORT2_ACTIVE |
>> @@ -1222,6 +1226,7 @@ struct em28xx_board em28xx_boards[] = {
>>  .name = "Terratec Cinergy 200 USB",
>>  .is_em2800= 1,
>>  .has_ir_i2c   = 1,
>> +.ir_codes = RC_MAP_EM_TERRATEC,
>>  .tuner_type   = TUNER_LG_TALN,
>>  .tda9887_conf = TDA9887_PRESENT,
>>  .decoder  = EM28XX_SAA711X,
>> @@ -2912,7 +2917,7 @@ static void request_module_async(struct work_struct 
>> *work)
>>  
>>  if (dev->board.has_dvb)
>>  request_module("em28xx-dvb");
>> -if (dev->board.ir_codes && !disable_ir)
>> +if ((dev->board.ir_codes || dev->board.has_ir_i2c) && !disable_ir)
>>  request_module("em28xx-rc");
>>  #endif /* CONFIG_MODULES */
>>  }
>> @@ -2935,8 +2940,6 @@ static void flush_request_modules(struct em28xx *dev)
>>  */
>>  void em28xx_release_resources(struct em28xx *dev)
>>  {
>> -/*FIXME: I2C IR should be disconnected */
>> -
>>  em28xx_release_analog_resources(dev);
>>  
>>  em28xx_i2c_unregister(dev);
>> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
>> b/drivers/media/usb/em28xx/em28xx-i2c.c
>> index 44533e4..39c5a3e 100644
>> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
>> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
>> @@ -470,6 +470,7 @@ static struct i2c_client em28xx_client_template = {
>>  static char *i2c_devs[128] = {
>>  [0x4a >> 1] = "saa7113h",
>>  [0x52 >> 1] = "drxk",
>> +[0x3e >> 1] = "remote IR sensor",
>>  [0x60 >> 1] = "remote IR sensor",
>>  [0x8e >> 1] = "remote IR sensor",
>>  [0x86 >> 1] = "tda9887",
>> diff --git a/drivers/media/usb/em28xx/em28xx-input.c 
>> b/drivers/media/usb/em28xx/em28xx-input.c
>> index 3598221..631e252 100644
>> --- a/drivers/media/usb/em28xx/em28xx-input.c
>> +++ b/drivers/media/usb/em28xx/em28xx-input.c
>> @@ -5,6 +5,7 @@
>>Markus Rechberger 
>>Mauro Carvalho Chehab 
>>Sascha Sommer 
>> +   Copyright (C) 2012 Frank Schäfer 
>>  
>>This program is free software; you can redistribute it and/or modify
>>it under the terms of the GNU General Public License as published by
>> @@ -34,6 +35,8 @@
>>  #define EM28XX_SBUTTON_QUERY_INTERVAL 500
>>  #define EM28XX_R0C_USBSUSP_SNAPSHOT 0x20
>>  
>> +#define

Re: [GIT PULL FOR 3.9] em28xx videobuf2 support and v4l2-compliance fixes

2013-01-05 Thread Frank Schäfer
Am 04.01.2013 20:41, schrieb Devin Heitmueller:
> Hello Mauro,
>
> Please pull the following series for 3.9, which ports em28xx to VB2 as
> well as applying Hans Verkuil's v4l2-compliance fixes for em28xx.
>
> Thanks,
>
> Devin
>
> The following changes since commit 8cd7085ff460ead3aba6174052a408f4ad52ac36:
>
>   [media] get_dvb_firmware: Fix the location of firmware for Terratec
> HTC (2013-01-01 11:18:26 -0200)
>
> are available in the git repository at:
>
>   git://git.kernellabs.com/dheitmueller/linuxtv.git v39staging
>
> for you to fetch changes up to 381abfc158c2dad81a558a3d3ff924fc7f80d277:
>
>   em28xx: convert to videobuf2 (2013-01-04 14:16:24 -0500)
>
> 
> Devin Heitmueller (1):
>   em28xx: convert to videobuf2
>
> Hans Verkuil (14):
>   em28xx: fix querycap.
>   em28xx: remove bogus input/audio ioctls for the radio device.
>   em28xx: fix VIDIOC_DBG_G_CHIP_IDENT compliance errors.
>   em28xx: fix tuner/frequency handling
>   v4l2-ctrls: add a notify callback.
>   em28xx: convert to the control framework.
>   em28xx: convert to v4l2_fh, fix priority handling.
>   em28xx: add support for control events.
>   em28xx: fill in readbuffers and fix incorrect return code.
>   em28xx: fix broken TRY_FMT.
>   tvp5150: remove compat control ops.
>   em28xx: std fixes: don't implement in webcam mode, and fix std changes.
>   em28xx: remove sliced VBI support.
>   em28xx: zero vbi_format reserved array and add try_vbi_fmt.
>
>  Documentation/video4linux/v4l2-controls.txt |   22 +-
>  drivers/media/i2c/tvp5150.c |7 -
>  drivers/media/usb/em28xx/Kconfig|3 +-
>  drivers/media/usb/em28xx/em28xx-cards.c |   31 +-
>  drivers/media/usb/em28xx/em28xx-dvb.c   |4 +-
>  drivers/media/usb/em28xx/em28xx-vbi.c   |  123 ++-
>  drivers/media/usb/em28xx/em28xx-video.c | 1159 
> ---
>  drivers/media/usb/em28xx/em28xx.h   |   38 +-
>  drivers/media/v4l2-core/v4l2-ctrls.c|   18 +
>  include/media/v4l2-ctrls.h  |   25 +
>  10 files changed, 504 insertions(+), 926 deletions(-)
>

The server is down ?


--
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] em28xx: fix audio input for TV mode of device Terratec Cinergy 250

2013-01-05 Thread Frank Schäfer
Remy Blank reported that audio over USB can be made working for the television
input if .amux is changed from EM28XX_AMUX_LINE_IN to EM28XX_AMUX_VIDEO.
An examination of his devices shows, that it is indeed supplied with an EM202
AC97 audio IC. We also use this setting for the Cinergy 200.

Remy Blank also provided the original version of this patch (many thanks !).

Fixes bug 14126 (see bug report for further device details).

Signed-off-by: Frank Schäfer 
Signed-off-by: Remy Blank 
Cc: sta...@kernel.org
---
 drivers/media/usb/em28xx/em28xx-cards.c |2 +-
 1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
b/drivers/media/usb/em28xx/em28xx-cards.c
index 4d849bf..0a5aa62 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -493,7 +493,7 @@ struct em28xx_board em28xx_boards[] = {
.input= { {
.type = EM28XX_VMUX_TELEVISION,
.vmux = SAA7115_COMPOSITE2,
-   .amux = EM28XX_AMUX_LINE_IN,
+   .amux = EM28XX_AMUX_VIDEO,
}, {
.type = EM28XX_VMUX_COMPOSITE1,
.vmux = SAA7115_COMPOSITE0,
-- 
1.7.10.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: [GIT PULL FOR 3.9] em28xx videobuf2 support and v4l2-compliance fixes

2013-01-05 Thread Frank Schäfer
First of all:
Hans, Devin thank you for working on this stuff !

Am 04.01.2013 21:59, schrieb Devin Heitmueller:
> On Fri, Jan 4, 2013 at 2:59 PM, Ezequiel Garcia  wrote:
>> Maybe I'm wrong, but weren't **all** changes supposed to be sent as a PATCH
>> to the mailing list for community review, before the PULL request was sent?
> No, you are correct.  I did a PULL specifically because Mauro asked me
> to. 

Interesting. Can you shed some light on whats going on behind the scenes
? ;)
How does this "request for pull-request" thing work and shouldn't this
happen on the public ML, too ?

> I'll resend the individual patches now to the list.

Do you still want me to review/test it ?
I'm willing to invest some time for it, but as there is already a pull
request, it seems to be ready and I really don't want to waste my time... !?

Regards,
Frank

> Devin
>

--
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: [BUG] crystalhd git.linuxtv.org kernel driver: unable to handle kernel paging requests, improper (spin)locking(?) and paging, null pointer oopses on SMP, libcrstalhd3-git i686 not interfacing to a

2013-01-05 Thread thomas schorpp

Hi Oliver,  hi crystalhd users and devs,  hello Broadcom Crystal HD staff,

1.
sorry for the delay, I had to upgrade my old debian i386 
stable...squeeze-backports userspace on the old core2duo machine to amd64 by 
full reinstall, otherwise the driver interface of libcrystalhd3 i686 to 
3.6.10...3.7.1amd64 SMP kernel.org kernels has failed permanently,

please (anyone still running such a setup or You) try to confirm this and 
report to this thread.

lspci still shows the same PCI-E errors (see my other posts to this list) with 
the working libcrystalhd3 amd64 and broadcom designed crystalhd driver now, so
this data reported from the chipset or lspci has to be considered faulty or 
stale and irelevant now, I will build the latest lspci from source to 
crosscheck this.


Please build ffmpeg rel. 1.0.1(non-MT version, not later version, git master 
showed up with an audio format bug, presenting wrong audio sample format as 
planar (sfltp, fltp, s16p) which bino cannot handle and makes mplayer cry for 
not having libavresample access but which is disabled by default in ffmpeg 
configure defaults and debian dmo source packages )
from Your distro source package with --disable-decoders='h264, h264_vdpau, 
h264_vda' and leave only h264_crystalhd need as h.264 decoder

and so trigger crystalhd by every app on your system accessing h.264 content 
for parsing or decoding and linked to libavcodec (check binaries with ldd if 
linked against this libavcodec54, , in libavcodec53 h264_crystalhd is flagged 
CAP_EXPERIMENTAL, which makes it unaccesible by other apps than the ffmpeg 
program (-strict -2), mplayer: -vc ffh264crystalhd, or will fail 
--disable-decoders='h264, h264_vdpau, h264_vda')  like mplayer (not statically 
linked), kaffeine, vlc, gnome nautilus media file properties (uses mplayer 
-identify) sequentially called and then in parallel and record and post the 
Ooopses in this thread here until the authors return from winter sports ;-)
Note for Bino users: System requirement is at least debian squeeze-backports X 
and DRI, otherwise bino will segfault the dri driver, i965 here and you need to 
build libGLEW(mx) 1.6 from source for debian stable systems, see 
http://savannah.nongnu.org/bugs/?33368 http://bino3d.org/help-wanted.html

2.
With the new amd64 userspace on 3.7.1 SMP PREEMPT kernel things got even more 
worse here now, got 5 kernel panics in IRQ handler of the crystalhd driver in 
1h while watching h.264 with
mplayer2/1 (single threaded decoding mode, stereo3d filter) resulting in system 
halting kernel crashes, I need to setup serial console debugging to get the 
logs, on my Pentium M i686 vdr machine, kernel has been able to continue at 
least after the null pointer oopses.
We need to have confirmation for this, too.

3.
Since the source code still states broadcom staff as module authors and the 
download on the broadcom website is packaged broken tar.bz2 and does not build 
here with recent kernels,
I'm CC'ing them now, too, and because it's their basic driver skeleton design 
and the quality and performance of this driver is far below the windows driver, 
which performs h.264 1080 great with http://mpc-hc.sourceforge.net at 5-10% cpu 
usage even on xp x64 on a i965, this should be the reference development target.

4.
The driver Makefile won't compile it with debian squeeze-backports 3.2 and 3.2 
bpo kbuild infrastructure, missing helper scripts and includes, it needs a full 
ready build kernel from sources:

schorpp@tom3:/usr/local/src/crystalhd/driver/linux$ LC_ALL=C make
make -C /lib/modules/3.2.0-0.bpo.4-amd64/build 
SUBDIRS=/mnt/data/usr/local/src/crystalhd/driver/linux modules
make[1]: Entering directory 
`/mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-amd64'
/mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/Makefile:287: 
/mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/scripts/Kbuild.include: 
Datei oder Verzeichnis nicht gefunden
/bin/bash: 
/mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/scripts/gcc-x86_64-has-stack-protector.sh:
 Datei oder Verzeichnis nicht gefunden
/mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/arch/x86/Makefile:81: 
stack protector enabled but no compiler support
/bin/bash: 
/mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/scripts/gcc-goto.sh: Datei 
oder Verzeichnis nicht gefunden
make: *** Leerer Variablenname.  Schluss.
make[3]: *** [_module_/mnt/data/usr/local/src/crystalhd/driver/linux] Fehler 2
make[2]: *** [sub-make] Error 2
make[1]: *** [all] Error 2
make[1]: Leaving directory `/mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-amd64'
make: *** [all] Error 2
schorpp@tom3:/usr/local/src/crystalhd/driver/linux$

5.
I'm focusing to the 3.x kernel source code delivered staging driver now (only 
BCM70012 support so far, no BCM70015) meanwhile unitil we get more information.

6.
Mythtv and xbmc people please join and report, too.

You can get a BCM70012 for just 10US$, a BCM700015 from 30US$ up on Ebay from 
china, mini-PCI-E to PCI-E adapter car

[REVIEW PATCH] Fix 3.7 radio modulator regression

2013-01-05 Thread Hans Verkuil
Hi all,

This is a fix for a regression introduced in 3.7. If there are no comments,
then I'll post the final version early next week.

The fix was tested with radio-keene.

Regards,

Hans

>From 86552330c91fff094a07c0018ca34a9f45362a64 Mon Sep 17 00:00:00 2001
Message-Id: 
<86552330c91fff094a07c0018ca34a9f45362a64.1357387528.git.hans.verk...@cisco.com>
From: Hans Verkuil 
Date: Sat, 5 Jan 2013 12:52:12 +0100
Subject: [PATCH] radio: set vfl_dir correctly to fix modulator regression.

The vfl_dir field should be set to indicate whether a device can receive
data, output data or can do both. This is used to let the v4l core know
which ioctls should be accepted and which can be refused.

Unfortunately, when this field was added the radio modulator drivers were
not updated: radio modulators transmit and so vfl_dir should be set to
VFL_DIR_TX (or VFL_DIR_M2M in the special case of wl128x).

Because of this omission it is not possible to call g/s_modulator for these
drivers, which effectively renders them useless.

This patch sets the correct vfl_dir value for these drivers, correcting
this bug.

Thanks to Paul Grinberg for bringing this to my attention.

Signed-off-by: Hans Verkuil 
Cc: sta...@vger.kernel.org  # for v3.7 and up
---
 drivers/media/radio/radio-keene.c   |1 +
 drivers/media/radio/radio-si4713.c  |1 +
 drivers/media/radio/radio-wl1273.c  |1 +
 drivers/media/radio/wl128x/fmdrv_v4l2.c |   10 ++
 4 files changed, 13 insertions(+)

diff --git a/drivers/media/radio/radio-keene.c 
b/drivers/media/radio/radio-keene.c
index e10e525..296941a 100644
--- a/drivers/media/radio/radio-keene.c
+++ b/drivers/media/radio/radio-keene.c
@@ -374,6 +374,7 @@ static int usb_keene_probe(struct usb_interface *intf,
radio->vdev.ioctl_ops = &usb_keene_ioctl_ops;
radio->vdev.lock = &radio->lock;
radio->vdev.release = video_device_release_empty;
+   radio->vdev.vfl_dir = VFL_DIR_TX;
 
radio->usbdev = interface_to_usbdev(intf);
radio->intf = intf;
diff --git a/drivers/media/radio/radio-si4713.c 
b/drivers/media/radio/radio-si4713.c
index a082e40..1507c9d 100644
--- a/drivers/media/radio/radio-si4713.c
+++ b/drivers/media/radio/radio-si4713.c
@@ -250,6 +250,7 @@ static struct video_device radio_si4713_vdev_template = {
.name   = "radio-si4713",
.release= video_device_release,
.ioctl_ops  = &radio_si4713_ioctl_ops,
+   .vfl_dir= VFL_DIR_TX,
 };
 
 /* Platform driver interface */
diff --git a/drivers/media/radio/radio-wl1273.c 
b/drivers/media/radio/radio-wl1273.c
index 6e55e93..8ba36c9 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -1971,6 +1971,7 @@ static struct video_device wl1273_viddev_template = {
.ioctl_ops  = &wl1273_ioctl_ops,
.name   = WL1273_FM_DRIVER_NAME,
.release= wl1273_vdev_release,
+   .vfl_dir= VFL_DIR_TX,
 };
 
 static int wl1273_fm_radio_remove(struct platform_device *pdev)
diff --git a/drivers/media/radio/wl128x/fmdrv_v4l2.c 
b/drivers/media/radio/wl128x/fmdrv_v4l2.c
index 048de45..0a8ee8f 100644
--- a/drivers/media/radio/wl128x/fmdrv_v4l2.c
+++ b/drivers/media/radio/wl128x/fmdrv_v4l2.c
@@ -518,6 +518,16 @@ static struct video_device fm_viddev_template = {
.ioctl_ops = &fm_drv_ioctl_ops,
.name = FM_DRV_NAME,
.release = video_device_release,
+   /*
+* To ensure both the tuner and modulator ioctls are accessible we
+* set the vfl_dir to M2M to indicate this.
+*
+* It is not really a mem2mem device of course, but it can both receive
+* and transmit using the same radio device. It's the only radio driver
+* that does this and it should really be split in two radio devices,
+* but that would affect applications using this driver.
+*/
+   .vfl_dir = VFL_DIR_M2M,
 };
 
 int fm_v4l2_init_video_device(struct fmdev *fmdev, int radio_nr)
-- 
1.7.10.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: [PATCH] ngene: fix commit 36a495a336c3fbbb2f4eeed2a94ab6d5be19d186

2013-01-05 Thread Patrice Chotard
Hi Mauro,

Yes, i confirm that without this patch, tuner_attach_dtt7520x() callback
was never called, so no tuning was possible.

Patrice

Le 05/01/2013 05:06, Mauro Carvalho Chehab a écrit :
> The above commit were applied only partially; it broke tuner and
> demod attach, but the part that added it to ngene_info was missing.
> 
> Not sure what happened there, but, without this patch, a regression
> would be happening.
> 
> Also, gcc complains about a defined but not used symbol.
> 
> So, apply manually the missing part.
> 
> Cc: Patrice Chotard 
> Cc: Antti Palosaari 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/pci/ngene/ngene-cards.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/pci/ngene/ngene-cards.c 
> b/drivers/media/pci/ngene/ngene-cards.c
> index 2a4895b..82318d1 100644
> --- a/drivers/media/pci/ngene/ngene-cards.c
> +++ b/drivers/media/pci/ngene/ngene-cards.c
> @@ -732,6 +732,7 @@ static struct ngene_info ngene_info_terratec = {
>   .name   = "Terratec Integra/Cinergy2400i Dual DVB-T",
>   .io_type= {NGENE_IO_TSIN, NGENE_IO_TSIN},
>   .demod_attach   = {demod_attach_drxd, demod_attach_drxd},
> + .tuner_attach   = {tuner_attach_dtt7520x, tuner_attach_dtt7520x},
>   .fe_config  = {&fe_terratec_dvbt_0, &fe_terratec_dvbt_1},
>   .i2c_access = 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