Re: how does v4l2 driver communicate a frequency lock to mythtv
Hi Devin, I have introduced t->signal = 0x; /* LOCKED */ on tm6000-video.c(vidioc_g_tuner) and mythtv indeed can locked the channel. xc2028_signal is showing the correct signal strength information. How can i call get_rf_strength to set the t->signal from vidioc_g_tuner? thanks, Hock. On Fri, Apr 2, 2010 at 9:59 PM, Devin Heitmueller wrote: > On Fri, Apr 2, 2010 at 9:48 AM, Bee Hock Goh wrote: >> Dear all, >> >> i have been doing some usb snoop and making some changes to the >> existing staging tm6000 to get my tm5600/xc2028 usb stick to work. >> Thanks to a lot of help from Stefan, I have some limited success and >> is able to get mythtv to do channel scan. However, mythtv is not able >> to logon to the channel even though usbmon shown the same in/out using >> usbmon and snoop on the stick windows application. >> >> Where should I be looking at to inform that a channel is to be locked? > > For most applications, the G_TUNER call must set the response struct > v4l2_tuner's "signal" field to a nonzero value. > > Devin > > -- > Devin J. Heitmueller - Kernel Labs > http://www.kernellabs.com > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core
Andy Walls wrote:
> On Thu, 2010-04-01 at 14:56 -0300, Mauro Carvalho Chehab wrote:
>> Adds a method to pass IR raw pulse/code events into ir-core. This is
>> needed in order to support LIRC. It also helps to move common code
>> from the drivers into the core.
>>
>> In order to allow testing, it implements a simple NEC protocol decoder
>> at ir-nec-decoder.c file. The logic is about the same used at saa7134
>> driver that handles Avermedia M135A and Encore FM53 boards.
>>
>> Signed-off-by: Mauro Carvalho Chehab
>>
>> create mode 100644 drivers/media/IR/ir-nec-decoder.c
>> create mode 100644 drivers/media/IR/ir-raw-event.c
>
> Hi Mauro,
>
> I haven't taken a very hard look at this since I'm very busy this month.
>
> It looks OK so far.
Thank you for your review.
One general comment: my main target of writing the NEC decoder is to have one
decoder for testing. I know that there are several other implementations, so I
didn't try to write a perfect decoder, but, instead, I wrote a code that
works, and that allows me to continue the Remote Controller subsystem design.
In other words, for sure there are lots of space for improvements there ;)
> I do have some comments
>
>
>> diff --git a/drivers/media/IR/Makefile b/drivers/media/IR/Makefile
>> index 171890e..18794c7 100644
>> --- a/drivers/media/IR/Makefile
>> +++ b/drivers/media/IR/Makefile
>> @@ -1,5 +1,5 @@
>> ir-common-objs := ir-functions.o ir-keymaps.o
>> -ir-core-objs:= ir-keytable.o ir-sysfs.o
>> +ir-core-objs:= ir-keytable.o ir-sysfs.o ir-raw-event.o
>> ir-nec-decoder.o
>>
>> obj-$(CONFIG_IR_CORE) += ir-core.o
>> obj-$(CONFIG_VIDEO_IR) += ir-common.o
>> diff --git a/drivers/media/IR/ir-nec-decoder.c
>> b/drivers/media/IR/ir-nec-decoder.c
>> new file mode 100644
>> index 000..16360eb
>> --- /dev/null
>> +++ b/drivers/media/IR/ir-nec-decoder.c
>> @@ -0,0 +1,131 @@
>> +/* ir-raw-event.c - handle IR Pulse/Space event
>> + *
>> + * Copyright (C) 2010 by Mauro Carvalho Chehab
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation version 2 of the License.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include
>> +
>> +/* Start time: 4.5 ms */
>> +#define MIN_START_TIME 390
>> +#define MAX_START_TIME 510
>
> Hmmm.
>
> An NEC header pulse is nominally16 * 560 us = 8.96 ms
> An NEC header space is nominally 8 * 560 us = 4.48 ms
> An NEC repeat header space is nominally 4 * 560 us = 2.24 ms
>
> I think you need a more explicit name than {MIN,MAX}_START_TIME.
Part of the problem with this decoder is that it was conceived to work with
the saa7134 driver. The driver is currently programmed to trigger IRQ on just
one of the edge (positive or negative, I need to double check). Due to that,
this time is just half of the time it should be.
I've changed on a latter patch my decoder to work with just the duration
of the bits.
After reviewing the datasheet, now I think I know how to program IRQ to
trigger on both edges. So, my idea is to enable it and rewrite the decoder.
>
>
>> +/* Pulse time: 560 us */
>> +#define MIN_PULSE_TIME 46
>> +#define MAX_PULSE_TIME 66
>> +
>> +/* Bit 1 space time: 2.25ms-560 us */
>> +#define MIN_BIT1_TIME 149
>> +#define MAX_BIT1_TIME 189
>> +
>> +/* Bit 0 space time: 1.12ms-560 us */
>> +#define MIN_BIT0_TIME 36
>> +#define MAX_BIT0_TIME 76
>> +
>
> The fundamental unit of time in the NEC protocol is ideally:
>
> 4192/197 cycles / 38 kHz = 559978.6 ns ~= 560 ns
>
> All other time durations in the NEC protocol are multiples of this unit.
Yes, I know. By max/min, I've meant to handle delta variations around
the main time, since the driver may miss the exact moment where it were
supposed to collect the timestamp.
> See:
>
> http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/2cfef53b95a2#l1.96
>
> If you define the your above constants in terms of that time unit, it
> makes the tolerances you added in explicitly visible when reading the
> source.
I'll take a look on your code and work to improve this decoder. The way you've
declared is for sure cleaner than mine.
>> +/** Decode NEC pulsecode. This code can take up to 76.5 ms to run.
>> +Unfortunately, using IRQ to decode pulse didn't work, since it uses
>> +a pulse train of 38KHz. This means one pulse on each 52 us
>> +*/
>> +
>> +int ir_nec_decode(struct input_dev *input_dev,
>> + struct ir_raw_event *evs,
>> + int len)
>> +{
>> +int i, count = -1;
>> +int ircode = 0, not_code = 0;
>> +#if 0
>> +/* Needed
Re: [PATCH 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core
Andy Walls wrote:
> On Fri, 2010-04-02 at 19:39 -0400, Andy Walls wrote:
>> On Thu, 2010-04-01 at 14:56 -0300, Mauro Carvalho Chehab wrote:
>
>>> +enum raw_event_type {
>>> + IR_SPACE= (1 << 0),
>>> + IR_PULSE= (1 << 1),
>>> + IR_START_EVENT = (1 << 2),
>>> + IR_STOP_EVENT = (1 << 3),
>>> +};
>>> +
>> Why are these events encoded as bit flags? Shouldn't they all be
>> orthogonal?
> ^^
> Argh, wrong word.
Why is it wrong? It seems appropriate to me.
>
> Shouldn't they all be mutually exclusive?
space x pulse are mutually exclusive, and start x stop are also
mutually exclusive, but you may have several possible combinations
for an event. The hole set of possibilities are:
IR_SPACE
IR_PULSE
IR_SPACE | IR_START_EVENT
IR_SPACE | IR_STOP_EVENT
IR_PULSE | IR_START_EVENT
IR_PULSE | IR_STOP_EVENT
With bit flags, it is possible to cover all the above combinations.
In a matter of fact, the driver is currently not using the stop events.
--
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 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core
On Fri, 2010-04-02 at 19:39 -0400, Andy Walls wrote:
> On Thu, 2010-04-01 at 14:56 -0300, Mauro Carvalho Chehab wrote:
> > +enum raw_event_type {
> > + IR_SPACE= (1 << 0),
> > + IR_PULSE= (1 << 1),
> > + IR_START_EVENT = (1 << 2),
> > + IR_STOP_EVENT = (1 << 3),
> > +};
> > +
>
> Why are these events encoded as bit flags? Shouldn't they all be
> orthogonal?
^^
Argh, wrong word.
Shouldn't they all be mutually exclusive?
Regards,
Andy
--
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] Serialization flag example
On Fri, 2010-04-02 at 18:15 -0300, Mauro Carvalho Chehab wrote: > Devin Heitmueller wrote: > In the case of a V4L x DVB type of lock, this is not to protect some memory, > but, > instead, to limit the usage of a hardware that is not capable of > simultaneously > provide V4L and DVB streams. This is a common case on almost all devices, > but, as > Hermann pointed, there are a few devices that are capable of doing both analog > and digital streams at the same time, but saa7134 driver currently doesn't > support. I know a driver that does: cx18 can handle simultaneous streaming of DTV and analog. Some cards have both an analog and digital tuner, so both DTV and analog can come from an RF source simultaneously. (No locking needed really.) Some cards only have one tuner, which means simultaneous streaming is limited to DTV from RF and analog from baseband inputs. Streaming analog from an RF source on these cards precludes streaming of DTV. (An odd locking ruleset when you consider MPEG, YUV, PCM, and VBI from the bridge chip can independently be streamed from the selected analog source to 4 different device nodes.) Regards, Andy -- 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] Serialization flag example
On Fri, 2010-04-02 at 10:57 +0200, Hans Verkuil wrote: > If needed one could allow drivers to override this function. But again, start > simple and only make it more complex if we really need to. Overengineering is > one of the worst mistakes one can make. I have seen too many projects fail > because of that. Gall's Law! http://en.wikipedia.org/wiki/Gall%27s_law Regards, Andy -- 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: V4L-DVB drivers and BKL
On Thu, 2010-04-01 at 17:29 -0400, Devin Heitmueller wrote: > On Thu, Apr 1, 2010 at 5:16 PM, Mauro Carvalho Chehab > I think though that we need to favor stability/reliability over > performance. You mean doing the wrong thing, as fast as you can, isn't performing? ;) I usually consider performance with two broad criteria - requirements correctly implemented (can be a weighted sum) - efficient use of limited resources (usually time or bandwidth) Regards, Andy -- 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 04/15] V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core
On Thu, 2010-04-01 at 14:56 -0300, Mauro Carvalho Chehab wrote:
> Adds a method to pass IR raw pulse/code events into ir-core. This is
> needed in order to support LIRC. It also helps to move common code
> from the drivers into the core.
>
> In order to allow testing, it implements a simple NEC protocol decoder
> at ir-nec-decoder.c file. The logic is about the same used at saa7134
> driver that handles Avermedia M135A and Encore FM53 boards.
>
> Signed-off-by: Mauro Carvalho Chehab
>
> create mode 100644 drivers/media/IR/ir-nec-decoder.c
> create mode 100644 drivers/media/IR/ir-raw-event.c
Hi Mauro,
I haven't taken a very hard look at this since I'm very busy this month.
It looks OK so far. I do have some comments
> diff --git a/drivers/media/IR/Makefile b/drivers/media/IR/Makefile
> index 171890e..18794c7 100644
> --- a/drivers/media/IR/Makefile
> +++ b/drivers/media/IR/Makefile
> @@ -1,5 +1,5 @@
> ir-common-objs := ir-functions.o ir-keymaps.o
> -ir-core-objs := ir-keytable.o ir-sysfs.o
> +ir-core-objs := ir-keytable.o ir-sysfs.o ir-raw-event.o ir-nec-decoder.o
>
> obj-$(CONFIG_IR_CORE) += ir-core.o
> obj-$(CONFIG_VIDEO_IR) += ir-common.o
> diff --git a/drivers/media/IR/ir-nec-decoder.c
> b/drivers/media/IR/ir-nec-decoder.c
> new file mode 100644
> index 000..16360eb
> --- /dev/null
> +++ b/drivers/media/IR/ir-nec-decoder.c
> @@ -0,0 +1,131 @@
> +/* ir-raw-event.c - handle IR Pulse/Space event
> + *
> + * Copyright (C) 2010 by Mauro Carvalho Chehab
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +
> +/* Start time: 4.5 ms */
> +#define MIN_START_TIME 390
> +#define MAX_START_TIME 510
Hmmm.
An NEC header pulse is nominally16 * 560 us = 8.96 ms
An NEC header space is nominally 8 * 560 us = 4.48 ms
An NEC repeat header space is nominally 4 * 560 us = 2.24 ms
I think you need a more explicit name than {MIN,MAX}_START_TIME.
> +/* Pulse time: 560 us */
> +#define MIN_PULSE_TIME 46
> +#define MAX_PULSE_TIME 66
> +
> +/* Bit 1 space time: 2.25ms-560 us */
> +#define MIN_BIT1_TIME149
> +#define MAX_BIT1_TIME189
> +
> +/* Bit 0 space time: 1.12ms-560 us */
> +#define MIN_BIT0_TIME36
> +#define MAX_BIT0_TIME76
> +
The fundamental unit of time in the NEC protocol is ideally:
4192/197 cycles / 38 kHz = 559978.6 ns ~= 560 ns
All other time durations in the NEC protocol are multiples of this unit.
See:
http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/2cfef53b95a2#l1.96
If you define the your above constants in terms of that time unit, it
makes the tolerances you added in explicitly visible when reading the
source.
> +/** Decode NEC pulsecode. This code can take up to 76.5 ms to run.
> + Unfortunately, using IRQ to decode pulse didn't work, since it uses
> + a pulse train of 38KHz. This means one pulse on each 52 us
> +*/
> +
> +int ir_nec_decode(struct input_dev *input_dev,
> + struct ir_raw_event *evs,
> + int len)
> +{
> + int i, count = -1;
> + int ircode = 0, not_code = 0;
> +#if 0
> + /* Needed only after porting the event code to the decoder */
> + struct ir_input_dev *ir = input_get_drvdata(input_dev);
> +#endif
> +
> + /* Be sure that the first event is an start one and is a pulse */
> + for (i = 0; i < len; i++) {
> + if (evs[i].type & (IR_START_EVENT | IR_PULSE))
> + break;
> + }
> + i++;/* First event doesn't contain data */
> +
> + if (i >= len)
> + return 0;
> +
> + /* First space should have 4.5 ms otherwise is not NEC protocol */
> + if ((evs[i].delta.tv_nsec < MIN_START_TIME) |
> + (evs[i].delta.tv_nsec > MAX_START_TIME) |
> + (evs[i].type != IR_SPACE))
> + goto err;
> +
> + /*
> + * FIXME: need to implement the repeat sequence
> + */
I have an NEC protocol decoder here:
http://linuxtv.org/hg/~awalls/cx23885-ir2/rev/2cfef53b95a2
If you would find it useful, please feel free to borrow ideas or parts
of the code to implement any features you are missing. (That code works
by converting a mark-space pair to an "nec_symbol", and then taking
action based on the symbol.)
I suspect you will want to implement the repeat sequence. It is hard
not to get a repeat sequence from a remote.
NEC ideally sends a repeat at intervals of:
4192 cycles * 38 kHz = 110.316 ms
> + count = 0;
> + for (i++; i < len;
Re: [RFC] Serialization flag example
Devin Heitmueller wrote: > On Fri, Apr 2, 2010 at 1:43 PM, Manu Abraham wrote: >> IMO, A framework shouldn't lock. Current DVB framework is locked with BKL. I agree that an unconditional lock like this is very bad. We need to get rid of it as soon as possible. > Hello Manu, > > The argument I am trying to make is that there are numerous cases > where you should not be able to use both a particular DVB and V4L > device at the same time. The implementation of such locking should be > handled by the v4l-dvb core, but the definition of the relationships > dictating which devices cannot be used in parallel is still the > responsibility of the driver. > > This way, a bridge driver can say "this DVB device cannot be used at > the same time as this V4L device" but the actual enforcement of the > locking is done in the core. For cases where the devices can be used > in parallel, the bridge driver doesn't have to do anything. Although both are some sort of locks, the BKL replacement lock is basically a memory barrier to serialize data, avoiding that the driver's internal structs to be corrupted or to return a wrong value. Only the driver really knows what should be protected. In the case of V4L/DVB devices, the struct to be protected is the struct *_dev (struct core, on cx88, struct em28xx on em28xx, struct saa7134_dev, and so on). Of course, if everything got serialized by the core, and assuming that the driver doesn't have IRQ's and/or other threads accessing the same data, the problem disappears, at the expense of a performance reduction that may or may not be relevant. That's said, except for the most simple drivers, like radio ones, there's always some sort of IRQ and/or threads evolved, touching on struct *_dev. For example, both cx88 and saa7134 have (depending on the hardware): - IRQ to announce that a data buffer was filled for video, vbi, alsa; - IRQ for IR; - IR polling; - video audio standard detection thread; A lock to protect the internal data structs should protect all the above. Even the most pedantic core-only lock won't solve it. Yet, a lock, on core, for ioctl/poll/open/close may be part of a protection, if the same lock is used also to protect the other usages of struct *_dev. So, I'm OK on having an optional mutex parameter to be passed to V4L core, to provide the BKL removal. In the case of a V4L x DVB type of lock, this is not to protect some memory, but, instead, to limit the usage of a hardware that is not capable of simultaneously provide V4L and DVB streams. This is a common case on almost all devices, but, as Hermann pointed, there are a few devices that are capable of doing both analog and digital streams at the same time, but saa7134 driver currently doesn't support. Some drivers, like cx88 has a sort of lock meant to protect such things. It is implemented on res_get/res_check/res_locked/res_free. Currently, the lock is only protecting simultaneous usage of the analog part of the driver. It works by not allowing to start two simultaneous video streams of the same memory access type, for example, while allowing multiple device opens, for example to call V4L controls, while another application is streaming. It also allows having a mmapped or overlay stream and a separate read() stream (used on applications like xawtv and xdtv to record a video on PCI devices that has enough bandwidth to provide two simultaneous streams). Maybe this kind of lock can be abstracted, and we can add a class of resource protectors inside the core, for the drivers to use it when needed. -- 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 1/2] OMAP2/3 V4L2: Add support for OMAP2/3 V4L2 driver on top of DSS2
Vaibhav,
I have some comments on this patch. Please address them.
> +
> +#include
Add a line here??
> +#include
> +#include
> +#include
> +#include
> +
> +#include "omap_voutlib.h"
> +#include "omap_voutdef.h"
> +
> +MODULE_AUTHOR("Texas Instruments");
> +MODULE_DESCRIPTION("OMAP Video for Linux Video out driver");
> +MODULE_LICENSE("GPL");
> +
> +
> +/* Driver Configuration macros */
> +#define VOUT_NAME "omap_vout"
> +
> +enum omap_vout_channels {
> + OMAP_VIDEO1 = 0,
Why do we have to initialize this to 0. It always start with a value 0
by default.
> + OMAP_VIDEO2,
> +};
> +
> +enum dma_channel_state {
> + DMA_CHAN_NOT_ALLOTED = 0,
Ditto.
> + DMA_CHAN_ALLOTED,
> +};
> +
> +#define QQVGA_WIDTH 160
> +#define QQVGA_HEIGHT 120
> +
> +/* Max Resolution supported by the driver */
> +#define VID_MAX_WIDTH 1280 /* Largest width */
> +#define VID_MAX_HEIGHT 720 /* Largest height */
> +
-
> +
> +module_param(debug, bool, S_IRUGO);
> +MODULE_PARM_DESC(debug, "Debug level (0-1)");
> +
> +/* Local Helper functions */
> +static void omap_vout_isr(void *arg, unsigned int irqstatus);
> +static void omap_vout_cleanup_device(struct omap_vout_device *vout);
> +
Is there a reason why you need these prototypes? I think we could
remove these prototypes and move the function ahead in the file before
it is called.
> +/* list of image formats supported by OMAP2 video pipelines */
> +const static struct v4l2_fmtdesc omap_formats[] = {
> + {
> + /* Note: V4L2 defines RGB565 as:
> + *
> + * Byte 0 Byte 1
> + * g2 g1 g0 r4 r3 r2 r1 r0 b4 b3 b2 b1 b0 g5 g4 g3
> + *
> + * We interpret RGB565 as:
> + *
> + * Byte 0 Byte 1
> + * g2 g1 g0 b4 b3 b2 b1 b0 r4 r3 r2 r1 r0 g5 g4 g3
> + */
> + .description = "RGB565, le",
> + .pixelformat = V4L2_PIX_FMT_RGB565,
> + },
> + {
> + /* Note: V4L2 defines RGB32 as: RGB-8-8-8-8 we use
> + * this for RGB24 unpack mode, the last 8 bits are ignored
> + * */
> + .description = "RGB32, le",
> + .pixelformat = V4L2_PIX_FMT_RGB32,
> + },
> + {
> + /* Note: V4L2 defines RGB24 as: RGB-8-8-8 we use
> + * this for RGB24 packed mode
> + *
> + */
> + .description = "RGB24, le",
> + .pixelformat = V4L2_PIX_FMT_RGB24,
> + },
> + {
> + .description = "YUYV (YUV 4:2:2), packed",
> + .pixelformat = V4L2_PIX_FMT_YUYV,
> + },
> + {
> + .description = "UYVY, packed",
> + .pixelformat = V4L2_PIX_FMT_UYVY,
> + },
> +};
> +
> +#define NUM_OUTPUT_FORMATS (ARRAY_SIZE(omap_formats))
> +
> +/*
> + * Allocate buffers
> + */
--
> +
> +/*
> + * omap_vout_uservirt_to_phys: This inline function is used to convert user
> + * space virtual address to physical address.
> + */
> +static u32 omap_vout_uservirt_to_phys(u32 virtp)
> +{
> + unsigned long physp = 0;
> + struct vm_area_struct *vma;
> + struct mm_struct *mm = current->mm;
> +
> + vma = find_vma(mm, virtp);
> + /* For kernel direct-mapped memory, take the easy way */
> + if (virtp >= PAGE_OFFSET) {
> + physp = virt_to_phys((void *) virtp);
> + } else if (vma && (vma->vm_flags & VM_IO)
> + && vma->vm_pgoff) {
> + /* this will catch, kernel-allocated,
> + mmaped-to-usermode addresses */
> + physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp -
> vma->vm_start);
> + } else {
> + /* otherwise, use get_user_pages() for general userland pages
> */
> + int res, nr_pages = 1;
> + struct page *pages;
> + down_read(¤t->mm->mmap_sem);
> +
> + res = get_user_pages(current, current->mm, virtp, nr_pages,
> + 1, 0, &pages, NULL);
> + up_read(¤t->mm->mmap_sem);
> +
> + if (res == nr_pages) {
> + physp = __pa(page_address(&pages[0]) +
> + (virtp & ~PAGE_MASK));
> + } else {
> + printk(KERN_WARNING VOUT_NAME
> + "get_user_pages failed\n");
> + return 0;
> + }
> + }
> +
> + return physp;
> +}
Shouldn't we remove omap_vout_uservirt_to_phys() and use videobuf_iolock()
instead as we have done in vpfe_capture.c?
-
> +
> +/*
> + * Convert
[cron job] v4l-dvb daily build 2.6.22 and up: ERRORS, 2.6.16-2.6.21: WARNINGS
This message is generated daily by a cron job that builds v4l-dvb for the kernels and architectures in the list below. Results of the daily build of v4l-dvb: date:Fri Apr 2 19:00:19 CEST 2010 path:http://www.linuxtv.org/hg/v4l-dvb changeset: 14536:a539e5b68945 git master: f6760aa024199cfbce564311dc4bc4d47b6fb349 git media-master: 8c69c6ed6c74c94fa7ad6fa24eda452e4b212d81 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: OK linux-2.6.33-armv5: OK linux-2.6.34-rc1-armv5: OK linux-2.6.32.6-armv5-davinci: WARNINGS linux-2.6.33-armv5-davinci: WARNINGS linux-2.6.34-rc1-armv5-davinci: WARNINGS linux-2.6.32.6-armv5-ixp: WARNINGS linux-2.6.33-armv5-ixp: WARNINGS linux-2.6.34-rc1-armv5-ixp: WARNINGS linux-2.6.32.6-armv5-omap2: WARNINGS linux-2.6.33-armv5-omap2: WARNINGS linux-2.6.34-rc1-armv5-omap2: WARNINGS linux-2.6.22.19-i686: WARNINGS linux-2.6.23.17-i686: WARNINGS linux-2.6.24.7-i686: WARNINGS linux-2.6.25.20-i686: WARNINGS 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-rc1-i686: WARNINGS linux-2.6.32.6-m32r: OK linux-2.6.33-m32r: OK linux-2.6.34-rc1-m32r: OK linux-2.6.32.6-mips: WARNINGS linux-2.6.33-mips: WARNINGS linux-2.6.34-rc1-mips: WARNINGS linux-2.6.32.6-powerpc64: WARNINGS linux-2.6.33-powerpc64: WARNINGS linux-2.6.34-rc1-powerpc64: WARNINGS linux-2.6.22.19-x86_64: WARNINGS linux-2.6.23.17-x86_64: WARNINGS linux-2.6.24.7-x86_64: WARNINGS linux-2.6.25.20-x86_64: WARNINGS 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-rc1-x86_64: WARNINGS linux-git-armv5: OK linux-git-armv5-davinci: OK linux-git-armv5-ixp: OK linux-git-armv5-omap2: OK linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS spec: ERRORS spec-git: OK sparse: ERRORS linux-2.6.16.62-i686: WARNINGS linux-2.6.17.14-i686: WARNINGS linux-2.6.18.8-i686: WARNINGS linux-2.6.19.7-i686: WARNINGS linux-2.6.20.21-i686: WARNINGS linux-2.6.21.7-i686: WARNINGS linux-2.6.16.62-x86_64: WARNINGS linux-2.6.17.14-x86_64: WARNINGS linux-2.6.18.8-x86_64: WARNINGS linux-2.6.19.7-x86_64: WARNINGS linux-2.6.20.21-x86_64: WARNINGS linux-2.6.21.7-x86_64: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/3] [PATCH] drivers/media/IR - improve keyup/keydown logic
The attached patch rewrites the keyup/keydown logic in
drivers/media/IR/ir-keytable.c.
(applies to http://git.linuxtv.org/mchehab/ir.git)
All knowledge of keystates etc is now internal to ir-keytable.c
and not scattered around ir-raw-event.c and ir-nec-decoder.c (where
it doesn't belong).
In addition, I've changed the API slightly so that ir_input_dev is
passed as the first argument rather than input_dev. If we're ever
going to support multiple keytables we need to move towards making
ir_input_dev the main interface from a driver POV and obscure away
the input_dev as an implementational detail in ir-core.
Mauro, once this patch is merged I'll start working on patches to
migrate drivers which use ir_input_* in ir-functions.c over to
the ir-keytable.c code instead.
Changes since last version:
Clarify a comment
Refreshed to apply on top of previous patch
Let the new functions take input_dev as their main argument
(as there's no agreement on this point yet)
Signed-off-by: David Härdeman
Index: ir/drivers/media/IR/ir-keytable.c
===
--- ir.orig/drivers/media/IR/ir-keytable.c 2010-04-02 12:32:13.015233729
+0200
+++ ir/drivers/media/IR/ir-keytable.c 2010-04-02 16:40:37.908276737 +0200
@@ -20,6 +20,9 @@
#define IR_TAB_MIN_SIZE256
#define IR_TAB_MAX_SIZE8192
+/* FIXME: IR_KEYPRESS_TIMEOUT should be protocol specific */
+#define IR_KEYPRESS_TIMEOUT 250
+
/**
* ir_resize_table() - resizes a scancode table if necessary
* @rc_tab:the ir_scancode_table to resize
@@ -250,56 +253,124 @@
/**
* ir_keyup() - generates input event to cleanup a key press
- * @input_dev: the struct input_dev descriptor of the device
+ * @ir:the struct ir_input_dev descriptor of the device
*
- * This routine is used by the input routines when a key is pressed at the
- * IR. It reports a keyup input event via input_report_key().
+ * This routine is used to signal that a key has been released on the
+ * remote control. It reports a keyup input event via input_report_key().
*/
-void ir_keyup(struct input_dev *dev)
+static void ir_keyup(struct ir_input_dev *ir)
{
- struct ir_input_dev *ir = input_get_drvdata(dev);
-
if (!ir->keypressed)
return;
- IR_dprintk(1, "keyup key 0x%04x\n", ir->keycode);
- input_report_key(dev, ir->keycode, 0);
- input_sync(dev);
- ir->keypressed = 0;
+ IR_dprintk(1, "keyup key 0x%04x\n", ir->last_keycode);
+ input_report_key(ir->input_dev, ir->last_keycode, 0);
+ input_sync(ir->input_dev);
+ ir->keypressed = false;
+}
+
+/**
+ * ir_timer_keyup() - generates a keyup event after a timeout
+ * @cookie:a pointer to struct ir_input_dev passed to setup_timer()
+ *
+ * This routine will generate a keyup event some time after a keydown event
+ * is generated when no further activity has been detected.
+ */
+static void ir_timer_keyup(unsigned long cookie)
+{
+ struct ir_input_dev *ir = (struct ir_input_dev *)cookie;
+ unsigned long flags;
+
+ /*
+* ir->keyup_jiffies is used to prevent a race condition if a
+* hardware interrupt occurs at this point and the keyup timer
+* event is moved further into the future as a result.
+*
+* The timer will then be reactivated and this function called
+* again in the future. We need to exit gracefully in that case
+* to allow the input subsystem to do its auto-repeat magic or
+* a keyup event might follow immediately after the keydown.
+*/
+ spin_lock_irqsave(&ir->keylock, flags);
+ if (time_is_after_eq_jiffies(ir->keyup_jiffies))
+ ir_keyup(ir);
+ spin_unlock_irqrestore(&ir->keylock, flags);
+}
+
+/**
+ * ir_repeat() - notifies the IR core that a key is still pressed
+ * @dev: the struct input_dev descriptor of the device
+ *
+ * This routine is used by IR decoders when a repeat message which does
+ * not include the necessary bits to reproduce the scancode has been
+ * received.
+ */
+void ir_repeat(struct input_dev *dev)
+{
+ unsigned long flags;
+ struct ir_input_dev *ir = input_get_drvdata(dev);
+
+ spin_lock_irqsave(&ir->keylock, flags);
+
+ if (!ir->keypressed)
+ goto out;
+
+ ir->keyup_jiffies = jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT);
+ mod_timer(&ir->timer_keyup, ir->keyup_jiffies);
+
+out:
+ spin_unlock_irqrestore(&ir->keylock, flags);
}
-EXPORT_SYMBOL_GPL(ir_keyup);
+EXPORT_SYMBOL_GPL(ir_repeat);
/**
* ir_keydown() - generates input event for a key press
- * @input_dev: the struct input_dev descriptor of the device
+ * @dev: the struct input_dev descriptor of the device
* @scancode: the scancode that we're seeking
+ * @toggle:the toggle value (protocol dependent, if the protocol doesn't
+ * support toggle values, this should be set to zero)
*
[patch 3/3] Convert drivers/media/dvb/ttpci/budget-ci.c to use ir-core
This patch converts drivers/media/dvb/ttpci/budget-ci.c to use ir-core
rather than rolling its own keydown timeout handler and reporting keys
via drivers/media/IR/ir-functions.c.
Signed-off-by: David Härdeman
Index: ir/drivers/media/dvb/ttpci/budget-ci.c
===
--- ir.orig/drivers/media/dvb/ttpci/budget-ci.c 2010-04-02 16:41:15.524206900
+0200
+++ ir/drivers/media/dvb/ttpci/budget-ci.c 2010-04-02 16:48:15.668239437
+0200
@@ -35,7 +35,7 @@
#include
#include
#include
-#include
+#include
#include "budget.h"
@@ -82,12 +82,6 @@
#define SLOTSTATUS_READY 8
#define SLOTSTATUS_OCCUPIED
(SLOTSTATUS_PRESENT|SLOTSTATUS_RESET|SLOTSTATUS_READY)
-/*
- * Milliseconds during which a key is regarded as pressed.
- * If an identical command arrives within this time, the timer will start over.
- */
-#define IR_KEYPRESS_TIMEOUT250
-
/* RC5 device wildcard */
#define IR_DEVICE_ANY 255
@@ -104,12 +98,9 @@
struct budget_ci_ir {
struct input_dev *dev;
struct tasklet_struct msp430_irq_tasklet;
- struct timer_list timer_keyup;
char name[72]; /* 40 + 32 for (struct saa7146_dev).name */
char phys[32];
- struct ir_input_state state;
int rc5_device;
- u32 last_raw;
u32 ir_key;
bool have_command;
};
@@ -124,18 +115,11 @@
u8 tuner_pll_address; /* used for philips_tdm1316l configs */
};
-static void msp430_ir_keyup(unsigned long data)
-{
- struct budget_ci_ir *ir = (struct budget_ci_ir *) data;
- ir_input_nokey(ir->dev, &ir->state);
-}
-
static void msp430_ir_interrupt(unsigned long data)
{
struct budget_ci *budget_ci = (struct budget_ci *) data;
struct input_dev *dev = budget_ci->ir.dev;
u32 command = ttpci_budget_debiread(&budget_ci->budget, DEBINOSWAP,
DEBIADDR_IR, 2, 1, 0) >> 8;
- u32 raw;
/*
* The msp430 chip can generate two different bytes, command and device
@@ -171,20 +155,12 @@
return;
budget_ci->ir.have_command = false;
+ /* FIXME: We should generate complete scancodes with device info */
if (budget_ci->ir.rc5_device != IR_DEVICE_ANY &&
budget_ci->ir.rc5_device != (command & 0x1f))
return;
- /* Is this a repeated key sequence? (same device, command, toggle) */
- raw = budget_ci->ir.ir_key | (command << 8);
- if (budget_ci->ir.last_raw != raw ||
!timer_pending(&budget_ci->ir.timer_keyup)) {
- ir_input_nokey(dev, &budget_ci->ir.state);
- ir_input_keydown(dev, &budget_ci->ir.state,
-budget_ci->ir.ir_key);
- budget_ci->ir.last_raw = raw;
- }
-
- mod_timer(&budget_ci->ir.timer_keyup, jiffies +
msecs_to_jiffies(IR_KEYPRESS_TIMEOUT));
+ ir_keydown(dev, budget_ci->ir.ir_key, (command & 0x20) ? 1 : 0);
}
static int msp430_ir_init(struct budget_ci *budget_ci)
@@ -251,11 +227,6 @@
ir_input_init(input_dev, &budget_ci->ir.state, IR_TYPE_RC5);
- /* initialise the key-up timeout handler */
- init_timer(&budget_ci->ir.timer_keyup);
- budget_ci->ir.timer_keyup.function = msp430_ir_keyup;
- budget_ci->ir.timer_keyup.data = (unsigned long) &budget_ci->ir;
- budget_ci->ir.last_raw = 0x; /* An impossible value */
error = ir_input_register(input_dev, ir_codes, NULL, MODULE_NAME);
if (error) {
printk(KERN_ERR "budget_ci: could not init driver for IR device
(code %d)\n", error);
@@ -284,9 +255,6 @@
saa7146_setgpio(saa, 3, SAA7146_GPIO_INPUT);
tasklet_kill(&budget_ci->ir.msp430_irq_tasklet);
- del_timer_sync(&dev->timer);
- ir_input_nokey(dev, &budget_ci->ir.state);
-
ir_input_unregister(dev);
}
--
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/3] [PATCH] drivers/media/IR - improve keytable code
The attached patch rewrites much of the keytable code in
drivers/media/IR/ir-keytable.c.
The scancodes are now inserted into the array in sorted
order which allows for a binary search on lookup.
The code has also been shrunk by about 150 lines.
In addition it fixes the following bugs:
Any use of ir_seek_table() was racy.
ir_dev->driver_name is leaked between ir_input_register() and
ir_input_unregister().
ir_setkeycode() unconditionally does clear_bit() on dev->keybit
when removing a mapping, but there might be another mapping with
a different scancode and the same keycode.
This version has been updated to incorporate patch feedback from
Mauro Carvalho Chehab.
Signed-off-by: David Härdeman
Index: ir/drivers/media/IR/ir-keymaps.c
===
--- ir.orig/drivers/media/IR/ir-keymaps.c 2010-04-01 17:38:16.24334
+0200
+++ ir/drivers/media/IR/ir-keymaps.c2010-04-02 12:23:14.952235145 +0200
@@ -39,6 +39,7 @@
struct ir_scancode_table tabname ## _table = { \
.scan = tabname,\
.size = ARRAY_SIZE(tabname),\
+ .len = ARRAY_SIZE(tabname), \
.ir_type = type,\
.name = #irname,\
}; \
Index: ir/drivers/media/IR/ir-keytable.c
===
--- ir.orig/drivers/media/IR/ir-keytable.c 2010-04-01 17:38:16.753724054
+0200
+++ ir/drivers/media/IR/ir-keytable.c 2010-04-02 12:32:13.015233729 +0200
@@ -16,344 +16,214 @@
#include
#include
-#define IR_TAB_MIN_SIZE32
-#define IR_TAB_MAX_SIZE1024
+/* Sizes are in bytes, 256 bytes allows for 32 entries on x64 */
+#define IR_TAB_MIN_SIZE256
+#define IR_TAB_MAX_SIZE8192
/**
- * ir_seek_table() - returns the element order on the table
- * @rc_tab:the ir_scancode_table with the keymap to be used
- * @scancode: the scancode that we're seeking
+ * ir_resize_table() - resizes a scancode table if necessary
+ * @rc_tab:the ir_scancode_table to resize
+ * @return:zero on success or a negative error code
*
- * This routine is used by the input routines when a key is pressed at the
- * IR. The scancode is received and needs to be converted into a keycode.
- * If the key is not found, it returns KEY_UNKNOWN. Otherwise, returns the
- * corresponding keycode from the table.
+ * This routine will shrink the ir_scancode_table if it has lots of
+ * unused entries and grow it if it is full.
*/
-static int ir_seek_table(struct ir_scancode_table *rc_tab, u32 scancode)
+static int ir_resize_table(struct ir_scancode_table *rc_tab)
{
- int rc;
- unsigned long flags;
- struct ir_scancode *keymap = rc_tab->scan;
-
- spin_lock_irqsave(&rc_tab->lock, flags);
-
- /* FIXME: replace it by a binary search */
+ unsigned int oldalloc = rc_tab->alloc;
+ unsigned int newalloc = oldalloc;
+ struct ir_scancode *oldscan = rc_tab->scan;
+ struct ir_scancode *newscan;
- for (rc = 0; rc < rc_tab->size; rc++)
- if (keymap[rc].scancode == scancode)
- goto exit;
-
- /* Not found */
- rc = -EINVAL;
+ if (rc_tab->size == rc_tab->len) {
+ /* All entries in use -> grow keytable */
+ if (rc_tab->alloc >= IR_TAB_MAX_SIZE)
+ return -ENOMEM;
-exit:
- spin_unlock_irqrestore(&rc_tab->lock, flags);
- return rc;
-}
+ newalloc *= 2;
+ IR_dprintk(1, "Growing table to %u bytes\n", newalloc);
+ }
-/**
- * ir_roundup_tablesize() - gets an optimum value for the table size
- * @n_elems: minimum number of entries to store keycodes
- *
- * This routine is used to choose the keycode table size.
- *
- * In order to have some empty space for new keycodes,
- * and knowing in advance that kmalloc allocates only power of two
- * segments, it optimizes the allocated space to have some spare space
- * for those new keycodes by using the maximum number of entries that
- * will be effectively be allocated by kmalloc.
- * In order to reduce the quantity of table resizes, it has a minimum
- * table size of IR_TAB_MIN_SIZE.
- */
-static int ir_roundup_tablesize(int n_elems)
-{
- size_t size;
+ if ((rc_tab->len * 3 < rc_tab->size) && (oldalloc > IR_TAB_MIN_SIZE)) {
+ /* Less than 1/3 of entries in use -> shrink keytable */
+ newalloc /= 2;
+ IR_dprintk(1, "Shrinking table to %u bytes\n", newalloc);
+ }
- if (n_elems < IR_TAB_MIN_SIZE)
- n_elems = IR_TAB_MIN_SIZE;
+ if (newalloc == oldalloc)
+ return 0;
- /*
-* As kmalloc only allocates sizes of
[patch 0/3] ir-core keytable patches
The following patchset provides a number of bug fixes and additional features to ir-core in drivers/media/IR. The first two patches are mostly cleanups to drivers/media/IR/ir-keytable.c while the third patch is an example of a conversion of drivers/media/dvb/ttpci/budget-ci.c to use the new functionality (rather than the drivers/media/IR/ir-functions.c functionality). -- David Härdeman -- 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] Serialization flag example
On Fri, Apr 2, 2010 at 2:24 PM, Manu Abraham wrote: > Hi Devin, > >> Hello Manu, >> >> The argument I am trying to make is that there are numerous cases >> where you should not be able to use both a particular DVB and V4L >> device at the same time. The implementation of such locking should be >> handled by the v4l-dvb core, but the definition of the relationships >> dictating which devices cannot be used in parallel is still the >> responsibility of the driver. >> >> This way, a bridge driver can say "this DVB device cannot be used at >> the same time as this V4L device" but the actual enforcement of the >> locking is done in the core. For cases where the devices can be used >> in parallel, the bridge driver doesn't have to do anything. > > I follow what you mean. Why I emphasized that it shouldn't be at the > core, but basically in the bridge driver: > > Case 1 > - there is a 1:n relation, In this case there is only 1 path and 3 > users sharing that path > In such a case, You can use such a mentioned scheme, where things do > look okay, since it is only about locking a single path. > > Case 2 > - there is a n:n relation, in this case there are n paths and n users > In such a case, it is hard to make the core aware of all the details, > since there could be more than 1 resource of the same category; > Mapping each of them properly won't be easy, as for the same chip > driver Resource A on Card A, would mean different for Resource A on > Card B. > > Case 3 > - there is a m:n relation, in this case, there are m paths and n users > This case is even more painful than the previous ones. > > In cases 2 & 3, the option to handle such cases is using a > configuration scheme based on the card type. I guess handling such > would be quite daunting and hard to get right. I guess it would be > much more efficient and useful to have such a feature to be made > available in the bridge driver as it is a resource of the card > configuration, rather than a common bridge resource. Hi Manu, I don't have any problem with a bridge choosing to implement some much more complicated scheme to meet its own special requirements. However, it feels like the vast majority of bridges would fall into scenario #1, and having that functionality in the core would mean that all of those bridges would work properly (only needing a 2 line code change). Hence, making the core handle the common case and still allowing the bridge maintainer to override the logic if necessary would seem like an ideal solution. Nothing I have suggested precludes the bridge maintainer from *not* adding the code making the association in the core and instead adding his/her own locking infrastructure to the bridge driver. Right now, I can think of five or six bridges all of which fall into category #1. Should we really add effectively the exact same locking code to all those bridges, running the risk of of screwing up the cut/paste? Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Serialization flag example
Hi Devin, > Hello Manu, > > The argument I am trying to make is that there are numerous cases > where you should not be able to use both a particular DVB and V4L > device at the same time. The implementation of such locking should be > handled by the v4l-dvb core, but the definition of the relationships > dictating which devices cannot be used in parallel is still the > responsibility of the driver. > > This way, a bridge driver can say "this DVB device cannot be used at > the same time as this V4L device" but the actual enforcement of the > locking is done in the core. For cases where the devices can be used > in parallel, the bridge driver doesn't have to do anything. I follow what you mean. Why I emphasized that it shouldn't be at the core, but basically in the bridge driver: Case 1 - there is a 1:n relation, In this case there is only 1 path and 3 users sharing that path In such a case, You can use such a mentioned scheme, where things do look okay, since it is only about locking a single path. Case 2 - there is a n:n relation, in this case there are n paths and n users In such a case, it is hard to make the core aware of all the details, since there could be more than 1 resource of the same category; Mapping each of them properly won't be easy, as for the same chip driver Resource A on Card A, would mean different for Resource A on Card B. Case 3 - there is a m:n relation, in this case, there are m paths and n users This case is even more painful than the previous ones. In cases 2 & 3, the option to handle such cases is using a configuration scheme based on the card type. I guess handling such would be quite daunting and hard to get right. I guess it would be much more efficient and useful to have such a feature to be made available in the bridge driver as it is a resource of the card configuration, rather than a common bridge resource. Manu -- 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] Serialization flag example
On Fri, Apr 2, 2010 at 1:43 PM, Manu Abraham wrote: > On Thu, Apr 1, 2010 at 10:24 PM, Mauro Carvalho Chehab > wrote: > >> You'll have issues also with -alsa and -dvb parts that are present on the >> drivers. >> >> Also, drivers like cx88 have another PCI device for mpeg-encoded streams. It >> behaves >> like two separate drivers (each with its own bind code at V4L core), but, as >> there's >> just one PCI bridge, and just one analog video decoder/analog audio decoder, >> the lock >> should cross between the different drivers. >> >> So, binding videobuf to v4l2_device won't solve the issue with most >> videobuf-aware drivers, >> nor the lock will work as expected, at least with most of the devices. >> >> Also, although this is not a real problem, your lock is too pedantic: it is >> blocking access to several ioctl's that don't need to be locked. For >> example, there are >> several ioctl's that just returns static: information: capabilities, >> supported video standards, >> etc. There are even some cases where dynamic ioctl's are welcome, like >> accepting QBUF/DQBUF >> without locking (or with a minimum lock), to allow different threads to >> handle the buffers. >> >> The big issue I see with this approach is that developers will become lazy >> on checking >> the locks inside the drivers: they'll just apply the flag and trust that all >> of their >> locking troubles were magically solved by the core. >> >> Maybe a better alternative would be to pass to the V4L2 core, optionally, >> some lock, >> used internally on the driver. This approach will still be pedantic, as all >> ioctls will >> keep being serialized, but at least the driver will need to explicitly >> handle the lock, >> and the same lock can be used on other parts of the driver. > > > IMO, A framework shouldn't lock. Why ? > > Different devices require different locking schemes, each and every > device require it in different ways. Some drivers might not even > require it, since they may be able to handle different systems > asynchronously. > > Locking and such device management, will be known to the driver alone > and not to any framework. While, this may benefit some few devices the > other set of devices will eventually be broken. Hello Manu, The argument I am trying to make is that there are numerous cases where you should not be able to use both a particular DVB and V4L device at the same time. The implementation of such locking should be handled by the v4l-dvb core, but the definition of the relationships dictating which devices cannot be used in parallel is still the responsibility of the driver. This way, a bridge driver can say "this DVB device cannot be used at the same time as this V4L device" but the actual enforcement of the locking is done in the core. For cases where the devices can be used in parallel, the bridge driver doesn't have to do anything. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Serialization flag example
On Thu, Apr 1, 2010 at 10:24 PM, Mauro Carvalho Chehab wrote: > You'll have issues also with -alsa and -dvb parts that are present on the > drivers. > > Also, drivers like cx88 have another PCI device for mpeg-encoded streams. It > behaves > like two separate drivers (each with its own bind code at V4L core), but, as > there's > just one PCI bridge, and just one analog video decoder/analog audio decoder, > the lock > should cross between the different drivers. > > So, binding videobuf to v4l2_device won't solve the issue with most > videobuf-aware drivers, > nor the lock will work as expected, at least with most of the devices. > > Also, although this is not a real problem, your lock is too pedantic: it is > blocking access to several ioctl's that don't need to be locked. For example, > there are > several ioctl's that just returns static: information: capabilities, > supported video standards, > etc. There are even some cases where dynamic ioctl's are welcome, like > accepting QBUF/DQBUF > without locking (or with a minimum lock), to allow different threads to > handle the buffers. > > The big issue I see with this approach is that developers will become lazy on > checking > the locks inside the drivers: they'll just apply the flag and trust that all > of their > locking troubles were magically solved by the core. > > Maybe a better alternative would be to pass to the V4L2 core, optionally, > some lock, > used internally on the driver. This approach will still be pedantic, as all > ioctls will > keep being serialized, but at least the driver will need to explicitly handle > the lock, > and the same lock can be used on other parts of the driver. IMO, A framework shouldn't lock. Why ? Different devices require different locking schemes, each and every device require it in different ways. Some drivers might not even require it, since they may be able to handle different systems asynchronously. Locking and such device management, will be known to the driver alone and not to any framework. While, this may benefit some few devices the other set of devices will eventually be broken. Manu -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] tm6000: tm6000_i2c_xfer: request labeling
From: Stefan Ringel
labeling the request after tuner reading and writeing
Signed-off-by: Stefan Ringel
---
drivers/staging/tm6000/tm6000-i2c.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/tm6000/tm6000-i2c.c
b/drivers/staging/tm6000/tm6000-i2c.c
index ec4c938..2ab632b 100644
--- a/drivers/staging/tm6000/tm6000-i2c.c
+++ b/drivers/staging/tm6000/tm6000-i2c.c
@@ -135,8 +135,8 @@ static int tm6000_i2c_xfer(struct i2c_adapter *i2c_adap,
i++;
if (addr == dev->tuner_addr << 1) {
- tm6000_set_reg(dev, 0x32, 0,0);
- tm6000_set_reg(dev, 0x33, 0,0);
+ tm6000_set_reg(dev, REQ_50_SET_START, 0, 0);
+ tm6000_set_reg(dev, REQ_51_SET_STOP, 0, 0);
}
if (i2c_debug >= 2)
for (byte = 0; byte < msgs[i].len; byte++)
@@ -150,8 +150,8 @@ static int tm6000_i2c_xfer(struct i2c_adapter *i2c_adap,
msgs[i].buf + 1, msgs[i].len - 1);
if (addr == dev->tuner_addr << 1) {
- tm6000_set_reg(dev, 0x32, 0,0);
- tm6000_set_reg(dev, 0x33, 0,0);
+ tm6000_set_reg(dev, REQ_50_SET_START, 0, 0);
+ tm6000_set_reg(dev, REQ_51_SET_STOP, 0, 0);
}
}
if (i2c_debug >= 2)
--
1.6.6.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
[PATCH 1/2] tm6000: request labeling board version check
From: Stefan Ringel
request labeling board version check
Signed-off-by: Stefan Ringel
---
drivers/staging/tm6000/tm6000-cards.c |4 ++--
drivers/staging/tm6000/tm6000-core.c | 18 --
drivers/staging/tm6000/tm6000.h |1 +
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/tm6000/tm6000-cards.c
b/drivers/staging/tm6000/tm6000-cards.c
index 2f0274d..f795a3e 100644
--- a/drivers/staging/tm6000/tm6000-cards.c
+++ b/drivers/staging/tm6000/tm6000-cards.c
@@ -480,9 +480,9 @@ int tm6000_cards_setup(struct tm6000_core *dev)
}
if (!i) {
- rc = tm6000_get_reg16(dev, 0x40, 0, 0);
+ rc = tm6000_get_reg32(dev, REQ_40_GET_VERSION, 0, 0);
if (rc >= 0)
- printk(KERN_DEBUG "board=%d\n", rc);
+ printk(KERN_DEBUG "board=0x%08x\n", rc);
}
}
diff --git a/drivers/staging/tm6000/tm6000-core.c
b/drivers/staging/tm6000/tm6000-core.c
index d9cade0..0b4dc64 100644
--- a/drivers/staging/tm6000/tm6000-core.c
+++ b/drivers/staging/tm6000/tm6000-core.c
@@ -139,6 +139,20 @@ int tm6000_get_reg16 (struct tm6000_core *dev, u8 req, u16
value, u16 index)
return buf[1]|buf[0]<<8;
}
+int tm6000_get_reg32 (struct tm6000_core *dev, u8 req, u16 value, u16 index)
+{
+ int rc;
+ u8 buf[4];
+
+ rc=tm6000_read_write_usb (dev, USB_DIR_IN | USB_TYPE_VENDOR, req,
+ value, index, buf, 4);
+
+ if (rc<0)
+ return rc;
+
+ return buf[3] | buf[2] << 8 | buf[1] << 16 | buf[0] << 24;
+}
+
void tm6000_set_fourcc_format(struct tm6000_core *dev)
{
if (dev->dev_type == TM6010) {
@@ -455,9 +469,9 @@ int tm6000_init (struct tm6000_core *dev)
msleep(5); /* Just to be conservative */
/* Check board version - maybe 10Moons specific */
- board=tm6000_get_reg16 (dev, 0x40, 0, 0);
+ board=tm6000_get_reg32 (dev, REQ_40_GET_VERSION, 0, 0);
if (board >=0) {
- printk (KERN_INFO "Board version = 0x%04x\n",board);
+ printk (KERN_INFO "Board version = 0x%08x\n",board);
} else {
printk (KERN_ERR "Error %i while retrieving board
version\n",board);
}
diff --git a/drivers/staging/tm6000/tm6000.h b/drivers/staging/tm6000/tm6000.h
index 172f7d7..d9d076b 100644
--- a/drivers/staging/tm6000/tm6000.h
+++ b/drivers/staging/tm6000/tm6000.h
@@ -224,6 +224,7 @@ int tm6000_read_write_usb (struct tm6000_core *dev, u8
reqtype, u8 req,
u16 value, u16 index, u8 *buf, u16 len);
int tm6000_get_reg (struct tm6000_core *dev, u8 req, u16 value, u16 index);
int tm6000_get_reg16(struct tm6000_core *dev, u8 req, u16 value, u16 index);
+int tm6000_get_reg32(struct tm6000_core *dev, u8 req, u16 value, u16 index);
int tm6000_set_reg (struct tm6000_core *dev, u8 req, u16 value, u16 index);
int tm6000_init (struct tm6000_core *dev);
--
1.6.6.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Serialization flag example
Hans Verkuil wrote: > On Thursday 01 April 2010 23:32:33 Mauro Carvalho Chehab wrote: >> Hans Verkuil wrote: Maybe a better alternative would be to pass to the V4L2 core, optionally, some lock, used internally on the driver. This approach will still be pedantic, as all ioctls will keep being serialized, but at least the driver will need to explicitly handle the lock, and the same lock can be used on other parts of the driver. >>> Well, I guess you could add a 'struct mutex *serialize;' field to >>> v4l2_device >>> that drivers can set. But I don't see much of a difference in practice. >> It makes easier to implement more complex approaches, where you may need to >> use more >> locks. Also, It makes no sense to make a DVB code or an alsa code dependent >> on a V4L >> header, just because it needs to see a mutex. > > What's in a name. v4l2_device is meant to be a top-level object in a driver. > There is nothing particularly v4l about it and it would be trivial to rename > it to media_device. This has nothing to do with a name. The point is that such mutex need to be used by other systems (for example, some drivers have alsa provided by snd-usb-audio). >> Also, a mutex at the driver need to be initialized inside the driver. It is >> not just one >> flag that someone writing a new driver will clone without really >> understanding what >> it is doing. > > Having a driver do mutex_init() really does not improve understanding. But > good documentation will. Creating a simple, easy to understand and well > documented locking scheme will go a long way to making our drivers better. Documentation is for sure needed. Having the mutex inside of the driver helps people to understand, as I doubt that most of the developers take a deep look inside V4L and Linux core implementations before writing a driver. I'm all for porting things that are common to the core, but things that depends on the driver internal logic, like the mutexes that protect the driver data structs, should be more visible (or be fully implemented) outside the core. > Now, having said all this, I do think upon reflection that using a pointer > to a mutex might be better. The main reason being that while I do think that > renaming v4l2_device to media_device is a good idea and that more code sharing > between v4l and dvb would benefit both (heck, perhaps there should be more > integration between v4l-dvb and alsa as well), the political reality is > different. With respect to v4l2_device: The reason should be technical, not political. A proper module/subsystem decoupling is interesting, to easy maintainership. As I said, back on LPC/2007, the core functions provided by v4l2_device makes sense and should also be used on DVB. That's my 2 cents. The reality is that migrating existing drivers to it will be very time consuming, and maybe not valuable enough, at least for those dvb drivers that are almost unmaintained nowadays, but I think that using it for new drivers and for the drivers where we have a constant pattern of maintainability would be a good thing. I think we should evolute to have a more integrated core for both V4L and DVB, that will provide the proper locking between the two subsystems. >>> Basically the CAP (capability) ioctls and all ENUM ioctls do not have to be >>> serialized. I am not sure whether the streaming ioctls should also be >>> included >>> here. I can't really grasp the consequences of whatever choice we make. If >>> we >>> want to be compatible with what happens today with ioctl and the BKL, then >>> we >>> need to lock and have videobuf unlock whenever it has to wait for an event. >> I suspect that the list of "must have" is driver-dependent. > > If needed one could allow drivers to override this function. But again, start > simple and only make it more complex if we really need to. Overengineering is > one of the worst mistakes one can make. I have seen too many projects fail > because of that. Ok. -- 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
V4L2 application compliance testing
Hi, we recently got around to test several video4linux2 applications and noticed that quite a few kernel drivers do not provide support properly testing applications for v4l2 compliance. http://sundtek.de/images/vivi.png we ported and modified the vivi to userspace, the testpackage is supposed to work with x86-64/x86-32/arm (eabi4/oabi)/mips and ppc(32/64), This virtual driver is also supposed to support all systems starting from Debian Potato (32bit) on. A quick shot for installing and testing: $ wget http://sundtek.de/media/sundtek_installer.sh $ chmod 777 sundtek_installer.sh $ ./sundtek_installer.sh adding a virtual driver (run this for each virtual interface you want to have): $ /opt/bin/mediaclient --tvdummy displaying the virtual device information: $ /opt/bin/mediaclient -e List of Media Hardware Devices device 1: [Virtual Video (vivi) Capture Board] ANALOG-TV [ANALOG-TV]: VIDEO0: /dev/video2 deleting the virtual device: $ /opt/bin/mediaclient --remove=[deviceid as indicated with mediaclient -e] to remove the package $ ./sundtek_installer -u So far we tested: * VLC (v4l1) * VLC (v4l2) * tvtime * mplayer * motion (v4l1) * ekiga * MythTV * xawtv * Zapping (poorly this application does not work with all distributions anymore since it's not maintained anymore). The virtual driver prints the frequency into the video when testing the tuner. If this driver works out with your application you can be sure that most legacy videodrivers will also work with your TV application. Aside of that we're also about to release the FreeBSD and Mac ports for the virtual driver (the package basically ports the V4L1/V4L2 interface to all other operating systems). Markus -- 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 does v4l2 driver communicate a frequency lock to mythtv
Thanks a lot for your advice! I will look into the g_tuner and signal function. On Fri, Apr 2, 2010 at 9:59 PM, Devin Heitmueller wrote: > On Fri, Apr 2, 2010 at 9:48 AM, Bee Hock Goh wrote: >> Dear all, >> >> i have been doing some usb snoop and making some changes to the >> existing staging tm6000 to get my tm5600/xc2028 usb stick to work. >> Thanks to a lot of help from Stefan, I have some limited success and >> is able to get mythtv to do channel scan. However, mythtv is not able >> to logon to the channel even though usbmon shown the same in/out using >> usbmon and snoop on the stick windows application. >> >> Where should I be looking at to inform that a channel is to be locked? > > For most applications, the G_TUNER call must set the response struct > v4l2_tuner's "signal" field to a nonzero value. > > Devin > > -- > Devin J. Heitmueller - Kernel Labs > http://www.kernellabs.com > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: em28xx vbi read timeout
On Fri, Apr 2, 2010 at 10:15 AM, Tim Stowell wrote: > Hi, > > I have a KWorld usb 2800D device and am using the newest em28xx > v4l-dvb drivers from linuxtv.org. I'm running Gentoo with a 2.6.31 > kernel. The driver compiles fine, and then I issue the following > commands: > > v4lctl setnorm NTSC > zvbi-ntsc-cc -c -d /dev/vbio -v > > > after that I just get constant "VBI Read Timeout (Ignored)" messages. > Any help is greatly appreciated. (I initially posted this question to > the kernellabs blog, I apologize I didn't know there was a mailing > list at the time, so I'm posting my question here now.) Thanks Hi Tim, Yeah, I'm a couple of hours behind on the blog postings and just getting caught up on email. I just sent a reply now. http://www.kernellabs.com/blog/?p=755#comment-1344 Cheers, Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
em28xx vbi read timeout
Hi, I have a KWorld usb 2800D device and am using the newest em28xx v4l-dvb drivers from linuxtv.org. I'm running Gentoo with a 2.6.31 kernel. The driver compiles fine, and then I issue the following commands: v4lctl setnorm NTSC zvbi-ntsc-cc -c -d /dev/vbio -v after that I just get constant "VBI Read Timeout (Ignored)" messages. Any help is greatly appreciated. (I initially posted this question to the kernellabs blog, I apologize I didn't know there was a mailing list at the time, so I'm posting my question here now.) Thanks -- 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 does v4l2 driver communicate a frequency lock to mythtv
On Fri, Apr 2, 2010 at 9:48 AM, Bee Hock Goh wrote: > Dear all, > > i have been doing some usb snoop and making some changes to the > existing staging tm6000 to get my tm5600/xc2028 usb stick to work. > Thanks to a lot of help from Stefan, I have some limited success and > is able to get mythtv to do channel scan. However, mythtv is not able > to logon to the channel even though usbmon shown the same in/out using > usbmon and snoop on the stick windows application. > > Where should I be looking at to inform that a channel is to be locked? For most applications, the G_TUNER call must set the response struct v4l2_tuner's "signal" field to a nonzero value. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
how does v4l2 driver communicate a frequency lock to mythtv
Dear all, i have been doing some usb snoop and making some changes to the existing staging tm6000 to get my tm5600/xc2028 usb stick to work. Thanks to a lot of help from Stefan, I have some limited success and is able to get mythtv to do channel scan. However, mythtv is not able to logon to the channel even though usbmon shown the same in/out using usbmon and snoop on the stick windows application. Where should I be looking at to inform that a channel is to be locked? here's small snapshot of the dmesg when mythtv is scanning. its basically calling on the VIDIOC_S_FREQUENCY and VIDIOC_G_FREQUENCY. If VIDIOC_G_FREQUENCY use in some form to inform that the correct freq has been selected? on the tm6000 code, it only return the frequency value. Hope that some of the you expert can offer me a advice. [ 3759.600038] tm6000: VIDIOC_G_FREQUENCY [ 3759.600046] xc2028 5-0061: xc2028_get_frequency called [ 3759.600108] tm6000: VIDIOC_G_TUNER [ 3759.625446] tm6000: VIDIOC_G_MODULATOR [ 3759.625454] tm6000: VIDIOC_S_FREQUENCY [ 3759.625461] xc2028 5-0061: xc2028_set_analog_freq called [ 3759.625466] xc2028 5-0061: generic_set_freq called [ 3759.625470] xc2028 5-0061: should set frequency 224250 kHz [ 3759.625473] xc2028 5-0061: check_firmware called [ 3759.625477] xc2028 5-0061: checking firmware, user requested type=F8MHZ (2), id 0010, scode_tbl (0), scode_nr 0 [ 3759.625487] xc2028 5-0061: BASE firmware not changed. [ 3759.625490] xc2028 5-0061: Std-specific firmware already loaded. [ 3759.625494] xc2028 5-0061: SCODE firmware already loaded. [ 3759.625498] xc2028 5-0061: xc2028_get_reg 0004 called [ 3759.660023] xc2028 5-0061: xc2028_get_reg 0008 called [ 3759.700027] xc2028 5-0061: Device is Xceive 3028 version 1.0, firmware version 2.4 [ 3760.070039] xc2028 5-0061: divisor= 00 00 38 10 (freq=224.250) [ 3760.070053] tm6000: VIDIOC_G_FREQUENCY [ 3760.070061] xc2028 5-0061: xc2028_get_frequency called [ 3760.070124] tm6000: VIDIOC_G_TUNER [ 3760.095457] tm6000: VIDIOC_G_MODULATOR [ 3760.095465] tm6000: VIDIOC_S_FREQUENCY [ 3760.095472] xc2028 5-0061: xc2028_set_analog_freq called [ 3760.095476] xc2028 5-0061: generic_set_freq called [ 3760.095480] xc2028 5-0061: should set frequency 487250 kHz [ 3760.095484] xc2028 5-0061: check_firmware called [ 3760.095487] xc2028 5-0061: checking firmware, user requested type=F8MHZ (2), id 0010, scode_tbl (0), scode_nr 0 [ 3760.095497] xc2028 5-0061: BASE firmware not changed. [ 3760.095501] xc2028 5-0061: Std-specific firmware already loaded. [ 3760.095504] xc2028 5-0061: SCODE firmware already loaded. [ 3760.095508] xc2028 5-0061: xc2028_get_reg 0004 called [ 3760.130027] xc2028 5-0061: xc2028_get_reg 0008 called [ 3760.170040] xc2028 5-0061: Device is Xceive 3028 version 1.0, firmware version 2.4 [ 3760.540035] xc2028 5-0061: divisor= 00 00 79 d0 (freq=487.250) [ 3760.540048] tm6000: VIDIOC_G_FREQUENCY [ 3760.540056] xc2028 5-0061: xc2028_get_frequency called [ 3760.540118] tm6000: VIDIOC_G_TUNER [ 3760.565446] tm6000: VIDIOC_G_MODULATOR [ 3760.565455] tm6000: VIDIOC_S_FREQUENCY [ 3760.565462] xc2028 5-0061: xc2028_set_analog_freq called [ 3760.565467] xc2028 5-0061: generic_set_freq called [ 3760.565471] xc2028 5-0061: should set frequency 495250 kHz [ 3760.565474] xc2028 5-0061: check_firmware called [ 3760.565478] xc2028 5-0061: checking firmware, user requested type=F8MHZ (2), id 0010, scode_tbl (0), scode_nr 0 [ 3760.565488] xc2028 5-0061: BASE firmware not changed. [ 3760.565491] xc2028 5-0061: Std-specific firmware already loaded. [ 3760.565495] xc2028 5-0061: SCODE firmware already loaded. [ 3760.565499] xc2028 5-0061: xc2028_get_reg 0004 called [ 3760.600031] xc2028 5-0061: xc2028_get_reg 0008 called [ 3760.640030] xc2028 5-0061: Device is Xceive 3028 version 1.0, firmware version 2.4 [ 3761.010032] xc2028 5-0061: divisor= 00 00 7b d0 (freq=495.250) [ 3761.010047] tm6000: VIDIOC_G_FREQUENCY [ 3761.010055] xc2028 5-0061: xc2028_get_frequency called thanks, Hock. -- 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] cx88: improve error handling
Return -EINVAL if we don't find the right query control id.
Signed-off-by: Dan Carpenter
diff --git a/drivers/media/video/cx88/cx88-video.c
b/drivers/media/video/cx88/cx88-video.c
index 48c450f..b4c80cb 100644
--- a/drivers/media/video/cx88/cx88-video.c
+++ b/drivers/media/video/cx88/cx88-video.c
@@ -1537,9 +1537,12 @@ static int radio_queryctrl (struct file *file, void
*priv,
c->id >= V4L2_CID_LASTP1)
return -EINVAL;
if (c->id == V4L2_CID_AUDIO_MUTE) {
- for (i = 0; i < CX8800_CTLS; i++)
+ for (i = 0; i < CX8800_CTLS; i++) {
if (cx8800_ctls[i].v.id == c->id)
break;
+ }
+ if (i == CX8800_CTLS)
+ return -EINVAL;
*c = cx8800_ctls[i].v;
} else
*c = no_ctl;
--
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] cx231xx: improve error handling
Return -EINVAL if we don't find the control id.
Signed-off-by: Dan Carpenter
diff --git a/drivers/media/video/cx231xx/cx231xx-video.c
b/drivers/media/video/cx231xx/cx231xx-video.c
index d4f546f..5a74ef8 100644
--- a/drivers/media/video/cx231xx/cx231xx-video.c
+++ b/drivers/media/video/cx231xx/cx231xx-video.c
@@ -1901,9 +1901,12 @@ static int radio_queryctrl(struct file *file, void *priv,
if (c->id < V4L2_CID_BASE || c->id >= V4L2_CID_LASTP1)
return -EINVAL;
if (c->id == V4L2_CID_AUDIO_MUTE) {
- for (i = 0; i < CX231XX_CTLS; i++)
+ for (i = 0; i < CX231XX_CTLS; i++) {
if (cx231xx_ctls[i].v.id == c->id)
break;
+ }
+ if (i == CX231XX_CTLS)
+ return -EINVAL;
*c = cx231xx_ctls[i].v;
} else
*c = no_ctl;
--
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 00/15] ir-core: Several improvements to allow adding LIRC and decoder plugins
On Thu, Apr 01, 2010 at 09:44:12PM -0400, Jon Smirl wrote: > On Thu, Apr 1, 2010 at 1:56 PM, Mauro Carvalho Chehab > wrote: > > This series of 15 patches improves support for IR, as discussed at the > > "What are the goals for the architecture of an in-kernel IR system?" > > thread. > > > > It basically adds a raw decoder layer at ir-core, allowing decoders to plug > > into IR core, and preparing for the addition of a lirc_dev driver that will > > allow raw IR codes to be sent to userspace. > > > > There's no lirc patch in this series. I have also a few other patches from > > David Härdeman that I'm about to test/review probably later today, but > > as I prefer to first merge what I have at V4L/DVB tree, before applying > > them. > > Has anyone ported the MSMCE driver onto these patches yet? That would > be a good check to make sure that rc-core has the necessary API. I still plan to make lots of changes to the rc-core API (I just have to convince Mauro first, but I'll get there). What I have done is to port your port of the msmce driver to the suggested rc-core subsystem I sent you in private a week or so ago, and it works fine (I've bought the hardware and tested it with 20 or so different protocols). The subsystem I suggested is basically what I'm using as inspiration while working with Mauro in improving rc-core so msmce should work well with the end product...but there's still some ground to cover. Porting the msmce driver to rc-core will be high on my list of priorities once I've done some more changes to the API. > Cooler if it works both through LIRC and with an internal protocol > decoder. The MSMCE driver in my old patches was very simplified, it > removed about half of the code from the LIRC version. Yes, and it was a great help to me at least...thanks :) -- David Härdeman -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.35] gspca for_2.6.35
Hi Mauro, The following changes since commit a6eb7bc8e0eea78f96ad1b0f0195ec52b88c6a00: Laurent Pinchart (1): V4L/DVB: uvcvideo: Use POLLOUT and POLLWRNORM for output devices are available in the git repository at: git://linuxtv.org/jfrancois/gspca.git for_2.6.35 Jean-François Moine (4): gspca: Change some copyrights and module authors. gspca - sonixj: Let the JPEG header in the device. gspca - sonixj: Add autogain for sensor gc0307. gspca - vc032x: Change the ov7670 format to YUYV. drivers/media/video/gspca/gspca.c |4 +- drivers/media/video/gspca/sonixj.c | 47 ++- drivers/media/video/gspca/vc032x.c | 747 +++- 3 files changed, 514 insertions(+), 284 deletions(-) Thanks. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- 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
CX88 TS overflows too often
Hello, I have a system with two Hauppauge Nova-S-Plus cards and a HVR4000Lite. Very often I'm getting errors like: cx88[0]/2-mpeg: general errors: 0x0100 cx88[2]/2-mpeg: general errors: 0x0100 Digging in the code, I found that is DMA related, meaning TS overflow. Looking further, I found that for TS handling, FIFO queue size is really small, only 4K, of available 32K. As long as the DVB-S2 streams are running up to 80Mbps, isn't this value too small a fluent running ? I presume that these overflows are caused by larger PCI latencies, causing delays in buffer transfers. As long as these cards are used most of the time for TS handling, isn't there any chance to increase the TS FIFO size ? Or optionally allocating more for this purpose ? The HVR4000Lite has no video or audio inputs, but there are allocated resources for them, wasted resources. Regards, Doru Marin -- 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: stv0903bab i2c-repeater question
2010/3/31 Andreas Regel : > Hi Sergey, > > Am 31.03.2010 12:14, schrieb Sergey Mironov: >> Hello maillist! >> I am integrating frontend with dvb-demux driver of one device >> called mdemux. >> >> The frontend includes following parts: >> - stv0903bab demodulator >> - stv6110a tuner >> - lnbp21 power supply controller >> >> stv6110a is connected to i2c bus via stv0903's repeater. >> >> My question is about setting up i2c repeater frequency divider (I2CRPT >> register). stv0903 datasheet says that "the speed of the i2c repeater >> obtained by >> dividing the internal chip frequency (that is, 135 MHz)" >> >> budget.c driver uses value STV090x_RPTLEVEL_16 for this divider. But >> 135*10^6/16 is still too high to be valid i2c freq. >> >> Please explain where I'm wrong. Does the base frequency really equals to >> 135 >> Mhz? Thanks. >> > > The frequency divider in I2CRPT controls the speed of the I2C repeater HW > unit inside the STV0903. The I2C clock itself has the same speed as the one > that is used to access the STV0903. The repeater basically just routes the > signals from one bus to the other and needs a higher internal frequency to > do that properly. That is the frequency you set up with I2CRPT. > > Regards > Andreas > Thanks, Andreas! Of cause, different i2c bus frequencies would require some buffer inside repeater. But there is no information about such things. I've checked carefully and it seems that ENARPT_LEVEL actually defines repeater delay. -- Sergey -- 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] Serialization flag example
On Thursday 01 April 2010 23:32:33 Mauro Carvalho Chehab wrote:
> Hans Verkuil wrote:
> >> Maybe a better alternative would be to pass to the V4L2 core, optionally,
> >> some lock,
> >> used internally on the driver. This approach will still be pedantic, as
> >> all ioctls will
> >> keep being serialized, but at least the driver will need to explicitly
> >> handle the lock,
> >> and the same lock can be used on other parts of the driver.
> >
> > Well, I guess you could add a 'struct mutex *serialize;' field to
> > v4l2_device
> > that drivers can set. But I don't see much of a difference in practice.
>
> It makes easier to implement more complex approaches, where you may need to
> use more
> locks. Also, It makes no sense to make a DVB code or an alsa code dependent
> on a V4L
> header, just because it needs to see a mutex.
What's in a name. v4l2_device is meant to be a top-level object in a driver.
There is nothing particularly v4l about it and it would be trivial to rename
it to media_device.
> Also, a mutex at the driver need to be initialized inside the driver. It is
> not just one
> flag that someone writing a new driver will clone without really
> understanding what
> it is doing.
Having a driver do mutex_init() really does not improve understanding. But
good documentation will. Creating a simple, easy to understand and well
documented locking scheme will go a long way to making our drivers better.
Now, having said all this, I do think upon reflection that using a pointer
to a mutex might be better. The main reason being that while I do think that
renaming v4l2_device to media_device is a good idea and that more code sharing
between v4l and dvb would benefit both (heck, perhaps there should be more
integration between v4l-dvb and alsa as well), the political reality is
different.
>
> > Regarding the 'pedantic approach': we can easily move the locking to
> > video_ioctl2:
> >
> > struct video_device *vdev = video_devdata(file);
> > int serialize = must_serialize_ioctl(vdev, cmd);
> >
> > if (serialize)
> > mutex_lock(&vdev->v4l2_dev->serialize_lock);
> > /* Handles IOCTL */
> > err = __video_do_ioctl(file, cmd, parg);
> > if (serialize)
> > mutex_unlock(&vdev->v4l2_dev->serialize_lock);
> >
> >
> > And must_serialize_ioctl() looks like this:
> >
> > static int must_serialize_ioctl(struct video_device *vdev, int cmd)
> > {
> > if (!vdev->v4l2_dev || !vdev->v4l2_dev->fl_serialize)
> > return 0;
> > switch (cmd) {
> > case VIDIOC_QUERYCAP:
> > case VIDIOC_ENUM_FMT:
> > ...
> > return 0;
> > }
> > return 1;
> > }
> >
> > Basically the CAP (capability) ioctls and all ENUM ioctls do not have to be
> > serialized. I am not sure whether the streaming ioctls should also be
> > included
> > here. I can't really grasp the consequences of whatever choice we make. If
> > we
> > want to be compatible with what happens today with ioctl and the BKL, then
> > we
> > need to lock and have videobuf unlock whenever it has to wait for an event.
>
> I suspect that the list of "must have" is driver-dependent.
If needed one could allow drivers to override this function. But again, start
simple and only make it more complex if we really need to. Overengineering is
one of the worst mistakes one can make. I have seen too many projects fail
because of that.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
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: adv7180 as SoC camera device
On Fri, Apr 02, 2010 at 09:09:30AM +0200, Guennadi Liakhovetski wrote:
> On Thu, 1 Apr 2010, Rodolfo Giometti wrote:
>
> > On Tue, Mar 30, 2010 at 04:06:11PM +0200, Rodolfo Giometti wrote:
> > > On Tue, Feb 23, 2010 at 12:19:13AM +0100, Richard Röjfors wrote:
> > > >
> > > > We use it as a subdev to a driver not yet committed from us. So I think
> > > > you should extend it, not move it.
> > >
> > > Finally I got something functional... but I'm puzzled to know how I
> > > can add platform data configuration struct by using the I2C's
> > > platform_data pointer if it is already used to hold struct
> > > soc_camera_device... O_o
> >
> > Here my solution:
> >
> > static __devinit int adv7180_probe(struct i2c_client *client,
> > const struct i2c_device_id *id)
> > {
> > struct adv7180_state *state;
> > #if defined(CONFIG_SOC_CAMERA)
> > struct soc_camera_device *icd = client->dev.platform_data;
> > struct soc_camera_link *icl;
> > struct adv7180_platform_data *pdata = NULL;
> > #else
> > struct adv7180_platform_data *pdata =
> > client->dev.platform_data;
> > #endif
>
> No, we don't want any ifdefs in drivers. And the current driver doesn't
> use the platform data, so, I would just leave it as it is - if NULL - no
> soc-camera and no platform data, if it set - soc-camera environment is
> used. That's until we standardise the use of platform data for v4l i2c
> devices.
Ok. Thanks! :)
Ciao,
Rodolfo
--
GNU/Linux Solutions e-mail: giome...@enneenne.com
Linux Device Driver giome...@linux.it
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
--
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: adv7180 as SoC camera device
On Thu, 1 Apr 2010, Rodolfo Giometti wrote:
> On Tue, Mar 30, 2010 at 04:06:11PM +0200, Rodolfo Giometti wrote:
> > On Tue, Feb 23, 2010 at 12:19:13AM +0100, Richard Röjfors wrote:
> > >
> > > We use it as a subdev to a driver not yet committed from us. So I think
> > > you should extend it, not move it.
> >
> > Finally I got something functional... but I'm puzzled to know how I
> > can add platform data configuration struct by using the I2C's
> > platform_data pointer if it is already used to hold struct
> > soc_camera_device... O_o
>
> Here my solution:
>
> static __devinit int adv7180_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> struct adv7180_state *state;
> #if defined(CONFIG_SOC_CAMERA)
> struct soc_camera_device *icd = client->dev.platform_data;
> struct soc_camera_link *icl;
> struct adv7180_platform_data *pdata = NULL;
> #else
> struct adv7180_platform_data *pdata =
> client->dev.platform_data;
> #endif
No, we don't want any ifdefs in drivers. And the current driver doesn't
use the platform data, so, I would just leave it as it is - if NULL - no
soc-camera and no platform data, if it set - soc-camera environment is
used. That's until we standardise the use of platform data for v4l i2c
devices.
> struct v4l2_subdev *sd;
> int i, ret;
>
> /* Check if the adapter supports the needed features */
> if (!i2c_check_functionality(client->adapter,
> I2C_FUNC_SMBUS_BYTE_DATA))
> return -EIO;
>
> v4l_info(client, "chip found @ 0x%02x (%s)\n",
> client->addr << 1, client->adapter->name);
>
> #if defined(CONFIG_SOC_CAMERA)
> if (icd) {
> icl = to_soc_camera_link(icd);
> if (!icl || !icl->priv) {
> v4l_err(client, "missing platform data!\n");
> return -EINVAL;
> }
> pdata = icl->priv;
>
> icd->ops = &adv7180_soc_ops;
> v4l_info(client, "soc-camera support enabled\n");
> }
> #endif
>
> state = kzalloc(sizeof(struct adv7180_state), GFP_KERNEL);
> if (state == NULL) {
> ret = -ENOMEM;
> goto err;
> }
>
> state->irq = client->irq;
> INIT_WORK(&state->work, adv7180_work);
> mutex_init(&state->mutex);
> state->autodetect = true;
> sd = &state->sd;
> v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
>
> if (pdata)
> for (i = 0; pdata[i].reg >= 0; i++) {
> printk("> %x %x\n", pdata[i].reg,
> pdata[i].val);
> ret = i2c_smbus_write_byte_data(client,
> pdata[i].reg, pdata[i].val);
> if (ret < 0)
> goto err_unreg_subdev;
> }
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: adv7180 as SoC camera device
On Tue, 30 Mar 2010, Rodolfo Giometti wrote:
> On Tue, Mar 30, 2010 at 04:40:17PM +0200, Guennadi Liakhovetski wrote:
> > On Tue, 30 Mar 2010, Rodolfo Giometti wrote:
> >
> > > On Tue, Feb 23, 2010 at 12:19:13AM +0100, Richard Röjfors wrote:
> > > >
> > > > We use it as a subdev to a driver not yet committed from us. So I think
> > > > you should extend it, not move it.
> > >
> > > Finally I got something functional... but I'm puzzled to know how I
> > > can add platform data configuration struct by using the I2C's
> > > platform_data pointer if it is already used to hold struct
> > > soc_camera_device... O_o
> >
> > As usual, looking at existing examples helps, e.g., in ov772x:
> >
> > priv->info = icl->priv;
> >
> > i.e., icl->priv is where you pass subdevice-driver specific data with
> > the soc-camera API.
>
> This driver was developed as v4l2-sub device and I just add the
> soc-camera support.
>
> Doing what are you suggesting is not compatible with the v4l2-sub
> device support, in fact the I2C platform_data is used to know if the
> driver must enable soc-camera or not...
Yes, this is a known limitation, I have tried to discuss this on this ML
in the past to get some reasonable solution for this, i.e., to standardise
the use of the platform-data pointer. However, this hasn't worked until
now. Maybe we manage it this time. So, what we would need is some standard
optional struct, to which platform-data may point. If it is not NULL, then
there could be either a "priv" member, that we would then use for
soc-camera specific data, or we could embed that common struct in an
soc-camera specific one. I would prefer a priv pointer.
>
> Look the code:
>
>static __devinit int adv7180_probe(struct i2c_client *client,
>const struct i2c_device_id *id)
>{
>struct adv7180_state *state;
>struct soc_camera_device *icd = client->dev.platform_data;
>
> icd is set as client->dev.platform_data. This can be use for normal
> I2C devices to hold custom platform_data initialization.
>
>struct soc_camera_link *icl;
>struct adv7180_platform_data *pdata = NULL;
>struct v4l2_subdev *sd;
>int ret;
>
>/* Check if the adapter supports the needed features */
>if (!i2c_check_functionality(client->adapter,
> I2C_FUNC_SMBUS_BYTE_DATA))
>return -EIO;
>
>v4l_info(client, "chip found @ 0x%02x (%s)\n",
>client->addr << 1, client->adapter->name);
>
>if (icd) {
>
> If the icd is set we assume it holds a soc-camera struct.
>
>icl = to_soc_camera_link(icd);
>if (!icl || !icl->priv) {
>v4l_err(client, "missing platform data!\n");
>return -EINVAL;
>}
>
>icd->ops = &adv7180_soc_ops;
>
> If soc-camera is defined we set some info in order to enable it...
>
>v4l_info(client, "soc-camera support enabled\n");
>}
> ...otherwise the driver works in the old way.
Exactly, now we just have to standardise the use of
client->dev.platform_data for v4l(2) I2C devices.
>
>state = kzalloc(sizeof(struct adv7180_state), GFP_KERNEL);
>if (state == NULL) {
>ret = -ENOMEM;
>goto err;
>}
>
>state->irq = client->irq;
>INIT_WORK(&state->work, adv7180_work);
>mutex_init(&state->mutex);
>state->autodetect = true;
>sd = &state->sd;
>v4l2_i2c_subdev_init(sd, client, &adv7180_ops);
>
> Ciao,
>
> Rodolfo
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
