Re: genius islim 310 webcam test
Jean-Francois Moine írta:
> On Wed, 31 Mar 2010 07:56:59 +0200
> Németh Márton wrote:
>
>> The next thing is that you need to learn how to compile the Linux
>> kernel from source code. There is a description for Ubuntu at
>> https://help.ubuntu.com/community/Kernel/Compile . After you are able
>> to compile and install your new kernel, you can try to apply the
>> patch in this email, recompile the kernel, install the kernel and the
>> modules, unload the gspca_pac7302 kernel module ("rmmod
>> gspca_pac7302"), and then plug the webcam in order it can load the
>> new kernel module. When you were successful with these steps you'll
>> see new messages in the output of "dmesg" command. Please send this
>> output also.
>
> Hello Németh and Sergei,
>
> I think the patch is not needed because it just gives the vend:prod
> which is already known by lsusb.
To avoid misunderstandings, the patch I sent is not just printing the
USB vendor ID and product ID but also really enables the pac7302 gspca
subdriver to actually work with the newly added USB IDs.
> On the other hand, compiling a full kernel is not needed with a small
> tarball distribution as the one I have in my page (actual gspca-2.9.10).
This is also a possible way to go, the important thing is that a kernel
module has to be built and the previous version of gspca_pac7302 kernel
module has to be replaced with the newly built one.
Regards,
Márton Németh
--
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: genius islim 310 webcam test
On Wed, 31 Mar 2010 07:56:59 +0200
Németh Márton wrote:
> The next thing is that you need to learn how to compile the Linux
> kernel from source code. There is a description for Ubuntu at
> https://help.ubuntu.com/community/Kernel/Compile . After you are able
> to compile and install your new kernel, you can try to apply the
> patch in this email, recompile the kernel, install the kernel and the
> modules, unload the gspca_pac7302 kernel module ("rmmod
> gspca_pac7302"), and then plug the webcam in order it can load the
> new kernel module. When you were successful with these steps you'll
> see new messages in the output of "dmesg" command. Please send this
> output also.
Hello Németh and Sergei,
I think the patch is not needed because it just gives the vend:prod
which is already known by lsusb.
On the other hand, compiling a full kernel is not needed with a small
tarball distribution as the one I have in my page (actual gspca-2.9.10).
Best regards.
--
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
Re: [RFC] What are the goals for the architecture of an in-kernel IR system?
David Härdeman wrote: > On Sun, Mar 28, 2010 at 09:51:17PM -0300, Mauro Carvalho Chehab wrote: >> I spoke too soon... removing the index causes a problem at the read ioctl: >> there's no way >> to retrieve just the non-sparsed values. >> >> There's one solution that would allow both read/write and compat to work >> nicely, >> but the API would become somewhat asymmetrical: >> >> At get (EVIOCGKEYCODEBIG): >> use index/len as input and keycode/scancode as output; >> >> At set (EVIOCSKEYCODEBIG): >> use scancode/keycode/len as input (and, optionally, index as output). >> > > This was exactly the approach I had in mind when I suggested using > indexes. Doesn't work perfectly. The asymmetry has a side effect on the internal logic: EVIOCGKEYCODEBIG should be implemented with a pseudo-code like: kt_entry = getkeycodebig_from_index(index); EVIOCSKEYCODEBIG should be implemented with a pseudo-code like: kt_entry = getkeycodebig_from_scan(scan, len); old_key = kt_entry->keycode; kt_entry->keycode = newkey; if (setkeycodebig(kt_entry) == 0) keyup(old_key); As you see, the input parameters for the getkeycodebig*() are different. So, this approach requires 3 ops instead of 2. Yet, as scancode->keycode is needed anyway, this doesn't actually hurts. I just added the patches that implement those two ioctls on my IR development tree. I tested only the original EVIOCGKEYCODE/EVIOSGKEYCODE and calling a clear_table function using EVIOCSKEYCODEBIG via emulation. My next step is to test the remaining ir-keytable functions via emulation, and then implement the *BIG ioctls at ir-core, for testing. I haven't test yet the *keycode*default methods. After having it fully tested, I'll submit the complete input ioctl patch via ML. -- 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: genius islim 310 webcam test
Hello Sergei,
Krivchikov Sergei wrote:
> Hi!
> I have a Genius iSlim 310 webcam and linux ubuntu 9.10 kernel 2.6.31. My
> developer level under linux - beginner (very beginner) :) but i'm ready
> to test this webcam undr linux. But I need the instructions:) How was
> your success in the launch of this webcam under Linux?
>
> Sincerely, Sergei Krivchikov.
Currently the Genius iSlim 310 webcam is not supported by the mainline Linux
kernel, but I think that the support can be added very easily. See the
description at http://linuxtv.org/wiki/index.php/PixArt_PAC7301/PAC7302 why.
First thing what I'll need that you connect your Genius iSlim 310 to the Linux
box, open an xterm and send the output of "lsusb" command. Then we can find
out the USB ID of the device.
Also, please send the output of dmesg. The easiest way to do this is to redirect
the standard output to a file like this: "dmesg >dmesg.txt" and then send the
created file as an attachment.
The next thing is that you need to learn how to compile the Linux kernel
from source code. There is a description for Ubuntu at
https://help.ubuntu.com/community/Kernel/Compile . After you are able to
compile and install your new kernel, you can try to apply the patch in this
email, recompile the kernel, install the kernel and the modules,
unload the gspca_pac7302 kernel module ("rmmod gspca_pac7302"), and then
plug the webcam in order it can load the new kernel module. When you were
successful with these steps you'll see new messages in the output of "dmesg"
command. Please send this output also.
Finally, what you will also need is that the libv4l-0 package of Ubuntu
( http://packages.ubuntu.com/intrepid/libv4l-0 ) is installed on your computer.
This package you may have already installed.
These are the main points. Let's see how far you can proceed. Let me know
if you need further assistance.
Regards,
Márton Németh
---
From: Márton Németh
On the schematics in PixArt PAC7301/PAC7302 datasheet
(http://www.pixart.com.tw/upload/PAC7301_7302%20%20Spec%20V1_20091228174030.pdf)
pages 19, 20, 21 and 22 there is a note titled "PID IO_TRAP" which describes
the possible product ID range 0x2620..0x262f. In this range there are some
known webcams, however, there are some PIDs with unknown or future devices.
Because PixArt PAC7301/PAC7302 is a System on a Chip (SoC) device is is
probable that this driver will work correctly independent of the used PID.
Signed-off-by: Márton Németh
---
diff -r dfa82cf98a85 linux/drivers/media/video/gspca/pac7302.c
--- a/linux/drivers/media/video/gspca/pac7302.c Sat Jan 30 20:03:02 2010 +0100
+++ b/linux/drivers/media/video/gspca/pac7302.c Sun Jan 31 11:08:21 2010 +0100
@@ -96,6 +96,7 @@
u8 flags;
#define FL_HFLIP 0x01 /* mirrored by default */
#define FL_VFLIP 0x02 /* vertical flipped by default */
+#define FL_EXPERIMENTAL 0x80 /* USB IDs based on heuristic without any known
product */
u8 sof_read;
u8 autogain_ignore_frames;
@@ -1220,17 +1221,33 @@
};
/* -- module initialisation -- */
+/* Note on FL_EXPERIMENTAL:
+ * On the schematics in PixArt PAC7301/PAC7302 datasheet
+ *
(http://www.pixart.com.tw/upload/PAC7301_7302%20%20Spec%20V1_20091228174030.pdf)
+ * pages 19, 20, 21 and 22 there is a note titled "PID IO_TRAP" which describes
+ * the possible product ID range 0x2620..0x262f. In this range there are some
+ * known webcams, however, there are some PIDs with unknown or future devices.
+ * Because PixArt PAC7301/PAC7302 is a System on a Chip (SoC) device is is
+ * probable that this driver will work correctly independent of the used PID.
+ */
static const struct usb_device_id device_table[] __devinitconst = {
{USB_DEVICE(0x06f8, 0x3009)},
{USB_DEVICE(0x093a, 0x2620)},
{USB_DEVICE(0x093a, 0x2621)},
{USB_DEVICE(0x093a, 0x2622), .driver_info = FL_VFLIP},
+ {USB_DEVICE(0x093a, 0x2623), .driver_info = FL_EXPERIMENTAL },
{USB_DEVICE(0x093a, 0x2624), .driver_info = FL_VFLIP},
+ {USB_DEVICE(0x093a, 0x2625), .driver_info = FL_EXPERIMENTAL },
{USB_DEVICE(0x093a, 0x2626)},
+ {USB_DEVICE(0x093a, 0x2627), .driver_info = FL_EXPERIMENTAL },
{USB_DEVICE(0x093a, 0x2628)},
{USB_DEVICE(0x093a, 0x2629), .driver_info = FL_VFLIP},
{USB_DEVICE(0x093a, 0x262a)},
+ {USB_DEVICE(0x093a, 0x262b), .driver_info = FL_EXPERIMENTAL },
{USB_DEVICE(0x093a, 0x262c)},
+ {USB_DEVICE(0x093a, 0x262d), .driver_info = FL_EXPERIMENTAL },
+ {USB_DEVICE(0x093a, 0x262e), .driver_info = FL_EXPERIMENTAL },
+ {USB_DEVICE(0x093a, 0x262f), .driver_info = FL_EXPERIMENTAL },
{}
};
MODULE_DEVICE_TABLE(usb, device_table);
@@ -1239,6 +1256,17 @@
static int __devinit sd_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
+ if ((u8)id->driver_info & FL_EXPERIMENTAL) {
+ PDEBUG(D_ERR | D_PROBE, "WARNING: USB device ID %04x:%04x is "
+
Re: GIGABYTE U8000-RH Analog source support ?
May you elaborate about the "huge undertaking" Fernando. > > 2010/3/3 RoboSK : > > Hi, i find this page *1 with last change from "3 May 2009" with text "no > > driver written for the CX25843-24Z" and then this *2 from "27 September > > 2009" with text "CX2584x chips are fully supported by Linux..." = have linux > > (now/future) support for Analog source with this USB stick ? > > > > thanks > > > > Robo > > The wiki page is just wrong. The reason that board is not supported > is not because of the cx25843. It's because the dib0700 bridge falls > under the dvb-usb framework, and the framework doesn't have analog > support at all. > > Adding such support would be a huge undertaking, but if it were done a > whole bunch of products would start getting analog support (all of the > dib0700 products which also have analog onboard). > > 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] Fix default state Beholder H6 tuner.
Hi Hermann
> Hi,
>
> Am Dienstag, den 30.03.2010, 16:02 +1000 schrieb Dmitri Belimov:
> > Hi
> >
> > The hybrid tuner FMD1216MEX_MK3 after cold start has disabled IF.
> > This tuner has internal I2C switch. This switch switch I2C bus
> > between DVB-T and IF part. Default state is DVB-T. When module
> > saa7134 is load it can't find IF tda9887 and disable analog TV mode.
> >
> > This patch set internal I2C switch of the tuner to IF by send
> > special value to the tuner as for receive analog TV from low band.
> > It can be usefule for other cards.
> >
> > I didn't set configure a tuner by a tuner model because this tuner
> > can has different I2C address. May be we can do it later after
> > discuss for more robust support a tuners.
>
> just as a reminder. It is the same for the FMD1216ME hybrid MK3.
> After every boot, analog mode fails with missing tda9887.
>
> Currently, after tuner modules are not independent anymore, one has to
> reload the saa7134 driver once.
>
> Relevant code in tuner.core.c.
>
> case TUNER_PHILIPS_FMD1216ME_MK3:
> buffer[0] = 0x0b;
> buffer[1] = 0xdc;
> buffer[2] = 0x9c;
> buffer[3] = 0x60;
> i2c_master_send(c, buffer, 4);
> mdelay(1);
> buffer[2] = 0x86;
> buffer[3] = 0x54;
> i2c_master_send(c, buffer, 4);
> if (!dvb_attach(simple_tuner_attach, &t->fe,
> t->i2c->adapter, t->i2c->addr,
> t->type)) goto attach_failed;
> break;
That is good. I'll try add case TUNER_PHILIPS_FMD1216MEX_MK3 here and test.
This is much better.
With my best regards, Dmitry.
> Hermann
>
> > diff -r 1ef0265456c8
> > linux/drivers/media/video/saa7134/saa7134-cards.c ---
> > a/linux/drivers/media/video/saa7134/saa7134-cards.c Fri Mar
> > 26 00:54:18 2010 -0300 +++
> > b/linux/drivers/media/video/saa7134/saa7134-cards.c Sun Mar
> > 28 08:21:10 2010 -0400 @@ -7450,6 +7450,21 @@ } break;
> > }
> > + case SAA7134_BOARD_BEHOLD_H6:
> > + {
> > + u8 data[] = { 0x09, 0x9f, 0x86, 0x11};
> > + struct i2c_msg msg = {.addr = 0x61, .flags =
> > 0, .buf = data,
> > + .len =
> > sizeof(data)}; +
> > + /* The tuner TUNER_PHILIPS_FMD1216MEX_MK3 after
> > hardware*/
> > + /* start has disabled IF and enabled DVB-T. When
> > saa7134*/
> > + /* scan I2C devices it not detect IF tda9887 and
> > can`t */
> > + /* watch TV without software reboot. For solve
> > this problem */
> > + /* switch the tuner to analog TV mode
> > manually. */
> > + if (i2c_transfer(&dev->i2c_adap, &msg, 1) != 1)
> > + printk(KERN_WARNING
> > + "%s: Unable to enable IF of
> > the tuner.\n",
> > + dev->name);
> > + break;
> > + }
> > } /* switch() */
> >
> > /* initialize tuner */
> >
> > Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov
> >
> >
> > With my best regards, Dmitry.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 2/10] V4L2 patches for Intel Moorestown Camera Imaging Drivers - part 1
Hi Hans,
See my inline comment.
Xiaolin
-Original Message-
From: Hans Verkuil [mailto:hverk...@xs4all.nl]
Sent: Monday, March 29, 2010 2:41 PM
To: Zhang, Xiaolin
Cc: linux-media@vger.kernel.org; Zhu, Daniel; Yu, Jinlu; Wang, Wen W; Huang,
Kai; Hu, Gang A; Ba, Zheng; Pawel Osciak
Subject: Re: [PATCH v2 2/10] V4L2 patches for Intel Moorestown Camera Imaging
Drivers - part 1
Hi Xiaolin,
On Sunday 28 March 2010 16:42:30 Zhang, Xiaolin wrote:
> From 1c18c41be33246e4b766d0e95e28a72dded87475 Mon Sep 17 00:00:00 2001
> From: Xiaolin Zhang
> Date: Sun, 28 Mar 2010 21:31:24 +0800
> Subject: [PATCH 2/10] This patch is second part of intel moorestown isp
> driver and c files collection which is v4l2 implementation.
>
> +struct videobuf_dma_contig_memory {
> + u32 magic;
> + void *vaddr;
> + dma_addr_t dma_handle;
> + unsigned long size;
> + int is_userptr;
> +};
> +
> +#define MAGIC_DC_MEM 0x0733ac61
> +#define MAGIC_CHECK(is, should)
> \
> + if (unlikely((is) != (should))) { \
> + pr_err("magic mismatch: %x expected %x\n", (is), (should)); \
> + BUG(); \
> + }
I will do a more in-depth review in a few days. However, I did notice that
you added your own dma_contig implementation. What were the reasons for doing
this? I've CC-ed Pawel since he will be interested in this as well.
[Xiaolin] good question. The basic reason is that we don't need video buffer
core (dma_contig) to allocate the memory because the camera memory is reserved
by BIOS and camera driver using ioremap the PCI bar1 address to access the
memory.
Another question that came up is: what is 'marvin'? It's clearly a codename,
but a codename for what? This should be documented at the top of some source
or header. Apologies if it is already documented, I didn't read everything yet.
[Xiaolin] Marvin is the code name of ISP IP core in Moorestown.
A final point I noticed: don't cast away a function result:
(void)ci_isp_set_bp_detection(NULL);
No need for (void). The gcc compiler won't warn about this unless the function
is annotated with __must_check__.
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: webcam problem after suspend/hibernate
Hi Mohamed, On Tuesday 30 March 2010 23:55:38 Mohamed Ikbel Boulabiar wrote: > Hi, > > After suspend/resume, I have my webcam no more working. > The /dev/video0 file still exist, but the webcam won't be used until I do > this : rmmod uvcvideo > modprobe uvcvideo > (2.6.31.8-0.1) > > This is may be caused by a bug somewhere. > These are more information about my hardware : > > I have Microdia webcam > `lsusb` > Bus 001 Device 004: ID 0c45:62c0 Microdia Sonix USB 2.0 Camera > > on openSUSE 11.2 `uname -a` > Linux linux-l365 2.6.31.8-0.1-desktop #1 SMP PREEMPT 2009-12-15 > 23:55:40 +0100 i686 i686 i386 GNU/Linux > > `hwinfo --usb` > > : USB 00.0: Unclassified device > > [Created at usb.122] > UDI: > /org/freedesktop/Hal/devices/usb_device_c45_62c0_1_3_2_1_7_if0_logicaldev_ > input Unique ID: Uc5H.F0c0EBqBP10 > Parent ID: k4bc.9T1GDCLyFd9 > SysFS ID: /devices/pci:00/:00:1d.7/usb1/1-4/1-4:1.0 > SysFS BusID: 1-4:1.0 > Hardware Class: unknown > Model: "Microdia LG Webcam" > Hotplug: USB > Vendor: usb 0x0c45 "Microdia" > Device: usb 0x62c0 "LG Webcam" > Revision: "32.17" > Serial ID: "1.3.2.1.7" > Driver: "uvcvideo" > Driver Modules: "uvcvideo" > Device File: /dev/input/event8 > Device Files: /dev/input/event8, /dev/char/13:72, > /dev/input/by-id/usb-LG_Innotek_LG_Webcam_1.3.2.1.7-event-if00, > /dev/input/by-path/pci-:00:1d.7-usb-0:4:1.0-event > Device Number: char 13:72 > Speed: 480 Mbps > Module Alias: "usb:v0C45p62C0d3217dcEFdsc02dp01ic0Eisc01ip00" > Driver Info #0: > Driver Status: uvcvideo is active > Driver Activation Cmd: "modprobe uvcvideo" > Config Status: cfg=no, avail=yes, need=no, active=unknown > Attached to: #4 (Hub) > > > If there is a scenario you propose me to do to detect from where comes > the problem, I can apply it. Could you please post the messages printed by the uvcvideo driver and USB core to the kernel log when you suspend and resume your system ? Thanks. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
webcam problem after suspend/hibernate
Hi, After suspend/resume, I have my webcam no more working. The /dev/video0 file still exist, but the webcam won't be used until I do this : rmmod uvcvideo modprobe uvcvideo (2.6.31.8-0.1) This is may be caused by a bug somewhere. These are more information about my hardware : I have Microdia webcam `lsusb` Bus 001 Device 004: ID 0c45:62c0 Microdia Sonix USB 2.0 Camera on openSUSE 11.2 `uname -a` Linux linux-l365 2.6.31.8-0.1-desktop #1 SMP PREEMPT 2009-12-15 23:55:40 +0100 i686 i686 i386 GNU/Linux `hwinfo --usb` : USB 00.0: Unclassified device [Created at usb.122] UDI: /org/freedesktop/Hal/devices/usb_device_c45_62c0_1_3_2_1_7_if0_logicaldev_input Unique ID: Uc5H.F0c0EBqBP10 Parent ID: k4bc.9T1GDCLyFd9 SysFS ID: /devices/pci:00/:00:1d.7/usb1/1-4/1-4:1.0 SysFS BusID: 1-4:1.0 Hardware Class: unknown Model: "Microdia LG Webcam" Hotplug: USB Vendor: usb 0x0c45 "Microdia" Device: usb 0x62c0 "LG Webcam" Revision: "32.17" Serial ID: "1.3.2.1.7" Driver: "uvcvideo" Driver Modules: "uvcvideo" Device File: /dev/input/event8 Device Files: /dev/input/event8, /dev/char/13:72, /dev/input/by-id/usb-LG_Innotek_LG_Webcam_1.3.2.1.7-event-if00, /dev/input/by-path/pci-:00:1d.7-usb-0:4:1.0-event Device Number: char 13:72 Speed: 480 Mbps Module Alias: "usb:v0C45p62C0d3217dcEFdsc02dp01ic0Eisc01ip00" Driver Info #0: Driver Status: uvcvideo is active Driver Activation Cmd: "modprobe uvcvideo" Config Status: cfg=no, avail=yes, need=no, active=unknown Attached to: #4 (Hub) If there is a scenario you propose me to do to detect from where comes the problem, I can apply it. Thanks, ik. -- 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] s2255drv: cleanup of driver disconnect code
On Tuesday 30 March 2010 23:09:49 Hans Verkuil wrote: > On Tuesday 30 March 2010 22:49:08 dean wrote: > > Thanks for this and the other feedback. > > > > The concern, without knowing the full history, is if video_device_alloc > > changes to do more than just allocate the whole structure with a single > > call to kzalloc? Otherwise, why have this extra indirection and > > overhead in most V4L drivers? > > It is unlikely that video_device_alloc() will change. I think it is just > historical. I have a preference for not allocating it at all, but embedding > it in a larger struct. This type of embedding is very common in the kernel. > > But if you do allocate it, then use video_device_alloc() rather than kzalloc, > if only because that makes it consistent and easier to grep on should we > need to replace it in the future. Just a quick follow-up: if someone is going to do some work on this driver, then it would be nice if the BKL (lock_kernel) can be removed as well. And also add struct v4l2_device. Admittedly, that doesn't do much (yet), but it will become more important in the near future. Eventually this struct will be required by all drivers. There are more things that can be simplified as well. Let me know if someone is interested in cleaning up/improving this driver. Regards, Hans > > Regards, > > Hans > > > The majority of V4L drivers are using video_device_alloc. Very few > > (bw-qcam.h, c-qcam.c, cpia.h, pvrusb2, usbvideo) are using "struct > > video_device" statically similar to solution 1. Three drivers(zoran, > > radio-gemtek, saa5249) are allocating their own video_device structure > > directly with kzalloc similar to solution #2. > > > > The call definitely needs checked, but I'd like some more feedback on this. > > > > Thanks and best regards, > > > > Dean > > > > > > > > > > David Ellingsworth wrote: > > > This patch looks good, but there was one other thing that caught my eye. > > > > > > In s2255_probe_v4l, video_device_alloc is called for each video > > > device, which is nothing more than a call to kzalloc, but the result > > > of the call is never verified. > > > > > > Given that this driver has a fixed number of video device nodes, the > > > array of video_device structs could be allocated within the s2255_dev > > > struct. This would remove the extra calls to video_device_alloc, > > > video_device_release, and the additional error checks that should have > > > been there. If you'd prefer to keep the array of video_device structs > > > independent of the s2255_dev struct, an alternative would be to > > > dynamically allocate the entire array at once using kcalloc and store > > > only the pointer to the array in the s2255_dev struct. In my opinion, > > > either of these methods would be better than calling > > > video_device_alloc for each video device that needs to be registered. > > > > > > Regards, > > > > > > David Ellingsworth > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-media" in > > > the body of a message to majord...@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-media" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- 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: [PATCH] s2255drv: cleanup of driver disconnect code
On Tuesday 30 March 2010 22:49:08 dean wrote: > Thanks for this and the other feedback. > > The concern, without knowing the full history, is if video_device_alloc > changes to do more than just allocate the whole structure with a single > call to kzalloc? Otherwise, why have this extra indirection and > overhead in most V4L drivers? It is unlikely that video_device_alloc() will change. I think it is just historical. I have a preference for not allocating it at all, but embedding it in a larger struct. This type of embedding is very common in the kernel. But if you do allocate it, then use video_device_alloc() rather than kzalloc, if only because that makes it consistent and easier to grep on should we need to replace it in the future. Regards, Hans > The majority of V4L drivers are using video_device_alloc. Very few > (bw-qcam.h, c-qcam.c, cpia.h, pvrusb2, usbvideo) are using "struct > video_device" statically similar to solution 1. Three drivers(zoran, > radio-gemtek, saa5249) are allocating their own video_device structure > directly with kzalloc similar to solution #2. > > The call definitely needs checked, but I'd like some more feedback on this. > > Thanks and best regards, > > Dean > > > > > David Ellingsworth wrote: > > This patch looks good, but there was one other thing that caught my eye. > > > > In s2255_probe_v4l, video_device_alloc is called for each video > > device, which is nothing more than a call to kzalloc, but the result > > of the call is never verified. > > > > Given that this driver has a fixed number of video device nodes, the > > array of video_device structs could be allocated within the s2255_dev > > struct. This would remove the extra calls to video_device_alloc, > > video_device_release, and the additional error checks that should have > > been there. If you'd prefer to keep the array of video_device structs > > independent of the s2255_dev struct, an alternative would be to > > dynamically allocate the entire array at once using kcalloc and store > > only the pointer to the array in the s2255_dev struct. In my opinion, > > either of these methods would be better than calling > > video_device_alloc for each video device that needs to be registered. > > > > Regards, > > > > David Ellingsworth > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-media" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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: [PATCH] s2255drv: cleanup of driver disconnect code
Thanks for this and the other feedback. The concern, without knowing the full history, is if video_device_alloc changes to do more than just allocate the whole structure with a single call to kzalloc? Otherwise, why have this extra indirection and overhead in most V4L drivers? The majority of V4L drivers are using video_device_alloc. Very few (bw-qcam.h, c-qcam.c, cpia.h, pvrusb2, usbvideo) are using "struct video_device" statically similar to solution 1. Three drivers(zoran, radio-gemtek, saa5249) are allocating their own video_device structure directly with kzalloc similar to solution #2. The call definitely needs checked, but I'd like some more feedback on this. Thanks and best regards, Dean David Ellingsworth wrote: This patch looks good, but there was one other thing that caught my eye. In s2255_probe_v4l, video_device_alloc is called for each video device, which is nothing more than a call to kzalloc, but the result of the call is never verified. Given that this driver has a fixed number of video device nodes, the array of video_device structs could be allocated within the s2255_dev struct. This would remove the extra calls to video_device_alloc, video_device_release, and the additional error checks that should have been there. If you'd prefer to keep the array of video_device structs independent of the s2255_dev struct, an alternative would be to dynamically allocate the entire array at once using kcalloc and store only the pointer to the array in the s2255_dev struct. In my opinion, either of these methods would be better than calling video_device_alloc for each video device that needs to be registered. Regards, David Ellingsworth -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[cron job] v4l-dvb daily build 2.6.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:Tue Mar 30 19:00:20 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/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Hauppauge WinTV HVR-900H
George Tellalov wrote: > On Mon, Mar 29, 2010 at 08:06:40PM +0200, Stefan Ringel wrote: >> >> Can you scan a cannel list? If yes, then have you the right audio and video >> setting in vlc? And if you use Kaffeine? The log looks ok, dvb signal has >> it, streams and feed is on. >> > > For some reason I can't get the dvbscan tool and the vlc to work. However > kaffeine works just fine - it performed the scan and played the channels, so I > can confirm now that dvb works. > > Now back to analog. Is there anything I can do to help debugging the oops? The bug is somewhere in the code that fills the video buffers, at tm6000-video. In order to fix, you'll likely need to have a separate machine, and use a serial cable for console, since you may eventually convert the bug into a Panic again and, without the console, you'll loose the error dumps. Alternatively, you may try to start your machine on text mode and use some program to capture the traffic, like capture-example (on v4l-utils tree). It should be noticed that older versions of the code worked without generating panic or oops. So, maybe digging into tm6000-video history, you may be able to find a code that won't cause panic/oops. The problem with the old code is that sometimes the code weren't able to produce an entire frame (lots of dropped/missed URB's). It should also be noticed that videobuf-vmalloc suffered a few changes along the time. So, if you backport a really old code, you'll likely need to re-apply some fixes on the code, due to the videobuf changes. -- 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] dib7000p.c: Fix for warning: the frame size of 1236 bytes is larger than 1024 bytes
On 03/30/10 11:02, Ricardo Maraschini wrote: > When compiling the last version of v4l-dvb tree I got the following message: > > /data/Projects/kernel/v4l-dvb/v4l/dib7000p.c: In function > 'dib7000p_i2c_enumeration': > /data/Projects/kernel/v4l-dvb/v4l/dib7000p.c:1393: warning: the frame size of > 1236 bytes is larger than 1024 bytes > > I believe that this problem is related to stack size, because we are > allocating memory for a big structure. > I changed the approach to dinamic allocated memory and the warning disappears. > The same problem appears on dib3000 as well, and I can fix that too if this > patch get in. > > Any comment on that? > I'll appreciate to read any comment from more experienced code makers. Hi, There is one caller of dib7000p_i2c_enumeration() that does not check its return value/error code. See drivers/media/dvb/dvb-usb/cxusb.c::cxusb_dualdig4_rev2_frontend_attach(): dib7000p_i2c_enumeration(&adap->dev->i2c_adap, 1, 18, &cxusb_dualdig4_rev2_config); That is in my (similar) patch and I also posted a dib3000 patch. Yes, it would be good if someone could review & merge them. https://patchwork.kernel.org/patch/77891/ https://patchwork.kernel.org/patch/77892/ -- ~Randy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Hauppauge WinTV HVR-900H
On Mon, Mar 29, 2010 at 08:06:40PM +0200, Stefan Ringel wrote: > > Can you scan a cannel list? If yes, then have you the right audio and video > setting in vlc? And if you use Kaffeine? The log looks ok, dvb signal has it, > streams and feed is on. > For some reason I can't get the dvbscan tool and the vlc to work. However kaffeine works just fine - it performed the scan and played the channels, so I can confirm now that dvb works. Now back to analog. Is there anything I can do to help debugging the oops? -- 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] dib7000p.c: Fix for warning: the frame size of 1236 bytes is larger than 1024 bytes
When compiling the last version of v4l-dvb tree I got the following message:
/data/Projects/kernel/v4l-dvb/v4l/dib7000p.c: In function
'dib7000p_i2c_enumeration':
/data/Projects/kernel/v4l-dvb/v4l/dib7000p.c:1393: warning: the frame size of
1236 bytes is larger than 1024 bytes
I believe that this problem is related to stack size, because we are allocating
memory for a big structure.
I changed the approach to dinamic allocated memory and the warning disappears.
The same problem appears on dib3000 as well, and I can fix that too if this
patch get in.
Any comment on that?
I'll appreciate to read any comment from more experienced code makers.
Signed-off-by: Ricardo Maraschini
--- a/linux/drivers/media/dvb/frontends/dib7000p.c Sat Mar 27 23:09:47
2010 -0300
+++ b/linux/drivers/media/dvb/frontends/dib7000p.c Tue Mar 30 13:03:59
2010 -0300
@@ -1349,46 +1349,57 @@
int dib7000p_i2c_enumeration(struct i2c_adapter *i2c, int no_of_demods, u8
default_addr, struct dib7000p_config cfg[])
{
- struct dib7000p_state st = { .i2c_adap = i2c };
+ struct dib7000p_state *st = NULL;
int k = 0;
u8 new_addr = 0;
+ st = kmalloc(sizeof(struct dib7000p_state), GFP_KERNEL);
+ if (!st) {
+ dprintk("DiB7000P: Unable to allocate memory\n");
+ return -ENOMEM;
+ }
+
+ st->i2c_adap = i2c;
+
+
for (k = no_of_demods-1; k >= 0; k--) {
- st.cfg = cfg[k];
+ st->cfg = cfg[k];
/* designated i2c address */
new_addr = (0x40 + k) << 1;
- st.i2c_addr = new_addr;
- dib7000p_write_word(&st, 1287, 0x0003); /* sram lead in, rdy */
- if (dib7000p_identify(&st) != 0) {
- st.i2c_addr = default_addr;
- dib7000p_write_word(&st, 1287, 0x0003); /* sram lead
in, rdy */
- if (dib7000p_identify(&st) != 0) {
+ st->i2c_addr = new_addr;
+ dib7000p_write_word(st, 1287, 0x0003); /* sram lead in, rdy */
+ if (dib7000p_identify(st) != 0) {
+ st->i2c_addr = default_addr;
+ dib7000p_write_word(st, 1287, 0x0003); /* sram lead in,
rdy */
+ if (dib7000p_identify(st) != 0) {
dprintk("DiB7000P #%d: not identified\n", k);
+ kfree(st);
return -EIO;
}
}
/* start diversity to pull_down div_str - just for
i2c-enumeration */
- dib7000p_set_output_mode(&st, OUTMODE_DIVERSITY);
+ dib7000p_set_output_mode(st, OUTMODE_DIVERSITY);
/* set new i2c address and force divstart */
- dib7000p_write_word(&st, 1285, (new_addr << 2) | 0x2);
+ dib7000p_write_word(st, 1285, (new_addr << 2) | 0x2);
dprintk("IC %d initialized (to i2c_address 0x%x)", k, new_addr);
}
for (k = 0; k < no_of_demods; k++) {
- st.cfg = cfg[k];
- st.i2c_addr = (0x40 + k) << 1;
+ st->cfg = cfg[k];
+ st->i2c_addr = (0x40 + k) << 1;
// unforce divstr
- dib7000p_write_word(&st, 1285, st.i2c_addr << 2);
+ dib7000p_write_word(st, 1285, st->i2c_addr << 2);
/* deactivate div - it was just for i2c-enumeration */
- dib7000p_set_output_mode(&st, OUTMODE_HIGH_Z);
+ dib7000p_set_output_mode(st, OUTMODE_HIGH_Z);
}
+ kfree(st);
return 0;
}
EXPORT_SYMBOL(dib7000p_i2c_enumeration);
--
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 -next] gspca: fix build for INPUT=m or INPUT=n
From: Randy Dunlap
Handle case of GSPCA=m, INPUT=m when building gspca core;
also handle case of INPUT=n by using stubs.
drivers/media/video/gspca/gspca.c:662: error: implicit declaration of function
'gspca_input_destroy_urb'
drivers/media/video/gspca/gspca.c:668: error: implicit declaration of function
'gspca_input_create_urb'
drivers/media/video/gspca/gspca.c:2284: error: implicit declaration of function
'gspca_input_connect'
Signed-off-by: Randy Dunlap
---
drivers/media/video/gspca/gspca.c | 23 ++-
drivers/media/video/gspca/gspca.h |6 +++---
2 files changed, 21 insertions(+), 8 deletions(-)
--- linux-next-20100330.orig/drivers/media/video/gspca/gspca.c
+++ linux-next-20100330/drivers/media/video/gspca/gspca.c
@@ -40,7 +40,7 @@
#include "gspca.h"
-#ifdef CONFIG_INPUT
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
#include
#include
#endif
@@ -115,7 +115,7 @@ static const struct vm_operations_struct
/*
* Input and interrupt endpoint handling functions
*/
-#ifdef CONFIG_INPUT
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
static void int_irq(struct urb *urb)
{
struct gspca_dev *gspca_dev = (struct gspca_dev *) urb->context;
@@ -279,6 +279,19 @@ static void gspca_input_destroy_urb(stru
usb_free_urb(urb);
}
}
+#else
+static inline void gspca_input_destroy_urb(struct gspca_dev *gspca_dev)
+{
+}
+
+static inline void gspca_input_create_urb(struct gspca_dev *gspca_dev)
+{
+}
+
+static inline int gspca_input_connect(struct gspca_dev *dev)
+{
+ return 0;
+}
#endif
/* get the current input frame buffer */
@@ -2310,7 +2323,7 @@ int gspca_dev_probe(struct usb_interface
return 0;
out:
-#ifdef CONFIG_INPUT
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
if (gspca_dev->input_dev)
input_unregister_device(gspca_dev->input_dev);
#endif
@@ -2329,7 +2342,7 @@ EXPORT_SYMBOL(gspca_dev_probe);
void gspca_disconnect(struct usb_interface *intf)
{
struct gspca_dev *gspca_dev = usb_get_intfdata(intf);
-#ifdef CONFIG_INPUT
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
struct input_dev *input_dev;
#endif
@@ -2343,7 +2356,7 @@ void gspca_disconnect(struct usb_interfa
wake_up_interruptible(&gspca_dev->wq);
}
-#ifdef CONFIG_INPUT
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
gspca_input_destroy_urb(gspca_dev);
input_dev = gspca_dev->input_dev;
if (input_dev) {
--- linux-next-20100330.orig/drivers/media/video/gspca/gspca.h
+++ linux-next-20100330/drivers/media/video/gspca/gspca.h
@@ -130,7 +130,7 @@ struct sd_desc {
cam_reg_op get_register;
#endif
cam_ident_op get_chip_ident;
-#ifdef CONFIG_INPUT
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
cam_int_pkt_op int_pkt_scan;
/* other_input makes the gspca core create gspca_dev->input even when
int_pkt_scan is NULL, for cams with non interrupt driven buttons */
@@ -158,7 +158,7 @@ struct gspca_dev {
struct module *module; /* subdriver handling the device */
struct usb_device *dev;
struct file *capt_file; /* file doing video capture */
-#ifdef CONFIG_INPUT
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
struct input_dev *input_dev;
char phys[64]; /* physical device path */
#endif
@@ -171,7 +171,7 @@ struct gspca_dev {
#define USB_BUF_SZ 64
__u8 *usb_buf; /* buffer for USB exchanges */
struct urb *urb[MAX_NURBS];
-#ifdef CONFIG_INPUT
+#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
struct urb *int_urb;
#endif
--
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, 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...
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.
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
--
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: [PATCH] s2255drv: cleanup of driver disconnect code
This patch looks good, but there was one other thing that caught my eye. In s2255_probe_v4l, video_device_alloc is called for each video device, which is nothing more than a call to kzalloc, but the result of the call is never verified. Given that this driver has a fixed number of video device nodes, the array of video_device structs could be allocated within the s2255_dev struct. This would remove the extra calls to video_device_alloc, video_device_release, and the additional error checks that should have been there. If you'd prefer to keep the array of video_device structs independent of the s2255_dev struct, an alternative would be to dynamically allocate the entire array at once using kcalloc and store only the pointer to the array in the s2255_dev struct. In my opinion, either of these methods would be better than calling video_device_alloc for each video device that needs to be registered. Regards, David Ellingsworth -- 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/RFC 0/1] v4l: Add support for binary controls
Kamil Debski wrote: > Hi Mauro, > > Mauro Carvalho Chehab wrote: >> Kamil Debski wrote: From: Hans Verkuil [mailto:hverk...@xs4all.nl] Hi Kamil! >>> Hi Hans, >>> On Monday 29 March 2010 11:53:05 Kamil Debski wrote: > Hello, > > This patch introduces new type of v4l2 control - the binary >> control. It > will be useful for exchanging raw binary data between the user >> space and > the driver/hardware. > > The patch is pretty small – basically it adds a new control type. > > 1. Reasons to include this new type > - Some devices require data which are not part of the stream, but there > are necessary for the device to work e.g. coefficients for transformation > matrices. > - String control is not suitable as it suggests that the data is a null > terminated string. This might be important when printing debug information - > one might output strings as they are and binary data in hex. > > 2. How does the binary control work > The binary control has been based on the string control. The principle of > use is the same. It uses v4l2_ext_control structure to pass the pointer and > size of the data. It is left for the driver to call the copy_from_user/ > copy_to_user function to copy the data. > > 3. About the patch > The patch is pretty small – it basically adds a new control type. > > Best wishes, > I don't think this is a good idea. Controls are not really meant to >> be used as an ioctl replacement. Controls can be used to control the hardware via a GUI (e.g. qv4l2). Obviously, this will fail for binary controls. Controls can also be used in >> cases where it is not known up front which controls are needed. This typically happens for bridge drivers that can use numerous combinations of i2c sub- >> devices. Each subdev can have its own controls. There is a grey area where you want to give the application access >> to low-level parameters but without showing them to the end-user. This is >> currently not possible, but it will be once the control framework is finished and once we have the possibility to create device nodes for subdevs. But what you want is to basically pass whole structs as a control. That's something that ioctls where invented for. Especially once we have subdev nodes this shouldn't be a problem. Just the fact that it is easy to implement doesn't mean it should be done :-) Do you have specific use-cases for your proposed binary control? >>> Yes, I have. I am working on a driver for a video codec which is >> using >>> the mem2mem framework. I have to admit it's a pretty difficult >>> hardware to work with. It was one of the reasons for Pawel Osciak >>> to add multiplane support to videobuf. >>> >>> Before decoding, the hardware has to parse the header of the video >>> stream to get all necessary parameters such as the number of buffers, >>> width, height and some internal, codec specific stuff. The video >> stream >>> is then demultiplexed and divided into encoded frames in software and >>> the hardware can only process one, separated frame at a time. >>> >>> The whole codec setup cannot be achieved by using VIDIOC_S_FMT call, >>> because hardware requires access to the header data. I wanted to use >>> this binary control to pass the header to the codec after setting the >>> right format with VIDIOC_S_FMT. Then video frames can be easily >> decoded >>> as a standard calls to QBUF/DQBUF pairs. >>> >>> It is similar for encoding - the basic parameters are set with >>> VIDIOC_S_FMT, then some codec specific/advanced are accessible as >>> standard v4l2 controls. Then the encoding engine is initialized and >> the >>> hardware returns an header of the output video stream. The header can >>> be acquired by getting the value of the binary control. After that >> the >>> frame to be encoded is provided and hw returns a single frame of >>> encoded stream. >>> >>> Using custom ioctls seems appropriate for a hardware specific driver. >>> Whereas the proposed binary control for getting and setting the video >>> stream header could be generic solution and can be used in many >> drivers >>> for hardware codecs. >>> >>> Do You have any better solution for such device? >> For sure using custom CTRL's is wrong. >> >> If I understood correctly, you want to send some mpeg transport stream >> (or something like) to the hardware and let it decode, right? >> >> For sure the current API spec don't cover such usecase. IMO, whatever >> decided, >> we need to add an example for this non-trivial usage. >> >> On a first glance, by using what we currently have for V4L2 API, it >> seems that >> the more correct approach is to format with S_FMT as MPEG, using a >> "resolution" >> that will allocate a bu
Re: adv7180 as SoC camera device
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.
> In fact my probe function looks like:
>
> 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;
> struct soc_camera_link *icl;
> 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) {
> icl = to_soc_camera_link(icd);
> if (!icl)
> return -EINVAL;
>
> icd->ops = &adv7180_soc_ops;
> v4l_info(client, "soc-camera support enabled\n");
> } else
> pdata = client->dev.platform_data;
This is a complicated way to say
pdata = NULL;
Thanks
Guennadi
>
> 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);
> ...
>
> Thanks in advance,
>
> 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
>
---
Guennadi Liakhovetski
--
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/RFC 0/1] v4l: Add support for binary controls
Hi Mauro, Mauro Carvalho Chehab wrote: > Kamil Debski wrote: > >> From: Hans Verkuil [mailto:hverk...@xs4all.nl] > >> > >> Hi Kamil! > > > > Hi Hans, > > > >> On Monday 29 March 2010 11:53:05 Kamil Debski wrote: > >>> Hello, > >>> > >>> This patch introduces new type of v4l2 control - the binary > control. > >> It > >>> will be useful for exchanging raw binary data between the user > space > >> and > >>> the driver/hardware. > >>> > >>> The patch is pretty small – basically it adds a new control type. > >>> > >>> 1. Reasons to include this new type > >>> - Some devices require data which are not part of the stream, but > >> there > >>> are necessary for the device to work e.g. coefficients for > >> transformation > >>> matrices. > >>> - String control is not suitable as it suggests that the data is a > >> null > >>> terminated string. This might be important when printing debug > >> information - > >>> one might output strings as they are and binary data in hex. > >>> > >>> 2. How does the binary control work > >>> The binary control has been based on the string control. The > >> principle of > >>> use is the same. It uses v4l2_ext_control structure to pass the > >> pointer and > >>> size of the data. It is left for the driver to call the > >> copy_from_user/ > >>> copy_to_user function to copy the data. > >>> > >>> 3. About the patch > >>> The patch is pretty small – it basically adds a new control type. > >>> > >>> Best wishes, > >>> > >> I don't think this is a good idea. Controls are not really meant to > be > >> used > >> as an ioctl replacement. > >> > >> Controls can be used to control the hardware via a GUI (e.g. qv4l2). > >> Obviously, > >> this will fail for binary controls. Controls can also be used in > cases > >> where > >> it is not known up front which controls are needed. This typically > >> happens for > >> bridge drivers that can use numerous combinations of i2c sub- > devices. > >> Each > >> subdev can have its own controls. > >> > >> There is a grey area where you want to give the application access > to > >> low-level > >> parameters but without showing them to the end-user. This is > currently > >> not > >> possible, but it will be once the control framework is finished and > >> once we > >> have the possibility to create device nodes for subdevs. > >> > >> But what you want is to basically pass whole structs as a control. > >> That's > >> something that ioctls where invented for. Especially once we have > >> subdev nodes > >> this shouldn't be a problem. > >> > >> Just the fact that it is easy to implement doesn't mean it should be > >> done :-) > >> > >> Do you have specific use-cases for your proposed binary control? > > > > Yes, I have. I am working on a driver for a video codec which is > using > > the mem2mem framework. I have to admit it's a pretty difficult > > hardware to work with. It was one of the reasons for Pawel Osciak > > to add multiplane support to videobuf. > > > > Before decoding, the hardware has to parse the header of the video > > stream to get all necessary parameters such as the number of buffers, > > width, height and some internal, codec specific stuff. The video > stream > > is then demultiplexed and divided into encoded frames in software and > > the hardware can only process one, separated frame at a time. > > > > The whole codec setup cannot be achieved by using VIDIOC_S_FMT call, > > because hardware requires access to the header data. I wanted to use > > this binary control to pass the header to the codec after setting the > > right format with VIDIOC_S_FMT. Then video frames can be easily > decoded > > as a standard calls to QBUF/DQBUF pairs. > > > > It is similar for encoding - the basic parameters are set with > > VIDIOC_S_FMT, then some codec specific/advanced are accessible as > > standard v4l2 controls. Then the encoding engine is initialized and > the > > hardware returns an header of the output video stream. The header can > > be acquired by getting the value of the binary control. After that > the > > frame to be encoded is provided and hw returns a single frame of > > encoded stream. > > > > Using custom ioctls seems appropriate for a hardware specific driver. > > Whereas the proposed binary control for getting and setting the video > > stream header could be generic solution and can be used in many > drivers > > for hardware codecs. > > > > Do You have any better solution for such device? > > For sure using custom CTRL's is wrong. > > If I understood correctly, you want to send some mpeg transport stream > (or something like) to the hardware and let it decode, right? > > For sure the current API spec don't cover such usecase. IMO, whatever > decided, > we need to add an example for this non-trivial usage. > > On a first glance, by using what we currently have for V4L2 API, it > seems that > the more correct approach is to format with S_FMT as MPEG, using a > "resolution" > that will allocate a buffer with enough size for sending the st
Re: adv7180 as SoC camera device
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
In fact my probe function looks like:
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;
struct soc_camera_link *icl;
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) {
icl = to_soc_camera_link(icd);
if (!icl)
return -EINVAL;
icd->ops = &adv7180_soc_ops;
v4l_info(client, "soc-camera support enabled\n");
} else
pdata = client->dev.platform_data;
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);
...
Thanks in advance,
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: [PATCH/RFC 0/1] v4l: Add support for binary controls
Hi Kamil, On Tuesday 30 March 2010 14:26:00 Kamil Debski wrote: > > From: Hans Verkuil [mailto:hverk...@xs4all.nl] > > On Monday 29 March 2010 11:53:05 Kamil Debski wrote: > > > Hello, > > > > > > This patch introduces new type of v4l2 control - the binary control. It > > > will be useful for exchanging raw binary data between the user space and > > > the driver/hardware. > > > > > > The patch is pretty small – basically it adds a new control type. > > > > > > 1. Reasons to include this new type > > > - Some devices require data which are not part of the stream, but there > > > are necessary for the device to work e.g. coefficients for > > > transformation matrices. > > > - String control is not suitable as it suggests that the data is a > > > null terminated string. This might be important when printing debug > > > information - one might output strings as they are and binary data in > > > hex. > > > > > > 2. How does the binary control work > > > The binary control has been based on the string control. The principle > > > of use is the same. It uses v4l2_ext_control structure to pass the > > > pointer and size of the data. It is left for the driver to call the > > > copy_from_user/copy_to_user function to copy the data. > > > > > > 3. About the patch > > > The patch is pretty small – it basically adds a new control type. > > > > > > > I don't think this is a good idea. Controls are not really meant to be > > used as an ioctl replacement. > > > > Controls can be used to control the hardware via a GUI (e.g. qv4l2). > > Obviously, this will fail for binary controls. Controls can also be used > > in cases where it is not known up front which controls are needed. This > > typically happens for bridge drivers that can use numerous combinations of > > i2c sub-devices. Each subdev can have its own controls. > > > > There is a grey area where you want to give the application access to > > low-level parameters but without showing them to the end-user. This is > > currently not possible, but it will be once the control framework is > > finished and once we have the possibility to create device nodes for > > subdevs. > > > > But what you want is to basically pass whole structs as a control. > > That's something that ioctls where invented for. Especially once we have > > subdev nodes this shouldn't be a problem. > > > > Just the fact that it is easy to implement doesn't mean it should be > > done :-) > > > > Do you have specific use-cases for your proposed binary control? > > Yes, I have. I am working on a driver for a video codec which is using > the mem2mem framework. I have to admit it's a pretty difficult > hardware to work with. It was one of the reasons for Pawel Osciak > to add multiplane support to videobuf. > > Before decoding, the hardware has to parse the header of the video > stream to get all necessary parameters such as the number of buffers, > width, height and some internal, codec specific stuff. The video stream > is then demultiplexed and divided into encoded frames in software and > the hardware can only process one, separated frame at a time. > > The whole codec setup cannot be achieved by using VIDIOC_S_FMT call, > because hardware requires access to the header data. I wanted to use > this binary control to pass the header to the codec after setting the > right format with VIDIOC_S_FMT. Then video frames can be easily decoded > as a standard calls to QBUF/DQBUF pairs. In that case I have to agree with Hans, a private ioctl is better. > It is similar for encoding - the basic parameters are set with > VIDIOC_S_FMT, then some codec specific/advanced are accessible as > standard v4l2 controls. Then the encoding engine is initialized and the > hardware returns an header of the output video stream. The header can > be acquired by getting the value of the binary control. After that the > frame to be encoded is provided and hw returns a single frame of > encoded stream. > > Using custom ioctls seems appropriate for a hardware specific driver. > Whereas the proposed binary control for getting and setting the video > stream header could be generic solution and can be used in many drivers > for hardware codecs. I don't think it would. The format of the data is pretty much hardware specific. If you can make the format generic, then the private ioctl could become generic. > Do You have any better solution for such device? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/1] v4l: Add support for binary controls
Kamil Debski wrote: >> From: Hans Verkuil [mailto:hverk...@xs4all.nl] >> >> Hi Kamil! > > Hi Hans, > >> On Monday 29 March 2010 11:53:05 Kamil Debski wrote: >>> Hello, >>> >>> This patch introduces new type of v4l2 control - the binary control. >> It >>> will be useful for exchanging raw binary data between the user space >> and >>> the driver/hardware. >>> >>> The patch is pretty small – basically it adds a new control type. >>> >>> 1. Reasons to include this new type >>> - Some devices require data which are not part of the stream, but >> there >>> are necessary for the device to work e.g. coefficients for >> transformation >>> matrices. >>> - String control is not suitable as it suggests that the data is a >> null >>> terminated string. This might be important when printing debug >> information - >>> one might output strings as they are and binary data in hex. >>> >>> 2. How does the binary control work >>> The binary control has been based on the string control. The >> principle of >>> use is the same. It uses v4l2_ext_control structure to pass the >> pointer and >>> size of the data. It is left for the driver to call the >> copy_from_user/ >>> copy_to_user function to copy the data. >>> >>> 3. About the patch >>> The patch is pretty small – it basically adds a new control type. >>> >>> Best wishes, >>> >> I don't think this is a good idea. Controls are not really meant to be >> used >> as an ioctl replacement. >> >> Controls can be used to control the hardware via a GUI (e.g. qv4l2). >> Obviously, >> this will fail for binary controls. Controls can also be used in cases >> where >> it is not known up front which controls are needed. This typically >> happens for >> bridge drivers that can use numerous combinations of i2c sub-devices. >> Each >> subdev can have its own controls. >> >> There is a grey area where you want to give the application access to >> low-level >> parameters but without showing them to the end-user. This is currently >> not >> possible, but it will be once the control framework is finished and >> once we >> have the possibility to create device nodes for subdevs. >> >> But what you want is to basically pass whole structs as a control. >> That's >> something that ioctls where invented for. Especially once we have >> subdev nodes >> this shouldn't be a problem. >> >> Just the fact that it is easy to implement doesn't mean it should be >> done :-) >> >> Do you have specific use-cases for your proposed binary control? > > Yes, I have. I am working on a driver for a video codec which is using > the mem2mem framework. I have to admit it's a pretty difficult > hardware to work with. It was one of the reasons for Pawel Osciak > to add multiplane support to videobuf. > > Before decoding, the hardware has to parse the header of the video > stream to get all necessary parameters such as the number of buffers, > width, height and some internal, codec specific stuff. The video stream > is then demultiplexed and divided into encoded frames in software and > the hardware can only process one, separated frame at a time. > > The whole codec setup cannot be achieved by using VIDIOC_S_FMT call, > because hardware requires access to the header data. I wanted to use > this binary control to pass the header to the codec after setting the > right format with VIDIOC_S_FMT. Then video frames can be easily decoded > as a standard calls to QBUF/DQBUF pairs. > > It is similar for encoding - the basic parameters are set with > VIDIOC_S_FMT, then some codec specific/advanced are accessible as > standard v4l2 controls. Then the encoding engine is initialized and the > hardware returns an header of the output video stream. The header can > be acquired by getting the value of the binary control. After that the > frame to be encoded is provided and hw returns a single frame of > encoded stream. > > Using custom ioctls seems appropriate for a hardware specific driver. > Whereas the proposed binary control for getting and setting the video > stream header could be generic solution and can be used in many drivers > for hardware codecs. > > Do You have any better solution for such device? For sure using custom CTRL's is wrong. If I understood correctly, you want to send some mpeg transport stream (or something like) to the hardware and let it decode, right? For sure the current API spec don't cover such usecase. IMO, whatever decided, we need to add an example for this non-trivial usage. On a first glance, by using what we currently have for V4L2 API, it seems that the more correct approach is to format with S_FMT as MPEG, using a "resolution" that will allocate a buffer with enough size for sending the stream to the hardware (this is basically the way other mpeg-capable encoders/decoders do). Then, send the header via the usual stream interface and retrieve the stream information via VIDIOC_G_* ioctls. Only after having the header processed by the hardware, you'll be able to mmap memory to rece
Re: [RFC] What are the goals for the architecture of an in-kernel IR system?
David Härdeman wrote: > On Sun, Mar 28, 2010 at 08:22:31PM -0300, Mauro Carvalho Chehab wrote: >> I also noticed another problem: kernel should have some way to report >> the expected >> size of the scancode to userspace, especially if we want to have the >> compatibility >> code (since, with compat, a scancode maximum size need to be 32 bits, >> otherwise >> the code won't work). >> >> I'll likely adding another control that returns the size of the scancode. > > And perhaps the interface should explicitly define that for the case > where userspace sends an undersized scancode, the real scancode will be > generated by zero-extending the undersized scancode into its expected > size. > > That way the interface will be binary-forwards-compatible even if > scancode sizes are increased at some later date. Makes sense. Padding an undersized scancode is endian-dependent. So, we'll likely need to add some padding functions. The better seems to add the logic at include/linux/byteorder/generic.h. -- 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 12/24] media/video: fix dangling pointers
Hans, > But this just feels like an i2c core thing to me. After remove() is called > the core should just set the client data to NULL. If there are drivers that > rely on the current behavior, then those drivers should be reviewed first as > to the reason why they need it. It will be done this way now. As you have taken part in the discussion, I guess the media-subsystem never really considered picking those patches up ;) Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
RE: [PATCH/RFC 0/1] v4l: Add support for binary controls
> From: Hans Verkuil [mailto:hverk...@xs4all.nl] > > Hi Kamil! Hi Hans, > > On Monday 29 March 2010 11:53:05 Kamil Debski wrote: > > > > Hello, > > > > This patch introduces new type of v4l2 control - the binary control. > It > > will be useful for exchanging raw binary data between the user space > and > > the driver/hardware. > > > > The patch is pretty small – basically it adds a new control type. > > > > 1. Reasons to include this new type > > - Some devices require data which are not part of the stream, but > there > > are necessary for the device to work e.g. coefficients for > transformation > > matrices. > > - String control is not suitable as it suggests that the data is a > null > > terminated string. This might be important when printing debug > information - > > one might output strings as they are and binary data in hex. > > > > 2. How does the binary control work > > The binary control has been based on the string control. The > principle of > > use is the same. It uses v4l2_ext_control structure to pass the > pointer and > > size of the data. It is left for the driver to call the > copy_from_user/ > > copy_to_user function to copy the data. > > > > 3. About the patch > > The patch is pretty small – it basically adds a new control type. > > > > Best wishes, > > > > I don't think this is a good idea. Controls are not really meant to be > used > as an ioctl replacement. > > Controls can be used to control the hardware via a GUI (e.g. qv4l2). > Obviously, > this will fail for binary controls. Controls can also be used in cases > where > it is not known up front which controls are needed. This typically > happens for > bridge drivers that can use numerous combinations of i2c sub-devices. > Each > subdev can have its own controls. > > There is a grey area where you want to give the application access to > low-level > parameters but without showing them to the end-user. This is currently > not > possible, but it will be once the control framework is finished and > once we > have the possibility to create device nodes for subdevs. > > But what you want is to basically pass whole structs as a control. > That's > something that ioctls where invented for. Especially once we have > subdev nodes > this shouldn't be a problem. > > Just the fact that it is easy to implement doesn't mean it should be > done :-) > > Do you have specific use-cases for your proposed binary control? Yes, I have. I am working on a driver for a video codec which is using the mem2mem framework. I have to admit it's a pretty difficult hardware to work with. It was one of the reasons for Pawel Osciak to add multiplane support to videobuf. Before decoding, the hardware has to parse the header of the video stream to get all necessary parameters such as the number of buffers, width, height and some internal, codec specific stuff. The video stream is then demultiplexed and divided into encoded frames in software and the hardware can only process one, separated frame at a time. The whole codec setup cannot be achieved by using VIDIOC_S_FMT call, because hardware requires access to the header data. I wanted to use this binary control to pass the header to the codec after setting the right format with VIDIOC_S_FMT. Then video frames can be easily decoded as a standard calls to QBUF/DQBUF pairs. It is similar for encoding - the basic parameters are set with VIDIOC_S_FMT, then some codec specific/advanced are accessible as standard v4l2 controls. Then the encoding engine is initialized and the hardware returns an header of the output video stream. The header can be acquired by getting the value of the binary control. After that the frame to be encoded is provided and hw returns a single frame of encoded stream. Using custom ioctls seems appropriate for a hardware specific driver. Whereas the proposed binary control for getting and setting the video stream header could be generic solution and can be used in many drivers for hardware codecs. Do You have any better solution for such device? Best regards, -- Kamil Debski Linux Platform Group Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/1] v4l: Add support for binary controls
On Tue, 2010-03-30 at 10:57 +0200, Laurent Pinchart wrote: > Hi Hans, > > On Tuesday 30 March 2010 08:41:47 Hans Verkuil wrote: > > On Monday 29 March 2010 11:53:05 Kamil Debski wrote: > > > Hello, > > > > > > This patch introduces new type of v4l2 control - the binary control. It > > > will be useful for exchanging raw binary data between the user space and > > > the driver/hardware. > > > > > > The patch is pretty small – basically it adds a new control type. > > > > > > 1. Reasons to include this new type > > > - Some devices require data which are not part of the stream, but there > > > are necessary for the device to work e.g. coefficients for transformation > > > matrices. > > > - String control is not suitable as it suggests that the data is a null > > > terminated string. This might be important when printing debug > > > information - one might output strings as they are and binary data in > > > hex. > > > > > > 2. How does the binary control work > > > The binary control has been based on the string control. The principle of > > > use is the same. It uses v4l2_ext_control structure to pass the pointer > > > and size of the data. It is left for the driver to call the > > > copy_from_user/ copy_to_user function to copy the data. > > > > > > 3. About the patch > > > The patch is pretty small – it basically adds a new control type. > > > > > > Best wishes, > > > > I don't think this is a good idea. Controls are not really meant to be used > > as an ioctl replacement. > > > > Controls can be used to control the hardware via a GUI (e.g. qv4l2). > > Obviously, this will fail for binary controls. Controls can also be used > > in cases where it is not known up front which controls are needed. This > > typically happens for bridge drivers that can use numerous combinations of > > i2c sub-devices. Each subdev can have its own controls. > > > > There is a grey area where you want to give the application access to > > low-level parameters but without showing them to the end-user. This is > > currently not possible, but it will be once the control framework is > > finished and once we have the possibility to create device nodes for > > subdevs. > > > > But what you want is to basically pass whole structs as a control. That's > > something that ioctls where invented for. Especially once we have subdev > > nodes this shouldn't be a problem. > > > > Just the fact that it is easy to implement doesn't mean it should be done > > :-) > > > > Do you have specific use-cases for your proposed binary control? > > As discussed yesterday, here are a few use cases for the OMAP3 ISP driver. > > - white balance matrix > - gamma correction tables > > In both cases, the driver needs an array (possible 2 dimensional) of values > to > configure the hardware. > > This can obviously be done using private ioctls, but what makes the red&blue > white balance gains different from the white balance matrix ? Why should the > first be controls and the later not ? I'll suggest some tests: 1. Can an operator easily manipulate the control in an intuitive fashion? 2. Can the kernel easily convey the visual form of the control to an application, such the application can render it with common widget sets not knowing much information apriori? So sliders for gain controls fit that criteria easily. I'm not sure about a matrix. I'm guessing one could have a "look up table" type control. It would visually (on a gui) be a matrix of text boxes that take numeric inputs; maybe with some kernel provided axes labels and row and column labaels. (I think that's better than an arbitrary binary blob, although more work to implement.) 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] What are the goals for the architecture of an in-kernel IR system?
On Sun, Mar 28, 2010 at 08:22:31PM -0300, Mauro Carvalho Chehab wrote: > I also noticed another problem: kernel should have some way to report > the expected > size of the scancode to userspace, especially if we want to have the > compatibility > code (since, with compat, a scancode maximum size need to be 32 bits, > otherwise > the code won't work). > > I'll likely adding another control that returns the size of the scancode. And perhaps the interface should explicitly define that for the case where userspace sends an undersized scancode, the real scancode will be generated by zero-extending the undersized scancode into its expected size. That way the interface will be binary-forwards-compatible even if scancode sizes are increased at some later date. -- 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] What are the goals for the architecture of an in-kernel IR system?
On Sun, Mar 28, 2010 at 09:51:17PM -0300, Mauro Carvalho Chehab wrote: > > I spoke too soon... removing the index causes a problem at the read ioctl: > there's no way > to retrieve just the non-sparsed values. > > There's one solution that would allow both read/write and compat to work > nicely, > but the API would become somewhat asymmetrical: > > At get (EVIOCGKEYCODEBIG): > use index/len as input and keycode/scancode as output; > > At set (EVIOCSKEYCODEBIG): > use scancode/keycode/len as input (and, optionally, index as output). > This was exactly the approach I had in mind when I suggested using indexes. > Having it asymmetrical doesn't sound good, but, on the other hand, > using index for > the set function also doesn't seem good, as the driver may reorder the > entries after > setting, for example to work with a binary tree or with hashes. I don't think the assymetry is really a problem. As I see it, there are basically two user cases: 1) Userspace wants scancode X to generate keypress Y (In which case userspace doesn't care one iota what the index is) 2) Userspace wants to get the current keytable from the kernel (In which case a loop with an index from 0 to n is appropriate) and, possibly: 3) Userspace wants to know what keycode (if any) scancode X generates (In which case approach 2 will work just as well, but this usecase seems a bit contrived anyway...) -- 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: [PATCH 1/2] V4L/DVB: buf-dma-sg.c: don't assume nr_pages == sglen
On Wednesday 24 March 2010 06:43:22, Sakari Ailus wrote:
> Hi Arnout,
>
> Thanks for the patch.
>
> Arnout Vandecappelle wrote:
[snip]
> > - dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->nr_pages,
> > dma->direction);
> > + dma_sync_sg_for_cpu(q->dev, dma->sglist, dma->sglen, dma->direction);
>
> I think the same problem still exists --- dma->sglen is not initialised
> anywhere, is it?
Yes it is.
In videobuf_dma_map (where dma->sglist is set), there are two conditions:
if (dma->bus_addr) {
dma->sglist = kmalloc(sizeof(struct scatterlist), GFP_KERNEL);
if (NULL != dma->sglist) {
dma->sglen = 1;
...
}
}
...
if (!dma->bus_addr) {
dma->sglen = dma_map_sg(q->dev, dma->sglist,
dma->nr_pages, dma->direction);
...
}
Regards,
Arnout
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286540
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, BelgiumBE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 31BB CF53 8660 6F88 345D 54CC A836 5879 20D7 CF43
--
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] Fix default state Beholder H6 tuner.
Hi,
Am Dienstag, den 30.03.2010, 16:02 +1000 schrieb Dmitri Belimov:
> Hi
>
> The hybrid tuner FMD1216MEX_MK3 after cold start has disabled IF.
> This tuner has internal I2C switch. This switch switch I2C bus between DVB-T
> and IF part.
> Default state is DVB-T. When module saa7134 is load it can't find IF tda9887
> and disable
> analog TV mode.
>
> This patch set internal I2C switch of the tuner to IF by send special value
> to the tuner as for receive
> analog TV from low band. It can be usefule for other cards.
>
> I didn't set configure a tuner by a tuner model because this tuner can has
> different I2C address.
> May be we can do it later after discuss for more robust support a tuners.
just as a reminder. It is the same for the FMD1216ME hybrid MK3.
After every boot, analog mode fails with missing tda9887.
Currently, after tuner modules are not independent anymore, one has to
reload the saa7134 driver once.
Relevant code in tuner.core.c.
case TUNER_PHILIPS_FMD1216ME_MK3:
buffer[0] = 0x0b;
buffer[1] = 0xdc;
buffer[2] = 0x9c;
buffer[3] = 0x60;
i2c_master_send(c, buffer, 4);
mdelay(1);
buffer[2] = 0x86;
buffer[3] = 0x54;
i2c_master_send(c, buffer, 4);
if (!dvb_attach(simple_tuner_attach, &t->fe,
t->i2c->adapter, t->i2c->addr, t->type))
goto attach_failed;
break;
Hermann
> diff -r 1ef0265456c8 linux/drivers/media/video/saa7134/saa7134-cards.c
> --- a/linux/drivers/media/video/saa7134/saa7134-cards.c Fri Mar 26
> 00:54:18 2010 -0300
> +++ b/linux/drivers/media/video/saa7134/saa7134-cards.c Sun Mar 28
> 08:21:10 2010 -0400
> @@ -7450,6 +7450,21 @@
> }
> break;
> }
> + case SAA7134_BOARD_BEHOLD_H6:
> + {
> + u8 data[] = { 0x09, 0x9f, 0x86, 0x11};
> + struct i2c_msg msg = {.addr = 0x61, .flags = 0, .buf = data,
> + .len = sizeof(data)};
> +
> + /* The tuner TUNER_PHILIPS_FMD1216MEX_MK3 after hardware*/
> + /* start has disabled IF and enabled DVB-T. When saa7134*/
> + /* scan I2C devices it not detect IF tda9887 and can`t */
> + /* watch TV without software reboot. For solve this problem */
> + /* switch the tuner to analog TV mode manually. */
> + if (i2c_transfer(&dev->i2c_adap, &msg, 1) != 1)
> + printk(KERN_WARNING
> + "%s: Unable to enable IF of the tuner.\n",
> +dev->name);
> + break;
> + }
> } /* switch() */
>
> /* initialize tuner */
>
> Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov
>
> With my best regards, Dmitry.
--
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: [Resubmit: PATCH-V2] Introducing ti-media directory
> -Original Message- > From: Mauro Carvalho Chehab [mailto:mche...@redhat.com] > Sent: Wednesday, March 24, 2010 5:32 PM > To: Laurent Pinchart > Cc: Karicheri, Muralidharan; Hiremath, Vaibhav; davinci-linux-open- > sou...@linux.davincidsp.com; linux-media@vger.kernel.org > Subject: Re: [Resubmit: PATCH-V2] Introducing ti-media directory > > Laurent Pinchart wrote: > > Hi Murali, > > > > On Tuesday 23 March 2010 18:52:44 Karicheri, Muralidharan wrote: > >> Laurent, > >> > I'm not too sure to like the ti-media name. It will soon get quite > crowded, and name collisions might occur (look at the linux-omap-camera > tree and the ISP driver in there for instance). Isn't there an internal > name to refer to both the DM6446 and AM3517 that could be used ? > >>> [Hiremath, Vaibhav] Laurent, > >>> > >>> ti-media directory is top level directory where we are putting all TI > >>> devices drivers. So having said that, we should worrying about what goes > >>> inside this directory. > >>> For me ISP is more generic, if you compare davinci and OMAP. > >>> > >>> Frankly, there are various naming convention we do have from device to > >>> device, even if the IP's are being reused. For example, the internal > name > >>> for OMAP is ISP but Davinci refers it as a VPSS. > >> Could you explain what name space issue you are referring to in > >> linux-omap-camera since I am not quite familiar with that tree? > > > > The linux-omap-camera tree contains a driver for the OMAP3 ISP. Basically, > > most source files start with the "isp" prefix and are stored in > > drivers/media/video/isp/. > > > > ISP is quite a generic name, and other vendors will probably develop an > ISP at > > some point (if not already done), so there's already a potential name > conflict > > today. > > > > Using a dedicated directory in drivers/media/video for TI-specific cores > is > > definitely a good idea (assuming the same IP cores won't be used by other > > vendors in the future). > > > > My concern is that, if we move the ISP driver in drivers/media/video/ti- > media, > > the directory will soon get quite crowded. If a new TI processor comes up > with > > a totally incompatible ISP, we will get a name conflict in > > drivers/media/video/ti-media. I was thinking about either replacing the > "isp" > > prefix with "omap3isp" (or similar), or moving the driver to > > drivers/media/video/ti-media/omap3isp, but that will impede code sharing > code > > between the Davinci and OMAP processor families. That's where my > uncertainty > > comes from. > > There are two separate points here. The first one is re-using a driver or > some > symbols. Whatever directory structure is used, this won't prevent to share > the code. > Just create the header files under include/media and be sure that the Kbuild > system will properly handle the dependencies, with "depends on". > > > > >> Myself and Vaibhav had discussed this in the past and ti-media is the > >> generic name that we could agree on. On DM SoCs (DM6446, DM355, DM365) I > >> expect ti-media to be the home for all vpfe and vpbe driver files. Since > >> we had a case of common IP across OMAP and DMxxx SoCs, we want to place > >> all OMAP and DMxxx video driver files in a common directory so that > >> sharing the drivers across the SoCs will be easy. We could discuss and > >> agree on another name if need be. Any suggestions? > > > > It's not the name ti-media that I don't agree on, it's just that this will > > move the problem one step further in the directory hierarchy without > actually > > solving it :-) > > > > Is it guaranteed today that no TI processors with new generation video > blocks > > will reuse the names ISP, VPFE and VPBE ? The OMAP3 datasheet refers to > VPFE > > and VPBE, but luckily those blocks are further divided into subblocks, and > the > > driver doesn't refer to the VPFE and VPBE directly. > > I agree that a name like ti-media would be too generic. Also, if we take a > look on > how the drivers are currently organized, the trees aren't vendor-based, but > chipset-design based. There are even some cases where there's just one or > two C files that > are directly stored under drivers/media/video (like tvp5150, for example). > > IMO, simpler drivers where just one or a couple of files are needed should > be stored > into drivers/media/video. Bigger drivers should be organized by family, and > not by vendor. > Otherwise, we would need to re-organize the tree to be coherent. > > One interesting example of the a per-family directory is cx25840. The same > driver > is used by several different chipsets. The driver supports a separate IC > chip (cx25836, > cx2584x), and two designs where the decoder logic is inside an IC chip with > the bridge > and other functional blocks (cx23885 and cx231xx). > [Hiremath, Vaibhav] Mauro and Laurent, What is the conclusion on this directory naming convention issue? Actually the V4L2 display driver patch (submitted by me) is depe
Re: [PATCH/RFC 0/1] v4l: Add support for binary controls
Hi Hans, On Tuesday 30 March 2010 08:41:47 Hans Verkuil wrote: > On Monday 29 March 2010 11:53:05 Kamil Debski wrote: > > Hello, > > > > This patch introduces new type of v4l2 control - the binary control. It > > will be useful for exchanging raw binary data between the user space and > > the driver/hardware. > > > > The patch is pretty small – basically it adds a new control type. > > > > 1. Reasons to include this new type > > - Some devices require data which are not part of the stream, but there > > are necessary for the device to work e.g. coefficients for transformation > > matrices. > > - String control is not suitable as it suggests that the data is a null > > terminated string. This might be important when printing debug > > information - one might output strings as they are and binary data in > > hex. > > > > 2. How does the binary control work > > The binary control has been based on the string control. The principle of > > use is the same. It uses v4l2_ext_control structure to pass the pointer > > and size of the data. It is left for the driver to call the > > copy_from_user/ copy_to_user function to copy the data. > > > > 3. About the patch > > The patch is pretty small – it basically adds a new control type. > > > > Best wishes, > > I don't think this is a good idea. Controls are not really meant to be used > as an ioctl replacement. > > Controls can be used to control the hardware via a GUI (e.g. qv4l2). > Obviously, this will fail for binary controls. Controls can also be used > in cases where it is not known up front which controls are needed. This > typically happens for bridge drivers that can use numerous combinations of > i2c sub-devices. Each subdev can have its own controls. > > There is a grey area where you want to give the application access to > low-level parameters but without showing them to the end-user. This is > currently not possible, but it will be once the control framework is > finished and once we have the possibility to create device nodes for > subdevs. > > But what you want is to basically pass whole structs as a control. That's > something that ioctls where invented for. Especially once we have subdev > nodes this shouldn't be a problem. > > Just the fact that it is easy to implement doesn't mean it should be done > :-) > > Do you have specific use-cases for your proposed binary control? As discussed yesterday, here are a few use cases for the OMAP3 ISP driver. - white balance matrix - gamma correction tables In both cases, the driver needs an array (possible 2 dimensional) of values to configure the hardware. This can obviously be done using private ioctls, but what makes the red&blue white balance gains different from the white balance matrix ? Why should the first be controls and the later not ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix default state Beholder H6 tuner.
Hi
The hybrid tuner FMD1216MEX_MK3 after cold start has disabled IF.
This tuner has internal I2C switch. This switch switch I2C bus between DVB-T
and IF part.
Default state is DVB-T. When module saa7134 is load it can't find IF tda9887
and disable
analog TV mode.
This patch set internal I2C switch of the tuner to IF by send special value to
the tuner as for receive
analog TV from low band. It can be usefule for other cards.
I didn't set configure a tuner by a tuner model because this tuner can has
different I2C address.
May be we can do it later after discuss for more robust support a tuners.
diff -r 1ef0265456c8 linux/drivers/media/video/saa7134/saa7134-cards.c
--- a/linux/drivers/media/video/saa7134/saa7134-cards.c Fri Mar 26 00:54:18
2010 -0300
+++ b/linux/drivers/media/video/saa7134/saa7134-cards.c Sun Mar 28 08:21:10
2010 -0400
@@ -7450,6 +7450,21 @@
}
break;
}
+ case SAA7134_BOARD_BEHOLD_H6:
+ {
+ u8 data[] = { 0x09, 0x9f, 0x86, 0x11};
+ struct i2c_msg msg = {.addr = 0x61, .flags = 0, .buf = data,
+ .len = sizeof(data)};
+
+ /* The tuner TUNER_PHILIPS_FMD1216MEX_MK3 after hardware*/
+ /* start has disabled IF and enabled DVB-T. When saa7134*/
+ /* scan I2C devices it not detect IF tda9887 and can`t */
+ /* watch TV without software reboot. For solve this problem */
+ /* switch the tuner to analog TV mode manually. */
+ if (i2c_transfer(&dev->i2c_adap, &msg, 1) != 1)
+ printk(KERN_WARNING
+ "%s: Unable to enable IF of the tuner.\n",
+ dev->name);
+ break;
+ }
} /* switch() */
/* initialize tuner */
Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov
With my best regards, Dmitry.diff -r 1ef0265456c8 linux/drivers/media/video/saa7134/saa7134-cards.c
--- a/linux/drivers/media/video/saa7134/saa7134-cards.c Fri Mar 26 00:54:18 2010 -0300
+++ b/linux/drivers/media/video/saa7134/saa7134-cards.c Sun Mar 28 08:21:10 2010 -0400
@@ -7450,6 +7450,21 @@
}
break;
}
+ case SAA7134_BOARD_BEHOLD_H6:
+ {
+ u8 data[] = { 0x09, 0x9f, 0x86, 0x11};
+ struct i2c_msg msg = {.addr = 0x61, .flags = 0, .buf = data,
+ .len = sizeof(data)};
+
+ /* The tuner TUNER_PHILIPS_FMD1216MEX_MK3 after hardware*/
+ /* start has disabled IF and enabled DVB-T. When saa7134*/
+ /* scan I2C devices it not detect IF tda9887 and can`t */
+ /* watch TV without software reboot. For solve this problem */
+ /* switch the tuner to analog TV mode manually. */
+ if (i2c_transfer(&dev->i2c_adap, &msg, 1) != 1)
+printk(KERN_WARNING
+ "%s: Unable to enable IF of the tuner.\n",
+ dev->name);
+ break;
+ }
} /* switch() */
/* initialize tuner */
Signed-off-by: Beholder Intl. Ltd. Dmitry Belimov
