Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
On Wed, 10 Jun 2009, Mauro Carvalho Chehab wrote: > Em Wed, 10 Jun 2009 23:36:32 +0200 (CEST) > Guennadi Liakhovetski escreveu: > > > someone writes a script to intercept all hg pull requests from lmml > > (procmail rule), forward that mail to a special media-patch list, and > > extract and post as replies to that mail all individual patches? And that > > list should be configured to only accept mails from that script or replies > > to its mails, so, it'd be spam-free. And that list would also be used for > > patch discussion. How does this sound? > > This can be done. If you write such script in perl or python [1], I can put > it to > run at linuxtv. You'll likely need to handle also the pull request replies. > > [1] since we don't have procmail (or exim) installed there. Hm, yeah, thanks, but I think, if I were to do this, I would just do a local script, that would cp -al v4l-dvb pull cd pull hg pull -u for $mail in @mails { hg extract $mail | mail ... } cd .. rm -rf pull and just mail the patches to me locally. But I don't think I'd be doing this, I don't have the time. I'll just subscribe to the commits ML and live with the fact, that the delta between patches in hg and in mainline git will grow, as more patches in hg wil have to be fixed by incremental patches, which you then will merge before importing into git... Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ov511.c: video_register_device() return zero on success
On Thu, 2009-06-11 at 01:40 -0300, Mauro Carvalho Chehab wrote: > Em Wed, 10 Jun 2009 22:39:51 -0300 > Mauro Carvalho Chehab escreveu: > > > Em Sun, 31 May 2009 14:41:52 +0800 > > "Figo.zhang" escreveu: > > > > > video_register_device() return zero on success, it would not return a > > > positive integer. > > > > > > Signed-off-by: Figo.zhang > > > --- > > > drivers/media/video/ov511.c |2 +- > > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > > > diff --git a/drivers/media/video/ov511.c b/drivers/media/video/ov511.c > > > index 9af5532..816427e 100644 > > > --- a/drivers/media/video/ov511.c > > > +++ b/drivers/media/video/ov511.c > > > @@ -5851,7 +5851,7 @@ ov51x_probe(struct usb_interface *intf, const > > > struct usb_device_id *id) > > > break; > > > > > > if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, > > > - unit_video[i]) >= 0) { > > > + unit_video[i]) == 0) { > > > break; > > > } > > > } > > > > Nack. > > > > Errors are always negative. So, any zero or positive value indicates that > > no error occurred. > > > > Yet, the logic for forcing ov51x to specific minor number seems broken: it > > will > > end by registering the device twice, if used. > > > > So, that part of the function needs a rewrite. I'll fix it. > > > > Hmm... the issue seems more complex than I've imagined... > > When ov511 were written, it was assumed that video_register_device(dev, v, nr) > would have the following behavior: > > if nr = -1, it would do dynamic minor allocation; > if nr >= 0, it would allocate to 'nr' minor or fail. > > With the original behavior. > > The ov511_probe registering loop is doing something like this (I did a cleanup > to allow a better understand of the logic): > > > #define OV511_MAX_UNIT_VIDEO 16 > ... > static int unit_video[OV511_MAX_UNIT_VIDEO]; > ... > module_param_array(unit_video, int, &num_uv, 0); > MODULE_PARM_DESC(unit_video, > "Force use of specific minor number(s). 0 is not allowed."); > ... > static int > ov51x_probe(struct usb_interface *intf, const struct usb_device_id *id) > { > ... > for (i = 0; i < OV511_MAX_UNIT_VIDEO; i++) { > /* Minor 0 cannot be specified; assume user wants autodetect > */ > if (unit_video[i] == 0) > break; > > if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, > unit_video[i]) >= 0) > goto register_succeded; > } > > /* Use the next available one */ > if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, -1) < 0) { > err("video_register_device failed"); > goto error; > > register_succeeded: > dev_info(&intf->dev, "Device at %s registered to minor %d\n", > ov->usb_path, ov->vdev->minor); > > > So, if you probe ov511 with a list of device numbers, for example: > > modprobe ov511 4,1,3 > you means specify the minior video device: /dev/vidoe4,/dev/video1, /dev/video3 ? it maybe : modprobe ov511 unit_video=4,1,3 > And assuming that you have 5 ov511 devices, and connect they one by one, > they'll gain the following device numbers (at the connection order): > /dev/video4 > /dev/video1 > /dev/video3 > /dev/video0 > /dev/video2 > > However, changeset 9133: > > changeset: 9133:64aed7485a43 > parent: 9073:4db9722caf4f > user:Hans Verkuil > date:Sat Oct 04 13:36:54 2008 +0200 > summary: v4l: disconnect kernel number from minor > > Changed the behavior video_register_device(dev, v, nr): > > + nr = find_next_zero_bit(video_nums[type], minor_cnt, nr == -1 ? 0 : nr); > > So, instead of accepting just -1 or nr, it will now do: > if nr = -1, it will do dynamic minor allocation (as before); > if nr >= 0, it will also do dynamic minor allocation, but starting from > nr. > > So, the new code won't fail if nr is already allocated, but it will return the > next unallocated nr. > > With the ov511 code, this means that you'll have, instead: > > /dev/video5 > /dev/video6 > /dev/video7 > /dev/video8 > /dev/video9 > > So, changeset 9133 actually broke the ov511 probe original behavior. > > In order to restore the original behavior, the probe logic should be replaced > by something else (like the approach taken by em28xx). > > Also, changeset 9133 may also cause other side effects on other drivers that > were expecting the original behavior. > > To make things worse, the function comment doesn't properly explain the > current > behavior. > > --- > > Figo, > > Since we are in the middle of a merge window, it is unlikely that I'll have > enough time to properly fix it right now (since my priority right now is to > merge the existing patches). > > As you started proposing a patch for it, maybe you could try to fix it and > check about similar troubles
About V4L2_MEMORY_OVERLAY
Hello everyone, What I'm curious is about what is V4L2_MEMORY_OVERLAY for. Even though I have been using overlay capability before, I always used V4L2_MEMORY_MMAP instead of that. And our v4l2 api document tells nothing about V4L2_MEMORY_MMAP (only [todo] is left..OMG ;-() But looking into videobuf, I can see some codes implemented for V4L2_MEMORY_OVERLAY. But I'm not sure about which point can make driver available for V4L2_MEMORY_OVERLAY. If the implementation of that method is still in progress, can I have any information or chance to participate? Until then I might be using V4L2_MEMORY_MMAP in my camera interface driver. Cheers, Nate -- = DongSoo, Nathaniel Kim Engineer Mobile S/W Platform Lab. Digital Media & Communications R&D Centre Samsung Electronics CO., LTD. e-mail : dongsoo@gmail.com dongsoo45@samsung.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] ov511.c: video_register_device() return zero on success
On Thursday 11 June 2009 06:40:14 Mauro Carvalho Chehab wrote: > Em Wed, 10 Jun 2009 22:39:51 -0300 > Mauro Carvalho Chehab escreveu: > > > Em Sun, 31 May 2009 14:41:52 +0800 > > "Figo.zhang" escreveu: > > > > > video_register_device() return zero on success, it would not return a > > > positive integer. > > > > > > Signed-off-by: Figo.zhang > > > --- > > > drivers/media/video/ov511.c |2 +- > > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > > > diff --git a/drivers/media/video/ov511.c b/drivers/media/video/ov511.c > > > index 9af5532..816427e 100644 > > > --- a/drivers/media/video/ov511.c > > > +++ b/drivers/media/video/ov511.c > > > @@ -5851,7 +5851,7 @@ ov51x_probe(struct usb_interface *intf, const > > > struct usb_device_id *id) > > > break; > > > > > > if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, > > > - unit_video[i]) >= 0) { > > > + unit_video[i]) == 0) { > > > break; > > > } > > > } > > > > Nack. > > > > Errors are always negative. So, any zero or positive value indicates that > > no error occurred. > > > > Yet, the logic for forcing ov51x to specific minor number seems broken: it > > will > > end by registering the device twice, if used. > > > > So, that part of the function needs a rewrite. I'll fix it. > > > > Hmm... the issue seems more complex than I've imagined... > > When ov511 were written, it was assumed that video_register_device(dev, v, nr) > would have the following behavior: > > if nr = -1, it would do dynamic minor allocation; > if nr >= 0, it would allocate to 'nr' minor or fail. > > With the original behavior. > > The ov511_probe registering loop is doing something like this (I did a cleanup > to allow a better understand of the logic): > > > #define OV511_MAX_UNIT_VIDEO 16 > ... > static int unit_video[OV511_MAX_UNIT_VIDEO]; > ... > module_param_array(unit_video, int, &num_uv, 0); > MODULE_PARM_DESC(unit_video, > "Force use of specific minor number(s). 0 is not allowed."); > ... > static int > ov51x_probe(struct usb_interface *intf, const struct usb_device_id *id) > { > ... > for (i = 0; i < OV511_MAX_UNIT_VIDEO; i++) { > /* Minor 0 cannot be specified; assume user wants autodetect > */ > if (unit_video[i] == 0) > break; > > if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, > unit_video[i]) >= 0) > goto register_succeded; > } > > /* Use the next available one */ > if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, -1) < 0) { > err("video_register_device failed"); > goto error; > > register_succeeded: > dev_info(&intf->dev, "Device at %s registered to minor %d\n", > ov->usb_path, ov->vdev->minor); > > > So, if you probe ov511 with a list of device numbers, for example: > > modprobe ov511 4,1,3 > > And assuming that you have 5 ov511 devices, and connect they one by one, > they'll gain the following device numbers (at the connection order): > /dev/video4 > /dev/video1 > /dev/video3 > /dev/video0 > /dev/video2 > > However, changeset 9133: > > changeset: 9133:64aed7485a43 > parent: 9073:4db9722caf4f > user:Hans Verkuil > date:Sat Oct 04 13:36:54 2008 +0200 > summary: v4l: disconnect kernel number from minor > > Changed the behavior video_register_device(dev, v, nr): > > + nr = find_next_zero_bit(video_nums[type], minor_cnt, nr == -1 ? 0 : nr); > > So, instead of accepting just -1 or nr, it will now do: > if nr = -1, it will do dynamic minor allocation (as before); > if nr >= 0, it will also do dynamic minor allocation, but starting from > nr. > > So, the new code won't fail if nr is already allocated, but it will return the > next unallocated nr. > > With the ov511 code, this means that you'll have, instead: > > /dev/video5 > /dev/video6 > /dev/video7 > /dev/video8 > /dev/video9 > > So, changeset 9133 actually broke the ov511 probe original behavior. > > In order to restore the original behavior, the probe logic should be replaced > by something else (like the approach taken by em28xx). > > Also, changeset 9133 may also cause other side effects on other drivers that > were expecting the original behavior. > > To make things worse, the function comment doesn't properly explain the > current > behavior. > > --- > > Figo, > > Since we are in the middle of a merge window, it is unlikely that I'll have > enough time to properly fix it right now (since my priority right now is to > merge the existing patches). > > As you started proposing a patch for it, maybe you could try to fix it and > check about similar troubles on other drivers. > > Cheers, > Mauro > Since I made that change I'm willing to look at this. Some comments definitely need impr
Re: pull requests x patches at linux-media ML - Was: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
Hi Mauro, thanks for replying and for the explanation. I'll skip most of your message, and just keep the bits I'd like to reply to. On Wed, 10 Jun 2009, Mauro Carvalho Chehab wrote: > The same happens here: All patches added at the staging tree are sent to > linuxtv-commits ML. So, they are there for discussions before my pull > requests. > > The main difference is that, in the case of Greg, his staging tree is a quilt > one. On our case, our staging tree is mercurial. Hm, I am not sure, is Greg's quilt tree publically available? And how many actually use it? Whereas your hg tree is publically available and it looks like a few do use it. > We're currently merging about 900 patches per kernel window, on a window of > about 10-12 weeks. This means about 90 patches per week, or about 13 patches > per day (for a 7 days week), or 18 patches per day (for a 5 days week). > > So, if people just send one email per patch, this will increase our traffic by > 18 emails by day. It can be worse than that, if we consider that patches can > be > replied, and that people use to write a patch 0 to describe a patch series. > > Considering about 50 messages per day, currently (today and yesterday's > statistics - not sure those are typical days), this would increase the ML by > about 36%. I still don't take this argument of increased traffic - I haven't seen a single complain "please, don't increase the traffic, it'll make it worse for me." > 1) trivial patches (typo fixes, coding style, simple board additions, Kbuild > fixes, etc); > > 2) bug fix patches at drivers; > > 3) new drivers; > > 4) core changes. > > However, several driver maintainers don't care (or just forget about they) > for (1) > type. Before patchwork, lots of such patches were lost forever in the middle > of > dvb and v4l mailing lists. They are happy when someone (me) get those patches > and apply at the tree or remind they to check and apply on their trees. > > patches of type (2) and (3) are in general sent via a driver maintainer and > generally doesn't generate discussions. I'm really happy we have subsistem maintainers that are such profecient in their work and such confident in their results that they don't need any reviews and discussions. I for one is not one of them, that's why I always first send my patches to the list. > Also, for the developer, using the pull request is better, since they can > better track when a patch series got merged. I never argumented against pull requests, I'm suggesting they should follow patches posted to the list. > The usage of a mix of PULL and PATCH requests has an extra trouble: it means > that we'll receive most of the patches duplicated. So, it means that I need to > manually mark all merged patches at patchwork queue, on _each_ pull request. Yes, I see what you mean, but 1) you cannot avoid it, there are always patches from various authors, that they send to the ML, that some driver-maintainer will then pull through his or her tree and ask you to pull it. So, we really have to learn to proces this case efficiently. > So, this adds an extra cost that will probably make life worse for everybody, > with almost no gain (since there are really very few complaints about badly > merged patches). > > That's said, I'm open to listen to opinions on how to improve our current > process Well, I guess, I will have to subscribe to that hg-commit list (or whatever it's called), and use it. The problem is - it is a bit too late. But it's the best option available so far. Another question, if you pull patches from someone's tree for review of one of those pull-requests (as you described in this mail, but I've deleted that piece already), how do you then quote the code if you want to comment on it? You export the patch again, hit reply on the pull-request, and paste the patch into it? And then add the quotation marks "> " manually?... Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC: PATCH: add new s_config subdev ops and v4l2_i2c_new_subdev_cfg/board calls
As per Mauro's request here is the patch adding the new core functionality. To quote from my original pull request: "This time I've only added new functions and left the existing ones in place. I did add a bit of code to the existing v4l2_i2c_new_(probed_)subdev functions to call the new s_config op if it is available. Existing subdev drivers never set this new op, so this code will not effect current behavior. But for new drivers that do set s_config it is important that it is called no matter what flavor of these functions is used. At the end of the 2.6.31 cycle we can replace the current v4l2_i2c_new_(probed_)subdev calls with the new one I had in my earlier patches." Comments are welcome. Hans # HG changeset patch # User Hans Verkuil # Date 1244578353 -7200 # Node ID d9d3f747395109de316eadeed1d1d52b8440f84b # Parent ed3781a79c734f35b800d0b55d276cd62d793141 v4l2: add new s_config subdev ops and v4l2_i2c_new_subdev_cfg/board calls From: Hans Verkuil Add a new s_config core ops call: this is called with the irq and platform data to be used to initialize the subdev. Added new v4l2_i2c_new_subdev_cfg and v4l2_i2c_new_subdev_board calls that allows you to pass these new arguments. The existing v4l2_i2c_new_subdev functions were modified to also call s_config. In the future the existing v4l2_i2c_new_subdev functions will be replaced by a single v4l2_i2c_new_subdev function similar to v4l2_i2c_new_subdev_cfg but without the irq and platform_data arguments. Priority: normal Signed-off-by: Hans Verkuil diff -r ed3781a79c73 -r d9d3f7473951 linux/drivers/media/video/v4l2-common.c --- a/linux/drivers/media/video/v4l2-common.c Sat Jun 06 16:31:34 2009 +0400 +++ b/linux/drivers/media/video/v4l2-common.c Tue Jun 09 22:12:33 2009 +0200 @@ -868,6 +868,17 @@ /* Decrease the module use count to match the first try_module_get. */ module_put(client->driver->driver.owner); + if (sd) { + /* We return errors from v4l2_subdev_call only if we have the + callback as the .s_config is not mandatory */ + int err = v4l2_subdev_call(sd, core, s_config, 0, NULL); + + if (err && err != -ENOIOCTLCMD) { + v4l2_device_unregister_subdev(sd); + sd = NULL; + } + } + error: #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 26) /* If we have a client but no subdev, then something went wrong and @@ -931,6 +942,17 @@ /* Decrease the module use count to match the first try_module_get. */ module_put(client->driver->driver.owner); + if (sd) { + /* We return errors from v4l2_subdev_call only if we have the + callback as the .s_config is not mandatory */ + int err = v4l2_subdev_call(sd, core, s_config, 0, NULL); + + if (err && err != -ENOIOCTLCMD) { + v4l2_device_unregister_subdev(sd); + sd = NULL; + } + } + error: #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 26) /* If we have a client but no subdev, then something went wrong and @@ -952,6 +974,150 @@ module_name, client_type, addrs); } EXPORT_SYMBOL_GPL(v4l2_i2c_new_probed_subdev_addr); + +/* Load an i2c sub-device. */ +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 26) +struct v4l2_subdev *v4l2_i2c_new_subdev_board(struct v4l2_device *v4l2_dev, + struct i2c_adapter *adapter, const char *module_name, + struct i2c_board_info *info, const unsigned short *probe_addrs) +{ + struct v4l2_subdev *sd = NULL; + struct i2c_client *client; + + BUG_ON(!v4l2_dev); + + if (module_name) + request_module(module_name); + + /* Create the i2c client */ + if (info->addr == 0 && probe_addrs) + client = i2c_new_probed_device(adapter, info, probe_addrs); + else + client = i2c_new_device(adapter, info); + + /* Note: by loading the module first we are certain that c->driver + will be set if the driver was found. If the module was not loaded + first, then the i2c core tries to delay-load the module for us, + and then c->driver is still NULL until the module is finally + loaded. This delay-load mechanism doesn't work if other drivers + want to use the i2c device, so explicitly loading the module + is the best alternative. */ + if (client == NULL || client->driver == NULL) + goto error; + + /* Lock the module so we can safely get the v4l2_subdev pointer */ + if (!try_module_get(client->driver->driver.owner)) + goto error; + sd = i2c_get_clientdata(client); + + /* Register with the v4l2_device which increases the module's + use count as well. */ + if (v4l2_device_register_subdev(v4l2_dev, sd)) + sd = NULL;
Re: About V4l2 overlay sequence
It seems to be totally described in v4l2 api specification document. The document is saying like this: VIDIOC_QUERYCAP video input and video standard ioctls VIDIOC_G_FBUF and VIDIOC_S_FBUF VIDIOC_G_FMT/S_FMT/TRY_FMT VIDIOC_OVERLAY Take a look at the document for detailed usage. Cheers, Nate On Wed, Jun 10, 2009 at 6:13 PM, xie wrote: > Dear all ~~ > > With your help I have implemented the preview with capture interface ~~ > Now i want to implement the preview with ovelay , and my camera support > s ovelay ~ > Who can tell me where I can find the document about ovelay sequcence ~ ? > Or does there have a standard example source code ~ ? > > Thanks a lot ~ > > Best regards > > -- = DongSoo, Nathaniel Kim Engineer Mobile S/W Platform Lab. Digital Media & Communications R&D Centre Samsung Electronics CO., LTD. e-mail : dongsoo@gmail.com dongsoo45@samsung.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] ov511.c: video_register_device() return zero on success
Em Wed, 10 Jun 2009 22:39:51 -0300 Mauro Carvalho Chehab escreveu: > Em Sun, 31 May 2009 14:41:52 +0800 > "Figo.zhang" escreveu: > > > video_register_device() return zero on success, it would not return a > > positive integer. > > > > Signed-off-by: Figo.zhang > > --- > > drivers/media/video/ov511.c |2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/media/video/ov511.c b/drivers/media/video/ov511.c > > index 9af5532..816427e 100644 > > --- a/drivers/media/video/ov511.c > > +++ b/drivers/media/video/ov511.c > > @@ -5851,7 +5851,7 @@ ov51x_probe(struct usb_interface *intf, const struct > > usb_device_id *id) > > break; > > > > if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, > > - unit_video[i]) >= 0) { > > + unit_video[i]) == 0) { > > break; > > } > > } > > Nack. > > Errors are always negative. So, any zero or positive value indicates that no > error occurred. > > Yet, the logic for forcing ov51x to specific minor number seems broken: it > will > end by registering the device twice, if used. > > So, that part of the function needs a rewrite. I'll fix it. > Hmm... the issue seems more complex than I've imagined... When ov511 were written, it was assumed that video_register_device(dev, v, nr) would have the following behavior: if nr = -1, it would do dynamic minor allocation; if nr >= 0, it would allocate to 'nr' minor or fail. With the original behavior. The ov511_probe registering loop is doing something like this (I did a cleanup to allow a better understand of the logic): #define OV511_MAX_UNIT_VIDEO 16 ... static int unit_video[OV511_MAX_UNIT_VIDEO]; ... module_param_array(unit_video, int, &num_uv, 0); MODULE_PARM_DESC(unit_video, "Force use of specific minor number(s). 0 is not allowed."); ... static int ov51x_probe(struct usb_interface *intf, const struct usb_device_id *id) { ... for (i = 0; i < OV511_MAX_UNIT_VIDEO; i++) { /* Minor 0 cannot be specified; assume user wants autodetect */ if (unit_video[i] == 0) break; if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, unit_video[i]) >= 0) goto register_succeded; } /* Use the next available one */ if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, -1) < 0) { err("video_register_device failed"); goto error; register_succeeded: dev_info(&intf->dev, "Device at %s registered to minor %d\n", ov->usb_path, ov->vdev->minor); So, if you probe ov511 with a list of device numbers, for example: modprobe ov511 4,1,3 And assuming that you have 5 ov511 devices, and connect they one by one, they'll gain the following device numbers (at the connection order): /dev/video4 /dev/video1 /dev/video3 /dev/video0 /dev/video2 However, changeset 9133: changeset: 9133:64aed7485a43 parent: 9073:4db9722caf4f user:Hans Verkuil date:Sat Oct 04 13:36:54 2008 +0200 summary: v4l: disconnect kernel number from minor Changed the behavior video_register_device(dev, v, nr): + nr = find_next_zero_bit(video_nums[type], minor_cnt, nr == -1 ? 0 : nr); So, instead of accepting just -1 or nr, it will now do: if nr = -1, it will do dynamic minor allocation (as before); if nr >= 0, it will also do dynamic minor allocation, but starting from nr. So, the new code won't fail if nr is already allocated, but it will return the next unallocated nr. With the ov511 code, this means that you'll have, instead: /dev/video5 /dev/video6 /dev/video7 /dev/video8 /dev/video9 So, changeset 9133 actually broke the ov511 probe original behavior. In order to restore the original behavior, the probe logic should be replaced by something else (like the approach taken by em28xx). Also, changeset 9133 may also cause other side effects on other drivers that were expecting the original behavior. To make things worse, the function comment doesn't properly explain the current behavior. --- Figo, Since we are in the middle of a merge window, it is unlikely that I'll have enough time to properly fix it right now (since my priority right now is to merge the existing patches). As you started proposing a patch for it, maybe you could try to fix it and check about similar troubles on other drivers. 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: s5h1411_readreg: readreg error (ret == -5)
On Sun, 7 Jun 2009, Roger wrote: > >From looking at "linux/drivers/media/dvb/frontends/s5h1411.c", The > s5h1411_readreg wants to see "2" but is getting "-5" from the i2c bus. > > --- Snip --- > > s5h1411_readreg: readreg error (ret == -5) > pvrusb2: unregistering DVB devices > device: 'dvb0.net0': device_unregister > > --- Snip --- > > What exactly does this mean? Roger: It means that the module attempted an I2C transfer and the transfer failed. The I2C adapter within the pvrusb2 driver will return either the number of bytes that it transferred or a failure code. The failure code, as is normal convention in the kernel, will be a negated errno value. Thus the expected value of 2 would be the fact that it probably tried a 2 byte transfer, while the actual value returned of -5 indicate an EIO error, which is what the pvrusb2 driver will return when the underlying I2C transaction has failed. Of course the real question is not that it failed but why it failed. And for that I unfortunately do not have an answer. It's possible that the s5h1411 driver did something that the chip didn't like and the chip responded by going deaf on the I2C bus. More than a few I2C-driven parts can behave this way. It's also possible that the part might have been busy and unable to respond - but usually in that case the driver for such a part will be written with this in mind and will know how / when to communicate with the hardware. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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] ov511.c: video_register_device() return zero on success
Em Sun, 31 May 2009 14:41:52 +0800 "Figo.zhang" escreveu: > video_register_device() return zero on success, it would not return a > positive integer. > > Signed-off-by: Figo.zhang > --- > drivers/media/video/ov511.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/media/video/ov511.c b/drivers/media/video/ov511.c > index 9af5532..816427e 100644 > --- a/drivers/media/video/ov511.c > +++ b/drivers/media/video/ov511.c > @@ -5851,7 +5851,7 @@ ov51x_probe(struct usb_interface *intf, const struct > usb_device_id *id) > break; > > if (video_register_device(ov->vdev, VFL_TYPE_GRABBER, > - unit_video[i]) >= 0) { > + unit_video[i]) == 0) { > break; > } > } Nack. Errors are always negative. So, any zero or positive value indicates that no error occurred. Yet, the logic for forcing ov51x to specific minor number seems broken: it will end by registering the device twice, if used. So, that part of the function needs a rewrite. I'll fix it. -- 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: pull requests x patches at linux-media ML - Was: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
Am Mittwoch, den 10.06.2009, 21:13 -0300 schrieb Mauro Carvalho Chehab: > First of all, I'm renaming the thread, since we're not discussing the pull > request per se. So, let's create a separate thread for it. > > Em Wed, 10 Jun 2009 23:18:52 +0200 > hermann pitton escreveu: > > > Hi, > > > > Am Mittwoch, den 10.06.2009, 22:39 +0200 schrieb Hans Verkuil: > > > On Wednesday 10 June 2009 22:20:03 Guennadi Liakhovetski wrote: > > > > On Wed, 10 Jun 2009, Hans Verkuil wrote: > > > > > On Wednesday 10 June 2009 20:17:53 Guennadi Liakhovetski wrote: > > > > > > On Tue, 9 Jun 2009, Hans Verkuil wrote: > > > > > > > Hi Mauro, > > > > > > > > > > > > > > Please pull from > > > > > > > http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2 for the > > > > > > > following: > > > > > > > > > > > > > > - v4l2: add new s_config subdev ops and > > > > > > > v4l2_i2c_new_subdev_cfg/board calls - v4l2-device: fix incorrect > > > > > > > kernel check > > > > > > > - v4l-compat: add I2C_ADDRS macro. > > > > > > > - v4l2: update framework documentation. > > > > > > > - v4l2-subdev: remove unnecessary check > > > > > > > > > > > > Do I understand this right, that these patches have not been posted > > > > > > to the list? > > > > > > > > > > The idea is that you click on the link and look at the patches through > > > > > the hg web frontend. > > > > > > > > And then? > > > > > > > > > > At least I haven't found them in online and in my local > > > > > > archives. If it's really so, sorry, this doesn't seem very > > > > > > productive > > > > > > to me... We have discussed this with Mauro on IRC, he didn't agree > > > > > > with me, he thought it was acceptable in many cases... Sorry, cannot > > > > > > agree. > > > > > > > > > > Both methods (a pull request or a patch series) are used and > > > > > personally > > > > > I have no preference, although currently I have a script that > > > > > simplifies these pull requests so I generally use that. A patch series > > > > > makes it easier to reply with review comments, while I think a pull > > > > > request reduces mailinglist traffic and actually makes it easier to do > > > > > the actual reviews. > > > > > > > > I think, patches posted to the list for reviews with a following pull > > > > request (if no rework needed of course) is the most reviewer-friendly, > > > > which is also what I so far have seen on all other lists I subscribe to > > > > (arm, ppc, usb, scsi, lkml, i2c,...). > > On LKML, several patches are sent as pull requests. > > > > > Have you seen those huge mailing > > > > threads from Greg K-H with all patches in his queue preceding his pull > > > > requests (I hope I reproduce his work flow correctly here, any mistakes > > > > are mine and unintended)? > > The same happens here: All patches added at the staging tree are sent to > linuxtv-commits ML. So, they are there for discussions before my pull > requests. > > The main difference is that, in the case of Greg, his staging tree is a quilt > one. On our case, our staging tree is mercurial. > > > > > Are you really saying that it's equally convenient for you to review / > > > > reply to patch on ML and to patch in some repository from a pull > > > > request? > > > > Especially when there are multiple patches in that pull and you have to > > > > click through them all, jumping back and forth between your mail client > > > > and a browser?... > > > > > > > > If all are so much concerned about the ML traffic (which I don't > > > > understand either, filtering and ignoring uninteresting mails is easy > > > > enough these days), maybe we should split into devel and user? Sorry, I > > > > really don't understand. I'll go ask members of other MLs what they > > > > think > > > > about "clicking" through patches... > > > > > > Um, you are asking the wrong person. It's one of the two methods used on > > > this list. Yes, pull requests are unusual compared to other lists (and so > > > is the use of hg instead of git for that matter due to historical > > > reasons). > > > > > > If Mauro says: use patch series, then I'll modify my workflow. If you > > > prefer > > > to see these subdev patches as a patch series, then I can do that for > > > you. > > > I have no preference myself. It's also a discussion I have no wish to go > > > into. > > > > > > So if you see a pull request from me and prefer to have it as a patch > > > series, just mail me and I'll do it. No problem. > > > > > > Regards, > > > > > > Hans > > > > on the pull requests is at least nothing new since years. > > > > Previously all patches were on video4linux and the linux-dvb ML and > > dealt with independently as far as possible. > > > > Because of all the hybrid devices that changed, but still someone having > > only analog TV reception likely doesn't want to read all about the > > digital stuff and in the other direction I assume in counts even more. > > > > So far the mercurial pull requests from the more active developers > >
Re: [patch 1/6] radio-mr800.c: missing mutex include
Em Wed, 10 Jun 2009 16:23:44 -0700 Andrew Morton escreveu: > On Thu, 11 Jun 2009 03:21:36 +0400 Alexey Klimov > wrote: > > > On Wed, Jun 10, 2009 at 11:44 PM, wrote: > > > From: Alessio Igor Bogani > > > > > > radio-mr800.c uses struct mutex, so while seems to be > > > pulled in indirectly by one of the headers it already includes, the right > > > thing is to include it directly. > > > > It was already applied to v4l-dvb tree (and probably to v4l git tree). > > It isn't in today's linux-next. Hi Andrew, Thanks for remind us about those 6 patches. I'll double check if those are already on my staging tree. If they aren't I'll apply or comment. Since I'm currently in the proccess updating my trees, I should be adding several pending patches on my -git tree, probably in time for tomorrow's linux-next tree. 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: soc-camera: status, roadmap
Em Wed, 10 Jun 2009 22:09:07 +0200 Hans Verkuil escreveu: > > 1.4. this is the actual conversion to v4l2-subdev. It depends on some > > bits and pieces in the v4l2-subdev framework, which are still in progress > > (e.g. v4l2_i2c_new_subdev_board), I believe (Hans, am I right? or what's > > the outcome of Mauro's last reply to you in the "[PULL] > > http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev"; thread?), so, it > > becomes practically impossible to also pull it for 2.6.31. > > I haven't seen a reaction yet from Mauro regarding my latest pull request: I > think it addresses all his concerns regarding existing functionality so I > actually hope this can be merged. It would help a lot with this and similar > efforts. As Guennadi pointed, it seems that those patches were not yet properly discussed at the ML. Since they touch at the core of the subsystem, could you please submit they as RFC, at linux-media? Let's give people some time to review the newly added functions. After that, you can send me V3 of your pull request. 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: Fwd: driver trident tm5600
ok i can compile it. only i have to change it: CONFIG_VIDEO_TM6000=y CONFIG_VIDEO_TM6000_DVB=m CONFIG_VIDEO_TM6000_ALSA=m but when i put modprobe tm6000-alsa card=5, still happend the previous error tm6000-alsa: Unknow symbol tm6000_get_reg tm6000-alsa: Unknow symbol tm6000_set_reg but also i try with make insmod tm6000-alsa ans make insmod tm6000-alsa card=5 appear this error /usr/lib/gcc/i486-linux-gnu/4.3.3/../../../../lib/crt1.o: In function `_start': /build/buildd/glibc-2.9/csu/../sysdeps/i386/elf/start.S:115: undefined reference to `main' tm6000-alsa.o: In function `dsp_buffer_free': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:148: undefined reference to `printk' tm6000-alsa.o: In function `tm6000_audio_init': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:369: undefined reference to `__this_module' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:369: undefined reference to `snd_card_new' tm6000-alsa.o: In function `kmalloc': /usr/src/linux-headers-2.6.28-13-generic/include/linux/slub_def.h:224: undefined reference to `kmalloc_caches' /usr/src/linux-headers-2.6.28-13-generic/include/linux/slub_def.h:224: undefined reference to `kmem_cache_alloc' tm6000-alsa.o: In function `tm6000_audio_init': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:388: undefined reference to `snd_component_add' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:400: undefined reference to `strlcat' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:402: undefined reference to `strlcat' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:408: undefined reference to `strlcat' tm6000-alsa.o: In function `snd_tm6000_pcm': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:336: undefined reference to `snd_pcm_new' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:341: undefined reference to `snd_pcm_set_ops' tm6000-alsa.o: In function `tm6000_audio_init': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:418: undefined reference to `snd_card_register' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:426: undefined reference to `snd_card_free' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:391: undefined reference to `usb_string' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:398: undefined reference to `strlcat' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:371: undefined reference to `printk' tm6000-alsa.o: In function `snd_tm6000_card_trigger': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:280: undefined reference to `_spin_lock' tm6000-alsa.o: In function `_tm6000_stop_audio_dma': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:135: undefined reference to `tm6000_get_reg' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:137: undefined reference to `tm6000_set_reg' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:139: undefined reference to `tm6000_set_reg' tm6000-alsa.o: In function `__raw_spin_unlock': /usr/src/linux-headers-2.6.28-13-generic/arch/x86/include/asm/paravirt.h:1411: undefined reference to `pv_lock_ops' tm6000-alsa.o: In function `_tm6000_start_audio_dma': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:116: undefined reference to `tm6000_get_reg' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:118: undefined reference to `tm6000_set_reg' tm6000-alsa.o: In function `_tm6000_stop_audio_dma': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:132: undefined reference to `printk' tm6000-alsa.o: In function `snd_tm6000_hw_params': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:230: undefined reference to `snd_pcm_format_physical_width' tm6000-alsa.o: In function `dsp_buffer_free': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:148: undefined reference to `printk' tm6000-alsa.o: In function `snd_tm6000_hw_params': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:236: undefined reference to `printk' tm6000-alsa.o: In function `snd_tm6000_pcm_open': /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:194: undefined reference to `snd_pcm_hw_constraint_pow2' /home/daniel/driver-tm5600/tm6010-3d58b6531a81/v4l/tm6000-alsa.c:205: undefined reference to `printk' tm6000-alsa.o:(__param+0x8): undefined reference to `param_set_int' tm6000-alsa.o:(__param+0xc): undefined reference to `param_get_int' tm6000-alsa.o:(__param+0x1c): undefined reference to `param_array_set' tm6000-alsa.o:(__param+0x20): undefined reference to `param_array_get' tm6000-alsa.o:(__param+0x30): undefined reference to `param_array_set' tm6000-alsa.o:(__param+0x34): undefined reference to `param_array_get' tm6000-alsa.o:(.rodata+0xcc): undefined reference to `param_set_int' tm6000-alsa.o:(.rodata+0xd0): undefined reference to `param_get_i
Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
Em Wed, 10 Jun 2009 23:36:32 +0200 (CEST) Guennadi Liakhovetski escreveu: > On Wed, 10 Jun 2009, hermann pitton wrote: > > > on the pull requests is at least nothing new since years. > > > > Previously all patches were on video4linux and the linux-dvb ML and > > dealt with independently as far as possible. > > > > Because of all the hybrid devices that changed, but still someone having > > only analog TV reception likely doesn't want to read all about the > > digital stuff and in the other direction I assume in counts even more. > > > > So far the mercurial pull requests from the more active developers > > worked quite well. Historically seen you would have had a need at some > > point to see _all_ patches on both lists, if you follow the rule _all_ > > patches must be on the list(s). > > > > Now, with linux-media, everybody subscribed has the traffic of both of > > the old lists. Means for most people 50% are off topic. > > > > But the really funny thing comes now, we have with you and all the > > others suddenly about 70% of traffic on the list about cams :) > > > > I'm sure that more than 90% of the old v4l and dvb list members are not > > interested in that stuff at all :) > > Sure, and that's fine, because I'm not interested in them being not > interested in that stuff at all:-) > > Now, how about this idea: > > someone writes a script to intercept all hg pull requests from lmml > (procmail rule), forward that mail to a special media-patch list, and > extract and post as replies to that mail all individual patches? And that > list should be configured to only accept mails from that script or replies > to its mails, so, it'd be spam-free. And that list would also be used for > patch discussion. How does this sound? This can be done. If you write such script in perl or python [1], I can put it to run at linuxtv. You'll likely need to handle also the pull request replies. [1] since we don't have procmail (or exim) installed there. 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: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
Em Wed, 10 Jun 2009 22:20:03 +0200 (CEST) Guennadi Liakhovetski escreveu: > Are you really saying that it's equally convenient for you to review / > reply to patch on ML and to patch in some repository from a pull request? > Especially when there are multiple patches in that pull and you have to > click through them all, jumping back and forth between your mail client > and a browser?... In my case (and I review 100% of the patches - so, I'm likely the worse case), reviewing they with a PULL request is faster, since I can just run hgimport script and see all of them locally even using tools like kdiff3. Also, I answer to just one email instead of tons of emails. 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
pull requests x patches at linux-media ML - Was: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
First of all, I'm renaming the thread, since we're not discussing the pull request per se. So, let's create a separate thread for it. Em Wed, 10 Jun 2009 23:18:52 +0200 hermann pitton escreveu: > Hi, > > Am Mittwoch, den 10.06.2009, 22:39 +0200 schrieb Hans Verkuil: > > On Wednesday 10 June 2009 22:20:03 Guennadi Liakhovetski wrote: > > > On Wed, 10 Jun 2009, Hans Verkuil wrote: > > > > On Wednesday 10 June 2009 20:17:53 Guennadi Liakhovetski wrote: > > > > > On Tue, 9 Jun 2009, Hans Verkuil wrote: > > > > > > Hi Mauro, > > > > > > > > > > > > Please pull from > > > > > > http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2 for the > > > > > > following: > > > > > > > > > > > > - v4l2: add new s_config subdev ops and > > > > > > v4l2_i2c_new_subdev_cfg/board calls - v4l2-device: fix incorrect > > > > > > kernel check > > > > > > - v4l-compat: add I2C_ADDRS macro. > > > > > > - v4l2: update framework documentation. > > > > > > - v4l2-subdev: remove unnecessary check > > > > > > > > > > Do I understand this right, that these patches have not been posted > > > > > to the list? > > > > > > > > The idea is that you click on the link and look at the patches through > > > > the hg web frontend. > > > > > > And then? > > > > > > > > At least I haven't found them in online and in my local > > > > > archives. If it's really so, sorry, this doesn't seem very productive > > > > > to me... We have discussed this with Mauro on IRC, he didn't agree > > > > > with me, he thought it was acceptable in many cases... Sorry, cannot > > > > > agree. > > > > > > > > Both methods (a pull request or a patch series) are used and personally > > > > I have no preference, although currently I have a script that > > > > simplifies these pull requests so I generally use that. A patch series > > > > makes it easier to reply with review comments, while I think a pull > > > > request reduces mailinglist traffic and actually makes it easier to do > > > > the actual reviews. > > > > > > I think, patches posted to the list for reviews with a following pull > > > request (if no rework needed of course) is the most reviewer-friendly, > > > which is also what I so far have seen on all other lists I subscribe to > > > (arm, ppc, usb, scsi, lkml, i2c,...). On LKML, several patches are sent as pull requests. > > > Have you seen those huge mailing > > > threads from Greg K-H with all patches in his queue preceding his pull > > > requests (I hope I reproduce his work flow correctly here, any mistakes > > > are mine and unintended)? The same happens here: All patches added at the staging tree are sent to linuxtv-commits ML. So, they are there for discussions before my pull requests. The main difference is that, in the case of Greg, his staging tree is a quilt one. On our case, our staging tree is mercurial. > > > Are you really saying that it's equally convenient for you to review / > > > reply to patch on ML and to patch in some repository from a pull request? > > > Especially when there are multiple patches in that pull and you have to > > > click through them all, jumping back and forth between your mail client > > > and a browser?... > > > > > > If all are so much concerned about the ML traffic (which I don't > > > understand either, filtering and ignoring uninteresting mails is easy > > > enough these days), maybe we should split into devel and user? Sorry, I > > > really don't understand. I'll go ask members of other MLs what they think > > > about "clicking" through patches... > > > > Um, you are asking the wrong person. It's one of the two methods used on > > this list. Yes, pull requests are unusual compared to other lists (and so > > is the use of hg instead of git for that matter due to historical reasons). > > > > If Mauro says: use patch series, then I'll modify my workflow. If you > > prefer > > to see these subdev patches as a patch series, then I can do that for you. > > I have no preference myself. It's also a discussion I have no wish to go > > into. > > > > So if you see a pull request from me and prefer to have it as a patch > > series, just mail me and I'll do it. No problem. > > > > Regards, > > > > Hans > > on the pull requests is at least nothing new since years. > > Previously all patches were on video4linux and the linux-dvb ML and > dealt with independently as far as possible. > > Because of all the hybrid devices that changed, but still someone having > only analog TV reception likely doesn't want to read all about the > digital stuff and in the other direction I assume in counts even more. > > So far the mercurial pull requests from the more active developers > worked quite well. Historically seen you would have had a need at some > point to see _all_ patches on both lists, if you follow the rule _all_ > patches must be on the list(s). > > Now, with linux-media, everybody subscribed has the traffic of both of > the old lists. Means for most people 50% are off topic. > > But the really funny t
Re: [patch 1/6] radio-mr800.c: missing mutex include
On Thu, 11 Jun 2009 03:21:36 +0400 Alexey Klimov wrote: > On Wed, Jun 10, 2009 at 11:44 PM, wrote: > > From: Alessio Igor Bogani > > > > radio-mr800.c uses struct mutex, so while seems to be > > pulled in indirectly by one of the headers it already includes, the right > > thing is to include it directly. > > It was already applied to v4l-dvb tree (and probably to v4l git tree). It isn't in today's linux-next. -- 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/6] radio-mr800.c: missing mutex include
On Wed, Jun 10, 2009 at 11:44 PM, wrote: > From: Alessio Igor Bogani > > radio-mr800.c uses struct mutex, so while seems to be > pulled in indirectly by one of the headers it already includes, the right > thing is to include it directly. It was already applied to v4l-dvb tree (and probably to v4l git tree). Thanks, -- regards, Klimov Alexey -- 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: mt9t031 (was RE: [PATCH] adding support for setting bus parameters in sub device)
On Wed, 10 Jun 2009, Karicheri, Muralidharan wrote: > So how do I know what frame-rate I get? Sensor output frame rate depends > on the resolution of the frame, blanking, exposure time etc. This is not supported. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] adding support for setting bus parameters in sub device
On Wed, 10 Jun 2009, Hans Verkuil wrote: > On Wednesday 10 June 2009 23:30:55 Guennadi Liakhovetski wrote: > > On Wed, 10 Jun 2009, Hans Verkuil wrote: > > > My view of this would be that the board specification specifies the > > > sensor (and possibly other chips) that are on the board. And to me it > > > makes sense that that also supplies the bus settings. I agree that it > > > is not complex code, but I think it is also unnecessary code. Why > > > negotiate if you can just set it? > > > > Why force all platforms to set it if the driver is perfectly capable do > > this itself? As I said - this is not a platform-specific feature, it's > > chip-specific. What good would it make to have all platforms using > > mt9t031 to specify, that yes, the chip can use both falling and rising > > pclk edge, but only active high vsync and hsync? > > ??? > > You will just tell the chip what to use. So you set 'use falling edge' and > either set 'active high vsync/hsync' or just leave that out since you know > the mt9t031 has that fixed. You don't specify in the platform data what the > chip can support, that's not relevant. You know what the host expects and > you pass that information on to the chip. > > A board designer knows what the host supports, No, he doesn't have to. That's not board specific, that's SoC specific. > knows what the sensor supports, Ditto, this is sensor-specific, not board-specific. > and knows if he added any inverters on the board, and based on > all that information he can just setup these parameters for the sensor > chip. Settings that are fixed on the sensor chip he can just ignore, he > only need to specify those settings that the sensor really needs. Of all the boards that I know of that use soc-camera only one (supposedly) had an inverter on one line, and even that one is not in the mainline. So, in the present soc-camera code not a single board have to bother with that. And now you want to add _all_ those polarity, master / slave flags to _all_ of them? Let me try again: you have an HSYNC output on the sensor. Its capabilities are known to the sensor driver you have an HSYNC input on the SoC. Its capabilities are known to the SoC-specific camera host driver these two lines are routed on the board to connect either directly or over some logic, hopefully, not more complex than an inverter. Now, this is _the_ _only_ bit of information, that is specific to the board. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
Am Mittwoch, den 10.06.2009, 23:36 +0200 schrieb Guennadi Liakhovetski: > On Wed, 10 Jun 2009, hermann pitton wrote: > > > on the pull requests is at least nothing new since years. > > > > Previously all patches were on video4linux and the linux-dvb ML and > > dealt with independently as far as possible. > > > > Because of all the hybrid devices that changed, but still someone having > > only analog TV reception likely doesn't want to read all about the > > digital stuff and in the other direction I assume in counts even more. > > > > So far the mercurial pull requests from the more active developers > > worked quite well. Historically seen you would have had a need at some > > point to see _all_ patches on both lists, if you follow the rule _all_ > > patches must be on the list(s). > > > > Now, with linux-media, everybody subscribed has the traffic of both of > > the old lists. Means for most people 50% are off topic. > > > > But the really funny thing comes now, we have with you and all the > > others suddenly about 70% of traffic on the list about cams :) > > > > I'm sure that more than 90% of the old v4l and dvb list members are not > > interested in that stuff at all :) > > Sure, and that's fine, because I'm not interested in them being not > interested in that stuff at all:-) Some do still follow ... ;) > Now, how about this idea: > > someone writes a script to intercept all hg pull requests from lmml > (procmail rule), forward that mail to a special media-patch list, and > extract and post as replies to that mail all individual patches? And that > list should be configured to only accept mails from that script or replies > to its mails, so, it'd be spam-free. And that list would also be used for > patch discussion. How does this sound? On v4l and dvb the most useful patches on the list for the users are those adding new devices on existing drivers. We don't have completely new drivers every day. To not lose them we have already patchwork employed and I'm not quite sure, if it really does what it should. The better is to have people dealing with all coming up 24/7 and issuing pull requests and pay with an additional SOB ;) for the rest of their live, but for the prize of having new bugs on always every new kernel because of API changes _others_ do need ... In opposite to linux-media in most cases, you are dealing with light sensors directly. Maybe this could make a difference. Cheers, Hermann -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: soc-camera: status, roadmap
On Wednesday 10 June 2009 23:49:23 Guennadi Liakhovetski wrote: > On Wed, 10 Jun 2009, Hans Verkuil wrote: > > On Wednesday 10 June 2009 18:45:36 Guennadi Liakhovetski wrote: > > [snip] > > > > 3. This also means, development will become more difficult, new > > > features and drivers will only be accepted on the top of my patch > > > stack, bugfixes will have to be accpeted against the mainline, which > > > then will mean extra porting work for me. > > > > If there is anything I can do to help this along, please let me know. > > In particular: what else besides the v4l2_i2c_new_subdev_board do you > > need? I didn't have much time in the past few weeks, but things are > > more relaxed now and I expect to be able to do a lot more in the coming > > weeks (fingers crossed :-) ). > > Thanks for your offer! > > Well, yes, at least we could start with these three things: > > 1. s_crop, g_crop, cropcap. Would be nice if you could add them just > following the standard API. No problem. > 2. bus parameter negotiation. I appreciate Murali's effort, but I don't > like re-inventing the wheel. So, we should now either bring his > implementation to support _at least_ all features so far supported in > soc-camera, or you could just copy the soc-camera implementation over, > just renaming functions and macros as appropriate. Obviously all features should be supported, but as you can no doubt tell I'm not sold (yet?) on the autonegotiation part of the soc-camera implementation. > 3. pixel format negotiation. The easiest would be to also just copy it > over. I haven't looked at this yet, but I will do so soon. > These 3 points would make the porting much easier. I'll certainly be > happy to answer any questions you might get working with soc-camera code. Thanks, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- 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,PATCH] VIDIOC_G_EXT_CTRLS does not handle NULL pointer correctly
On Wednesday 10 June 2009 15:53:57 Mauro Carvalho Chehab wrote: > Em Wed, 10 Jun 2009 10:52:28 -0300 > > Mauro Carvalho Chehab escreveu: > > Em Mon, 25 May 2009 11:16:34 -0300 > > > > Mauro Carvalho Chehab escreveu: > > > Em Mon, 25 May 2009 13:17:02 +0200 > > > > > > Laurent Pinchart escreveu: > > > > Hi everybody, > > > > > > > > Márton Németh found an integer overflow bug in the extended control > > > > ioctl handling code. This affects both video_usercopy and > > > > video_ioctl2. See http://bugzilla.kernel.org/show_bug.cgi?id=13357 > > > > for a detailed description of the problem. > > > > > > > > > > > > Restricting v4l2_ext_controls::count to values smaller than > > > > KMALLOC_MAX_SIZE / sizeof(struct v4l2_ext_control) should be enough, > > > > but we might want to restrict the value even further. I'd like > > > > opinions on this. > > > > > > Seems fine to my eyes, but being so close to kmalloc size doesn't seem > > > to be a good idea. It seems better to choose an arbitrary size big > > > enough to handle all current needs. > > > > I'll apply the current version, but I still think we should restrict it > > to a lower value. > > Hmm... SOB is missing. Márton and Laurent, could you please sign it Signed-off-by: Laurent Pinchart Cheers, 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] adding support for setting bus parameters in sub device
On Wednesday 10 June 2009 23:30:55 Guennadi Liakhovetski wrote: > On Wed, 10 Jun 2009, Hans Verkuil wrote: > > My view of this would be that the board specification specifies the > > sensor (and possibly other chips) that are on the board. And to me it > > makes sense that that also supplies the bus settings. I agree that it > > is not complex code, but I think it is also unnecessary code. Why > > negotiate if you can just set it? > > Why force all platforms to set it if the driver is perfectly capable do > this itself? As I said - this is not a platform-specific feature, it's > chip-specific. What good would it make to have all platforms using > mt9t031 to specify, that yes, the chip can use both falling and rising > pclk edge, but only active high vsync and hsync? ??? You will just tell the chip what to use. So you set 'use falling edge' and either set 'active high vsync/hsync' or just leave that out since you know the mt9t031 has that fixed. You don't specify in the platform data what the chip can support, that's not relevant. You know what the host expects and you pass that information on to the chip. A board designer knows what the host supports, knows what the sensor supports, and knows if he added any inverters on the board, and based on all that information he can just setup these parameters for the sensor chip. Settings that are fixed on the sensor chip he can just ignore, he only need to specify those settings that the sensor really needs. > > BTW, looking at the code there doesn't seem to be a bus type (probably > > only Bayer is used), correct? How is the datawidth negotiation done? Is > > the largest available width chosen? I assume that the datawidth is > > generally fixed by the host? I don't quite see how that can be > > negotiated since what is chosen there is linked to how the video data > > is transferred to memory. E.g., chosing a bus width of 8 or 10 bits can > > be the difference between transferring 8 bit or 16 bit data (with 6 > > bits padding) when the image is transferred to memory. If I would > > implement negotiation I would probably limit it to those one-bit > > settings and not include bus type and width. > > Well, this is indeed hairy. And hardware developers compete in creativity > making us invent new and new algorithms:-( I think, so far _practically_ > I have only worked with the following setups: > > 8 bit parallel with syncs and clocks. Data can be either Bayer or YUV. > This is most usual in my practice so far. > > 10 bit parallel (PXA270) with syncs and clocks bus with an 8 bit sensor > connected to most significant lanes... This is achieved by providing an > additional I2C controlled switch... > > 10 bit parallel with a 10 bit sensor, data 0-extended to 16 bit, raw > Bayer. > > 15 bit parallel bus (i.MX31) with 8 bit sensor connected to least > significant lanes, because i.MX31 is smart enough to use them correctly. > > ATM this is a part of soc-camera pixel format negotiation code, you can > look at various .set_bus_param() methods in sensor drivers. E.g., look in > mt9m001 and mt9v022 for drivers, using external bus-width switches... > > Now, I do not know how standard these various data packing methods on the > bus are, I think, we will have to give it a good thought when designing > an API, comments from developers familiar with various setups are much > appreciated. I think we should not attempt to be too fancy with this part of the API. Something like the pixel format API in the v4l2 spec which is basically just a random number with an associated format specification and no attempt and trying high-level format descriptions. In this case a simple enum might be enough. I'm not even sure whether we should specify the bus width but instead have it implicit in the bus type. I'm sure we are never going to be able to come up with something that will work on all hardware, so perhaps we shouldn't try to. Each different format gets its own type. Which is OK I think as long as that format is properly documented. Regards, Hans > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- 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: soc-camera: status, roadmap
On Wed, 10 Jun 2009, Hans Verkuil wrote: > On Wednesday 10 June 2009 18:45:36 Guennadi Liakhovetski wrote: [snip] > > 3. This also means, development will become more difficult, new features > > and drivers will only be accepted on the top of my patch stack, bugfixes > > will have to be accpeted against the mainline, which then will mean extra > > porting work for me. > > If there is anything I can do to help this along, please let me know. In > particular: what else besides the v4l2_i2c_new_subdev_board do you need? I > didn't have much time in the past few weeks, but things are more relaxed > now and I expect to be able to do a lot more in the coming weeks (fingers > crossed :-) ). Thanks for your offer! Well, yes, at least we could start with these three things: 1. s_crop, g_crop, cropcap. Would be nice if you could add them just following the standard API. 2. bus parameter negotiation. I appreciate Murali's effort, but I don't like re-inventing the wheel. So, we should now either bring his implementation to support _at least_ all features so far supported in soc-camera, or you could just copy the soc-camera implementation over, just renaming functions and macros as appropriate. 3. pixel format negotiation. The easiest would be to also just copy it over. These 3 points would make the porting much easier. I'll certainly be happy to answer any questions you might get working with soc-camera code. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: mt9t031 (was RE: [PATCH] adding support for setting bus parameters in sub device)
>> >So, what is missing in the driver apart from the ability to specify >> >a frame-rate? >> > >> [MK] Does the mt9t031 output one frame (snapshot) like in a camera or >> can it output frame continuously along with PCLK, Hsync and Vsync >> signals like in a video streaming device. VPFE capture can accept frames >> continuously from the sensor synchronized to PCLK, HSYNC and VSYNC and >> output frames to application using QBUF/DQBUF. In our implementation, we >> have timing parameters for the sensor to do streaming at various >> resolutions and fps. So application calls S_STD to set these timings. I >> am not sure if this is an acceptable way of implementing it. Any >> comments? > >Yes, it is streaming. > So how do I know what frame-rate I get? Sensor output frame rate depends on the resolution of the frame, blanking, exposure time etc. Murali >Thanks >Guennadi >--- >Guennadi Liakhovetski, Ph.D. >Freelance Open-Source Software Developer >http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
On Wed, 10 Jun 2009, Hans Verkuil wrote: > Um, you are asking the wrong person. It's one of the two methods used on > this list. Yes, pull requests are unusual compared to other lists (and so > is the use of hg instead of git for that matter due to historical reasons). No, they are not unusual, but - they come _after_ all patches to be pulled have been posted (and reviewed) on the list. For exanple, on ARM, they have sub-maintainers for various ARM CPUs, those sub-maintainers collect patches from contributors for their platforms from the list, and post their own patches to the list too. After that they form pull requests with patches that have been reviewed on the list and that they (and everyone else) are happy about. > If Mauro says: use patch series, then I'll modify my workflow. If you prefer > to see these subdev patches as a patch series, then I can do that for you. > I have no preference myself. It's also a discussion I have no wish to go > into. > > So if you see a pull request from me and prefer to have it as a patch > series, just mail me and I'll do it. No problem. This would help yes, thanks. Or - see my idea in an earlier mail in this thread. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: S_FMT vs. S_CROP
On Wed, 10 Jun 2009, Hans Verkuil wrote: > On Wednesday 10 June 2009 18:02:39 Guennadi Liakhovetski wrote: > > This question - how S_FMT and S_CROP affest image geometry - has been > > discussed at least twice before - that's only with my participation, > > don't know if and how often it has come up before. But the fact, that in > > two discussions we came up with different results seems to suggest, that > > this is not something trivially known by all except me. > > > > First time I asked this question in this thread > > > > http://www.mail-archive.com/linux-media@vger.kernel.org/msg00052.html > > > > and Mauro replied (see above thread for a complete reply): > > > > On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote: > > > On Wed, 7 Jan 2009 10:14:31 +0100 (CET) > > > Guennadi Liakhovetski wrote: > > > > [snip] > > > > > > For example on mt9t031 > > > > binning and skipping are used for that. Whereas CROP uses the current > > > > scaling configuration and selects a sub-window, so, once you've done > > > > S_FMT to 320x240, a crop request for 640x480 might well fail. > > > > > > I also understand this way. You cannot crop with a resolution bigger > > > than what you've selected. > > > > (Let's call this statement M1:-)) > > If I read the spec correctly, in particular section 1.11.1, then cropping > comes before scaling, so you can crop to 640x480 (S_CROP) and scale that to > 320x240 (S_FMT). S_FMT scales the cropped rectangle. This is my understanding of how it's supposed to work as well. -- 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: mt9t031 (was RE: [PATCH] adding support for setting bus parameters in sub device)
On Wed, 10 Jun 2009, Karicheri, Muralidharan wrote: > > > >> We need > >> streaming capability in the driver. This is how our driver works > >> with mt9t031 sensor > >> raw-bus (10 bit) > >> vpfe-capture - mt9t031 driver > >> || > >> V V > >>VPFEMT9T031 > >> > >> VPFE hardware has internal timing and DMA controller to > >> copy data frame by frame from the sensor output to SDRAM. > >> The PCLK form the sensor is used to generate the internal > >> timing. > > > >So, what is missing in the driver apart from the ability to specify > >a frame-rate? > > > [MK] Does the mt9t031 output one frame (snapshot) like in a camera or > can it output frame continuously along with PCLK, Hsync and Vsync > signals like in a video streaming device. VPFE capture can accept frames > continuously from the sensor synchronized to PCLK, HSYNC and VSYNC and > output frames to application using QBUF/DQBUF. In our implementation, we > have timing parameters for the sensor to do streaming at various > resolutions and fps. So application calls S_STD to set these timings. I > am not sure if this is an acceptable way of implementing it. Any > comments? Yes, it is streaming. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
On Wed, 10 Jun 2009, hermann pitton wrote: > on the pull requests is at least nothing new since years. > > Previously all patches were on video4linux and the linux-dvb ML and > dealt with independently as far as possible. > > Because of all the hybrid devices that changed, but still someone having > only analog TV reception likely doesn't want to read all about the > digital stuff and in the other direction I assume in counts even more. > > So far the mercurial pull requests from the more active developers > worked quite well. Historically seen you would have had a need at some > point to see _all_ patches on both lists, if you follow the rule _all_ > patches must be on the list(s). > > Now, with linux-media, everybody subscribed has the traffic of both of > the old lists. Means for most people 50% are off topic. > > But the really funny thing comes now, we have with you and all the > others suddenly about 70% of traffic on the list about cams :) > > I'm sure that more than 90% of the old v4l and dvb list members are not > interested in that stuff at all :) Sure, and that's fine, because I'm not interested in them being not interested in that stuff at all:-) Now, how about this idea: someone writes a script to intercept all hg pull requests from lmml (procmail rule), forward that mail to a special media-patch list, and extract and post as replies to that mail all individual patches? And that list should be configured to only accept mails from that script or replies to its mails, so, it'd be spam-free. And that list would also be used for patch discussion. How does this sound? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: S_FMT vs. S_CROP
On Wednesday 10 June 2009 18:02:39 Guennadi Liakhovetski wrote: > This question - how S_FMT and S_CROP affest image geometry - has been > discussed at least twice before - that's only with my participation, > don't know if and how often it has come up before. But the fact, that in > two discussions we came up with different results seems to suggest, that > this is not something trivially known by all except me. > > First time I asked this question in this thread > > http://www.mail-archive.com/linux-media@vger.kernel.org/msg00052.html > > and Mauro replied (see above thread for a complete reply): > > On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote: > > On Wed, 7 Jan 2009 10:14:31 +0100 (CET) > > Guennadi Liakhovetski wrote: > > [snip] > > > > For example on mt9t031 > > > binning and skipping are used for that. Whereas CROP uses the current > > > scaling configuration and selects a sub-window, so, once you've done > > > S_FMT to 320x240, a crop request for 640x480 might well fail. > > > > I also understand this way. You cannot crop with a resolution bigger > > than what you've selected. > > (Let's call this statement M1:-)) If I read the spec correctly, in particular section 1.11.1, then cropping comes before scaling, so you can crop to 640x480 (S_CROP) and scale that to 320x240 (S_FMT). S_FMT scales the cropped rectangle. > > > > For this you have > > > to issue a S_FMT, i.e., change scaling. Or would one have to re-scale > > > transparently? > > > > > > Is this interpretation correct? It seems to reflect the API as > > > documented on http://v4l2spec.bytesex.org/spec/book1.htm correctly. > > > > > > If it is correct, then what should CROP_CAP return as maximum > > > supported window sizes? Should it return them according to the > > > current scaling or according to scale 1? > > > > I understand that it should return against the current scaling. > > (and this one M2, whereas I understand it as "current scaling" means > "current scaling coefficient", not "current scaled output windof") > > Then in another thread > > http://www.mail-archive.com/linux-media@vger.kernel.org/msg03512.html > > Stefan motivated for an incomatibly different interpretation of the > standard: > > On Thu, 2 Apr 2009, Stefan Herbrechtsmeier wrote: > > [snip] > > > The user doesn't have to remember the scale anyway. Only the ways a > > different. You interpret S_CROP > > as something like a cutting of the S_FMT window. I interpret S_FMT as a > > output format selection > > and S_CROP as a sensor window selection. > > which I now interpret as > > S_FMT(640x480) means "scale whatever rectangle has been selected on the > sensor to produce an output window of 640x480" and S_CROP(2048x1536) > means "take a window of 2048x1536 sensor pixels from the sensor and scale > it to whatever output window has been or will be selected by S_FMT." This > contradicts M1, because you certainly can crop a larger (sensor) window. > Also, I now believe, that [GS]_CROP and, logically, CROPCAP operate in > sensor pixels and shall not depend on any scales, which contradicts (my > understanding of) M2. > > It now seems to be quite simple to me: > > {G,S,TRY}_FMT configure output geometry in user pixels > [GS]_CROP, CROPCAP configure input window in sensor pixels Agreed. > The thus configured input window should be mapped (scaled) into the > output window. > > Now, which one is correct? Your interpretation is correct to the best of my knowledge. I think the cropping API remains one of the worst explained ioctls in the spec. There are some weird things you can get into when dealing with S-Video (PAL/NTSC) like signals, but for sensors like this it is as you described. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- 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] adding support for setting bus parameters in sub device
On Wed, 10 Jun 2009, Hans Verkuil wrote: > My view of this would be that the board specification specifies the sensor > (and possibly other chips) that are on the board. And to me it makes sense > that that also supplies the bus settings. I agree that it is not complex > code, but I think it is also unnecessary code. Why negotiate if you can > just set it? Why force all platforms to set it if the driver is perfectly capable do this itself? As I said - this is not a platform-specific feature, it's chip-specific. What good would it make to have all platforms using mt9t031 to specify, that yes, the chip can use both falling and rising pclk edge, but only active high vsync and hsync? > BTW, looking at the code there doesn't seem to be a bus type (probably only > Bayer is used), correct? How is the datawidth negotiation done? Is the > largest available width chosen? I assume that the datawidth is generally > fixed by the host? I don't quite see how that can be negotiated since what > is chosen there is linked to how the video data is transferred to memory. > E.g., chosing a bus width of 8 or 10 bits can be the difference between > transferring 8 bit or 16 bit data (with 6 bits padding) when the image is > transferred to memory. If I would implement negotiation I would probably > limit it to those one-bit settings and not include bus type and width. Well, this is indeed hairy. And hardware developers compete in creativity making us invent new and new algorithms:-( I think, so far _practically_ I have only worked with the following setups: 8 bit parallel with syncs and clocks. Data can be either Bayer or YUV. This is most usual in my practice so far. 10 bit parallel (PXA270) with syncs and clocks bus with an 8 bit sensor connected to most significant lanes... This is achieved by providing an additional I2C controlled switch... 10 bit parallel with a 10 bit sensor, data 0-extended to 16 bit, raw Bayer. 15 bit parallel bus (i.MX31) with 8 bit sensor connected to least significant lanes, because i.MX31 is smart enough to use them correctly. ATM this is a part of soc-camera pixel format negotiation code, you can look at various .set_bus_param() methods in sensor drivers. E.g., look in mt9m001 and mt9v022 for drivers, using external bus-width switches... Now, I do not know how standard these various data packing methods on the bus are, I think, we will have to give it a good thought when designing an API, comments from developers familiar with various setups are much appreciated. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/10 - v2] ARM: DaVinci: Video: DM355/DM6446 VPFE Capture driver
Hans, Great! I look forward for your comments. Murali Karicheri email: m-kariche...@ti.com >-Original Message- >From: Hans Verkuil [mailto:hverk...@xs4all.nl] >Sent: Wednesday, June 10, 2009 5:28 PM >To: Karicheri, Muralidharan >Cc: linux-media@vger.kernel.org; davinci-linux-open- >sou...@linux.davincidsp.com; Muralidharan Karicheri >Subject: Re: [PATCH 0/10 - v2] ARM: DaVinci: Video: DM355/DM6446 VPFE >Capture driver > >On Tuesday 09 June 2009 20:46:44 m-kariche...@ti.com wrote: >> From: Muralidharan Karicheri >> >> VPFE Capture driver for DaVinci Media SOCs :- DM355 and DM6446 >> >> This is the version v2 of the patch series. This is the reworked >> version of the driver based on comments received against the last >> version of the patch. > >I'll be reviewing this Friday or Saturday. > >Regards, > > Hans > >> >> +++ >> These patches add support for VPFE (Video Processing Front End) based >> video capture on DM355 and DM6446 EVMs. For more details on the hardware >> configuration and capabilities, please refer the vpfe_capture.c header. >> This patch set consists of following:- >> >> Patch 1: VPFE Capture bridge driver >> Patch 2: CCDC hw device header file >> Patch 3: DM355 CCDC hw module >> Patch 4: DM644x CCDC hw module >> Patch 5: common types used across CCDC modules >> Patch 6: Makefile and config files for the driver >> Patch 7: DM355 platform and board setup >> Patch 8: DM644x platform and board setup >> Patch 9: Remove outdated driver files from davinci git tree >> Patch 10: common vpss hw module for video drivers >> >> NOTE: >> >> Dependent on the TVP514x decoder driver patch for migrating the >> driver to sub device model from Vaibhav Hiremath >> >> Following tests are performed. >> 1) Capture and display video (PAL & NTSC) from tvp5146 decoder. >> Displayed using fbdev device driver available on davinci git tree >> 2) Tested with driver built statically and dynamically >> >> Muralidhara Karicheri >> >> Reviewed By "Hans Verkuil". >> Reviewed By "Laurent Pinchart". >> >> Signed-off-by: Muralidharan Karicheri >> -- >> 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 Telecom -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/10 - v2] ARM: DaVinci: Video: DM355/DM6446 VPFE Capture driver
On Tuesday 09 June 2009 20:46:44 m-kariche...@ti.com wrote: > From: Muralidharan Karicheri > > VPFE Capture driver for DaVinci Media SOCs :- DM355 and DM6446 > > This is the version v2 of the patch series. This is the reworked > version of the driver based on comments received against the last > version of the patch. I'll be reviewing this Friday or Saturday. Regards, Hans > > +++ > These patches add support for VPFE (Video Processing Front End) based > video capture on DM355 and DM6446 EVMs. For more details on the hardware > configuration and capabilities, please refer the vpfe_capture.c header. > This patch set consists of following:- > > Patch 1: VPFE Capture bridge driver > Patch 2: CCDC hw device header file > Patch 3: DM355 CCDC hw module > Patch 4: DM644x CCDC hw module > Patch 5: common types used across CCDC modules > Patch 6: Makefile and config files for the driver > Patch 7: DM355 platform and board setup > Patch 8: DM644x platform and board setup > Patch 9: Remove outdated driver files from davinci git tree > Patch 10: common vpss hw module for video drivers > > NOTE: > > Dependent on the TVP514x decoder driver patch for migrating the > driver to sub device model from Vaibhav Hiremath > > Following tests are performed. > 1) Capture and display video (PAL & NTSC) from tvp5146 decoder. > Displayed using fbdev device driver available on davinci git tree > 2) Tested with driver built statically and dynamically > > Muralidhara Karicheri > > Reviewed By "Hans Verkuil". > Reviewed By "Laurent Pinchart". > > Signed-off-by: Muralidharan Karicheri > -- > 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 Telecom -- 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: mt9t031 (was RE: [PATCH] adding support for setting bus parameters in sub device)
>> We need >> streaming capability in the driver. This is how our driver works >> with mt9t031 sensor >>raw-bus (10 bit) >> vpfe-capture - mt9t031 driver >>|| >>V V >> VPFEMT9T031 >> >> VPFE hardware has internal timing and DMA controller to >> copy data frame by frame from the sensor output to SDRAM. >> The PCLK form the sensor is used to generate the internal >> timing. > >So, what is missing in the driver apart from the ability to specify >a frame-rate? > [MK] Does the mt9t031 output one frame (snapshot) like in a camera or can it output frame continuously along with PCLK, Hsync and Vsync signals like in a video streaming device. VPFE capture can accept frames continuously from the sensor synchronized to PCLK, HSYNC and VSYNC and output frames to application using QBUF/DQBUF. In our implementation, we have timing parameters for the sensor to do streaming at various resolutions and fps. So application calls S_STD to set these timings. I am not sure if this is an acceptable way of implementing it. Any comments? Thanks Murali >Thanks >Guennadi >--- >Guennadi Liakhovetski, Ph.D. >Freelance Open-Source Software Developer >http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
Hi, Am Mittwoch, den 10.06.2009, 22:39 +0200 schrieb Hans Verkuil: > On Wednesday 10 June 2009 22:20:03 Guennadi Liakhovetski wrote: > > On Wed, 10 Jun 2009, Hans Verkuil wrote: > > > On Wednesday 10 June 2009 20:17:53 Guennadi Liakhovetski wrote: > > > > On Tue, 9 Jun 2009, Hans Verkuil wrote: > > > > > Hi Mauro, > > > > > > > > > > Please pull from > > > > > http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2 for the > > > > > following: > > > > > > > > > > - v4l2: add new s_config subdev ops and > > > > > v4l2_i2c_new_subdev_cfg/board calls - v4l2-device: fix incorrect > > > > > kernel check > > > > > - v4l-compat: add I2C_ADDRS macro. > > > > > - v4l2: update framework documentation. > > > > > - v4l2-subdev: remove unnecessary check > > > > > > > > Do I understand this right, that these patches have not been posted > > > > to the list? > > > > > > The idea is that you click on the link and look at the patches through > > > the hg web frontend. > > > > And then? > > > > > > At least I haven't found them in online and in my local > > > > archives. If it's really so, sorry, this doesn't seem very productive > > > > to me... We have discussed this with Mauro on IRC, he didn't agree > > > > with me, he thought it was acceptable in many cases... Sorry, cannot > > > > agree. > > > > > > Both methods (a pull request or a patch series) are used and personally > > > I have no preference, although currently I have a script that > > > simplifies these pull requests so I generally use that. A patch series > > > makes it easier to reply with review comments, while I think a pull > > > request reduces mailinglist traffic and actually makes it easier to do > > > the actual reviews. > > > > I think, patches posted to the list for reviews with a following pull > > request (if no rework needed of course) is the most reviewer-friendly, > > which is also what I so far have seen on all other lists I subscribe to > > (arm, ppc, usb, scsi, lkml, i2c,...). Have you seen those huge mailing > > threads from Greg K-H with all patches in his queue preceding his pull > > requests (I hope I reproduce his work flow correctly here, any mistakes > > are mine and unintended)? > > > > Are you really saying that it's equally convenient for you to review / > > reply to patch on ML and to patch in some repository from a pull request? > > Especially when there are multiple patches in that pull and you have to > > click through them all, jumping back and forth between your mail client > > and a browser?... > > > > If all are so much concerned about the ML traffic (which I don't > > understand either, filtering and ignoring uninteresting mails is easy > > enough these days), maybe we should split into devel and user? Sorry, I > > really don't understand. I'll go ask members of other MLs what they think > > about "clicking" through patches... > > Um, you are asking the wrong person. It's one of the two methods used on > this list. Yes, pull requests are unusual compared to other lists (and so > is the use of hg instead of git for that matter due to historical reasons). > > If Mauro says: use patch series, then I'll modify my workflow. If you prefer > to see these subdev patches as a patch series, then I can do that for you. > I have no preference myself. It's also a discussion I have no wish to go > into. > > So if you see a pull request from me and prefer to have it as a patch > series, just mail me and I'll do it. No problem. > > Regards, > > Hans on the pull requests is at least nothing new since years. Previously all patches were on video4linux and the linux-dvb ML and dealt with independently as far as possible. Because of all the hybrid devices that changed, but still someone having only analog TV reception likely doesn't want to read all about the digital stuff and in the other direction I assume in counts even more. So far the mercurial pull requests from the more active developers worked quite well. Historically seen you would have had a need at some point to see _all_ patches on both lists, if you follow the rule _all_ patches must be on the list(s). Now, with linux-media, everybody subscribed has the traffic of both of the old lists. Means for most people 50% are off topic. But the really funny thing comes now, we have with you and all the others suddenly about 70% of traffic on the list about cams :) I'm sure that more than 90% of the old v4l and dvb list members are not interested in that stuff at all :) Cheers, Hermann -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] adding support for setting bus parameters in sub device
Hi, Now that there is discussion on this let me put how I see it working.. The bridge driver is aware of all the sub devices it is working with. So for each of the sub device, bridge driver could define per sub device configuration such as bus type, data width, and polarities as a platform data. So when bridge driver loads the sub device, first thing it will do is to read these settings and call s_bus() and configure the interface. vpfe capture works with tvp514x and mt9t031. The first one uses YUV bus and second one uses Bayer bus. For each of these, I will have per platform specific bus parameters. Now if any other bridge driver needs to use any these sub device, it is a matter of customizing these platform data. One example is mt9t031 driver which needs to work with vpfe capture as well soc camera system. What is the advantage of using querying versus defining them statically in the platform data? Murali email: m-kariche...@ti.com >-Original Message- >From: Hans Verkuil [mailto:hverk...@xs4all.nl] >Sent: Wednesday, June 10, 2009 4:52 PM >To: Guennadi Liakhovetski >Cc: Karicheri, Muralidharan; Linux Media Mailing List; davinci-linux-open- >sou...@linux.davincidsp.com >Subject: Re: [PATCH] adding support for setting bus parameters in sub >device > >On Wednesday 10 June 2009 21:59:13 Guennadi Liakhovetski wrote: >> On Wed, 10 Jun 2009, Hans Verkuil wrote: >> > On Wednesday 10 June 2009 20:32:25 Guennadi Liakhovetski wrote: >> > > On Tue, 9 Jun 2009, m-kariche...@ti.com wrote: >> > > > From: Muralidharan Karicheri >> > > > >> > > > This patch adds support for setting bus parameters such as bus type >> > > > (BT.656, BT.1120 etc), width (example 10 bit raw image data bus) >> > > > and polarities (vsync, hsync, field etc) in sub device. This allows >> > > > bridge driver to configure the sub device for a specific set of bus >> > > > parameters through s_bus() function call. >> > > >> > > Yes, this is required, but this is not enough. Firstly, you're >> > > missing at least one more flag - master or slave. Secondly, it is not >> > > enough to provide a s_bus function. Many hosts and sensors can >> > > configure one of several alternate configurations - they can select >> > > signal polarities, data widths, master / slave role, etc. Whereas >> > > others have some or all of these parameters fixed. That's why we have >> > > a query method in soc-camera, which delivers all supported >> > > configurations, and then the host can select some mutually acceptable >> > > subset. No, just returning an error code is not enough. >> > >> > Why would you want to query this? I would expect this to be fixed >> > settings: something that is determined by the architecture. Actually, I >> > would expect this to be part of the platform_data in many cases and >> > setup when the i2c driver is initialized and not touched afterwards. >> > >> > If we want to negotiate these bus settings, then we indeed need >> > something more. But it seems unnecessarily complex to me to implement >> > autonegotiation when this is in practice a fixed setting determined by >> > how the sensor is hooked up to the host. >> >> On the platform level I have so far seen two options: signal connected >> directly or via an inverter. For that you need platform data, yes. But >> otherwise - why? say, if both your sensor and your host can select hsync >> polarity in software - what should platform tell about it? This knowledge >> belongs in the respective drivers - they know, that they can configure >> arbitrary polarity and they advertise that. Another sensor, that is >> statically active high - what does platform have to do with it? The >> driver knows perfectly, that it can only do one polarity, and it >> negotiates that. Earlier we also had this flags configured in platform >> code, but then we realised that this wasn't correct. This information and >> configuration methods are chip-specific, not platform-specific. >> >> And the negotiation code is not that complex - just copy respective >> soc-camera functions, later the originals will be removed. > >My view of this would be that the board specification specifies the sensor >(and possibly other chips) that are on the board. And to me it makes sense >that that also supplies the bus settings. I agree that it is not complex >code, but I think it is also unnecessary code. Why negotiate if you can >just set it? > >BTW, looking at the code there doesn't seem to be a bus type (probably only >Bayer is used), correct? How is the datawidth negotiation done? Is the >largest available width chosen? I assume that the datawidth is generally >fixed by the host? I don't quite see how that can be negotiated since what >is chosen there is linked to how the video data is transferred to memory. >E.g., chosing a bus width of 8 or 10 bits can be the difference between >transferring 8 bit or 16 bit data (with 6 bits padding) when the image is >transferred to memory. If I would implement negotiation I would
mt9t031 (was RE: [PATCH] adding support for setting bus parameters in sub device)
On Wed, 10 Jun 2009, Karicheri, Muralidharan wrote: > Guennadi, > > Thanks for responding. I acknowledge I need to add > master & slave information in the structure. Querying > the capabilities from the sub device is a good feature. > I will look into your references and let you know if I > have any question. > > I will send an updated patch based on this. > > BTW, I have a question about the mt9t031.c driver. Could > I use this driver to stream VGA frames from the sensor? Sure, any size the chip supports (up to 2048x1536) and your host can handle. > Is it possible to configure the driver to stream at a > specific fps? No, patches welcome. > We have a version of the driver used internally > and it can do capture and stream of Bayer RGB data at VGA, > 480p, 576p and 720p resolutions. I have started integrating > your driver with my vpfe capture driver and it wasn't very > clear to me. Looks like driver calculates the timing parameters > based on the width and height of the capture area. Yes, it provides exposure control by setting shutter timing, and it emulates autoexposure by calculating shutter times from window geometry. > We need > streaming capability in the driver. This is how our driver works > with mt9t031 sensor > raw-bus (10 bit) > vpfe-capture - mt9t031 driver > || > V V > VPFEMT9T031 > > VPFE hardware has internal timing and DMA controller to > copy data frame by frame from the sensor output to SDRAM. > The PCLK form the sensor is used to generate the internal > timing. So, what is missing in the driver apart from the ability to specify a frame-rate? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] adding support for setting bus parameters in sub device
On Wednesday 10 June 2009 21:59:13 Guennadi Liakhovetski wrote: > On Wed, 10 Jun 2009, Hans Verkuil wrote: > > On Wednesday 10 June 2009 20:32:25 Guennadi Liakhovetski wrote: > > > On Tue, 9 Jun 2009, m-kariche...@ti.com wrote: > > > > From: Muralidharan Karicheri > > > > > > > > This patch adds support for setting bus parameters such as bus type > > > > (BT.656, BT.1120 etc), width (example 10 bit raw image data bus) > > > > and polarities (vsync, hsync, field etc) in sub device. This allows > > > > bridge driver to configure the sub device for a specific set of bus > > > > parameters through s_bus() function call. > > > > > > Yes, this is required, but this is not enough. Firstly, you're > > > missing at least one more flag - master or slave. Secondly, it is not > > > enough to provide a s_bus function. Many hosts and sensors can > > > configure one of several alternate configurations - they can select > > > signal polarities, data widths, master / slave role, etc. Whereas > > > others have some or all of these parameters fixed. That's why we have > > > a query method in soc-camera, which delivers all supported > > > configurations, and then the host can select some mutually acceptable > > > subset. No, just returning an error code is not enough. > > > > Why would you want to query this? I would expect this to be fixed > > settings: something that is determined by the architecture. Actually, I > > would expect this to be part of the platform_data in many cases and > > setup when the i2c driver is initialized and not touched afterwards. > > > > If we want to negotiate these bus settings, then we indeed need > > something more. But it seems unnecessarily complex to me to implement > > autonegotiation when this is in practice a fixed setting determined by > > how the sensor is hooked up to the host. > > On the platform level I have so far seen two options: signal connected > directly or via an inverter. For that you need platform data, yes. But > otherwise - why? say, if both your sensor and your host can select hsync > polarity in software - what should platform tell about it? This knowledge > belongs in the respective drivers - they know, that they can configure > arbitrary polarity and they advertise that. Another sensor, that is > statically active high - what does platform have to do with it? The > driver knows perfectly, that it can only do one polarity, and it > negotiates that. Earlier we also had this flags configured in platform > code, but then we realised that this wasn't correct. This information and > configuration methods are chip-specific, not platform-specific. > > And the negotiation code is not that complex - just copy respective > soc-camera functions, later the originals will be removed. My view of this would be that the board specification specifies the sensor (and possibly other chips) that are on the board. And to me it makes sense that that also supplies the bus settings. I agree that it is not complex code, but I think it is also unnecessary code. Why negotiate if you can just set it? BTW, looking at the code there doesn't seem to be a bus type (probably only Bayer is used), correct? How is the datawidth negotiation done? Is the largest available width chosen? I assume that the datawidth is generally fixed by the host? I don't quite see how that can be negotiated since what is chosen there is linked to how the video data is transferred to memory. E.g., chosing a bus width of 8 or 10 bits can be the difference between transferring 8 bit or 16 bit data (with 6 bits padding) when the image is transferred to memory. If I would implement negotiation I would probably limit it to those one-bit settings and not include bus type and width. Regards, Hans > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- 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: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
On Wednesday 10 June 2009 22:20:03 Guennadi Liakhovetski wrote: > On Wed, 10 Jun 2009, Hans Verkuil wrote: > > On Wednesday 10 June 2009 20:17:53 Guennadi Liakhovetski wrote: > > > On Tue, 9 Jun 2009, Hans Verkuil wrote: > > > > Hi Mauro, > > > > > > > > Please pull from > > > > http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2 for the > > > > following: > > > > > > > > - v4l2: add new s_config subdev ops and > > > > v4l2_i2c_new_subdev_cfg/board calls - v4l2-device: fix incorrect > > > > kernel check > > > > - v4l-compat: add I2C_ADDRS macro. > > > > - v4l2: update framework documentation. > > > > - v4l2-subdev: remove unnecessary check > > > > > > Do I understand this right, that these patches have not been posted > > > to the list? > > > > The idea is that you click on the link and look at the patches through > > the hg web frontend. > > And then? > > > > At least I haven't found them in online and in my local > > > archives. If it's really so, sorry, this doesn't seem very productive > > > to me... We have discussed this with Mauro on IRC, he didn't agree > > > with me, he thought it was acceptable in many cases... Sorry, cannot > > > agree. > > > > Both methods (a pull request or a patch series) are used and personally > > I have no preference, although currently I have a script that > > simplifies these pull requests so I generally use that. A patch series > > makes it easier to reply with review comments, while I think a pull > > request reduces mailinglist traffic and actually makes it easier to do > > the actual reviews. > > I think, patches posted to the list for reviews with a following pull > request (if no rework needed of course) is the most reviewer-friendly, > which is also what I so far have seen on all other lists I subscribe to > (arm, ppc, usb, scsi, lkml, i2c,...). Have you seen those huge mailing > threads from Greg K-H with all patches in his queue preceding his pull > requests (I hope I reproduce his work flow correctly here, any mistakes > are mine and unintended)? > > Are you really saying that it's equally convenient for you to review / > reply to patch on ML and to patch in some repository from a pull request? > Especially when there are multiple patches in that pull and you have to > click through them all, jumping back and forth between your mail client > and a browser?... > > If all are so much concerned about the ML traffic (which I don't > understand either, filtering and ignoring uninteresting mails is easy > enough these days), maybe we should split into devel and user? Sorry, I > really don't understand. I'll go ask members of other MLs what they think > about "clicking" through patches... Um, you are asking the wrong person. It's one of the two methods used on this list. Yes, pull requests are unusual compared to other lists (and so is the use of hg instead of git for that matter due to historical reasons). If Mauro says: use patch series, then I'll modify my workflow. If you prefer to see these subdev patches as a patch series, then I can do that for you. I have no preference myself. It's also a discussion I have no wish to go into. So if you see a pull request from me and prefer to have it as a patch series, just mail me and I'll do it. No problem. Regards, Hans > > Thanks > Guennadi > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- 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] adding support for setting bus parameters in sub device
Guennadi, Thanks for responding. I acknowledge I need to add master & slave information in the structure. Querying the capabilities from the sub device is a good feature. I will look into your references and let you know if I have any question. I will send an updated patch based on this. BTW, I have a question about the mt9t031.c driver. Could I use this driver to stream VGA frames from the sensor? Is it possible to configure the driver to stream at a specific fps? We have a version of the driver used internally and it can do capture and stream of Bayer RGB data at VGA, 480p, 576p and 720p resolutions. I have started integrating your driver with my vpfe capture driver and it wasn't very clear to me. Looks like driver calculates the timing parameters based on the width and height of the capture area. We need streaming capability in the driver. This is how our driver works with mt9t031 sensor raw-bus (10 bit) vpfe-capture - mt9t031 driver || V V VPFEMT9T031 VPFE hardware has internal timing and DMA controller to copy data frame by frame from the sensor output to SDRAM. The PCLK form the sensor is used to generate the internal timing. Thanks. Murali email: m-kariche...@ti.com >-Original Message- >From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] >Sent: Wednesday, June 10, 2009 2:32 PM >To: Karicheri, Muralidharan >Cc: linux-media@vger.kernel.org; davinci-linux-open- >sou...@linux.davincidsp.com; Muralidharan Karicheri >Subject: Re: [PATCH] adding support for setting bus parameters in sub >device > >On Tue, 9 Jun 2009, m-kariche...@ti.com wrote: > >> From: Muralidharan Karicheri >> >> This patch adds support for setting bus parameters such as bus type >> (BT.656, BT.1120 etc), width (example 10 bit raw image data bus) >> and polarities (vsync, hsync, field etc) in sub device. This allows >> bridge driver to configure the sub device for a specific set of bus >> parameters through s_bus() function call. > >Yes, this is required, but this is not enough. Firstly, you're missing at >least one more flag - master or slave. Secondly, it is not enough to >provide a s_bus function. Many hosts and sensors can configure one of >several alternate configurations - they can select signal polarities, data >widths, master / slave role, etc. Whereas others have some or all of these >parameters fixed. That's why we have a query method in soc-camera, which >delivers all supported configurations, and then the host can select some >mutually acceptable subset. No, just returning an error code is not >enough. > >So, you would need to request supported flags, the sensor would return a >bitmask, and the host would then issue a s_bus call with a selected subset >and configure itself. And then you realise, that one bit per parameter is >not enough - you need a bit for both, e.g., vsync active low and active >high. > >Have a look at >include/media/soc_camera.h::soc_camera_bus_param_compatible() and macros >defined there, then you might want to have a look at >drivers/media/video/pxa_camera.c::pxa_camera_set_bus_param() to see how >the whole machinery works. And you also want inverter flags, see >drivers/media/video/soc_camera.c::soc_camera_apply_sensor_flags(). > >So, this is a step in the right direction, but it doesn't seem final yet. > >Thanks >Guennadi > >> >> Reviewed By "Hans Verkuil". >> Signed-off-by: Muralidharan Karicheri >> --- >> Applies to v4l-dvb repository >> >> include/media/v4l2-subdev.h | 36 >> 1 files changed, 36 insertions(+), 0 deletions(-) >> >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h >> index 1785608..c1cfb3b 100644 >> --- a/include/media/v4l2-subdev.h >> +++ b/include/media/v4l2-subdev.h >> @@ -37,6 +37,41 @@ struct v4l2_decode_vbi_line { >> u32 type; /* VBI service type (V4L2_SLICED_*). 0 if no >service found */ >> }; >> >> +/* >> + * Some sub-devices are connected to the bridge device through a bus >that >> + * carries * the clock, vsync, hsync and data. Some interfaces such as >BT.656 >> + * carries the sync embedded in the data where as others have separate >line >> + * carrying the sync signals. The structure below is used by bridge >driver to >> + * set the desired bus parameters in the sub device to work with it. >> + */ >> +enum v4l2_subdev_bus_type { >> +/* BT.656 interface. Embedded sync */ >> +V4L2_SUBDEV_BUS_BT_656, >> +/* BT.1120 interface. Embedded sync */ >> +V4L2_SUBDEV_BUS_BT_1120, >> +/* 8 bit muxed YCbCr bus, separate sync and field signals */ >> +V4L2_SUBDEV_BUS_YCBCR_8, >> +/* 16 bit YCbCr bus, separate sync and field signals */ >> +V4L2_SUBDEV_BUS_YCBCR_16, >> +/* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */ >> +V4L2_SUBDEV_BUS_RAW_BAYER >> +}; >> + >
Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
On Wed, 10 Jun 2009, Hans Verkuil wrote: > On Wednesday 10 June 2009 20:17:53 Guennadi Liakhovetski wrote: > > On Tue, 9 Jun 2009, Hans Verkuil wrote: > > > Hi Mauro, > > > > > > Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2 > > > for the following: > > > > > > - v4l2: add new s_config subdev ops and v4l2_i2c_new_subdev_cfg/board > > > calls - v4l2-device: fix incorrect kernel check > > > - v4l-compat: add I2C_ADDRS macro. > > > - v4l2: update framework documentation. > > > - v4l2-subdev: remove unnecessary check > > > > Do I understand this right, that these patches have not been posted to > > the list? > > The idea is that you click on the link and look at the patches through the > hg web frontend. And then? > > At least I haven't found them in online and in my local > > archives. If it's really so, sorry, this doesn't seem very productive to > > me... We have discussed this with Mauro on IRC, he didn't agree with me, > > he thought it was acceptable in many cases... Sorry, cannot agree. > > Both methods (a pull request or a patch series) are used and personally I > have no preference, although currently I have a script that simplifies > these pull requests so I generally use that. A patch series makes it easier > to reply with review comments, while I think a pull request reduces > mailinglist traffic and actually makes it easier to do the actual reviews. I think, patches posted to the list for reviews with a following pull request (if no rework needed of course) is the most reviewer-friendly, which is also what I so far have seen on all other lists I subscribe to (arm, ppc, usb, scsi, lkml, i2c,...). Have you seen those huge mailing threads from Greg K-H with all patches in his queue preceding his pull requests (I hope I reproduce his work flow correctly here, any mistakes are mine and unintended)? Are you really saying that it's equally convenient for you to review / reply to patch on ML and to patch in some repository from a pull request? Especially when there are multiple patches in that pull and you have to click through them all, jumping back and forth between your mail client and a browser?... If all are so much concerned about the ML traffic (which I don't understand either, filtering and ignoring uninteresting mails is easy enough these days), maybe we should split into devel and user? Sorry, I really don't understand. I'll go ask members of other MLs what they think about "clicking" through patches... Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: soc-camera: status, roadmap
Hi Guennadi, Let me start with a big Thank You! for all your hard work on this! Much appreciated! On Wednesday 10 June 2009 18:45:36 Guennadi Liakhovetski wrote: > Hi all > > for those interested here's a (not so) short status report and a proposed > roadmap for general soc-camera development, and, of course, its ongoing > conversion to v4l2-subdev API. > > 1. v4l2-subdev conversion. I have posted several versions of the > conversion patch series to the list, of which the last takes an IMHO > correct approach of a graduate conversion, avoiding mega-patches, > modifying multiple platforms and drivers at once. With this approach the > roadmap consists of the following steps: > > 1.1. preparatory patch to soc-camera core, allowing parallel existence of > "legacy" (all in the mainline) platforms and converted platforms (pcm037 > i.MX31 platform so far) by introducing some backwards compatibility code. > This patch is currently in v4l next and in linux-next, i.e., it is going > in with 2.6.31. > > 1.2. convert all (around 7) mainline platforms to the new layout. This > step is necessary for further conversions, but it depends on 1.1. > Therefore this can only be done later in 2.6.31 merge window, when 1.1. > is in the mainline. > > 1.3. convert soc-camera core and drivers to an intermediate state, with > which all cameras are registered by platforms as platform devices, later > soc-camera core probes them and dynamically registers respective i2c (or > other, as in soc_camera_platform.c case) devices. This patch depends on > 1.2., and it is hard to expect to be able to push these three steps > within the 2.6.31 merge window. It could be possible, we could request to > accept this patch after -rc1, maybe we would be allowed to do this, but > > 1.4. this is the actual conversion to v4l2-subdev. It depends on some > bits and pieces in the v4l2-subdev framework, which are still in progress > (e.g. v4l2_i2c_new_subdev_board), I believe (Hans, am I right? or what's > the outcome of Mauro's last reply to you in the "[PULL] > http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev"; thread?), so, it > becomes practically impossible to also pull it for 2.6.31. I haven't seen a reaction yet from Mauro regarding my latest pull request: I think it addresses all his concerns regarding existing functionality so I actually hope this can be merged. It would help a lot with this and similar efforts. > Now, I do not want to have soc-camera in the intermediate 1.3. state for > a whole 2.6.31 kernel, which means, we have to postpone 1.3. and 1.4. > until 2.6.32. > > 2. The above means, I'll have to maintain and update my patches for a > whole 2.6.31 development cycle. In this time I'll try to update and > upload them as a quilt patch series and announce it on the list a couple > of times. > > 3. This also means, development will become more difficult, new features > and drivers will only be accepted on the top of my patch stack, bugfixes > will have to be accpeted against the mainline, which then will mean extra > porting work for me. If there is anything I can do to help this along, please let me know. In particular: what else besides the v4l2_i2c_new_subdev_board do you need? I didn't have much time in the past few weeks, but things are more relaxed now and I expect to be able to do a lot more in the coming weeks (fingers crossed :-) ). Regards, Hans > 4. In a message I posted a few minutes ago > > http://www.mail-archive.com/linux-media@vger.kernel.org/msg06294.html > > I'm asking about a correct interpretation of S_CROP and S_FMT operations. > I suspect, what soc-camera framework and all drivers thereunder are doing > is wrong, and have to be fixed rather sooner than later. However, I'd be > very much against fixing this in the present stack, because that would > mean a _lot_ of porting. So, this will remain standard-non-compliant > until 2.6.32 too. > > 5. The conversion described in (1) is only partial, in its current form > it does not replace the existing soc-camera API between sensor drivers > and the soc-camera core with v4l2-subdev operations completely. Partly > because many of the current soc-camera methods are still missing in > v4l2-subdev, partly because it just makes more sense to first push the > principle conversion in the mainline, which at least removes soc-camera > device registration and switches to i2c driver autoloading and replaces > some trivially replaceable methods, like [gs]_fmt, [gs]_register, > [gs]_control. Some of the missing methods, like [gs]_crop should be easy > to add, others, like pixel format and bus parameter negotiations would > require some thinking and substantial work. Which makes this all some > 2.6.33 > material... but who wants to think that far... > > 6. As you see, this all looks like a lot of work, and I so far have been > doing all of this in my free time. But it would become difficult with > these amounts of work. So, I would welcome if either someone would st
Re: [PATCH] adding support for setting bus parameters in sub device
On Wed, 10 Jun 2009, Hans Verkuil wrote: > On Wednesday 10 June 2009 20:32:25 Guennadi Liakhovetski wrote: > > On Tue, 9 Jun 2009, m-kariche...@ti.com wrote: > > > From: Muralidharan Karicheri > > > > > > This patch adds support for setting bus parameters such as bus type > > > (BT.656, BT.1120 etc), width (example 10 bit raw image data bus) > > > and polarities (vsync, hsync, field etc) in sub device. This allows > > > bridge driver to configure the sub device for a specific set of bus > > > parameters through s_bus() function call. > > > > Yes, this is required, but this is not enough. Firstly, you're missing at > > least one more flag - master or slave. Secondly, it is not enough to > > provide a s_bus function. Many hosts and sensors can configure one of > > several alternate configurations - they can select signal polarities, > > data widths, master / slave role, etc. Whereas others have some or all of > > these parameters fixed. That's why we have a query method in soc-camera, > > which delivers all supported configurations, and then the host can select > > some mutually acceptable subset. No, just returning an error code is not > > enough. > > Why would you want to query this? I would expect this to be fixed settings: > something that is determined by the architecture. Actually, I would expect > this to be part of the platform_data in many cases and setup when the i2c > driver is initialized and not touched afterwards. > > If we want to negotiate these bus settings, then we indeed need something > more. But it seems unnecessarily complex to me to implement autonegotiation > when this is in practice a fixed setting determined by how the sensor is > hooked up to the host. On the platform level I have so far seen two options: signal connected directly or via an inverter. For that you need platform data, yes. But otherwise - why? say, if both your sensor and your host can select hsync polarity in software - what should platform tell about it? This knowledge belongs in the respective drivers - they know, that they can configure arbitrary polarity and they advertise that. Another sensor, that is statically active high - what does platform have to do with it? The driver knows perfectly, that it can only do one polarity, and it negotiates that. Earlier we also had this flags configured in platform code, but then we realised that this wasn't correct. This information and configuration methods are chip-specific, not platform-specific. And the negotiation code is not that complex - just copy respective soc-camera functions, later the originals will be removed. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
On Wednesday 10 June 2009 20:17:53 Guennadi Liakhovetski wrote: > On Tue, 9 Jun 2009, Hans Verkuil wrote: > > Hi Mauro, > > > > Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2 > > for the following: > > > > - v4l2: add new s_config subdev ops and v4l2_i2c_new_subdev_cfg/board > > calls - v4l2-device: fix incorrect kernel check > > - v4l-compat: add I2C_ADDRS macro. > > - v4l2: update framework documentation. > > - v4l2-subdev: remove unnecessary check > > Do I understand this right, that these patches have not been posted to > the list? The idea is that you click on the link and look at the patches through the hg web frontend. > At least I haven't found them in online and in my local > archives. If it's really so, sorry, this doesn't seem very productive to > me... We have discussed this with Mauro on IRC, he didn't agree with me, > he thought it was acceptable in many cases... Sorry, cannot agree. Both methods (a pull request or a patch series) are used and personally I have no preference, although currently I have a script that simplifies these pull requests so I generally use that. A patch series makes it easier to reply with review comments, while I think a pull request reduces mailinglist traffic and actually makes it easier to do the actual reviews. Regards, Hans > > Thanks > Guennadi > > > This time I've only added new functions and left the existing ones in > > place. I did add a bit of code to the existing > > v4l2_i2c_new_(probed_)subdev functions to call the new s_config op if > > it is available. Existing subdev drivers never set this new op, so this > > code will not effect current behavior. But for new drivers that do set > > s_config it is important that it is called no matter what flavor of > > these functions is used. > > > > At the end of the 2.6.31 cycle we can replace the current > > v4l2_i2c_new_(probed_)subdev calls with the new one I had in my earlier > > patches. > > > > Thanks, > > > > Hans > > > > diffstat: > > linux/Documentation/video4linux/v4l2-framework.txt | 24 +++ > > linux/drivers/media/video/v4l2-common.c| 166 > > + > > linux/drivers/media/video/v4l2-device.c|2 > > linux/include/media/v4l2-common.h | 18 ++ > > linux/include/media/v4l2-subdev.h |9 - > > v4l/compat.h |6 > > 6 files changed, 222 insertions(+), 3 deletions(-) > > > > -- > > Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom > > -- > > 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 > > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- 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] adding support for setting bus parameters in sub device
On Wednesday 10 June 2009 20:32:25 Guennadi Liakhovetski wrote: > On Tue, 9 Jun 2009, m-kariche...@ti.com wrote: > > From: Muralidharan Karicheri > > > > This patch adds support for setting bus parameters such as bus type > > (BT.656, BT.1120 etc), width (example 10 bit raw image data bus) > > and polarities (vsync, hsync, field etc) in sub device. This allows > > bridge driver to configure the sub device for a specific set of bus > > parameters through s_bus() function call. > > Yes, this is required, but this is not enough. Firstly, you're missing at > least one more flag - master or slave. Secondly, it is not enough to > provide a s_bus function. Many hosts and sensors can configure one of > several alternate configurations - they can select signal polarities, > data widths, master / slave role, etc. Whereas others have some or all of > these parameters fixed. That's why we have a query method in soc-camera, > which delivers all supported configurations, and then the host can select > some mutually acceptable subset. No, just returning an error code is not > enough. Why would you want to query this? I would expect this to be fixed settings: something that is determined by the architecture. Actually, I would expect this to be part of the platform_data in many cases and setup when the i2c driver is initialized and not touched afterwards. If we want to negotiate these bus settings, then we indeed need something more. But it seems unnecessarily complex to me to implement autonegotiation when this is in practice a fixed setting determined by how the sensor is hooked up to the host. I may be missing something, of course. Regards, Hans > So, you would need to request supported flags, the sensor would return a > bitmask, and the host would then issue a s_bus call with a selected > subset and configure itself. And then you realise, that one bit per > parameter is not enough - you need a bit for both, e.g., vsync active low > and active high. > > Have a look at > include/media/soc_camera.h::soc_camera_bus_param_compatible() and macros > defined there, then you might want to have a look at > drivers/media/video/pxa_camera.c::pxa_camera_set_bus_param() to see how > the whole machinery works. And you also want inverter flags, see > drivers/media/video/soc_camera.c::soc_camera_apply_sensor_flags(). > > So, this is a step in the right direction, but it doesn't seem final yet. > > Thanks > Guennadi > > > Reviewed By "Hans Verkuil". > > Signed-off-by: Muralidharan Karicheri > > --- > > Applies to v4l-dvb repository > > > > include/media/v4l2-subdev.h | 36 > > 1 files changed, 36 insertions(+), > > 0 deletions(-) > > > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > > index 1785608..c1cfb3b 100644 > > --- a/include/media/v4l2-subdev.h > > +++ b/include/media/v4l2-subdev.h > > @@ -37,6 +37,41 @@ struct v4l2_decode_vbi_line { > > u32 type; /* VBI service type (V4L2_SLICED_*). 0 if no > > service found > > */ }; > > > > +/* > > + * Some sub-devices are connected to the bridge device through a bus > > that + * carries * the clock, vsync, hsync and data. Some interfaces > > such as BT.656 + * carries the sync embedded in the data where as > > others have separate line + * carrying the sync signals. The structure > > below is used by bridge driver to + * set the desired bus parameters in > > the sub device to work with it. + */ > > +enum v4l2_subdev_bus_type { > > + /* BT.656 interface. Embedded sync */ > > + V4L2_SUBDEV_BUS_BT_656, > > + /* BT.1120 interface. Embedded sync */ > > + V4L2_SUBDEV_BUS_BT_1120, > > + /* 8 bit muxed YCbCr bus, separate sync and field signals */ > > + V4L2_SUBDEV_BUS_YCBCR_8, > > + /* 16 bit YCbCr bus, separate sync and field signals */ > > + V4L2_SUBDEV_BUS_YCBCR_16, > > + /* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */ > > + V4L2_SUBDEV_BUS_RAW_BAYER > > +}; > > + > > +struct v4l2_subdev_bus { > > + enum v4l2_subdev_bus_type type; > > + u8 width; > > + /* 0 - active low, 1 - active high */ > > + unsigned pol_vsync:1; > > + /* 0 - active low, 1 - active high */ > > + unsigned pol_hsync:1; > > + /* 0 - low to high , 1 - high to low */ > > + unsigned pol_field:1; > > + /* 0 - sample at falling edge , 1 - sample at rising edge */ > > + unsigned pol_pclock:1; > > + /* 0 - active low , 1 - active high */ > > + unsigned pol_data:1; > > +}; > > + > > /* Sub-devices are devices that are connected somehow to the main > > bridge device. These devices are usually audio/video > > muxers/encoders/decoders or sensors and webcam controllers. > > @@ -109,6 +144,7 @@ struct v4l2_subdev_core_ops { > > int (*querymenu)(struct v4l2_subdev *sd, struct v4l2_querymenu *qm); > > int (*s_std)(struct v4l2_subdev *sd, v4l2_std_id norm); > > long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg); > > + int (*s_bus)(struct v4l2_subd
[patch 2/6] dvb-core: fix potential mutex_unlock without mutex_lock in dvb_dvr_read
From: Simon Arlott dvb_dvr_read may unlock the dmxdev mutex and return -ENODEV, except this function is a file op and will never be called with the mutex held. There's existing mutex_lock and mutex_unlock around the actual read but it's commented out. These should probably be uncommented but the read blocks and this could block another non-blocking reader on the mutex instead. This change comments out the extra mutex_unlock. [a...@linux-foundation.org: cleanups, simplification] Signed-off-by: Simon Arlott Cc: Mauro Carvalho Chehab Signed-off-by: Andrew Morton --- drivers/media/dvb/dvb-core/dmxdev.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff -puN drivers/media/dvb/dvb-core/dmxdev.c~dvb-core-fix-potential-mutex_unlock-without-mutex_lock-in-dvb_dvr_read drivers/media/dvb/dvb-core/dmxdev.c --- a/drivers/media/dvb/dvb-core/dmxdev.c~dvb-core-fix-potential-mutex_unlock-without-mutex_lock-in-dvb_dvr_read +++ a/drivers/media/dvb/dvb-core/dmxdev.c @@ -244,19 +244,13 @@ static ssize_t dvb_dvr_read(struct file { struct dvb_device *dvbdev = file->private_data; struct dmxdev *dmxdev = dvbdev->priv; - int ret; - if (dmxdev->exit) { - mutex_unlock(&dmxdev->mutex); + if (dmxdev->exit) return -ENODEV; - } - //mutex_lock(&dmxdev->mutex); - ret = dvb_dmxdev_buffer_read(&dmxdev->dvr_buffer, -file->f_flags & O_NONBLOCK, -buf, count, ppos); - //mutex_unlock(&dmxdev->mutex); - return ret; + return dvb_dmxdev_buffer_read(&dmxdev->dvr_buffer, + file->f_flags & O_NONBLOCK, + buf, count, ppos); } static int dvb_dvr_set_buffer_size(struct dmxdev *dmxdev, _ -- 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 6/6] tvp514x: try_count off by one
From: Roel Kluin With `while (try_count-- > 0)' try_count reaches -1 after the loop. Signed-off-by: Roel Kluin Cc: Mauro Carvalho Chehab Signed-off-by: Andrew Morton --- drivers/media/video/tvp514x.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff -puN drivers/media/video/tvp514x.c~tvp514x-try_count-off-by-one drivers/media/video/tvp514x.c --- a/drivers/media/video/tvp514x.c~tvp514x-try_count-off-by-one +++ a/drivers/media/video/tvp514x.c @@ -692,7 +692,7 @@ static int ioctl_s_routing(struct v4l2_i break; /* Input detected */ } - if ((current_std == STD_INVALID) || (try_count <= 0)) + if ((current_std == STD_INVALID) || (try_count < 0)) return -EINVAL; decoder->current_std = current_std; _ -- 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 4/6] V4L/pwc: use usb_interface as parent, not usb_device
From: Lennart Poettering The current code creates a sysfs device path where the video4linux device is child of the usb device itself instead of the interface it belongs to. That is evil and confuses udev. This patch does basically the same thing as Kay's similar patch for the ov511 driver: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=ce96d0a44a4f8d1bb3dc12b5e98cb688c1bc730d Cc: Kay Sievers Cc: Mauro Carvalho Chehab Signed-off-by: Lennart Poettering Signed-off-by: Andrew Morton --- drivers/media/video/pwc/pwc-if.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff -puN drivers/media/video/pwc/pwc-if.c~v4l-pwc-use-usb_interface-as-parent-not-usb_device drivers/media/video/pwc/pwc-if.c --- a/drivers/media/video/pwc/pwc-if.c~v4l-pwc-use-usb_interface-as-parent-not-usb_device +++ a/drivers/media/video/pwc/pwc-if.c @@ -1783,7 +1783,7 @@ static int usb_pwc_probe(struct usb_inte return -ENOMEM; } memcpy(pdev->vdev, &pwc_template, sizeof(pwc_template)); - pdev->vdev->parent = &(udev->dev); + pdev->vdev->parent = &intf->dev; strcpy(pdev->vdev->name, name); video_set_drvdata(pdev->vdev, pdev); _ -- 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 5/6] v4l: generate KEY_CAMERA instead of BTN_0 key events on input devices
From: Lennart Poettering A bunch of V4L drivers generate BTN_0 instead of KEY_CAMERA key presses. X11 is able to handle KEY_CAMERA automatically these days while BTN_0 is not treated at all. Thus it would be of big benefit if the camera drivers would consistently generate KEY_CAMERA. Some drivers (uvc) already do, this patch updates the remaining drivers to do the same. I only possess a limited set of webcams, so this isn't tested with all cameras. The patch is rather trivial and compile tested, so I'd say it's still good enough to get merged. Cc: Mauro Carvalho Chehab Signed-off-by: Lennart Poettering Signed-off-by: Andrew Morton --- drivers/media/video/pwc/pwc-if.c |4 ++-- drivers/media/video/usbvideo/konicawc.c |4 ++-- drivers/media/video/usbvideo/quickcam_messenger.c |4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff -puN drivers/media/video/pwc/pwc-if.c~v4l-generate-key_camera-instead-of-btn_0-key-events-on-input-devices drivers/media/video/pwc/pwc-if.c --- a/drivers/media/video/pwc/pwc-if.c~v4l-generate-key_camera-instead-of-btn_0-key-events-on-input-devices +++ a/drivers/media/video/pwc/pwc-if.c @@ -601,7 +601,7 @@ static void pwc_snapshot_button(struct p #ifdef CONFIG_USB_PWC_INPUT_EVDEV if (pdev->button_dev) { - input_report_key(pdev->button_dev, BTN_0, down); + input_report_key(pdev->button_dev, KEY_CAMERA, down); input_sync(pdev->button_dev); } #endif @@ -1847,7 +1847,7 @@ static int usb_pwc_probe(struct usb_inte usb_to_input_id(pdev->udev, &pdev->button_dev->id); pdev->button_dev->dev.parent = &pdev->udev->dev; pdev->button_dev->evbit[0] = BIT_MASK(EV_KEY); - pdev->button_dev->keybit[BIT_WORD(BTN_0)] = BIT_MASK(BTN_0); + pdev->button_dev->keybit[BIT_WORD(KEY_CAMERA)] = BIT_MASK(KEY_CAMERA); rc = input_register_device(pdev->button_dev); if (rc) { diff -puN drivers/media/video/usbvideo/konicawc.c~v4l-generate-key_camera-instead-of-btn_0-key-events-on-input-devices drivers/media/video/usbvideo/konicawc.c --- a/drivers/media/video/usbvideo/konicawc.c~v4l-generate-key_camera-instead-of-btn_0-key-events-on-input-devices +++ a/drivers/media/video/usbvideo/konicawc.c @@ -240,7 +240,7 @@ static void konicawc_register_input(stru input_dev->dev.parent = &dev->dev; input_dev->evbit[0] = BIT_MASK(EV_KEY); - input_dev->keybit[BIT_WORD(BTN_0)] = BIT_MASK(BTN_0); + input_dev->keybit[BIT_WORD(KEY_CAMERA)] = BIT_MASK(KEY_CAMERA); error = input_register_device(cam->input); if (error) { @@ -263,7 +263,7 @@ static void konicawc_unregister_input(st static void konicawc_report_buttonstat(struct konicawc *cam) { if (cam->input) { - input_report_key(cam->input, BTN_0, cam->buttonsts); + input_report_key(cam->input, KEY_CAMERA, cam->buttonsts); input_sync(cam->input); } } diff -puN drivers/media/video/usbvideo/quickcam_messenger.c~v4l-generate-key_camera-instead-of-btn_0-key-events-on-input-devices drivers/media/video/usbvideo/quickcam_messenger.c --- a/drivers/media/video/usbvideo/quickcam_messenger.c~v4l-generate-key_camera-instead-of-btn_0-key-events-on-input-devices +++ a/drivers/media/video/usbvideo/quickcam_messenger.c @@ -103,7 +103,7 @@ static void qcm_register_input(struct qc input_dev->dev.parent = &dev->dev; input_dev->evbit[0] = BIT_MASK(EV_KEY); - input_dev->keybit[BIT_WORD(BTN_0)] = BIT_MASK(BTN_0); + input_dev->keybit[BIT_WORD(KEY_CAMERA)] = BIT_MASK(KEY_CAMERA); error = input_register_device(cam->input); if (error) { @@ -126,7 +126,7 @@ static void qcm_unregister_input(struct static void qcm_report_buttonstat(struct qcm *cam) { if (cam->input) { - input_report_key(cam->input, BTN_0, cam->button_sts); + input_report_key(cam->input, KEY_CAMERA, cam->button_sts); input_sync(cam->input); } } _ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/6] radio-mr800.c: missing mutex include
From: Alessio Igor Bogani radio-mr800.c uses struct mutex, so while seems to be pulled in indirectly by one of the headers it already includes, the right thing is to include it directly. Signed-off-by: Alessio Igor Bogani Cc: Mauro Carvalho Chehab Signed-off-by: Andrew Morton --- drivers/media/radio/radio-mr800.c |1 + 1 file changed, 1 insertion(+) diff -puN drivers/media/radio/radio-mr800.c~radio-mr800c-missing-mutex-include drivers/media/radio/radio-mr800.c --- a/drivers/media/radio/radio-mr800.c~radio-mr800c-missing-mutex-include +++ a/drivers/media/radio/radio-mr800.c @@ -64,6 +64,7 @@ #include #include #include /* for KERNEL_VERSION MACRO */ +#include /* driver and module definitions */ #define DRIVER_AUTHOR "Alexey Klimov " _ -- 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 3/6] vino: replace dma_sync_single with dma_sync_single_for_cpu
From: FUJITA Tomonori This replaces dma_sync_single() with dma_sync_single_for_cpu() because dma_sync_single() is an obsolete API; include/linux/dma-mapping.h says: /* Backwards compat, remove in 2.7.x */ #define dma_sync_single dma_sync_single_for_cpu #define dma_sync_sg dma_sync_sg_for_cpu Signed-off-by: FUJITA Tomonori Cc: Mauro Carvalho Chehab Signed-off-by: Andrew Morton --- drivers/media/video/vino.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff -puN drivers/media/video/vino.c~vino-replace-dma_sync_single-with-dma_sync_single_for_cpu drivers/media/video/vino.c --- a/drivers/media/video/vino.c~vino-replace-dma_sync_single-with-dma_sync_single_for_cpu +++ a/drivers/media/video/vino.c @@ -868,9 +868,9 @@ static void vino_sync_buffer(struct vino dprintk("vino_sync_buffer():\n"); for (i = 0; i < fb->desc_table.page_count; i++) - dma_sync_single(NULL, - fb->desc_table.dma_cpu[VINO_PAGE_RATIO * i], - PAGE_SIZE, DMA_FROM_DEVICE); + dma_sync_single_for_cpu(NULL, + fb->desc_table.dma_cpu[VINO_PAGE_RATIO * i], + PAGE_SIZE, DMA_FROM_DEVICE); } /* Framebuffer fifo functions (need to be locked externally) */ _ -- 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] adding support for setting bus parameters in sub device
On Tue, 9 Jun 2009, m-kariche...@ti.com wrote: > From: Muralidharan Karicheri > > This patch adds support for setting bus parameters such as bus type > (BT.656, BT.1120 etc), width (example 10 bit raw image data bus) > and polarities (vsync, hsync, field etc) in sub device. This allows > bridge driver to configure the sub device for a specific set of bus > parameters through s_bus() function call. Yes, this is required, but this is not enough. Firstly, you're missing at least one more flag - master or slave. Secondly, it is not enough to provide a s_bus function. Many hosts and sensors can configure one of several alternate configurations - they can select signal polarities, data widths, master / slave role, etc. Whereas others have some or all of these parameters fixed. That's why we have a query method in soc-camera, which delivers all supported configurations, and then the host can select some mutually acceptable subset. No, just returning an error code is not enough. So, you would need to request supported flags, the sensor would return a bitmask, and the host would then issue a s_bus call with a selected subset and configure itself. And then you realise, that one bit per parameter is not enough - you need a bit for both, e.g., vsync active low and active high. Have a look at include/media/soc_camera.h::soc_camera_bus_param_compatible() and macros defined there, then you might want to have a look at drivers/media/video/pxa_camera.c::pxa_camera_set_bus_param() to see how the whole machinery works. And you also want inverter flags, see drivers/media/video/soc_camera.c::soc_camera_apply_sensor_flags(). So, this is a step in the right direction, but it doesn't seem final yet. Thanks Guennadi > > Reviewed By "Hans Verkuil". > Signed-off-by: Muralidharan Karicheri > --- > Applies to v4l-dvb repository > > include/media/v4l2-subdev.h | 36 > 1 files changed, 36 insertions(+), 0 deletions(-) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 1785608..c1cfb3b 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -37,6 +37,41 @@ struct v4l2_decode_vbi_line { > u32 type; /* VBI service type (V4L2_SLICED_*). 0 if no > service found */ > }; > > +/* > + * Some sub-devices are connected to the bridge device through a bus that > + * carries * the clock, vsync, hsync and data. Some interfaces such as BT.656 > + * carries the sync embedded in the data where as others have separate line > + * carrying the sync signals. The structure below is used by bridge driver to > + * set the desired bus parameters in the sub device to work with it. > + */ > +enum v4l2_subdev_bus_type { > + /* BT.656 interface. Embedded sync */ > + V4L2_SUBDEV_BUS_BT_656, > + /* BT.1120 interface. Embedded sync */ > + V4L2_SUBDEV_BUS_BT_1120, > + /* 8 bit muxed YCbCr bus, separate sync and field signals */ > + V4L2_SUBDEV_BUS_YCBCR_8, > + /* 16 bit YCbCr bus, separate sync and field signals */ > + V4L2_SUBDEV_BUS_YCBCR_16, > + /* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */ > + V4L2_SUBDEV_BUS_RAW_BAYER > +}; > + > +struct v4l2_subdev_bus { > + enum v4l2_subdev_bus_type type; > + u8 width; > + /* 0 - active low, 1 - active high */ > + unsigned pol_vsync:1; > + /* 0 - active low, 1 - active high */ > + unsigned pol_hsync:1; > + /* 0 - low to high , 1 - high to low */ > + unsigned pol_field:1; > + /* 0 - sample at falling edge , 1 - sample at rising edge */ > + unsigned pol_pclock:1; > + /* 0 - active low , 1 - active high */ > + unsigned pol_data:1; > +}; > + > /* Sub-devices are devices that are connected somehow to the main bridge > device. These devices are usually audio/video muxers/encoders/decoders or > sensors and webcam controllers. > @@ -109,6 +144,7 @@ struct v4l2_subdev_core_ops { > int (*querymenu)(struct v4l2_subdev *sd, struct v4l2_querymenu *qm); > int (*s_std)(struct v4l2_subdev *sd, v4l2_std_id norm); > long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg); > + int (*s_bus)(struct v4l2_subdev *sd, struct v4l2_subdev_bus *bus); > #ifdef CONFIG_VIDEO_ADV_DEBUG > int (*g_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register > *reg); > int (*s_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register > *reg); > -- > 1.6.0.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo
Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2
On Tue, 9 Jun 2009, Hans Verkuil wrote: > Hi Mauro, > > Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev2 for the > following: > > - v4l2: add new s_config subdev ops and v4l2_i2c_new_subdev_cfg/board calls > - v4l2-device: fix incorrect kernel check > - v4l-compat: add I2C_ADDRS macro. > - v4l2: update framework documentation. > - v4l2-subdev: remove unnecessary check Do I understand this right, that these patches have not been posted to the list? At least I haven't found them in online and in my local archives. If it's really so, sorry, this doesn't seem very productive to me... We have discussed this with Mauro on IRC, he didn't agree with me, he thought it was acceptable in many cases... Sorry, cannot agree. Thanks Guennadi > > This time I've only added new functions and left the existing ones in place. > I did add a bit of code to the existing v4l2_i2c_new_(probed_)subdev > functions to call the new s_config op if it is available. Existing subdev > drivers never set this new op, so this code will not effect current > behavior. But for new drivers that do set s_config it is important that it > is called no matter what flavor of these functions is used. > > At the end of the 2.6.31 cycle we can replace the current > v4l2_i2c_new_(probed_)subdev calls with the new one I had in my earlier > patches. > > Thanks, > > Hans > > diffstat: > linux/Documentation/video4linux/v4l2-framework.txt | 24 +++ > linux/drivers/media/video/v4l2-common.c| 166 > + > linux/drivers/media/video/v4l2-device.c|2 > linux/include/media/v4l2-common.h | 18 ++ > linux/include/media/v4l2-subdev.h |9 - > v4l/compat.h |6 > 6 files changed, 222 insertions(+), 3 deletions(-) > > -- > Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom > -- > 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 > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[cron job] v4l-dvb daily build 2.6.22 and up: ERRORS, 2.6.16-2.6.21: ERRORS
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:Wed Jun 10 19:00:03 CEST 2009 path:http://www.linuxtv.org/hg/v4l-dvb changeset: 11930:ed3781a79c73 gcc version: gcc (GCC) 4.3.1 hardware:x86_64 host os: 2.6.26 linux-2.6.22.19-armv5: OK linux-2.6.23.12-armv5: OK linux-2.6.24.7-armv5: OK linux-2.6.25.11-armv5: OK linux-2.6.26-armv5: OK linux-2.6.27-armv5: WARNINGS linux-2.6.28-armv5: WARNINGS linux-2.6.29.1-armv5: OK linux-2.6.30-rc7-armv5: OK linux-2.6.27-armv5-ixp: WARNINGS linux-2.6.28-armv5-ixp: WARNINGS linux-2.6.29.1-armv5-ixp: WARNINGS linux-2.6.30-rc7-armv5-ixp: WARNINGS linux-2.6.28-armv5-omap2: WARNINGS linux-2.6.29.1-armv5-omap2: WARNINGS linux-2.6.30-rc7-armv5-omap2: WARNINGS linux-2.6.22.19-i686: OK linux-2.6.23.12-i686: WARNINGS linux-2.6.24.7-i686: WARNINGS linux-2.6.25.11-i686: WARNINGS linux-2.6.26-i686: WARNINGS linux-2.6.27-i686: WARNINGS linux-2.6.28-i686: WARNINGS linux-2.6.29.1-i686: WARNINGS linux-2.6.30-rc7-i686: WARNINGS linux-2.6.23.12-m32r: OK linux-2.6.24.7-m32r: OK linux-2.6.25.11-m32r: OK linux-2.6.26-m32r: OK linux-2.6.27-m32r: OK linux-2.6.28-m32r: OK linux-2.6.29.1-m32r: OK linux-2.6.30-rc7-m32r: OK linux-2.6.22.19-mips: ERRORS linux-2.6.26-mips: ERRORS linux-2.6.27-mips: ERRORS linux-2.6.28-mips: ERRORS linux-2.6.29.1-mips: ERRORS linux-2.6.30-rc7-mips: ERRORS linux-2.6.27-powerpc64: WARNINGS linux-2.6.28-powerpc64: WARNINGS linux-2.6.29.1-powerpc64: WARNINGS linux-2.6.30-rc7-powerpc64: WARNINGS linux-2.6.22.19-x86_64: OK linux-2.6.23.12-x86_64: OK linux-2.6.24.7-x86_64: OK linux-2.6.25.11-x86_64: OK linux-2.6.26-x86_64: OK linux-2.6.27-x86_64: OK linux-2.6.28-x86_64: OK linux-2.6.29.1-x86_64: OK linux-2.6.30-rc7-x86_64: WARNINGS sparse (linux-2.6.29.1): OK sparse (linux-2.6.30-rc7): OK linux-2.6.16.61-i686: ERRORS linux-2.6.17.14-i686: ERRORS linux-2.6.18.8-i686: ERRORS linux-2.6.19.5-i686: OK linux-2.6.20.21-i686: OK linux-2.6.21.7-i686: OK linux-2.6.16.61-x86_64: ERRORS linux-2.6.17.14-x86_64: ERRORS linux-2.6.18.8-x86_64: ERRORS linux-2.6.19.5-x86_64: OK linux-2.6.20.21-x86_64: OK linux-2.6.21.7-x86_64: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The V4L2 specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/v4l2.html The DVB API specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/dvbapi.pdf -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/10 - v2] ARM: DaVinci: Video: DM355/DM6446 VPFE Capture driver
Hello all, My patch 1/10 of this series somehow doesn't make it to linux-me...@vger.kernel.org. I can see it locally. Here is the header part of the patch. I can't see any thing wrong. I have tried re-sending with subject changed as follows, but nothing helped. Do you know what could cause this? [PATCH 1/10 - v2] vpfe capture bridge driver fro DM355 & DM6446 [PATCH 1/10 - v2] vpfe capture bridge driver fro DM355 and DM6446 [PATCH 1/10 - v2] vpfe-capture bridge driver fro DM355 & DM6446 From: Muralidharan Karicheri VPFE Capture bridge driver This is version, v2 of vpfe capture bridge driver for doing video capture on DM355 and DM6446 evms. The ccdc hw modules register with the driver and are used for configuring the CCD Controller for a specific decoder interface. The driver also registers the sub devices required for a specific evm. More than one sub devices can be registered. This allows driver to switch dynamically to capture video from any sub device that is registered. Currently only one sub device (tvp5146) is supported. But in future this driver is expected to do capture from sensor devices such as Micron's MT9T001,MT9T031 and MT9P031 etc. The driver currently supports MMAP based IO. Following are the updates based on review comments:- 1) minor number is allocated dynamically 2) updates to QUERYCAP handling 3) eliminated intermediate vpfe pixel format 4) refactored few functions 5) reworked isr routines for reducing indentation 6) reworked vpfe_check_format and added a documentation for algorithm 7) fixed memory leak in probe() TODO list : 1) load sub device from bridge driver. Hans has enhanced the v4l2-subdevice framework to do this. Will be updated soon to pick this. Reviewed By "Hans Verkuil". Reviewed By "Laurent Pinchart". Signed-off-by: Muralidharan Karicheri --- Applies to v4l-dvb repository Murali Karicheri Software Design Engineer Texas Instruments Inc. Germantown, MD 20874 Phone : 301-515-3736 email: m-kariche...@ti.com >-Original Message- >From: Karicheri, Muralidharan >Sent: Tuesday, June 09, 2009 2:47 PM >To: linux-media@vger.kernel.org >Cc: davinci-linux-open-sou...@linux.davincidsp.com; Muralidharan Karicheri; >Karicheri, Muralidharan >Subject: [PATCH 0/10 - v2] ARM: DaVinci: Video: DM355/DM6446 VPFE Capture >driver > >From: Muralidharan Karicheri > >VPFE Capture driver for DaVinci Media SOCs :- DM355 and DM6446 > >This is the version v2 of the patch series. This is the reworked >version of the driver based on comments received against the last >version of the patch. > >+++ >These patches add support for VPFE (Video Processing Front End) based >video capture on DM355 and DM6446 EVMs. For more details on the hardware >configuration and capabilities, please refer the vpfe_capture.c header. >This patch set consists of following:- > >Patch 1: VPFE Capture bridge driver >Patch 2: CCDC hw device header file >Patch 3: DM355 CCDC hw module >Patch 4: DM644x CCDC hw module >Patch 5: common types used across CCDC modules >Patch 6: Makefile and config files for the driver >Patch 7: DM355 platform and board setup >Patch 8: DM644x platform and board setup >Patch 9: Remove outdated driver files from davinci git tree >Patch 10: common vpss hw module for video drivers > >NOTE: > >Dependent on the TVP514x decoder driver patch for migrating the >driver to sub device model from Vaibhav Hiremath > >Following tests are performed. > 1) Capture and display video (PAL & NTSC) from tvp5146 decoder. > Displayed using fbdev device driver available on davinci git tree > 2) Tested with driver built statically and dynamically > >Muralidhara Karicheri > >Reviewed By "Hans Verkuil". >Reviewed By "Laurent Pinchart". > >Signed-off-by: Muralidharan Karicheri -- 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] [Resend] cxusb, d680 dmbth use unified lgs8gxx code instead of lgs8gl5
Use unified lgs8gxx frontend instead of reverse engineered lgs8gl5 frontend. After this patch, lgs8gl5 frontend could be mark as deprecated. Future development should base on unified lgs8gxx frontend. Signed-off-by: David T.L. Wong gmail.com> changeset: 11831:2fe3ef06a8e2 tag: tip user:mar...@dhm200 date:Mon May 18 17:03:36 2009 +0800 summary: cxusb: d680 switch from lgs8gl5 to unified lgs8gxx frontend driver diff -r 32e66a2bd568 -r 2fe3ef06a8e2 linux/drivers/media/dvb/dvb-usb/cxusb.c --- a/linux/drivers/media/dvb/dvb-usb/cxusb.c Mon May 18 16:01:15 2009 +0800 +++ b/linux/drivers/media/dvb/dvb-usb/cxusb.c Mon May 18 17:03:36 2009 +0800 @@ -38,7 +38,7 @@ #include "mxl5005s.h" #include "dib7000p.h" #include "dib0070.h" -#include "lgs8gl5.h" +#include "lgs8gxx.h" /* debug */ static int dvb_usb_cxusb_debug; @@ -1097,8 +1097,18 @@ return -EIO; } -static struct lgs8gl5_config lgs8gl5_cfg = { +static struct lgs8gxx_config d680_lgs8gl5_cfg = { + .prod = LGS8GXX_PROD_LGS8GL5, .demod_address = 0x19, + .serial_ts = 0, + .ts_clk_pol = 0, + .ts_clk_gated = 1, + .if_clk_freq = 30400, /* 30.4 MHz */ + .if_freq = 5725, /* 5.725 MHz */ + .if_neg_center = 0, + .ext_adc = 0, + .adc_signed = 0, + .if_neg_edge = 0, }; static int cxusb_d680_dmb_frontend_attach(struct dvb_usb_adapter *adap) @@ -1138,7 +1148,7 @@ msleep(100); /* Attach frontend */ - adap->fe = dvb_attach(lgs8gl5_attach, &lgs8gl5_cfg, &d->i2c_adap); + adap->fe = dvb_attach(lgs8gxx_attach, &d680_lgs8gl5_cfg, &d->i2c_adap); if (adap->fe == NULL) return -EIO;
[RFC PATCH] adding support for setting bus parameters in sub device
From: Muralidharan Karicheri This patch adds support for setting bus parameters such as bus type (Raw Bayer or Raw YUV image data bus), bus width (example 10 bit raw image data bus, 10 bit BT.656 etc.), and polarities (vsync, hsync, field etc) in sub device. This allows bridge driver to configure the sub device interface for a specific set of bus parameters through s_bus() function call. Reviewed By "Hans Verkuil". Signed-off-by: Muralidharan Karicheri --- Applies to v4l-dvb repository include/media/v4l2-subdev.h | 36 1 files changed, 36 insertions(+), 0 deletions(-) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 1785608..8e719c4 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -37,6 +37,39 @@ struct v4l2_decode_vbi_line { u32 type; /* VBI service type (V4L2_SLICED_*). 0 if no service found */ }; +/* + * Some sub-devices are connected to the bridge device through a bus that + * carries * the clock, vsync, hsync and data. Some interfaces such as BT.656 + * carries the sync embedded in the data where as others have separate line + * carrying the sync signals. The structure below is used by bridge driver to + * set the desired bus parameters in the sub device to work with it. + */ +enum v4l2_subdev_bus_type { + /* Raw YUV image data bus */ + V4L2_SUBDEV_BUS_RAW_YUV, + /* Raw Bayer image data bus */ + V4L2_SUBDEV_BUS_RAW_BAYER +}; + +struct v4l2_subdev_bus { + /* yuv or bayer image data bus */ + enum v4l2_subdev_bus_type type; + /* bus width */ + u8 width; + /* embedded sync, set this when sync is embedded in the data stream */ + unsigned embedded_sync:1; + /* 0 - active low, 1 - active high */ + unsigned pol_vsync:1; + /* 0 - active low, 1 - active high */ + unsigned pol_hsync:1; + /* 0 - low to high , 1 - high to low */ + unsigned pol_field:1; + /* 0 - sample at falling edge , 1 - sample at rising edge */ + unsigned pol_pclock:1; + /* 0 - active low , 1 - active high */ + unsigned pol_data:1; +}; + /* Sub-devices are devices that are connected somehow to the main bridge device. These devices are usually audio/video muxers/encoders/decoders or sensors and webcam controllers. @@ -199,6 +232,8 @@ struct v4l2_subdev_audio_ops { s_routing: see s_routing in audio_ops, except this version is for video devices. + + s_bus: set bus parameters in sub device to configure the interface */ struct v4l2_subdev_video_ops { int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32 config); @@ -219,6 +254,7 @@ struct v4l2_subdev_video_ops { int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param); int (*enum_framesizes)(struct v4l2_subdev *sd, struct v4l2_frmsizeenum *fsize); int (*enum_frameintervals)(struct v4l2_subdev *sd, struct v4l2_frmivalenum *fival); + int (*s_bus)(struct v4l2_subdev *sd, const struct v4l2_subdev_bus *bus); }; struct v4l2_subdev_ops { -- 1.6.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] vino: replace dma_sync_single with dma_sync_single_for_cpu
Em Mon, 1 Jun 2009 11:08:26 +0900 FUJITA Tomonori escreveu: > On Thu, 28 May 2009 08:02:14 +0200 > Jiri Slaby wrote: > > > On 05/28/2009 03:10 AM, FUJITA Tomonori wrote: > > > This replaces dma_sync_single() with dma_sync_single_for_cpu() because > > > dma_sync_single() is an obsolete API; include/linux/dma-mapping.h says: > > > > > > /* Backwards compat, remove in 2.7.x */ > > > #define dma_sync_single dma_sync_single_for_cpu > > > #define dma_sync_sg dma_sync_sg_for_cpu > > > > > > Signed-off-by: FUJITA Tomonori > > > --- > > > drivers/media/video/vino.c |6 +++--- > > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/media/video/vino.c b/drivers/media/video/vino.c > > > index 43e0998..97b082f 100644 > > > --- a/drivers/media/video/vino.c > > > +++ b/drivers/media/video/vino.c > > > @@ -868,9 +868,9 @@ static void vino_sync_buffer(struct vino_framebuffer > > > *fb) > > > dprintk("vino_sync_buffer():\n"); > > > > > > for (i = 0; i < fb->desc_table.page_count; i++) > > > - dma_sync_single(NULL, > > > - fb->desc_table.dma_cpu[VINO_PAGE_RATIO * i], > > > - PAGE_SIZE, DMA_FROM_DEVICE); > > > + dma_sync_single_for_cpu(NULL, > > > + fb->desc_table.dma_cpu[VINO_PAGE_RATIO > > > * i], > > > + PAGE_SIZE, DMA_FROM_DEVICE); > > > > Shouldn't be there sync_for_device in vino_dma_setup (or somewhere) > > then? If I understand the API correctly this won't (and didn't) work on > > some platforms. Well, this driver is bound to an specific architecture: config VIDEO_VINO tristate "SGI Vino Video For Linux (EXPERIMENTAL)" depends on I2C && SGI_IP22 && EXPERIMENTAL && VIDEO_V4L2 So, it works only with a few SGI machines. > > Yeah, you might be right. > > However, looks like this driver does only DMA_FROM_DEVICE transfer and > cpu doesn't modify the DMA buffer. > > So we don't need to worry that the hardware gets old data. And it's > not possible that we write back old data in cache to the main memory > after DMA. It means that the driver doesn't need > sync_single_for_device(), I think. > > Note that this patch doesn't break the driver (this patch doesn't > change anything). If this patch doesn't work, then this driver is > already broken. This driver were written a long time ago. I'm not sure if it still works fine, since all patches we receive are related to API changes. Yet, it seems better to apply your patch, since it doesn't hurt. For now, I'll apply it. It would be interesting to have someone to test the removal of this call, since it looks that this call is not needed. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
soc-camera: status, roadmap
Hi all for those interested here's a (not so) short status report and a proposed roadmap for general soc-camera development, and, of course, its ongoing conversion to v4l2-subdev API. 1. v4l2-subdev conversion. I have posted several versions of the conversion patch series to the list, of which the last takes an IMHO correct approach of a graduate conversion, avoiding mega-patches, modifying multiple platforms and drivers at once. With this approach the roadmap consists of the following steps: 1.1. preparatory patch to soc-camera core, allowing parallel existence of "legacy" (all in the mainline) platforms and converted platforms (pcm037 i.MX31 platform so far) by introducing some backwards compatibility code. This patch is currently in v4l next and in linux-next, i.e., it is going in with 2.6.31. 1.2. convert all (around 7) mainline platforms to the new layout. This step is necessary for further conversions, but it depends on 1.1. Therefore this can only be done later in 2.6.31 merge window, when 1.1. is in the mainline. 1.3. convert soc-camera core and drivers to an intermediate state, with which all cameras are registered by platforms as platform devices, later soc-camera core probes them and dynamically registers respective i2c (or other, as in soc_camera_platform.c case) devices. This patch depends on 1.2., and it is hard to expect to be able to push these three steps within the 2.6.31 merge window. It could be possible, we could request to accept this patch after -rc1, maybe we would be allowed to do this, but 1.4. this is the actual conversion to v4l2-subdev. It depends on some bits and pieces in the v4l2-subdev framework, which are still in progress (e.g. v4l2_i2c_new_subdev_board), I believe (Hans, am I right? or what's the outcome of Mauro's last reply to you in the "[PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-subdev"; thread?), so, it becomes practically impossible to also pull it for 2.6.31. Now, I do not want to have soc-camera in the intermediate 1.3. state for a whole 2.6.31 kernel, which means, we have to postpone 1.3. and 1.4. until 2.6.32. 2. The above means, I'll have to maintain and update my patches for a whole 2.6.31 development cycle. In this time I'll try to update and upload them as a quilt patch series and announce it on the list a couple of times. 3. This also means, development will become more difficult, new features and drivers will only be accepted on the top of my patch stack, bugfixes will have to be accpeted against the mainline, which then will mean extra porting work for me. 4. In a message I posted a few minutes ago http://www.mail-archive.com/linux-media@vger.kernel.org/msg06294.html I'm asking about a correct interpretation of S_CROP and S_FMT operations. I suspect, what soc-camera framework and all drivers thereunder are doing is wrong, and have to be fixed rather sooner than later. However, I'd be very much against fixing this in the present stack, because that would mean a _lot_ of porting. So, this will remain standard-non-compliant until 2.6.32 too. 5. The conversion described in (1) is only partial, in its current form it does not replace the existing soc-camera API between sensor drivers and the soc-camera core with v4l2-subdev operations completely. Partly because many of the current soc-camera methods are still missing in v4l2-subdev, partly because it just makes more sense to first push the principle conversion in the mainline, which at least removes soc-camera device registration and switches to i2c driver autoloading and replaces some trivially replaceable methods, like [gs]_fmt, [gs]_register, [gs]_control. Some of the missing methods, like [gs]_crop should be easy to add, others, like pixel format and bus parameter negotiations would require some thinking and substantial work. Which makes this all some 2.6.33 material... but who wants to think that far... 6. As you see, this all looks like a lot of work, and I so far have been doing all of this in my free time. But it would become difficult with these amounts of work. So, I would welcome if either someone would step forward to help with this work, or if some company would volunteer to support this work financially. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-dvb] FlyDVB Trio PCI simultanious DVB-S/DVB-T?
On Wednesday 10 June 2009 17:36:34 sacha wrote: > I was googling quite a lot without result. Is it possible to run > FlyDVB Trio PCI in simultaneous mode with both DVB-S and DVB-T > frontends? It is working under Windows. > > KR > > no, it's not possible, not even in windows: they share a part of the hardware. What's possible is using an analog and a digital source at the same time. -- 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: S_FMT vs. S_CROP
This question - how S_FMT and S_CROP affest image geometry - has been discussed at least twice before - that's only with my participation, don't know if and how often it has come up before. But the fact, that in two discussions we came up with different results seems to suggest, that this is not something trivially known by all except me. First time I asked this question in this thread http://www.mail-archive.com/linux-media@vger.kernel.org/msg00052.html and Mauro replied (see above thread for a complete reply): On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote: > On Wed, 7 Jan 2009 10:14:31 +0100 (CET) > Guennadi Liakhovetski wrote: [snip] > > For example on mt9t031 > > binning and skipping are used for that. Whereas CROP uses the current > > scaling configuration and selects a sub-window, so, once you've done S_FMT > > to 320x240, a crop request for 640x480 might well fail. > > I also understand this way. You cannot crop with a resolution bigger than what > you've selected. (Let's call this statement M1:-)) > > For this you have > > to issue a S_FMT, i.e., change scaling. Or would one have to re-scale > > transparently? > > > > Is this interpretation correct? It seems to reflect the API as documented > > on http://v4l2spec.bytesex.org/spec/book1.htm correctly. > > > > If it is correct, then what should CROP_CAP return as maximum supported > > window sizes? Should it return them according to the current scaling or > > according to scale 1? > > I understand that it should return against the current scaling. (and this one M2, whereas I understand it as "current scaling" means "current scaling coefficient", not "current scaled output windof") Then in another thread http://www.mail-archive.com/linux-media@vger.kernel.org/msg03512.html Stefan motivated for an incomatibly different interpretation of the standard: On Thu, 2 Apr 2009, Stefan Herbrechtsmeier wrote: [snip] > The user doesn't have to remember the scale anyway. Only the ways a different. > You interpret S_CROP > as something like a cutting of the S_FMT window. I interpret S_FMT as a output > format selection > and S_CROP as a sensor window selection. which I now interpret as S_FMT(640x480) means "scale whatever rectangle has been selected on the sensor to produce an output window of 640x480" and S_CROP(2048x1536) means "take a window of 2048x1536 sensor pixels from the sensor and scale it to whatever output window has been or will be selected by S_FMT." This contradicts M1, because you certainly can crop a larger (sensor) window. Also, I now believe, that [GS]_CROP and, logically, CROPCAP operate in sensor pixels and shall not depend on any scales, which contradicts (my understanding of) M2. It now seems to be quite simple to me: {G,S,TRY}_FMT configure output geometry in user pixels [GS]_CROP, CROPCAP configure input window in sensor pixels The thus configured input window should be mapped (scaled) into the output window. Now, which one is correct? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
help needed: DVB-T nearly working with a Terratec Cynergy Hybrid XS 0ccd:005e
Hi ! I've modified the sources of a 2.6.29 kernel to try to have my Terratec Hybrid usb key working in DVB-T mode. Basically I modified em28xx-cards.c, and used the settings of a WINTV HVR-900 for my card. [EM2882_BOARD_TERRATEC_HYBRID_XS] = { .name = "Terratec Hybrid XS (em2882)", .valid= EM28XX_BOARD_NOT_VALIDATED, .tuner_type = TUNER_XC2028, .tuner_gpio = default_tuner_gpio, //.mts_firmware = 1, with or without same results .has_dvb = 1, .dvb_gpio = hauppauge_wintv_hvr_900_digital, It nearly works: only the 1st tuning works, then that's as if the tuner is locked up. For example scan, tunes properly for the 1st frequency: # scan /usr/share/dvb/dvb-t/fr-Metz scanning /usr/share/dvb/dvb-t/fr-Metz using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0' initial transponder 570166000 0 2 9 3 1 0 0 initial transponder 594166000 0 2 9 3 1 0 0 initial transponder 754166000 0 2 9 3 1 0 0 initial transponder 770166000 0 2 9 3 1 0 0 initial transponder 498166000 0 2 9 3 1 0 0 initial transponder 794166000 0 2 9 3 1 0 0 >>> tune to: 570166000:INVERSION_AUTO:BANDWIDTH_8_MHZ:FEC_2_3:FEC_AUTO:QAM_64:TRANSMISSION_MODE_8K:GUARD_INTERVAL_1_32:HIERARCHY_NONE Network Name 'F' 0x 0x0101: pmt_pid 0x006e GR1 -- France 2 (running) 0x 0x0104: pmt_pid 0x0136 GR1 -- France 5 (running) 0x 0x0105: pmt_pid 0x01fe GR1 -- ARTE (running) 0x 0x0106: pmt_pid 0x0262 GR1 -- LCP (running) 0x 0x0111: pmt_pid 0x00d2 National -- France 3 (running) 0x 0x01ff: pmt_pid 0x03f2 (null) -- (null) (running) >>> tune to: 594166000:INVERSION_AUTO:BANDWIDTH_8_MHZ:FEC_2_3:FEC_AUTO:QAM_64:TRANSMISSION_MODE_8K:GUARD_INTERVAL_1_32:HIERARCHY_NONE WARNING: filter timeout pid 0x0011 WARNING: filter timeout pid 0x WARNING: filter timeout pid 0x0010 >>> tune to: 754166000:INVERSION_AUTO:BANDWIDTH_8_MHZ:FEC_2_3:FEC_AUTO:QAM_64:TRANSMISSION_MODE_8K:GUARD_INTERVAL_1_32:HIERARCHY_NONE WARNING: >>> tuning failed!!! If I change the 1st freq in the config file and restart scan it works. So only the 1st tuning works when the FE is opened only one time, after the firmware is sent: --- xc2028 4-0061: Loading firmware for type=BASE F8MHZ (3), id . xc2028 4-0061: Loading firmware for type=D2633 DTV8 (210), id . xc2028 4-0061: Loading SCODE for type=DTV6 QAM DTV7 DTV78 DTV8 ZARLINK456 SCODE HAS_IF_4760 (620003e0), id . --- How could I solve this stupid tuning problem ? (BTW, this usb device seems to work properly in analog mode) Cheers, -- Ludovic Drolez. http://www.geeksback.com - Secure File Backups for Geeks http://www.palmopensource.com - The PalmOS Open Source Portal -- 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,PATCH] VIDIOC_G_EXT_CTRLS does not handle NULL pointer correctly
Mauro Carvalho Chehab wrote: > Em Wed, 10 Jun 2009 10:52:28 -0300 > Mauro Carvalho Chehab escreveu: > >> Em Mon, 25 May 2009 11:16:34 -0300 >> Mauro Carvalho Chehab escreveu: >> >>> Em Mon, 25 May 2009 13:17:02 +0200 >>> Laurent Pinchart escreveu: >>> Hi everybody, Márton Németh found an integer overflow bug in the extended control ioctl handling code. This affects both video_usercopy and video_ioctl2. See http://bugzilla.kernel.org/show_bug.cgi?id=13357 for a detailed description of the problem. Restricting v4l2_ext_controls::count to values smaller than KMALLOC_MAX_SIZE / sizeof(struct v4l2_ext_control) should be enough, but we might want to restrict the value even further. I'd like opinions on this. >>> Seems fine to my eyes, but being so close to kmalloc size doesn't seem to >>> be a >>> good idea. It seems better to choose an arbitrary size big enough to handle >>> all current needs. >> I'll apply the current version, but I still think we should restrict it to a >> lower value. > > > Hmm... SOB is missing. Márton and Laurent, could you please sign it As I wrote at http://bugzilla.kernel.org/show_bug.cgi?id=13357#c6 : Tested-by: Márton Németh 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: [PATCH RFC] adding support for setting bus parameters in sub device
>> > >> > +/* >> > + * Some sub-devices are connected to the bridge device through a bus >> > that + * carries * the clock, vsync, hsync and data. Some interfaces >> > such as BT.656 + * carries the sync embedded in the data where as >> > others have separate line + * carrying the sync signals. The structure >> > below is used by bridge driver to + * set the desired bus parameters in >> > the sub device to work with it. + */ >> > +enum v4l2_subdev_bus_type { >> > + /* BT.656 interface. Embedded sync */ >> > + V4L2_SUBDEV_BUS_BT_656, >> > + /* BT.1120 interface. Embedded sync */ >> > + V4L2_SUBDEV_BUS_BT_1120, >> > + /* 8 bit muxed YCbCr bus, separate sync and field signals */ >> > + V4L2_SUBDEV_BUS_YCBCR_8, >> > + /* 16 bit YCbCr bus, separate sync and field signals */ >> > + V4L2_SUBDEV_BUS_YCBCR_16, >> >> Hmm, what do you mean with "8 bit muxed YCbCr bus"? It's not clear to me >> what the format of these YCBCR bus types is exactly. >> [MK]I spent sometime yesterday looking at different interfaces that we support in our soc. Here is the list... BT.656, which is 8 bit or 10 bit multiplexed YCbCr bus BT.1120, which is 16 bit or 20 bit YCbCr bus YUV bus with separate sync signals (hsync, vsync and field)which could be 8 bit, 10 bit, 16 bit and 20 bit Since BT_656 & BT_1120 also carries YUV data, we could call them as YUV bus. So we can classify bus type based on the type of data it carries as below.. enum v4l2_subdev_bus_type { /* Raw YUV image data bus, such as BT.656, BT.1120, or with * separate sync */ V4L2_SUBDEV_BUS_RAW_YUV, /* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */ V4L2_SUBDEV_BUS_RAW_BAYER }; Since we have width to describe the bus size, we could define all of the above bus types using above bus types and width. In addition we could add another Boolean to describe if sync is sent embedded or separate as below. Just want to do it right. If anyone has any comments about the classification, please reply. I will sent an updated patch soon. >> > + >> > +struct v4l2_subdev_bus{ >> > + enum v4l2_subdev_bus_type type; >> > + u8 width; unsigned embedded_sync:1; >> > + /* 0 - active low, 1 - active high */ >> > + unsigned pol_vsync:1; >> > + /* 0 - active low, 1 - active high */ >> > + unsigned pol_hsync:1; >> > + /* 0 - low to high , 1 - high to low */ >> > + unsigned pol_field:1; >> > + /* 0 - sample at falling edge , 1 - sample at rising edge */ >> > + unsigned pol_pclock:1; >> > + /* 0 - active low , 1 - active high */ >> > + unsigned pol_data:1; >> > +}; >> > + >> > /* Sub-devices are devices that are connected somehow to the main >> > bridge device. These devices are usually audio/video >> > muxers/encoders/decoders or sensors and webcam controllers. >> > @@ -109,6 +144,7 @@ struct v4l2_subdev_core_ops { >> >int (*querymenu)(struct v4l2_subdev *sd, struct v4l2_querymenu *qm); >> >int (*s_std)(struct v4l2_subdev *sd, v4l2_std_id norm); >> >long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg); >> > + int (*s_bus)(struct v4l2_subdev *sd, struct v4l2_subdev_bus *bus); >> >> Make this 'const struct v4l2_subdev_bus *bus'. > >And move it to the video ops. This op is only relevant for video, after all. >Yes, I know I said to add it to core initially; so sue me :-) > >Regards, > > Hans > >> >> > #ifdef CONFIG_VIDEO_ADV_DEBUG >> >int (*g_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register >> > *reg); int (*s_register)(struct v4l2_subdev *sd, struct >> > v4l2_dbg_register *reg); >> >> Regards, >> >> Hans > > > >-- >Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: how to mmap in videobuf-dma-sg.c
On Thu, 2009-05-21 at 18:48 +0800, Figo.zhang wrote: > On Thu, 2009-05-21 at 07:35 -0300, Mauro Carvalho Chehab wrote: > > Em Thu, 21 May 2009 12:46:04 +0800 > > "Figo.zhang" escreveu: > > > > > hi,all, > > > I am puzzle that how to mmap ( V4L2_MEMORY_MMAP) in videobuf-dma-sg.c? > > > > > > In this file, it alloc the momery using vmalloc_32() , and put this > > > momery into sglist table,and then use dma_map_sg() to create sg dma at > > > __videobuf_iolock() function. but in __videobuf_mmap_mapper(), i canot > > > understand how it do the mmap? > > > why it not use the remap_vmalloc_range() to do the mmap? > > > > The answer is simple: remap_vmalloc_range() is newer than videobuf code. > > This > > part of the code was written back to kernel 2.4, and nobody cared to update > > it > > to use those newer functions, and simplify its code. > > > > If you want, feel free to propose some cleanups on it > > > > > > > > Cheers, > > Mauro > > hi mauro, > Thank you! > But i canot found the similar function code of remap_vmalloc_range() in > the videobuf-dma-contig.c file. So i want to know the how is work in > __videobuf_mmap_mapper() function? > > hi mauro: Thank you. But i still have a puzzle question about mmap() in videobuf-dma-sg.c. I canot find how to remap the dma buffer (which alloc by vmalloc_32()) to the vma area in __videobuf_mmap_mapper()? there is my test driver code about dma-sg,it work well. i use remap_pfn_range() to remap the dma buffer to vma area in mmap method. so would you like to give me some detail about it? Best Regards, Figo.zhang static int mydev_alloc_dma_sg(struct mydev_device *dev, struct mydev_buf *buf) { int nr_pages; int i; struct page *pg; unsigned char * virt; nr_pages = mydev_buffer_pages(buf->size); buf->nr_pages = nr_pages; dprintk("%s:: buf->nr_pages =%d\n", __func__, buf->nr_pages); buf->sglist = kcalloc(buf->nr_pages, sizeof(struct scatterlist), GFP_KERNEL); if (NULL == buf->sglist) return NULL; sg_init_table(buf->sglist, buf->nr_pages); buf->vmalloc = vmalloc_32(buf->nr_pages << PAGE_SHIFT); memset(buf->vmalloc,0,buf->nr_pages << PAGE_SHIFT); virt = buf->vmalloc; for(i = 0; i< buf->nr_pages; i++,virt += PAGE_SIZE){ pg = vmalloc_to_page(virt); if (NULL == pg) goto nopage; BUG_ON(PageHighMem(pg)); sg_set_page(&buf->sglist[i], pg, PAGE_SIZE, 0); } buf->sglen = dma_map_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, DMA_FROM_DEVICE); return 0; nopage: dprintk("sgl: oops - no page\n"); kfree(buf->sglist); return 0; } in mmap(); static int do_mmap_sg(struct mydev_device *dev, struct vm_area_struct * vma) { struct videobuf_mapping *map; unsigned long pos,page; unsigned long start = vma->vm_start; unsigned long size = vma->vm_end - vma->vm_start; unsigned long offset = vma->vm_pgoff << PAGE_SHIFT; unsigned int first, last, end; int retval = -EINVAL; int i; /* look for first buffer to map */ for (first = 0; first < VIDEO_MAX_FRAME; first++) { if (dev->ktbuf[first].boff == (vma->vm_pgoff << PAGE_SHIFT)) break; } if (VIDEO_MAX_FRAME == first) { dprintk("mmap app bug: offset invalid [offset=0x%lx]\n", (vma->vm_pgoff << PAGE_SHIFT)); goto done; } /* create mapping + update buffer list */ retval = -ENOMEM; map = kmalloc(sizeof(struct videobuf_mapping),GFP_KERNEL); if (NULL == map) goto done; pos = (unsigned long)dev->ktbuf[first].vmalloc; while (size > 0) { page = vmalloc_to_pfn((void *)pos); if (remap_pfn_range(vma, start, page, PAGE_SIZE, PAGE_SHARED)) { return -EAGAIN; } start += PAGE_SIZE; pos += PAGE_SIZE; if (size > PAGE_SIZE) size -= PAGE_SIZE; else size = 0; } map->count= 1; map->start= vma->vm_start; map->end = vma->vm_end; vma->vm_ops = &mydev_vm_ops; vma->vm_flags |=/* VM_DONTEXPAND |*/ VM_RESERVED; vma->vm_flags &= ~VM_IO; /* using shared anonymous pages */ vma->vm_private_data = map; mydev_vm_open(vma); retval = 0; done: return retval; } -- 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/majo
Re: [PATCH]V4L:some v4l drivers have error for video_register_device
Em Thu, 04 Jun 2009 17:20:50 +0800 "figo.zhang" escreveu: > On Thu, 2009-06-04 at 11:18 +0200, Laurent Pinchart wrote: > > Hi, > > > > On Thursday 04 June 2009 06:20:07 figo.zhang wrote: > > > The function video_register_device() will call the > > > video_register_device_index(). In this function, firtly it will do some > > > argments check , if failed,it will return a negative number such as > > > -EINVAL, and then do cdev_alloc() and device_register(), if success return > > > zero. so video_register_device_index() canot return a a positive number. > > > > > > for example, see the drivers/media/video/stk-webcam.c (line 1325): > > > > > > err = video_register_device(&dev->vdev, VFL_TYPE_GRABBER, -1); > > > if (err) > > > STK_ERROR("v4l registration failed\n"); > > > else > > > STK_INFO("Syntek USB2.0 Camera is now controlling video device" > > > " /dev/video%d\n", dev->vdev.num); > > > > > > in my opinion, it will be cleaner to do something like this: > > > > > > err = video_register_device(&dev->vdev, VFL_TYPE_GRABBER, -1); > > > if (err != 0) > > > STK_ERROR("v4l registration failed\n"); > > > else > > > STK_INFO("Syntek USB2.0 Camera is now controlling video device" > > > " /dev/video%d\n", dev->vdev.num); > > > > What's the difference ? (err != 0) and (err) are identical. > > > > Best regards, > > > > Laurent Pinchart > > yes, it is the same, but it is easy for reading. if (err) is easier for reading, since it is closer to the natural language. Also, as CodingStyle says, "C is a Spartan language". So, using just if (err) is better than if (err != 0). Also, since positive values don't indicate errors (on Linux, all errors are negative values), using err != 0 looks wrong. Yet, changing they to err < 0 would require a careful review of the returned values by each function. In summary, for newer code, the better is to always use if (err < 0), being sure that the called function will return a negative value. However, I don't see much sense on changing the current code. 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: cx18, s5h1409: chronic bit errors, only under Linux
David Ward wrote: On 06/09/2009 03:26 PM, Steven Toth wrote: 30db for the top end of ATSC sounds about right. David, when you ran the windows signal monitor - did it claim QAM64 or 256 when it was reporting 30db? Steven and Devin, All the digital signals are 256 QAM. 39 is very good, exceptional. And did they do as I suggested, which is measure db across the high channels? ... and ideally against your problematic channel? I assume not. Comcast checked the outlet on channels 2 (41 dB) and 83 (39 dB). I looked afterwards and saw that the first of those is analog programming, but the second just appears as analog noise on my TV set. (??) I asked them to check a specific ATSC channel, but it seems that their meter was fixed to those two frequencies, which doesn't really help. The ATSC rebroadcasts by Comcast are on high frequencies; the program I am testing primarily is on channel 79 (tunes at 555 MHz). Under Windows I'm now seeing 34.5-34.8 dB for lower frequency QAM, 32.5-32.7 dB for higher frequency QAM, and about 30.5 dB for ATSC. Under Linux with azap, the corresponding BER/UNC values are 0x0140, 0x0134, and 0x0132. I'm not exactly sure what numbers I should be going by here...again, wish I had my own meter. Which of these three values is UNC/BER and which is snr? I don't understand, I need you to be more specific. 34 is good, normal. However 30.5 is still edgy under windows, assuming QAM 256. Did you get a chance to review the signal monitor to determine whether it was 64 or 256? If you have any way to attenuate the signal then you'll find that very quickly the windows 30.5 will drop just a little and you'll begin to see real uncorrectable errors. I alluded to this yesterday. With 30.5 your just a fraction above 'working' reliably. If you were to insert attenuation through some barrel connectors, or join some other cables together to impede the RF, you'd see that 30.5 drop quickly and the errors would begin to appear. I suspect this will still occur, as I mentioned yesterday. The windows drivers is working slightly better for you but it's still no where near good enough RF for reliable 24x7x365 viewing. You'll find the RF on your local cable rings varies during an average day. It certainly does for me on various products. What looks great today (when you're on the edge) can look ugly at 9pm in the evening or 7am thursday morning. I wouldn't expect pristine recordings with Microsoft MCE (or other apps) (for any random moment in the week) with a 30.5 reading. -- Steven Toth - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] flexcop-pci: add suspend/resume support
Em Tue, 26 May 2009 21:09:28 +0200 Matthias Schwarzott escreveu: > Hi Patrick! Hi list! > > This patch adds suspend/resume support to flexcop-pci driver. > > I could only test this patch with the bare card, but without having a DVB-S > signal. I checked it with and without running szap (obviously getting no > lock). > It works fine here with suspend-to-disk on a tuxonice kernel. > > Setting of hw-filter in resume is done the same way as the watchdog does it: > Just looping over fc->demux.feed_list and running flexcop_pid_feed_control. > Where I am unsure is the order at resume. For now hw filters get started > first, then dma is re-started. > > Do I need to give special care to irq handling? It would be important to have a test with real signals to see if the patch is really working, and see if it will still work if you suspend while streaming. Also, you missed your SOB. 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: [RFC,PATCH] VIDIOC_G_EXT_CTRLS does not handle NULL pointer correctly
Em Wed, 10 Jun 2009 10:52:28 -0300 Mauro Carvalho Chehab escreveu: > Em Mon, 25 May 2009 11:16:34 -0300 > Mauro Carvalho Chehab escreveu: > > > Em Mon, 25 May 2009 13:17:02 +0200 > > Laurent Pinchart escreveu: > > > > > Hi everybody, > > > > > > Márton Németh found an integer overflow bug in the extended control ioctl > > > handling code. This affects both video_usercopy and video_ioctl2. See > > > http://bugzilla.kernel.org/show_bug.cgi?id=13357 for a detailed > > > description of > > > the problem. > > > > > > > > Restricting v4l2_ext_controls::count to values smaller than > > > KMALLOC_MAX_SIZE / > > > sizeof(struct v4l2_ext_control) should be enough, but we might want to > > > restrict the value even further. I'd like opinions on this. > > > > Seems fine to my eyes, but being so close to kmalloc size doesn't seem to > > be a > > good idea. It seems better to choose an arbitrary size big enough to handle > > all current needs. > > I'll apply the current version, but I still think we should restrict it to a > lower value. Hmm... SOB is missing. Márton and Laurent, could you please sign it 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: [RFC,PATCH] VIDIOC_G_EXT_CTRLS does not handle NULL pointer correctly
Em Mon, 25 May 2009 11:16:34 -0300 Mauro Carvalho Chehab escreveu: > Em Mon, 25 May 2009 13:17:02 +0200 > Laurent Pinchart escreveu: > > > Hi everybody, > > > > Márton Németh found an integer overflow bug in the extended control ioctl > > handling code. This affects both video_usercopy and video_ioctl2. See > > http://bugzilla.kernel.org/show_bug.cgi?id=13357 for a detailed description > > of > > the problem. > > > > > Restricting v4l2_ext_controls::count to values smaller than > > KMALLOC_MAX_SIZE / > > sizeof(struct v4l2_ext_control) should be enough, but we might want to > > restrict the value even further. I'd like opinions on this. > > Seems fine to my eyes, but being so close to kmalloc size doesn't seem to be a > good idea. It seems better to choose an arbitrary size big enough to handle > all current needs. I'll apply the current version, but I still think we should restrict it to a lower value. 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
Mantis CI (Azurewave AD SP400 CI (VP-1041) cards)
Can any body confirm if the CI works on Mantis-based cards, in particular I'm interested in getting the Azurewave AD SP400 CI (VP-1041) working. Are there any other boards with CI that work under Linux? Many thanks in advance. Regards -- Brad. -- 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
About V4l2 overlay sequence
Dear all ~~ With your help I have implemented the preview with capture interface ~~ Now i want to implement the preview with ovelay , and my camera support s ovelay ~ Who can tell me where I can find the document about ovelay sequcence ~ ? Or does there have a standard example source code ~ ? Thanks a lot ~ Best regards -- 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: cx18, s5h1409: chronic bit errors, only under Linux
On 06/09/2009 03:26 PM, Steven Toth wrote: 30db for the top end of ATSC sounds about right. David, when you ran the windows signal monitor - did it claim QAM64 or 256 when it was reporting 30db? Steven and Devin, All the digital signals are 256 QAM. 39 is very good, exceptional. And did they do as I suggested, which is measure db across the high channels? ... and ideally against your problematic channel? I assume not. Comcast checked the outlet on channels 2 (41 dB) and 83 (39 dB). I looked afterwards and saw that the first of those is analog programming, but the second just appears as analog noise on my TV set. (??) I asked them to check a specific ATSC channel, but it seems that their meter was fixed to those two frequencies, which doesn't really help. The ATSC rebroadcasts by Comcast are on high frequencies; the program I am testing primarily is on channel 79 (tunes at 555 MHz). Under Windows I'm now seeing 34.5-34.8 dB for lower frequency QAM, 32.5-32.7 dB for higher frequency QAM, and about 30.5 dB for ATSC. Under Linux with azap, the corresponding BER/UNC values are 0x0140, 0x0134, and 0x0132. I'm not exactly sure what numbers I should be going by here...again, wish I had my own meter. I admit that I ruled out the idea of RF issues too soon, which I really should know better than. After reading the thread at http://forums.gbpvr.com/showthread.php?t=36049 I'm now realizing why reception on the TV and tuner card isn't going to be equal, due to limited size and shielding of tuner circuitry on a PCI form factor card vs. on a TV set. Makes sense. Still, I am continuing to see uncorrectable bit errors at the same rate as before under Linux, while Windows sees errors but corrects them. I would think that both drivers should be receiving identical streams of data from the chipset, and should be able to process them the same way? That's what is confusing me. Ideally I would like to have access to a lot more equipment to control all the variables and make it easier to reproduce what I am seeing...but I don't here... Or do you guys think that this is still primarily an RF problem? I don't know what else I could do about that though. Since the SNR did not improve when I hooked the tuner card directly into the cable input from the street, I'm concerned that putting an amplifier would not help and could just make things worse. And clearly Comcast now considers me to be within their quality thresholds. I really appreciate your help and patience with me, especially to the extent that this is going outside the realm of DVB drivers. David -- 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