Re: [PATCHv4 0/6] media: convert to c99 format
On Tuesday 18 September 2012 05:52 PM, Shubhrajyoti D wrote: The series tries to convert the i2c_msg to c99 struct. This may avoid issues like below if someone tries to add an element to the structure. http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg08972.html Special thanks to Julia Lawall for helping it automate. By the below script. http://www.mail-archive.com/cocci@diku.dk/msg02753.html Changelogs - Remove the zero inititialisation of the flags. ping. Shubhrajyoti D (6): media: Convert struct i2c_msg initialization to C99 format media: Convert struct i2c_msg initialization to C99 format media: Convert struct i2c_msg initialization to C99 format media: Convert struct i2c_msg initialization to C99 format media: Convert struct i2c_msg initialization to C99 format media: Convert struct i2c_msg initialization to C99 format drivers/media/i2c/ks0127.c| 13 +++- drivers/media/i2c/msp3400-driver.c| 40 + drivers/media/i2c/tvaudio.c | 13 +++- drivers/media/radio/radio-tea5764.c | 13 ++-- drivers/media/radio/saa7706h.c| 15 - drivers/media/radio/si470x/radio-si470x-i2c.c | 23 ++ 6 files changed, 96 insertions(+), 21 deletions(-) -- 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/4] [media] mem2mem_testdev: Fix incorrect location of v4l2_m2m_release()
v4l2_m2m_release() was placed after the return statement and outside any of the goto labels and hence was not getting executed under the error exit path. This patch moves it under the exit path label. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/mem2mem_testdev.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index 771a84f..fc95559 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -1067,8 +1067,8 @@ static int m2mtest_probe(struct platform_device *pdev) return 0; - v4l2_m2m_release(dev-m2m_dev); err_m2m: + v4l2_m2m_release(dev-m2m_dev); video_unregister_device(dev-vfd); rel_vdev: video_device_release(vfd); -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] [media] mem2mem_testdev: Add missing braces around sizeof
Fixes the following checkpatch warnings: WARNING: sizeof *ctx should be sizeof(*ctx) WARNING: sizeof *dev should be sizeof(*dev) Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/mem2mem_testdev.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index fc95559..570e880 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -895,7 +895,7 @@ static int m2mtest_open(struct file *file) if (mutex_lock_interruptible(dev-dev_mutex)) return -ERESTARTSYS; - ctx = kzalloc(sizeof *ctx, GFP_KERNEL); + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) { rc = -ENOMEM; goto open_unlock; @@ -1020,7 +1020,7 @@ static int m2mtest_probe(struct platform_device *pdev) struct video_device *vfd; int ret; - dev = kzalloc(sizeof *dev, GFP_KERNEL); + dev = kzalloc(sizeof(*dev), GFP_KERNEL); if (!dev) return -ENOMEM; -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] [media] mem2mem_testdev: Use pr_err instead of printk
printk(KERN_ERR...) is replaced with pr_err to silence checkpatch warning. WARNING: Prefer netdev_err(netdev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/mem2mem_testdev.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index 570e880..f7d15ec 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -397,8 +397,7 @@ static void device_isr(unsigned long priv) curr_ctx = v4l2_m2m_get_curr_priv(m2mtest_dev-m2m_dev); if (NULL == curr_ctx) { - printk(KERN_ERR - Instance released before the end of transaction\n); + pr_err(Instance released before the end of transaction\n); return; } -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] [media] mem2mem_testdev: Use devm_kzalloc() in probe
devm_kzalloc() makes error handling and cleanup simpler. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/mem2mem_testdev.c |7 ++- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index f7d15ec..cd1c844 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -1019,7 +1019,7 @@ static int m2mtest_probe(struct platform_device *pdev) struct video_device *vfd; int ret; - dev = kzalloc(sizeof(*dev), GFP_KERNEL); + dev = devm_kzalloc(pdev-dev, sizeof(*dev), GFP_KERNEL); if (!dev) return -ENOMEM; @@ -1027,7 +1027,7 @@ static int m2mtest_probe(struct platform_device *pdev) ret = v4l2_device_register(pdev-dev, dev-v4l2_dev); if (ret) - goto free_dev; + return ret; atomic_set(dev-num_inst, 0); mutex_init(dev-dev_mutex); @@ -1073,8 +1073,6 @@ rel_vdev: video_device_release(vfd); unreg_dev: v4l2_device_unregister(dev-v4l2_dev); -free_dev: - kfree(dev); return ret; } @@ -1089,7 +1087,6 @@ static int m2mtest_remove(struct platform_device *pdev) del_timer_sync(dev-timer); video_unregister_device(dev-vfd); v4l2_device_unregister(dev-v4l2_dev); - kfree(dev); return 0; } -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Freeze or Oops on recent kernels
I'm not going to file a bug for this on bugzilla because it's ancient and not a new bug. But I can forward it to linux-media and the get_maintainer.pl people for cx23885_video. HINT: We only care about the very most recent kernel. If you can take a photo of the stack trace, then file a bug report and attach the .jpg. regards, dan carpenter On Fri, Sep 07, 2012 at 09:24:13PM +1000, yvahk-xre...@zacglen.net wrote: I am getting either a a kernel Oops or freeze (without any console output) on recent kernels. I have tested on 2.6.32.26 PAE, 3.1.9 PAE, and 3.4.9 PAE all with similar results. The hardware involved comprises mainly: 12 GB ram Intel DX58S02 motherboard Intel Xeon E5220 2 x onboard ethernet Intel PRO/1000 PT Dual Port ethernet Hauppauge HVR1700 DVB-T PCIe card Technisat SkyStar2 DVB-T PCI card The oops or freeze occurs when both DVB cards are recording simultaneously. With either card installed on their own there is never any problem. I should also add that the exact same kernel and cards on a Gigabyte GA-P31-S3G motherboard + Intel Pentium 4 the problem NEVER occurs. So there may be a DX58S02/timing/interrupt issue. When there is an oops it is like this (hand-transcribed from 2.6.32.26 PAE kernel): c0789444 panic + 3e/e9 oops_end + 97/a6 no_context + 13b/145 __bad_area_nosemaphore + ec/f4 ? do_page_fault + 0/29f bad_area_nosemaphore + 12/15 do_page_fault + 139/29f ? do_page_fault + 139/29f c078b54b error_code + 73/78 f82084fb ? cx23885_video_irq + d8/1dc f820b04d cx23885_irq + 3df/3fe c04834ac handle_irq_event + 57/fd handle_fasteoi_irq + 6f/a2 handle_irq + 40/4d do_IRQ + 46/9a common_interrupt + 30/38 c045b7a9 ? prepare_to_wait + 14c f7fef5a5 ? videobug_waitm + 90/133 c045b601 ? autoremove_waker_function + 0/34 f805e5bc videobuf_dvb_thread + 73/135 f805e4f9 ? videobuf_dvb_thread + 0/135 c045b3c9 kthread + 64/69 ? kthread + 0/69 kernel_thread_helper + 7/10 Although I am a very experienced programmer I have next to zero kernel expertise except for minor patching of a few drivers. My guess from the stack trace is that there might be an issue with page fault recursion (if that is at all possible). Anyhow I don't want to waste too much of my time or anybody elses on this - although with the problem occuring with 3.4.9 kernel which has significant interrupt handling changes it probably is something that somebody might want to know about. If anybody can spot a clue as to where I should be looking and how I should go about isolating the problem (if only kernel core dumped!) please let me know and I will possibly try and assist. I need some guidance. Regards John W. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- 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/4] [media] mem2mem_testdev: Fix incorrect location of v4l2_m2m_release()
Hello, On Monday, September 24, 2012 8:18 AM Sachin Kamat wrote: v4l2_m2m_release() was placed after the return statement and outside any of the goto labels and hence was not getting executed under the error exit path. This patch moves it under the exit path label. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Acked-by: Marek Szyprowski m.szyprow...@samsung.com --- drivers/media/platform/mem2mem_testdev.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index 771a84f..fc95559 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -1067,8 +1067,8 @@ static int m2mtest_probe(struct platform_device *pdev) return 0; - v4l2_m2m_release(dev-m2m_dev); err_m2m: + v4l2_m2m_release(dev-m2m_dev); video_unregister_device(dev-vfd); rel_vdev: video_device_release(vfd); -- 1.7.4.1 Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/4] [media] mem2mem_testdev: Use devm_kzalloc() in probe
Hello, On Monday, September 24, 2012 8:18 AM Sachin Kamat wrote: devm_kzalloc() makes error handling and cleanup simpler. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Acked-by: Marek Szyprowski m.szyprow...@samsung.com --- drivers/media/platform/mem2mem_testdev.c |7 ++- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c index f7d15ec..cd1c844 100644 --- a/drivers/media/platform/mem2mem_testdev.c +++ b/drivers/media/platform/mem2mem_testdev.c @@ -1019,7 +1019,7 @@ static int m2mtest_probe(struct platform_device *pdev) struct video_device *vfd; int ret; - dev = kzalloc(sizeof(*dev), GFP_KERNEL); + dev = devm_kzalloc(pdev-dev, sizeof(*dev), GFP_KERNEL); if (!dev) return -ENOMEM; @@ -1027,7 +1027,7 @@ static int m2mtest_probe(struct platform_device *pdev) ret = v4l2_device_register(pdev-dev, dev-v4l2_dev); if (ret) - goto free_dev; + return ret; atomic_set(dev-num_inst, 0); mutex_init(dev-dev_mutex); @@ -1073,8 +1073,6 @@ rel_vdev: video_device_release(vfd); unreg_dev: v4l2_device_unregister(dev-v4l2_dev); -free_dev: - kfree(dev); return ret; } @@ -1089,7 +1087,6 @@ static int m2mtest_remove(struct platform_device *pdev) del_timer_sync(dev-timer); video_unregister_device(dev-vfd); v4l2_device_unregister(dev-v4l2_dev); - kfree(dev); return 0; } -- 1.7.4.1 Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
HVR 4000 and DVB-API
The WinTV HVR-4000-HD is a multi-tuners TV card with 2 dvb tuners. It look like its driver doesn't have been updated to the new DVB-API. The problem I get with dvr is than vdr only seam to recognize and use the first tuner of my Hauppauge WinTV HVR-4000-HD. The devices are in /dev/dvb/adapter0: # ls /dev/dvb/adapter0 demux0 demux1 dvr0 dvr1 frontend0 frontend1 net0 net1 frontend0 is the dvb-s tuner and frontedn1 is the dvb-t/c tuner, and I want to be able to use both of them. It is a hardware limitation in that card (among other) that make not possible to use those 2 tuners at the same time. When starting vdr, the only error message I get at the console is, when I make udev to create symlinks like /dev/dvb/adapter1/*0 to /dev/dvb/adapter0/*1: * VDR errors from /var/log/messages: * ERROR: /dev/dvb/adapter1/frontend0: Device or resource busy I can use mplayer instead of vdr. It work with the symiliks with mplayer dvb://2@, and without the symlinks by acceding directly the adapter0/drv1 device after tuning with tzap. When trying to access one of the dvb-t channels from shmclient, it tell me Channel not available. vdr holds an opened handle on the frontend during its running time. Since the DVB-S frontend is the first one, DVB-T does not work. Some months ago the linux-media DVB-API was extended so multiple delivery systems can be accessed through one frontend, if they are mutually exclusive. Sadly the driver for the HVR 4000 was forgotten. I don't want to buy another TV card or a TV set. So, can I hope than, in the reasonably near future, someone on this list will get some time to fix and update the HVR 4000 driver? Dominique -- We have the heroes we deserve. -- 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] Timestamps and V4L2
On Sun September 23 2012 15:07:24 Sakari Ailus wrote: Hi Hans, Hans Verkuil wrote: On Sat September 22 2012 14:38:07 Sakari Ailus wrote: Hi Hans, Thanks for the comments. Hans Verkuil wrote: On Thu September 20 2012 22:21:22 Sakari Ailus wrote: ... Capability flag for monotonic timestamps A capability flag can be used to tell whether the timestamp is monotonic. However, it's not extensible cleanly to provide selectable timestamps. These are not features that are needed right now, though. The upside of this option is ease of implementation and use, but it's not extensible. Also we're left with a flag that's set for all drivers: in the end it provides no information to the user and is only noise in the spec. Control for timestamp type -- Using a control to tell the type of the timestamp is extensible but not as easy to implement than the capability flag: each and every device would get an additional control. The value should likely be also file handle specific, and we do not have file handle specific controls yet. Yes, we do. You can make per-file handle controls. M2M devices need that. Thanks for correcting me. I'm not sure why this would be filehandle specific, BTW. Good point. I thought that as other properties of the buffers are specific to file handles, including format when using CREATE_BUFS, it'd make sense to make the timestamp source file-handle specific as well. What do you think? I don't think it makes sense to have different streams from the same device use different clocks. In the meantime the control could be read-only, and later made read-write when the timestamp type can be made selectable. Much of he work of timestamping can be done by the framework: drivers can use a single helper function and need to create one extra standard control. Should the control also have an effect on the types of the timestamps in V4L2 events? Likely yes. You are missing one other option: Using v4l2_buffer flags to report the clock --- By defining flags like this: V4L2_BUF_FLAG_CLOCK_MASK 0x7000 /* Possible Clocks */ V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x /* system or monotonic, we don't know */ V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 you could tell the application which clock is used. This does allow for more clocks to be added in the future and clock selection would then be done by a control or possibly an ioctl. For now there are no plans to do such things, so this flag should be sufficient. And it can be implemented very efficiently. It works with existing drivers as well, since they will report CLOCK_UNKNOWN. I am very much in favor of this approach. Thanks for adding this. I knew I was forgetting something but didn't remember what --- I swear it was unintentional! :-) If we'd add more clocks without providing an ability to choose the clock from the user space, how would the clock be selected? It certainly isn't the driver's job, nor I think it should be system-specific either (platform data on embedded systems). IF a driver supports more than one clock (which I really don't see happening anytime soon), then we either need a control to select the clock or an ioctl. The timestamps can be taken from any standard clock using a helper function. Right now drivers call do_gettimeofday() or ktime_get_ts(), it's the same whether the function was called e.g. v4l2_get_ts(). The driver doesn't even need to know which timer is being used to make that timestamp. It used to be the realtime clock and soon it's the monotonic clock. So I really don't see why the timestamp source should depend on the driver. It rather depends on what the user wants. That said, I also don't see use for other clocks _right now_ than the monotonic one. And something as well to enumerate the available clocks. I'm leaning towards ioctls, but I think this should be decided if we ever get an actual use-case for this. I think it should be a control --- I see no reason to add new IOCTLs when controls are equally, or even better, fit for the same job. But let's decide this only when the functionality is needed. Yes, please. I think it is premature to talk about how to select a clock, since I doubt we will ever need it. We just need a way to tell the app which clock is being used (which in practice is either UNKNOWN or MONOTONIC), and keep the option of adding other clocks open. For M2M devices it is possible to select different clocks per-filehandle, so that precludes having a QUERYCAP capability (that was a good point that Sylwester made). Adding a clock mask to the buffer flags does exactly what we need IMHO. It's up to the application and its needs. That would suggest we should always provide monotonic
Re: [PATCH] [media] s5p-tv: Fix potential NULL pointer dereference error
On 09/22/2012 09:39 AM, Sachin Kamat wrote: When mdev is NULL, the error print statement will try to dereference the NULL pointer. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/s5p-tv/mixer_drv.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/platform/s5p-tv/mixer_drv.c b/drivers/media/platform/s5p-tv/mixer_drv.c index a15ca05..ca0f297 100644 --- a/drivers/media/platform/s5p-tv/mixer_drv.c +++ b/drivers/media/platform/s5p-tv/mixer_drv.c @@ -384,7 +384,7 @@ static int __devinit mxr_probe(struct platform_device *pdev) mdev = kzalloc(sizeof *mdev, GFP_KERNEL); if (!mdev) { - mxr_err(mdev, not enough memory.\n); + dev_err(dev, not enough memory.\n); ret = -ENOMEM; goto fail; } Acked-by: Tomasz Stanislawski t.stanisl...@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
[PATCH] [media] s5p-fimc: Use the new linux/sizes.h header file
Replaces asm/sizes.h with linux/sizes.h. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/s5p-fimc/fimc-core.h |2 +- drivers/media/platform/s5p-fimc/fimc-lite.h |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-core.h b/drivers/media/platform/s5p-fimc/fimc-core.h index 808ccc6..e787f65 100644 --- a/drivers/media/platform/s5p-fimc/fimc-core.h +++ b/drivers/media/platform/s5p-fimc/fimc-core.h @@ -17,7 +17,7 @@ #include linux/types.h #include linux/videodev2.h #include linux/io.h -#include asm/sizes.h +#include linux/sizes.h #include media/media-entity.h #include media/videobuf2-core.h diff --git a/drivers/media/platform/s5p-fimc/fimc-lite.h b/drivers/media/platform/s5p-fimc/fimc-lite.h index 44424ee..8dc3e9b 100644 --- a/drivers/media/platform/s5p-fimc/fimc-lite.h +++ b/drivers/media/platform/s5p-fimc/fimc-lite.h @@ -9,7 +9,7 @@ #ifndef FIMC_LITE_H_ #define FIMC_LITE_H_ -#include asm/sizes.h +#include linux/sizes.h #include linux/io.h #include linux/irqreturn.h #include linux/platform_device.h -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 2/14] drivers/media/platform/soc_camera/mx2_camera.c: fix error return code
From: Peter Senna Tschudin peter.se...@gmail.com Convert a nonnegative error return code to a negative one, as returned elsewhere in the function. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // smpl ( if@p1 (\(ret 0\|ret != 0\)) { ... return ret; } | ret@p1 = 0 ) ... when != ret = e1 when != ret *if(...) { ... when != ret = e2 when forall return ret; } // /smpl Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- drivers/media/platform/soc_camera/mx2_camera.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c index 256187f..01d7c11 100644 --- a/drivers/media/platform/soc_camera/mx2_camera.c +++ b/drivers/media/platform/soc_camera/mx2_camera.c @@ -1800,12 +1800,14 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev) if (!res_emma || !irq_emma) { dev_err(pdev-dev, no EMMA resources\n); + err = -ENODEV; goto exit_free_irq; } pcdev-res_emma = res_emma; pcdev-irq_emma = irq_emma; - if (mx27_camera_emma_init(pcdev)) + err = mx27_camera_emma_init(pcdev); + if (err) goto exit_free_irq; } -- 1.7.11.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 v2] drivers/media/platform/s5p-tv/sdo_drv.c: fix error return code
Hi. On 09/06/2012 10:38 AM, Peter Senna Tschudin wrote: From: Peter Senna Tschudin peter.se...@gmail.com Convert a nonnegative error return code to a negative one, as returned elsewhere in the function. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // smpl ( if@p1 (\(ret 0\|ret != 0\)) { ... return ret; } | ret@p1 = 0 ) ... when != ret = e1 when != ret *if(...) { ... when != ret = e2 when forall return ret; } // /smpl Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com Acked-by: Tomasz Stanislawski t.stanisl...@samsung.com --- drivers/media/platform/s5p-tv/sdo_drv.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/s5p-tv/sdo_drv.c b/drivers/media/platform/s5p-tv/sdo_drv.c index ad68bbe..58cf56d 100644 --- a/drivers/media/platform/s5p-tv/sdo_drv.c +++ b/drivers/media/platform/s5p-tv/sdo_drv.c @@ -369,6 +369,7 @@ static int __devinit sdo_probe(struct platform_device *pdev) sdev-fout_vpll = clk_get(dev, fout_vpll); if (IS_ERR_OR_NULL(sdev-fout_vpll)) { dev_err(dev, failed to get clock 'fout_vpll'\n); + ret = -ENXIO; goto fail_dacphy; } dev_info(dev, fout_vpll.rate = %lu\n, clk_get_rate(sclk_vpll)); @@ -377,11 +378,13 @@ static int __devinit sdo_probe(struct platform_device *pdev) sdev-vdac = devm_regulator_get(dev, vdd33a_dac); if (IS_ERR_OR_NULL(sdev-vdac)) { dev_err(dev, failed to get regulator 'vdac'\n); + ret = -ENXIO; goto fail_fout_vpll; } sdev-vdet = devm_regulator_get(dev, vdet); if (IS_ERR_OR_NULL(sdev-vdet)) { dev_err(dev, failed to get regulator 'vdet'\n); + ret = -ENXIO; goto fail_fout_vpll; } -- 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: [PATCH v2] drivers/media/platform/s5p-tv/sdo_drv.c: fix error return code
On 09/23/2012 09:30 PM, Mauro Carvalho Chehab wrote: Sylwester, Please review. Regards, Mauro Mensagem original Assunto: [PATCH v2] drivers/media/platform/s5p-tv/sdo_drv.c: fix error return code Data: Thu, 6 Sep 2012 10:38:29 +0200 De: Peter Senna Tschudin peter.se...@gmail.com Para: peter.se...@gmail.com, Mauro Carvalho Chehab mche...@infradead.org CC: linux-media@vger.kernel.org, linux-ker...@vger.kernel.org From: Peter Senna Tschudin peter.se...@gmail.com Convert a nonnegative error return code to a negative one, as returned elsewhere in the function. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // smpl ( if@p1 (\(ret 0\|ret != 0\)) { ... return ret; } | ret@p1 = 0 ) ... when != ret = e1 when != ret *if(...) { ... when != ret = e2 when forall return ret; } // /smpl Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- drivers/media/platform/s5p-tv/sdo_drv.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/s5p-tv/sdo_drv.c b/drivers/media/platform/s5p-tv/sdo_drv.c index ad68bbe..58cf56d 100644 --- a/drivers/media/platform/s5p-tv/sdo_drv.c +++ b/drivers/media/platform/s5p-tv/sdo_drv.c @@ -369,6 +369,7 @@ static int __devinit sdo_probe(struct platform_device *pdev) sdev-fout_vpll = clk_get(dev, fout_vpll); if (IS_ERR_OR_NULL(sdev-fout_vpll)) { dev_err(dev, failed to get clock 'fout_vpll'\n); + ret = -ENXIO; This patch improves the situation but it doesn't look like a proper fix to me. First of all clk_get() return code should be checked only with IS_ERR() and the error code should be propagated up to the caller, rather than overwriting it with -ENXIO. It's important especially in cases where, e.g we are getting EPROBE_DEFER instead of a valid resource. goto fail_dacphy; } dev_info(dev, fout_vpll.rate = %lu\n, clk_get_rate(sclk_vpll)); @@ -377,11 +378,13 @@ static int __devinit sdo_probe(struct platform_device *pdev) sdev-vdac = devm_regulator_get(dev, vdd33a_dac); if (IS_ERR_OR_NULL(sdev-vdac)) { dev_err(dev, failed to get regulator 'vdac'\n); + ret = -ENXIO; Same situation here.. goto fail_fout_vpll; } sdev-vdet = devm_regulator_get(dev, vdet); if (IS_ERR_OR_NULL(sdev-vdet)) { dev_err(dev, failed to get regulator 'vdet'\n); + ret = -ENXIO; goto fail_fout_vpll; } end here. I think something more like this is needed (I'm going to queue this patch for 3.7): 8-- From 8d3ce303461a2c09991efc5b86a0f56789045fa9 Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki s.nawro...@samsung.com Date: Mon, 24 Sep 2012 10:37:51 +0200 Subject: [PATCH] s5p-tv: Fix return value in sdo_probe() on error paths Use proper return value test for clk_get() and devm_regulator_get() functions and propagate any errors from the clock and the regulator subsystem to the driver core. In two cases a proper error code is now returned rather than 0. Reported-by: Peter Senna Tschudin peter.se...@gmail.com Cc: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/s5p-tv/sdo_drv.c | 25 ++--- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/s5p-tv/sdo_drv.c b/drivers/media/platform/s5p-tv/sdo_drv.c index ad68bbe..2d1a654 100644 --- a/drivers/media/platform/s5p-tv/sdo_drv.c +++ b/drivers/media/platform/s5p-tv/sdo_drv.c @@ -341,47 +341,50 @@ static int __devinit sdo_probe(struct platform_device *pdev) /* acquire clocks */ sdev-sclk_dac = clk_get(dev, sclk_dac); - if (IS_ERR_OR_NULL(sdev-sclk_dac)) { + if (IS_ERR(sdev-sclk_dac)) { dev_err(dev, failed to get clock 'sclk_dac'\n); - ret = -ENXIO; + ret = PTR_ERR(sdev-sclk_dac); goto fail; } sdev-dac = clk_get(dev, dac); - if (IS_ERR_OR_NULL(sdev-dac)) { + if (IS_ERR(sdev-dac)) { dev_err(dev, failed to get clock 'dac'\n); - ret = -ENXIO; + ret = PTR_ERR(sdev-dac); goto fail_sclk_dac; } sdev-dacphy = clk_get(dev, dacphy); - if (IS_ERR_OR_NULL(sdev-dacphy)) { + if (IS_ERR(sdev-dacphy)) { dev_err(dev, failed to get clock 'dacphy'\n); - ret = -ENXIO; + ret = PTR_ERR(sdev-dacphy); goto fail_dac; } sclk_vpll = clk_get(dev, sclk_vpll); - if (IS_ERR_OR_NULL(sclk_vpll)) { + if (IS_ERR(sclk_vpll)) { dev_err(dev, failed to get clock 'sclk_vpll'\n); -
[PATCH] media: davinci: vpif: display: separate out subdev from output
From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 138 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 185 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PIN GPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPONENT_ID, + }, + { + .output = { + .index = 2, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config dm646x_vpif_display_config = { .set_clock = set_vpif_clock, .subdevinfo = dm646x_vpif_subdev, .subdev_count = ARRAY_SIZE(dm646x_vpif_subdev), - .output = output, - .output_count = ARRAY_SIZE(output), + .chan_config[0] = { + .outputs = dm6467_ch0_outputs, + .output_count = ARRAY_SIZE(dm6467_ch0_outputs), + }, .card_name = DM646x EVM, }; diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c index 8d1ce09..b218f3a 100644 --- a/drivers/media/platform/davinci/vpif_display.c +++
[PATCH RFC] s5p-tv: Report only multi-plane capabilities in vidioc_querycap
The mixer video node supports only multi-planar API so the driver should not be setting V4L2_CAP_VIDEO_OUTPUT flags. Fix this and also switch to device_caps. Additionally fix the VIDIOC_ENUM_FMT ioctl handler which now works for V4L2_BUF_TYPE_CAPTURE, rather than expected V4L2_BUF_TYPE_CAPTURE_MPLANE. Cc: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/s5p-tv/mixer_video.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/s5p-tv/mixer_video.c b/drivers/media/platform/s5p-tv/mixer_video.c index 8649de01..9876bd9 100644 --- a/drivers/media/platform/s5p-tv/mixer_video.c +++ b/drivers/media/platform/s5p-tv/mixer_video.c @@ -164,9 +164,8 @@ static int mxr_querycap(struct file *file, void *priv, strlcpy(cap-driver, MXR_DRIVER_NAME, sizeof cap-driver); strlcpy(cap-card, layer-vfd.name, sizeof cap-card); sprintf(cap-bus_info, %d, layer-idx); - cap-version = KERNEL_VERSION(0, 1, 0); - cap-capabilities = V4L2_CAP_STREAMING | - V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VIDEO_OUTPUT_MPLANE; + cap-device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_OUTPUT_MPLANE; + cap-capabilities = cap-device_caps | V4L2_CAP_DEVICE_CAPS; return 0; } @@ -727,7 +726,7 @@ static int mxr_streamoff(struct file *file, void *priv, enum v4l2_buf_type i) static const struct v4l2_ioctl_ops mxr_ioctl_ops = { .vidioc_querycap = mxr_querycap, /* format handling */ - .vidioc_enum_fmt_vid_out = mxr_enum_fmt, + .vidioc_enum_fmt_vid_out_mplane = mxr_enum_fmt, .vidioc_s_fmt_vid_out_mplane = mxr_s_fmt, .vidioc_g_fmt_vid_out_mplane = mxr_g_fmt, /* buffer control */ -- 1.7.11.3 -- 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] media: davinci: vpif: display: separate out subdev from output
On Mon September 24 2012 12:02:26 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 138 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 185 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PINGPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPONENT_ID, + }, + { + .output = { + .index = 2, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config dm646x_vpif_display_config = { .set_clock = set_vpif_clock, .subdevinfo = dm646x_vpif_subdev, .subdev_count = ARRAY_SIZE(dm646x_vpif_subdev), - .output = output, - .output_count = ARRAY_SIZE(output), + .chan_config[0] = { + .outputs = dm6467_ch0_outputs, + .output_count = ARRAY_SIZE(dm6467_ch0_outputs), + }, .card_name = DM646x EVM, }; diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c index 8d1ce09..b218f3a 100644 ---
Re: [PATCH] media: davinci: vpif: display: separate out subdev from output
Hi Hans, Thanks for the review. On Mon, Sep 24, 2012 at 3:39 PM, Hans Verkuil hverk...@xs4all.nl wrote: On Mon September 24 2012 12:02:26 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 138 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 185 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PINGPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPONENT_ID, + }, + { + .output = { + .index = 2, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config dm646x_vpif_display_config = { .set_clock = set_vpif_clock, .subdevinfo = dm646x_vpif_subdev, .subdev_count = ARRAY_SIZE(dm646x_vpif_subdev), - .output = output, - .output_count = ARRAY_SIZE(output), + .chan_config[0] = { + .outputs = dm6467_ch0_outputs, + .output_count = ARRAY_SIZE(dm6467_ch0_outputs), + }, .card_name = DM646x EVM, }; diff --git a/drivers/media/platform/davinci/vpif_display.c
[PATCH v2] media: davinci: vpif: display: separate out subdev from output
From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sekhar Nori nsek...@ti.com --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) Changes for V2: 1: Changed v4l2_device_call_until_err() call to v4l2_subdev_call() for s_routing, since this call is for specific subdev, pointed out by Hans. arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 136 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 183 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PIN GPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPONENT_ID, + }, + { + .output = { + .index = 2, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config dm646x_vpif_display_config = { .set_clock = set_vpif_clock, .subdevinfo = dm646x_vpif_subdev, .subdev_count = ARRAY_SIZE(dm646x_vpif_subdev), - .output = output, - .output_count = ARRAY_SIZE(output), + .chan_config[0] = { + .outputs = dm6467_ch0_outputs, + .output_count = ARRAY_SIZE(dm6467_ch0_outputs), + }, .card_name = DM646x EVM, }; diff --git
Re: [PATCH 4/4] gspca_pac7302: add support for green balance adjustment
Hi, Sorry for the slow response ... On 09/20/2012 01:54 PM, Frank Schäfer wrote: Hi, Am 20.09.2012 11:08, schrieb Hans de Goede: snip Many webcams have RGB gains, but we don't have standard CID-s for these, instead we've Blue and Red Balance. This has grown historically because of the bttv cards which actually have Blue and Red balance controls in hardware, rather then the usual RGB gain triplet. Various gspca drivers cope with this in different ways. If you look at the pac7302 driver before your latest 4 patches it has a Red and Blue balance control controlling the Red and Blue gain, and a Whitebalance control, which is not White balance at all, but simply controls the green gain... Ok, so if I understand you right, red+green+blue balance = white balance. And because we already have defined red, blue and whitebalance controls for historical reasons, we don't need green balance ? Maybe that matches physics, but I don't think it's a sane approach for a user interface... No what I was trying to say is that the balance controls are for hardware which actually has balance controls and not per color gains (such as the bt87x chips), but they are being abused by many drivers to add support for per color gains. And then you miss one control. And in the case of the pac7302 driver the original route was taken of using whitebalance to control the green gain. Which is wrong IMHO, but it is what the driver does know. A proper fix would be to introduce new controls for all 3 gains, and use those instead of using the balance controls + adding a 3th balance control being discussed in the thread titled: Gain controls in v4l2-ctrl framework. And as said other drivers have similar (albeit usually different) hacks. At a minimum I would like you to rework your patches to: 1) Not add the new Green balance, and instead modify the existing whitebalance to control the new green gain you've found. Keeping things as broken as they are, but not worse; and I prefer waiting for the results of the discussion you are proposing further down. I see in your next mail that you've changed your mind. So I would like to move forward with this by adding your 2 patches + 1 more patch to also make the whitebalance control (which really is the green gain control) use 0x02 rather then 0xc6. To do this we must make sure that 0xc6 has a proper fixed / default setting. So what does the windows driver use for this? 1 like with 0xc5 and 0xc7 ? And can you do a 3th patch to make the whitebalance control control 0x02 rather then 0xc6 like you did for red and blue balance? Then later on when the Gain controls in v4l2-ctrl framework discussion is done we can change these controls to the new controls. 2) Try to use both the page 0 reg 0x01 - 0x03 and page 0 reg 0xc5 - 0xc7 at the same time to get a wider ranged control. Maybe 0xc5 - 0xc7 are simply the most significant bits of a wider ranged gain ? I don't think so. The windows driver does not use them. It even doesn't use the full range of registers 0x01-0x03. Of course, I have expermiented with the full range and it works, but it doesn't make much sense to use it. Ok. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi, On 09/23/2012 01:26 PM, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE Not all sensors have separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE, so we will need a separate control for sensors which have one combined gain called simply V4L2_CID_GAIN_GREEN Also do we really need separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE controls? I know hardware has them, but in my experience that is only done as it is simpler to make the hardware this way (fully symmetric sensor grid), have you ever tried actually using different gain settings for the 2 different green rows ? I've and that always results in an ugly checker board pattern. So I think we can and should only have a V4L2_CID_GAIN_GREEN, and for sensors with 2 green gains have that control both, forcing both to always have the same setting, which is really what you want anyways ... 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET GAIN_OFFSET that sounds a bit weird... GAIN_OFFSET sounds like it is a number which gets added to the 3/4 gain settings before the gain gets applied, but I assume that you just mean a number which gets added to the value from the pixel, either before or after the gain is applied and I must admit I cannot come up with a better name. I believe (not sure) that some sensors have these per color ... The question is if it makes sense to actually control this per color though, I don't think it does as it is meant to compensate for any fixed measuring errors, which are the same for all 3/4 colors. Note that all the sensor cells are exactly the same, later on a color grid gets added on top of the sensors to turn them into r/g/b cells, but physically they are the same cells, so with the same process and temperature caused measuring errors... Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators
This patch adds support for prepare/finish callbacks in VB2 allocators. These callback are used for buffer flushing. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Federico Vaga federico.v...@gmail.com --- drivers/media/v4l2-core/videobuf2-core.c | 11 +++ include/media/videobuf2-core.h | 7 +++ 2 file modificati, 18 inserzioni(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 4da3df6..079fa79 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) { struct vb2_queue *q = vb-vb2_queue; unsigned long flags; + unsigned int plane; if (vb-state != VB2_BUF_STATE_ACTIVE) return; @@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) dprintk(4, Done processing on buffer %d, state: %d\n, vb-v4l2_buf.index, vb-state); + /* sync buffers */ + for (plane = 0; plane vb-num_planes; ++plane) + call_memop(q, finish, vb-planes[plane].mem_priv); + /* Add the buffer to the done buffers list */ spin_lock_irqsave(q-done_lock, flags); vb-state = state; @@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b) static void __enqueue_in_driver(struct vb2_buffer *vb) { struct vb2_queue *q = vb-vb2_queue; + unsigned int plane; vb-state = VB2_BUF_STATE_ACTIVE; atomic_inc(q-queued_count); + + /* sync buffers */ + for (plane = 0; plane vb-num_planes; ++plane) + call_memop(q, prepare, vb-planes[plane].mem_priv); + q-ops-buf_queue(vb); } diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 8dd9b6c..2508609 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -41,6 +41,10 @@ struct vb2_fileio_data; * argument to other ops in this structure * @put_userptr: inform the allocator that a USERPTR buffer will no longer * be used + * @prepare: called every time the buffer is passed from userspace to the + * driver, usefull for cache synchronisation, optional + * @finish:called every time the buffer is passed back from the driver + * to the userspace, also optional * @vaddr: return a kernel virtual address to a given memory buffer * associated with the passed private structure or NULL if no * such mapping exists @@ -65,6 +69,9 @@ struct vb2_mem_ops { unsigned long size, int write); void(*put_userptr)(void *buf_priv); + void(*prepare)(void *buf_priv); + void(*finish)(void *buf_priv); + void*(*vaddr)(void *buf_priv); void*(*cookie)(void *buf_priv); -- 1.7.11.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
[PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
The DMA streaming allocator is similar to the DMA contig but it use the DMA streaming interface (dma_map_single, dma_unmap_single). The allocator allocates buffers and immediately map the memory for DMA transfer. For each buffer prepare/finish it does a DMA synchronization. Signed-off-by: Federico Vaga federico.v...@gmail.com --- drivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 1 + drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++ include/media/videobuf2-dma-streaming.h | 32 4 file modificati, 243 inserzioni(+) create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c create mode 100644 include/media/videobuf2-dma-streaming.h diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig index 0c54e19..60548a7 100644 --- a/drivers/media/v4l2-core/Kconfig +++ b/drivers/media/v4l2-core/Kconfig @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG #depends on HAS_DMA select VIDEOBUF2_CORE select VIDEOBUF2_MEMOPS + +config VIDEOBUF2_DMA_STREAMING + select VIDEOBUF2_CORE + select VIDEOBUF2_MEMOPS + tristate diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index c2d61d4..0b2756f 100644 --- a/drivers/media/v4l2-core/Makefile +++ b/drivers/media/v4l2-core/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o ccflags-y += -I$(srctree)/drivers/media/dvb-core ccflags-y += -I$(srctree)/drivers/media/dvb-frontends diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2-core/videobuf2-dma-streaming.c new file mode 100644 index 000..c839e05 --- /dev/null +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c @@ -0,0 +1,205 @@ +/* + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2 + * + * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/module.h +#include linux/slab.h +#include linux/pagemap.h +#include linux/dma-mapping.h + +#include media/videobuf2-core.h +#include media/videobuf2-dma-streaming.h +#include media/videobuf2-memops.h + +struct vb2_streaming_conf { + struct device *dev; +}; +struct vb2_streaming_buf { + struct vb2_streaming_conf *conf; + void*vaddr; + + dma_addr_t dma_handle; + + unsigned long size; + struct vm_area_struct *vma; + + atomic_trefcount; + struct vb2_vmarea_handler handler; +}; + +static void vb2_dma_streaming_put(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (atomic_dec_and_test(buf-refcount)) { + dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size, +DMA_FROM_DEVICE); + free_pages_exact(buf-vaddr, buf-size); + kfree(buf); + } + +} + +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size) +{ + struct vb2_streaming_conf *conf = alloc_ctx; + struct vb2_streaming_buf *buf; + int err; + + buf = kzalloc(sizeof(struct vb2_streaming_buf), GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA); + if (!buf-vaddr) { + err = -ENOMEM; + goto out; + } + buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size, +DMA_FROM_DEVICE); + err = dma_mapping_error(conf-dev, buf-dma_handle); + if (err) { + dev_err(conf-dev, dma_map_single failed\n); + + free_pages_exact(buf-vaddr, size); + buf-vaddr = NULL; + goto out_pages; + } + buf-conf = conf; + buf-size = size; + buf-handler.refcount = buf-refcount; + buf-handler.put = vb2_dma_streaming_put; + buf-handler.arg = buf; + + atomic_inc(buf-refcount); + return buf; + +out_pages: + free_pages_exact(buf-vaddr, buf-size); +out: + kfree(buf); + return ERR_PTR(err); +} + +static void *vb2_dma_streaming_cookie(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return buf-dma_handle; +} + +static void *vb2_dma_streaming_vaddr(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (!buf) + return NULL; + return buf-vaddr;
[PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
This patch re-write the driver and use the videobuf2 interface instead of the old videobuf. Moreover, it uses also the control framework which allows the driver to inherit controls from its subdevice (ADV7180) Signed-off-by: Federico Vaga federico.v...@gmail.com Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com --- drivers/media/pci/sta2x11/Kconfig |2 +- drivers/media/pci/sta2x11/sta2x11_vip.c | 1238 ++- 2 file modificati, 407 inserzioni(+), 833 rimozioni(-) diff --git a/drivers/media/pci/sta2x11/Kconfig b/drivers/media/pci/sta2x11/Kconfig index 6749f67..654339f 100644 --- a/drivers/media/pci/sta2x11/Kconfig +++ b/drivers/media/pci/sta2x11/Kconfig @@ -2,7 +2,7 @@ config STA2X11_VIP tristate STA2X11 VIP Video For Linux depends on STA2X11 select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT - select VIDEOBUF_DMA_CONTIG + select VIDEOBUF2_DMA_STREAMING depends on PCI VIDEO_V4L2 VIRT_TO_BUS help Say Y for support for STA2X11 VIP (Video Input Port) capture diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c index 4c10205..b9ff926 100644 --- a/drivers/media/pci/sta2x11/sta2x11_vip.c +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c @@ -1,6 +1,7 @@ /* * This is the driver for the STA2x11 Video Input Port. * + * Copyright (C) 2012 ST Microelectronics * Copyright (C) 2010 WindRiver Systems, Inc. * * This program is free software; you can redistribute it and/or modify it @@ -19,36 +20,30 @@ * The full GNU General Public License is included in this distribution in * the file called COPYING. * - * Author: Andreas Kies andreas.k...@windriver.com - * Vlad Lungu vlad.lu...@windriver.com - * */ #include linux/types.h #include linux/kernel.h #include linux/module.h #include linux/init.h -#include linux/vmalloc.h - #include linux/videodev2.h - #include linux/kmod.h - #include linux/pci.h #include linux/interrupt.h -#include linux/mutex.h #include linux/io.h #include linux/gpio.h #include linux/i2c.h #include linux/delay.h #include media/v4l2-common.h #include media/v4l2-device.h +#include media/v4l2-ctrls.h #include media/v4l2-ioctl.h -#include media/videobuf-dma-contig.h +#include media/v4l2-fh.h +#include media/v4l2-event.h +#include media/videobuf2-dma-streaming.h #include sta2x11_vip.h -#define DRV_NAME sta2x11_vip #define DRV_VERSION 1.3 #ifndef PCI_DEVICE_ID_STMICRO_VIP @@ -63,8 +58,8 @@ #define DVP_TFS0x08 #define DVP_BFO0x0C #define DVP_BFS0x10 -#define DVP_VTP 0x14 -#define DVP_VBP 0x18 +#define DVP_VTP0x14 +#define DVP_VBP0x18 #define DVP_VMP0x1C #define DVP_ITM0x98 #define DVP_ITS0x9C @@ -84,43 +79,20 @@ #define DVP_HLFLN_SD 0x0001 -#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg)) -#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg)) - #define SAVE_COUNT 8 #define AUX_COUNT 3 #define IRQ_COUNT 1 -/** - * struct sta2x11_vip - All internal data for one instance of device - * @v4l2_dev: device registered in v4l layer - * @video_dev: properties of our device - * @pdev: PCI device - * @adapter: contains I2C adapter information - * @register_save_area: All relevant register are saved here during suspend - * @decoder: contains information about video DAC - * @format: pixel format, fixed UYVY - * @std: video standard (e.g. PAL/NTSC) - * @input: input line for video signal ( 0 or 1 ) - * @users: Number of open of device ( max. 1 ) - * @disabled: Device is in power down state - * @mutex: ensures exclusive opening of device - * @slock: for excluse acces of registers - * @vb_vidq: queue maintained by videobuf layer - * @capture: linked list of capture buffer - * @active: struct videobuf_buffer currently beingg filled - * @started: device is ready to capture frame - * @closing: device will be shut down - * @tcount: Number of top frames - * @bcount: Number of bottom frames - * @overflow: Number of FIFO overflows - * @mem_spare: small buffer of unused frame - * @dma_spare: dma addres of mem_spare - * @iomem: hardware base address - * @config: I2C and gpio config from platform - * - * All non-local data is accessed via this structure. - */ + +struct vip_buffer { + struct vb2_buffer vb; + struct list_headlist; + dma_addr_t dma; +}; +static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2) +{ + return container_of(vb2, struct vip_buffer, vb); +} struct sta2x11_vip { struct v4l2_device v4l2_dev; @@ -129,21 +101,27 @@ struct sta2x11_vip { struct i2c_adapter *adapter; unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT]; struct v4l2_subdev *decoder; - struct v4l2_pix_format format; - v4l2_std_id std; -
[PATCH v3 4/4] adv7180: remove {query/g_/s_}ctrl
All drivers which use this subdevice use also the control framework. The v4l2_subdev_core_ops operations {query/g_/s_}ctrl are useless because device drivers will inherit controls from this subdevice. Signed-off-by: Federico Vaga federico.v...@gmail.com --- drivers/media/i2c/adv7180.c | 3 --- 1 file modificato, 3 rimozioni(-) diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c index 45ecf8d..43bc2b9 100644 --- a/drivers/media/i2c/adv7180.c +++ b/drivers/media/i2c/adv7180.c @@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = { static const struct v4l2_subdev_core_ops adv7180_core_ops = { .g_chip_ident = adv7180_g_chip_ident, .s_std = adv7180_s_std, - .queryctrl = v4l2_subdev_queryctrl, - .g_ctrl = v4l2_subdev_g_ctrl, - .s_ctrl = v4l2_subdev_s_ctrl, }; static const struct v4l2_subdev_ops adv7180_ops = { -- 1.7.11.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 v2] media: davinci: vpif: display: separate out subdev from output
On Mon September 24 2012 12:44:11 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sekhar Nori nsek...@ti.com Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) Changes for V2: 1: Changed v4l2_device_call_until_err() call to v4l2_subdev_call() for s_routing, since this call is for specific subdev, pointed out by Hans. arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 136 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 183 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PINGPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPONENT_ID, + }, + { + .output = { + .index = 2, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config dm646x_vpif_display_config = { .set_clock = set_vpif_clock, .subdevinfo = dm646x_vpif_subdev, .subdev_count = ARRAY_SIZE(dm646x_vpif_subdev), - .output = output, - .output_count = ARRAY_SIZE(output), + .chan_config[0] = { + .outputs = dm6467_ch0_outputs, +
Re: Gain controls in v4l2-ctrl framework
Hi Hans, On Monday 24 September 2012 12:55:53 Hans de Goede wrote: On 09/23/2012 01:26 PM, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE Not all sensors have separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE, so we will need a separate control for sensors which have one combined gain called simply V4L2_CID_GAIN_GREEN Also do we really need separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE controls? I know hardware has them, but in my experience that is only done as it is simpler to make the hardware this way (fully symmetric sensor grid), have you ever tried actually using different gain settings for the 2 different green rows ? I've and that always results in an ugly checker board pattern. So I think we can and should only have a V4L2_CID_GAIN_GREEN, and for sensors with 2 green gains have that control both, forcing both to always have the same setting, which is really what you want anyways ... I've never had to set different gains for the two green components either, although I haven't done much with them. 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET GAIN_OFFSET that sounds a bit weird... GAIN_OFFSET sounds like it is a number which gets added to the 3/4 gain settings before the gain gets applied, but I assume that you just mean a number which gets added to the value from the pixel, either before or after the gain is applied and I must admit I cannot come up with a better name. I believe (not sure) that some sensors have these per color ... Some might at least. The question is if it makes sense to actually control this per color though, I don't think it does as it is meant to compensate for any fixed measuring errors, which are the same for all 3/4 colors. The offset is usually applied after the gain, so you might need different offsets to compensate for a fixed error that is multiplied by different gains. Note that all the sensor cells are exactly the same, later on a color grid gets added on top of the sensors to turn them into r/g/b cells, but physically they are the same cells, so with the same process and temperature caused measuring errors... -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi, On 09/24/2012 01:00 PM, Laurent Pinchart wrote: Hi Hans, On Monday 24 September 2012 12:55:53 Hans de Goede wrote: On 09/23/2012 01:26 PM, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE Not all sensors have separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE, so we will need a separate control for sensors which have one combined gain called simply V4L2_CID_GAIN_GREEN Also do we really need separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE controls? I know hardware has them, but in my experience that is only done as it is simpler to make the hardware this way (fully symmetric sensor grid), have you ever tried actually using different gain settings for the 2 different green rows ? I've and that always results in an ugly checker board pattern. So I think we can and should only have a V4L2_CID_GAIN_GREEN, and for sensors with 2 green gains have that control both, forcing both to always have the same setting, which is really what you want anyways ... I've never had to set different gains for the two green components either, although I haven't done much with them. 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET GAIN_OFFSET that sounds a bit weird... GAIN_OFFSET sounds like it is a number which gets added to the 3/4 gain settings before the gain gets applied, but I assume that you just mean a number which gets added to the value from the pixel, either before or after the gain is applied and I must admit I cannot come up with a better name. I believe (not sure) that some sensors have these per color ... Some might at least. The question is if it makes sense to actually control this per color though, I don't think it does as it is meant to compensate for any fixed measuring errors, which are the same for all 3/4 colors. The offset is usually applied after the gain, so you might need different offsets to compensate for a fixed error that is multiplied by different gains. Hmm, so some have per color, some don't, so then we need: V4L2_CID_GAIN_OFFSET V4L2_CID_BLUE_OFFSET V4L2_CID_RED_OFFSET V4L2_CID_GREEN_OFFSET Where GAIN_OFFSET is for the ones with just 1 offset register. Anyone have a better name for that ? Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
On Sunday 23 September 2012 19:27:03 Sakari Ailus wrote: Laurent Pinchart wrote: On Sunday 23 September 2012 16:20:06 Sakari Ailus wrote: Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET I need your opinion's to get moving to add them. We already have a V4L2_CID_GAIN control and a V4L2_CID_CHROMA_GAIN control in the user controls class. I'd like to document how those controls and the new proposed gain controls interact. At first glance they don't interact at all, devices should not implement both, the user class gain controls are higher- level than the controls you proposed - this should still be documented though, to make sure driver and application authors will not get confused. A couple of quick questions about the new controls. Do we also need a common gain controls for monochrome sensors ? Is the offset always common for the 4 I think we should have a common gain control for sensors in general, whether monochrome or not. Many sensors support global digital gain, either only or besides the per-channel gains. Agreed. The documentation should clearly state that drivers must not implement the common gain control as a shortcut to set all color gains to the same value. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] media-devnode: Replace printk with pr_*
Fixes checkpatch warnings related to printk. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/media-devnode.c | 14 -- 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c index f6b52d5..023b2a1 100644 --- a/drivers/media/media-devnode.c +++ b/drivers/media/media-devnode.c @@ -30,6 +30,8 @@ * counting. */ +#define pr_fmt(fmt) KBUILD_MODNAME : fmt + #include linux/errno.h #include linux/init.h #include linux/module.h @@ -215,7 +217,7 @@ int __must_check media_devnode_register(struct media_devnode *mdev) minor = find_next_zero_bit(media_devnode_nums, MEDIA_NUM_DEVICES, 0); if (minor == MEDIA_NUM_DEVICES) { mutex_unlock(media_devnode_lock); - printk(KERN_ERR could not get a free minor\n); + pr_err(could not get a free minor\n); return -ENFILE; } @@ -230,7 +232,7 @@ int __must_check media_devnode_register(struct media_devnode *mdev) ret = cdev_add(mdev-cdev, MKDEV(MAJOR(media_dev_t), mdev-minor), 1); if (ret 0) { - printk(KERN_ERR %s: cdev_add failed\n, __func__); + pr_err(%s: cdev_add failed\n, __func__); goto error; } @@ -243,7 +245,7 @@ int __must_check media_devnode_register(struct media_devnode *mdev) dev_set_name(mdev-dev, media%d, mdev-minor); ret = device_register(mdev-dev); if (ret 0) { - printk(KERN_ERR %s: device_register failed\n, __func__); + pr_err(%s: device_register failed\n, __func__); goto error; } @@ -287,18 +289,18 @@ static int __init media_devnode_init(void) { int ret; - printk(KERN_INFO Linux media interface: v0.10\n); + pr_info(Linux media interface: v0.10\n); ret = alloc_chrdev_region(media_dev_t, 0, MEDIA_NUM_DEVICES, MEDIA_NAME); if (ret 0) { - printk(KERN_WARNING media: unable to allocate major\n); + pr_warn(unable to allocate major\n); return ret; } ret = bus_register(media_bus_type); if (ret 0) { unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES); - printk(KERN_WARNING media: bus_register failed\n); + pr_warn(bus_register failed\n); return -EIO; } -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Fix several warnings on new fc2580 tuner driver
Hi all, this small patch series fixes the warnings generated compiling the new fc2580 tuner driver on a Ubuntu system with the 2.6.32-43 32 bit kernel and GCC 4.4.3. Compile tested only. Best regards, Gianluca Gennari Gianluca Gennari (3): fc2580: define const as UL to silence a warning fc2580: silence uninitialized variable warning fc2580: use macro for 64 bit division drivers/media/tuners/fc2580.c |7 +++ 1 files changed, 3 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] fc2580: define const as UL to silence a warning
fc2580.c: In function 'fc2580_set_params': fc2580.c:150: warning: this decimal constant is unsigned only in ISO C90 Signed-off-by: Gianluca Gennari gennar...@gmail.com --- drivers/media/tuners/fc2580.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index afc0491..036e94b 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -147,7 +147,7 @@ static int fc2580_set_params(struct dvb_frontend *fe) f_vco = c-frequency; f_vco *= fc2580_pll_lut[i].div; - if (f_vco = 26) + if (f_vco = 26UL) tmp_val = 0x0e | fc2580_pll_lut[i].band; else tmp_val = 0x06 | fc2580_pll_lut[i].band; -- 1.7.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
[PATCH 2/3] fc2580: silence uninitialized variable warning
fc2580.c: In function 'fc2580_set_params': fc2580.c:118: warning: 'ret' may be used uninitialized in this function Signed-off-by: Gianluca Gennari gennar...@gmail.com --- drivers/media/tuners/fc2580.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index 036e94b..3ad68e9 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -115,7 +115,7 @@ static int fc2580_set_params(struct dvb_frontend *fe) { struct fc2580_priv *priv = fe-tuner_priv; struct dtv_frontend_properties *c = fe-dtv_property_cache; - int ret, i; + int ret=0, i; unsigned int r_val, n_val, k_val, k_val_reg, f_ref; u8 tmp_val, r18_val; u64 f_vco; -- 1.7.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
[PATCH 3/3] fc2580: use macro for 64 bit division and reminder
Fixes the following warnings on a 32 bit system with GCC 4.4.3 and kernel Ubuntu 2.6.32-43 32 bit: WARNING: __udivdi3 [fc2580.ko] undefined! WARNING: __umoddi3 [fc2580.ko] undefined! Signed-off-by: Gianluca Gennari gennar...@gmail.com --- drivers/media/tuners/fc2580.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index 3ad68e9..2e8ebac 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -168,8 +168,7 @@ static int fc2580_set_params(struct dvb_frontend *fe) } f_ref = 2UL * priv-cfg-clock / r_val; - n_val = f_vco / f_ref; - k_val = f_vco % f_ref; + n_val = div_u64_rem(f_vco, f_ref, k_val); k_val_reg = 1UL * k_val * (1 20) / f_ref; ret = fc2580_wr_reg(priv, 0x18, r18_val | ((k_val_reg 16) 0xff)); -- 1.7.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 v2] media: davinci: vpif: display: separate out subdev from output
On Mon September 24 2012 12:59:11 Hans Verkuil wrote: On Mon September 24 2012 12:44:11 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sekhar Nori nsek...@ti.com Acked-by: Hans Verkuil hans.verk...@cisco.com I'm retracting this Ack. I did see something that wasn't right but I thought it was harmless, but after thinking some more I believe it should be fixed. Luckily, it's easy to fix. See below. Since we need a new version anyway I also added a comment to a few minor issues that can be fixed at the same time. Regards, Hans --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) Changes for V2: 1: Changed v4l2_device_call_until_err() call to v4l2_subdev_call() for s_routing, since this call is for specific subdev, pointed out by Hans. arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 136 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 183 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PIN GPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPONENT_ID, + }, + { + .output = { + .index = 2, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + },
Re: [PATCH 1/3] fc2580: define const as UL to silence a warning
On 09/24/2012 02:37 PM, Gianluca Gennari wrote: fc2580.c: In function 'fc2580_set_params': fc2580.c:150: warning: this decimal constant is unsigned only in ISO C90 Signed-off-by: Gianluca Gennari gennar...@gmail.com Acked-by: Antti Palosaari cr...@iki.fi Reviewed-by: Antti Palosaari cr...@iki.fi --- drivers/media/tuners/fc2580.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index afc0491..036e94b 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -147,7 +147,7 @@ static int fc2580_set_params(struct dvb_frontend *fe) f_vco = c-frequency; f_vco *= fc2580_pll_lut[i].div; - if (f_vco = 26) + if (f_vco = 26UL) tmp_val = 0x0e | fc2580_pll_lut[i].band; else tmp_val = 0x06 | fc2580_pll_lut[i].band; -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] fc2580: silence uninitialized variable warning
On 09/24/2012 02:37 PM, Gianluca Gennari wrote: fc2580.c: In function 'fc2580_set_params': fc2580.c:118: warning: 'ret' may be used uninitialized in this function Signed-off-by: Gianluca Gennari gennar...@gmail.com --- drivers/media/tuners/fc2580.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index 036e94b..3ad68e9 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -115,7 +115,7 @@ static int fc2580_set_params(struct dvb_frontend *fe) { struct fc2580_priv *priv = fe-tuner_priv; struct dtv_frontend_properties *c = fe-dtv_property_cache; - int ret, i; + int ret=0, i; Nack. This is Codingstyle violation. See rules around line 206 from Documentation/CodingStyle That replace warning with Codingstyle mistake. Change it to meet CodingStyle and resend. unsigned int r_val, n_val, k_val, k_val_reg, f_ref; u8 tmp_val, r18_val; u64 f_vco; Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] fc2580: use macro for 64 bit division and reminder
On 09/24/2012 02:37 PM, Gianluca Gennari wrote: Fixes the following warnings on a 32 bit system with GCC 4.4.3 and kernel Ubuntu 2.6.32-43 32 bit: WARNING: __udivdi3 [fc2580.ko] undefined! WARNING: __umoddi3 [fc2580.ko] undefined! Signed-off-by: Gianluca Gennari gennar...@gmail.com Acked-by: Antti Palosaari cr...@iki.fi Reviewed-by: Antti Palosaari cr...@iki.fi --- drivers/media/tuners/fc2580.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index 3ad68e9..2e8ebac 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -168,8 +168,7 @@ static int fc2580_set_params(struct dvb_frontend *fe) } f_ref = 2UL * priv-cfg-clock / r_val; - n_val = f_vco / f_ref; - k_val = f_vco % f_ref; + n_val = div_u64_rem(f_vco, f_ref, k_val); k_val_reg = 1UL * k_val * (1 20) / f_ref; ret = fc2580_wr_reg(priv, 0x18, r18_val | ((k_val_reg 16) 0xff)); -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: davinci: vpif: display: separate out subdev from output
On Mon September 24 2012 13:50:00 Hans Verkuil wrote: On Mon September 24 2012 12:59:11 Hans Verkuil wrote: On Mon September 24 2012 12:44:11 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sekhar Nori nsek...@ti.com Acked-by: Hans Verkuil hans.verk...@cisco.com I'm retracting this Ack. I did see something that wasn't right but I thought it was harmless, but after thinking some more I believe it should be fixed. Luckily, it's easy to fix. See below. Since we need a new version anyway I also added a comment to a few minor issues that can be fixed at the same time. Regards, Hans --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) Changes for V2: 1: Changed v4l2_device_call_until_err() call to v4l2_subdev_call() for s_routing, since this call is for specific subdev, pointed out by Hans. arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 136 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 183 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PINGPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPONENT_ID, + }, + { + .output = { + .index = 2, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, +
Re: [PATCH 5/5] drivers/media/platform/omap3isp/isp.c: fix error return code
Hi, On Sunday 23 September 2012 19:46:53 Peter Senna Tschudin wrote: On Sun, Sep 23, 2012 at 7:39 PM, Mauro Carvalho Chehab wrote: Laurent, Could you please review this patch? Peter, Please, always c/c the driver maintainer/author on patches you submit. You can check it with scripts/get_maintainer.pl. Sorry for that. I'll be more careful next time. Thanks! Regards, Mauro Mensagem original Assunto: [PATCH 5/5] drivers/media/platform/omap3isp/isp.c: fix error return code Data: Tue, 4 Sep 2012 18:14:25 +0200 De: Peter Senna Tschudin peter.se...@gmail.com Para: Mauro Carvalho Chehab mche...@infradead.org CC: kernel-janit...@vger.kernel.org, julia.law...@lip6.fr, linux-media@vger.kernel.org, linux-ker...@vger.kernel.org From: Peter Senna Tschudin peter.se...@gmail.com Convert a nonnegative error return code to a negative one, as returned elsewhere in the function. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // smpl ( if@p1 (\(ret 0\|ret != 0\)) { ... return ret; } ret@p1 = 0 ) ... when != ret = e1 when != ret *if(...) { ... when != ret = e2 when forall return ret; } // /smpl Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- drivers/media/platform/omap3isp/isp.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index e0096e0..91fcaef 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -2102,8 +2102,10 @@ static int __devinit isp_probe(struct platform_device *pdev) if (ret 0) goto error; - if (__omap3isp_get(isp, false) == NULL) + if (__omap3isp_get(isp, false) == NULL) { + ret = -EBUSY; /* Not sure if EBUSY is best for here */ I've replaced -EBUSY with -ENODEV, removed the comment and applied the patch to my tree. Thanks. goto error; + } ret = isp_reset(isp); if (ret 0) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 02/26] Documentation: media: description of DMABUF importing in V4L2
Hi Hans, Thank you for review. On 08/22/2012 12:47 PM, Hans Verkuil wrote: On Tue August 14 2012 17:34:32 Tomasz Stanislawski wrote: This patch adds description and usage examples for importing DMABUF file descriptor in V4L2. Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com CC: linux-...@vger.kernel.org --- Documentation/DocBook/media/v4l/compat.xml |4 + Documentation/DocBook/media/v4l/io.xml | 180 .../DocBook/media/v4l/vidioc-create-bufs.xml |3 +- Documentation/DocBook/media/v4l/vidioc-qbuf.xml| 15 ++ Documentation/DocBook/media/v4l/vidioc-reqbufs.xml | 47 ++--- 5 files changed, 226 insertions(+), 23 deletions(-) [snip] +v4l2-plane; in the multi-planar API case). The driver must be switched into +DMABUF I/O mode by calling the VIDIOC-REQBUFS; with the desired buffer type. +No buffers (planes) are allocated beforehand, consequently they are not indexed +and cannot be queried like mapped buffers with the +constantVIDIOC_QUERYBUF/constant ioctl./para I disagree with that. Userptr buffers can use QUERYBUF just fine. Even for the userptr you still have to fill in the buffer index when calling QBUF. So I see no reason why you couldn't use QUERYBUF in the DMABUF case. The only difference is that the fd field is undefined (set to -1 perhaps?) if the bufffer isn't queued. QUERYBUF can be very useful for debugging, for example to see what the status is of each buffer and how many are queued. Ok. I agree that QUERYBUF can be useful for debugging. The value of fd field should be the last value passed using QBUF. It would simplify streaming because an application would not have to keep the file descriptor around. + +example + titleInitiating streaming I/O with DMABUF file descriptors/title + + programlisting +v4l2-requestbuffers; reqbuf; + +memset (amp;reqbuf, 0, sizeof (reqbuf)); +reqbuf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; +reqbuf.memory = V4L2_MEMORY_DMABUF; +reqbuf.count = 1; + +if (ioctl (fd, VIDIOC-REQBUFS;, amp;reqbuf) == -1) { +if (errno == EINVAL) +printf (Video capturing or DMABUF streaming is not supported\n); +else +perror (VIDIOC_REQBUFS); + +exit (EXIT_FAILURE); Let's stick to the kernel coding style, so no ' ' before '(' in function calls. Same for the other program examples below. The ' ' before function was used for userptr and mmap usage examples. These examples should be fixed too. +} + /programlisting +/example + +paraBuffer (plane) file descriptor is passed on the fly with the s/Buffer/The buffer/ +VIDIOC-QBUF; ioctl. In case of multiplanar buffers, every plane can be 'Can be', 'should be' or 'must be'? Does it ever make sense to have the same fd for different planes? Do we have restrictions on this in the userptr case? I think that we should keep to 'can be'. I see no good reason to prevent the same dmabuf to be used for different planes. Allowing reusing of dmabufs with assistance of data_offset field would allow to pass a 2-planar YUV420 from V4L2-single-plane API to a driver with V4L2-multi-plane API. [snip] diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml index 77ff5be..436d21c 100644 --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml @@ -109,6 +109,21 @@ they cannot be swapped out to disk. Buffers remain locked until dequeued, until the VIDIOC-STREAMOFF; or VIDIOC-REQBUFS; ioctl is called, or until the device is closed./para +paraTo enqueue a link linkend=dmabufDMABUF/link buffer applications +set the structfieldmemory/structfield field to +constantV4L2_MEMORY_DMABUF/constant and the structfieldm.fd/structfield +field to a file descriptor associated with a DMABUF buffer. When the +multi-planar API is used structfieldm.fd/structfield of the passed array of multi-planar API is used the structfieldm.fd/structfield fields of the passed array of +v4l2-plane; have to be used instead. When constantVIDIOC_QBUF/constant is +called with a pointer to this structure the driver sets the +constantV4L2_BUF_FLAG_QUEUED/constant flag and clears the +constantV4L2_BUF_FLAG_MAPPED/constant and +constantV4L2_BUF_FLAG_DONE/constant flags in the +structfieldflags/structfield field, or it returns an error code. This +ioctl locks the buffer. Buffers remain locked until dequeued, until the +VIDIOC-STREAMOFF; or VIDIOC-REQBUFS; ioctl is called, or until the device is +closed./para You need to explain what a 'locked buffer' means. I propose following definition: Locking a buffer means passing it to a driver for an access by hardware. If an application accesses (reads/writes) a locked buffer then the result is undefined. What is your opinion about
Re: [PULL FOR v3.7] OMAP3 ISP patches
Hi Mauro, Here's an additional patch if there's still time. The following changes since commit f9040ef3fab8f6f5f6fced5583203695d08efde3: [media] stv090x: add support for multistream (2012-09-23 21:27:19 -0300) are available in the git repository at: git://linuxtv.org/pinchartl/media.git omap3isp-omap3isp-next Laurent Pinchart (1): omap3isp: Use monotonic timestamps for statistics buffers Peter Senna Tschudin (1): omap3isp: Fix error return code in probe function Wanlong Gao (1): omap3isp: Fix up ENOIOCTLCMD error handling drivers/media/platform/omap3isp/isp.c |4 +++- drivers/media/platform/omap3isp/ispstat.c |2 +- drivers/media/platform/omap3isp/ispstat.h |2 +- drivers/media/platform/omap3isp/ispvideo.c |8 include/linux/omap3isp.h |7 ++- 5 files changed, 15 insertions(+), 8 deletions(-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mt9t031 driver support on OMAP3 system
Hi Pete, On Sunday 23 September 2012 00:04:20 Guennadi Liakhovetski wrote: On Sat, 22 Sep 2012, P Jackson wrote: I'm trying to incorporate an MT9T031-based sensor into a commercial small form-factor OMAP3 DM3730-based system which is supplied with sources for a 2.6.37 kernel and is somewhat out-dated.The application I'm working on requires a single image to be acquired from the sensor every once in a while which is then processed off-line by another algorithm on the OMAP3 I'd appreciate any advice from the list as to what the most suitable up to-date compatible kernel I should use would be and what other work I need to do in order to get things sorted. I would certainly advise to use a current kernel (e.g., 3.5). As for how, I know, that Laurent has worked on a similar tasks, you can find his posts in mailing list archives, or maybe he'll be able to advise you more specifically. You can have a look at http://git.linuxtv.org/pinchartl/media.git/shortlog/refs/heads/omap3isp- sensors-board to see how I've modified the ov772x driver to make it usable with the OMAP3 ISP. The patches are not upstreamable as such, I still need to work on them. I've explained the issues in detail on the mailing list. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3isp: wrong image after resizer with mt9v034 sensor
Hi Enric, On Monday 24 September 2012 10:33:42 Enric Balletbò i Serra wrote: Hi everybody, I'm trying to add support for MT9V034 Aptina image sensor to current mainline, as a base of my current work I start using the latest omap3isp-next branch from Laurent's git tree [1]. The MT9V034 image sensor is very similar to MT9V032 sensor, so I modified current driver to accept MT9V034 sensor adding the chip ID. The driver recognizes the sensor and I'm able to capture some frames. I started capturing directly frames using the pipeline Sensor - CCDC ./media-ctl -r ./media-ctl -l 'mt9v032 3-005c:0-OMAP3 ISP CCDC:0[1]' ./media-ctl -l 'OMAP3 ISP CCDC:1-OMAP3 ISP CCDC output:0[1]' ./media-ctl -f 'mt9v032 3-005c:0 [SGRBG10 752x480]' ./media-ctl -f 'OMAP3 ISP CCDC:1 [SGRBG10 752x480]' # Test pattern ./yavta --set-control '0x00981901 1' /dev/v4l-subdev8 # ./yavta -p -f SGRBG10 -s 752x480 -n 4 --capture=3 /dev/video2 --file=img-#.bin To convert to jpg I used bayer2rgb [2] program executing following command, $ convert -size 752x480 GRBG_BAYER:./img-00.bin img-00.jpg And the result image looks like this http://downloads.isee.biz/pub/files/patterns/img-from-sensor.jpg Seems good, so I tried to use following pipeline Sensor - CCDC - Preview - Resizer ./media-ctl -r ./media-ctl -l 'mt9v032 3-005c:0-OMAP3 ISP CCDC:0[1]' ./media-ctl -l 'OMAP3 ISP CCDC:2-OMAP3 ISP preview:0[1]' ./media-ctl -l 'OMAP3 ISP preview:1-OMAP3 ISP resizer:0[1]' ./media-ctl -l 'OMAP3 ISP resizer:1-OMAP3 ISP resizer output:0[1]' ./media-ctl -V 'mt9v032 3-005c:0[SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP CCDC:0 [SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP CCDC:2 [SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP preview:1 [UYVY 752x480]' ./media-ctl -V 'OMAP3 ISP resizer:1 [UYVY 752x480]' # Set Test pattern ./yavta --set-control '0x00981901 1' /dev/v4l-subdev8 ./yavta -f UYVY -s 752x480 --capture=3 --file=img-#.uyvy /dev/video6 I used 'convert' program to pass from UYVY to jpg, $ convert -size 752x480 img-00.uyvy img-00.jpg and the result image looks like this http://downloads.isee.biz/pub/files/patterns/img-from-resizer.jpg As you can see, the image is wrong and I'm not sure if the problem is from the sensor, from the previewer, from the resizer or from my conversion. Anyone have idea where should I look ? Or which is the source of the problem ? Could you please post the output of all the above media-ctl and yavta runs, as well as the captured raw binary frame ? [1] http://git.linuxtv.org/pinchartl/media.git/shortlog/refs/heads/omap3isp- omap3isp-next [2] https://github.com/jdthomas/bayer2rgb -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Hello, On Monday, September 24, 2012 12:59 PM Federico Vaga wrote: The DMA streaming allocator is similar to the DMA contig but it use the DMA streaming interface (dma_map_single, dma_unmap_single). The allocator allocates buffers and immediately map the memory for DMA transfer. For each buffer prepare/finish it does a DMA synchronization. Signed-off-by: Federico Vaga federico.v...@gmail.com Acked-by: Marek Szyprowski m.szyprow...@samsung.com --- drivers/media/v4l2-core/Kconfig | 5 + drivers/media/v4l2-core/Makefile | 1 + drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++ include/media/videobuf2-dma-streaming.h | 32 4 file modificati, 243 inserzioni(+) create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c create mode 100644 include/media/videobuf2-dma-streaming.h diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig index 0c54e19..60548a7 100644 --- a/drivers/media/v4l2-core/Kconfig +++ b/drivers/media/v4l2-core/Kconfig @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG #depends on HAS_DMA select VIDEOBUF2_CORE select VIDEOBUF2_MEMOPS + +config VIDEOBUF2_DMA_STREAMING + select VIDEOBUF2_CORE + select VIDEOBUF2_MEMOPS + tristate diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile index c2d61d4..0b2756f 100644 --- a/drivers/media/v4l2-core/Makefile +++ b/drivers/media/v4l2-core/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o ccflags-y += -I$(srctree)/drivers/media/dvb-core ccflags-y += -I$(srctree)/drivers/media/dvb-frontends diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2- core/videobuf2-dma-streaming.c new file mode 100644 index 000..c839e05 --- /dev/null +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c @@ -0,0 +1,205 @@ +/* + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2 + * + * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include linux/module.h +#include linux/slab.h +#include linux/pagemap.h +#include linux/dma-mapping.h + +#include media/videobuf2-core.h +#include media/videobuf2-dma-streaming.h +#include media/videobuf2-memops.h + +struct vb2_streaming_conf { + struct device *dev; +}; +struct vb2_streaming_buf { + struct vb2_streaming_conf *conf; + void*vaddr; + + dma_addr_t dma_handle; + + unsigned long size; + struct vm_area_struct *vma; + + atomic_trefcount; + struct vb2_vmarea_handler handler; +}; + +static void vb2_dma_streaming_put(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + if (atomic_dec_and_test(buf-refcount)) { + dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size, + DMA_FROM_DEVICE); + free_pages_exact(buf-vaddr, buf-size); + kfree(buf); + } + +} + +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size) +{ + struct vb2_streaming_conf *conf = alloc_ctx; + struct vb2_streaming_buf *buf; + int err; + + buf = kzalloc(sizeof(struct vb2_streaming_buf), GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA); + if (!buf-vaddr) { + err = -ENOMEM; + goto out; + } + buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size, + DMA_FROM_DEVICE); + err = dma_mapping_error(conf-dev, buf-dma_handle); + if (err) { + dev_err(conf-dev, dma_map_single failed\n); + + free_pages_exact(buf-vaddr, size); + buf-vaddr = NULL; + goto out_pages; + } + buf-conf = conf; + buf-size = size; + buf-handler.refcount = buf-refcount; + buf-handler.put = vb2_dma_streaming_put; + buf-handler.arg = buf; + + atomic_inc(buf-refcount); + return buf; + +out_pages: + free_pages_exact(buf-vaddr, buf-size); +out: + kfree(buf); + return ERR_PTR(err); +} + +static void *vb2_dma_streaming_cookie(void *buf_priv) +{ + struct vb2_streaming_buf *buf = buf_priv; + + return buf-dma_handle; +} + +static void
RE: [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators
Hello, It would be great if you could keep the correct authorship of the patch by adding the following line on top of the patch (git will handle it automatically after applying): --8-- From: Marek Szyprowski m.szyprow...@samsung.com --8-- On Monday, September 24, 2012 12:59 PM Federico Vaga wrote: This patch adds support for prepare/finish callbacks in VB2 allocators. These callback are used for buffer flushing. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Federico Vaga federico.v...@gmail.com --- drivers/media/v4l2-core/videobuf2-core.c | 11 +++ include/media/videobuf2-core.h | 7 +++ 2 file modificati, 18 inserzioni(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2- core.c index 4da3df6..079fa79 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) { struct vb2_queue *q = vb-vb2_queue; unsigned long flags; + unsigned int plane; if (vb-state != VB2_BUF_STATE_ACTIVE) return; @@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) dprintk(4, Done processing on buffer %d, state: %d\n, vb-v4l2_buf.index, vb-state); + /* sync buffers */ + for (plane = 0; plane vb-num_planes; ++plane) + call_memop(q, finish, vb-planes[plane].mem_priv); + /* Add the buffer to the done buffers list */ spin_lock_irqsave(q-done_lock, flags); vb-state = state; @@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b) static void __enqueue_in_driver(struct vb2_buffer *vb) { struct vb2_queue *q = vb-vb2_queue; + unsigned int plane; vb-state = VB2_BUF_STATE_ACTIVE; atomic_inc(q-queued_count); + + /* sync buffers */ + for (plane = 0; plane vb-num_planes; ++plane) + call_memop(q, prepare, vb-planes[plane].mem_priv); + q-ops-buf_queue(vb); } diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 8dd9b6c..2508609 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -41,6 +41,10 @@ struct vb2_fileio_data; *argument to other ops in this structure * @put_userptr: inform the allocator that a USERPTR buffer will no longer *be used + * @prepare: called every time the buffer is passed from userspace to the + * driver, usefull for cache synchronisation, optional + * @finish: called every time the buffer is passed back from the driver + * to the userspace, also optional * @vaddr: return a kernel virtual address to a given memory buffer * associated with the passed private structure or NULL if no * such mapping exists @@ -65,6 +69,9 @@ struct vb2_mem_ops { unsigned long size, int write); void(*put_userptr)(void *buf_priv); + void(*prepare)(void *buf_priv); + void(*finish)(void *buf_priv); + void*(*vaddr)(void *buf_priv); void*(*cookie)(void *buf_priv); -- 1.7.11.4 Best regards -- Marek Szyprowski Samsung Poland RD Center -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3 V2] fc2580: silence uninitialized variable warning
fc2580.c: In function 'fc2580_set_params': fc2580.c:118: warning: 'ret' may be used uninitialized in this function V2: fixed coding style. Signed-off-by: Gianluca Gennari gennar...@gmail.com --- drivers/media/tuners/fc2580.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c index 036e94b..3ad68e9 100644 --- a/drivers/media/tuners/fc2580.c +++ b/drivers/media/tuners/fc2580.c @@ -115,7 +115,7 @@ static int fc2580_set_params(struct dvb_frontend *fe) { struct fc2580_priv *priv = fe-tuner_priv; struct dtv_frontend_properties *c = fe-dtv_property_cache; - int ret, i; + int ret = 0, i; unsigned int r_val, n_val, k_val, k_val_reg, f_ref; u8 tmp_val, r18_val; u64 f_vco; -- 1.7.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
[PATCH v5] media: v4l2-ctrl: add a helper function to add standard control with driver specific menu
From: Lad, Prabhakar prabhakar@ti.com Add helper function v4l2_ctrl_new_std_menu_items(), which adds a standard menu control, with driver specific menu. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Rob Landley r...@landley.net --- Changes for v5: 1: Fixed some grammatical issues pointed by Hans. Changes for v4: 1: Rather then adding a function to modify the menu, added a helper function, that creates a new standard control with user specific menu. Changes for v3: 1: Fixed style/grammer issues as pointed by Hans. Thanks Hans for providing the description. Changes for v2: 1: Fixed review comments from Hans, to have return type as void, add WARN_ON() for fail conditions, allow this fucntion to modify the menu of custom controls. Documentation/video4linux/v4l2-controls.txt | 24 + drivers/media/v4l2-core/v4l2-ctrls.c| 30 +++ include/media/v4l2-ctrls.h | 23 3 files changed, 77 insertions(+), 0 deletions(-) diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt index 43da22b..b0cb0b8 100644 --- a/Documentation/video4linux/v4l2-controls.txt +++ b/Documentation/video4linux/v4l2-controls.txt @@ -136,11 +136,25 @@ Or alternatively for integer menu controls, by calling v4l2_ctrl_new_int_menu: const struct v4l2_ctrl_ops *ops, u32 id, s32 max, s32 def, const s64 *qmenu_int); +Standard menu controls with a driver specific menu are added by calling +v4l2_ctrl_new_std_menu_items: + + struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items( + struct v4l2_ctrl_handler *hdl, + const struct v4l2_ctrl_ops *ops, u32 id, s32 max, + s32 skip_mask, s32 def, const char * const *qmenu); + These functions are typically called right after the v4l2_ctrl_handler_init: static const s64 exp_bias_qmenu[] = { -2, -1, 0, 1, 2 }; + static const char * const test_pattern[] = { + Disabled, + Vertical Bars, + Solid Black, + Solid White, + }; v4l2_ctrl_handler_init(foo-ctrl_handler, nr_of_controls); v4l2_ctrl_new_std(foo-ctrl_handler, foo_ctrl_ops, @@ -156,6 +170,9 @@ These functions are typically called right after the v4l2_ctrl_handler_init: ARRAY_SIZE(exp_bias_qmenu) - 1, ARRAY_SIZE(exp_bias_qmenu) / 2 - 1, exp_bias_qmenu); + v4l2_ctrl_new_std_menu_items(foo-ctrl_handler, foo_ctrl_ops, + V4L2_CID_TEST_PATTERN, ARRAY_SIZE(test_pattern) - 1, 0, + 0, test_pattern); ... if (foo-ctrl_handler.error) { int err = foo-ctrl_handler.error; @@ -185,6 +202,13 @@ v4l2_ctrl_new_std_menu in that it doesn't have the mask argument and takes as the last argument an array of signed 64-bit integers that form an exact menu item list. +The v4l2_ctrl_new_std_menu_items function is very similar to +v4l2_ctrl_new_std_menu but takes an extra parameter qmenu, which is the driver +specific menu for an otherwise standard menu control. A good example for this +control is the test pattern control for capture/display/sensors devices that +have the capability to generate test patterns. These test patterns are hardware +specific, so the contents of the menu will vary from device to device. + Note that if something fails, the function will return NULL or an error and set ctrl_handler-error to the error code. If ctrl_handler-error was already set, then it will just return and do nothing. This is also true for diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 41b7732..96c5e73 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -1643,6 +1643,36 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl, } EXPORT_SYMBOL(v4l2_ctrl_new_std_menu); +/* Helper function for standard menu controls with driver defined menu */ +struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl, + const struct v4l2_ctrl_ops *ops, u32 id, s32 max, + s32 mask, s32 def, const char * const *qmenu) +{ + enum v4l2_ctrl_type type; + const char *name; + u32 flags; + s32 step; + s32 min; + + /* v4l2_ctrl_new_std_menu_items() should only be called
[PATCH v3] media: v4l2-ctrls: add control for test pattern
From: Lad, Prabhakar prabhakar@ti.com add V4L2_CID_TEST_PATTERN of type menu, which determines the internal test pattern selected by the device. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Acked-by: Sakari Ailus sakari.ai...@iki.fi Cc: Hans Verkuil hans.verk...@cisco.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Rob Landley r...@landley.net --- This patches has one checkpatch warning for line over 80 characters altough it can be avoided I have kept it for consistency. Changes for v3: 1: Removed the menu for test pattern, pointed by Sakari. Changes for v2: 1: Included display devices in the description for test pattern as pointed by Hans. 2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as pointed by Sylwester. 3: Removed the test patterns from menu as the are hardware specific as pointed by Sakari. Documentation/DocBook/media/v4l/controls.xml | 10 ++ drivers/media/v4l2-core/v4l2-ctrls.c |2 ++ include/linux/videodev2.h|1 + 3 files changed, 13 insertions(+), 0 deletions(-) diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index f0fb08d..51e9c4e 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -4313,6 +4313,16 @@ interface and may change in the future./para /tbody /entrytbl /row + row + entry spanname=idconstantV4L2_CID_TEST_PATTERN/constant/entry + entrymenu/entry + /row + row id=v4l2-test-pattern + entry spanname=descr The Capture/Display/Sensors have the capability + to generate internal test patterns and this are hardware specific. This + test patterns are used to test a device is properly working and can generate + the desired waveforms that it supports./entry + /row rowentry/entry/row /tbody /tgroup diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 8f2f40b..41b7732 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -740,6 +740,7 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_LINK_FREQ:return Link Frequency; case V4L2_CID_PIXEL_RATE: return Pixel Rate; case V4L2_CID_DPCM_PREDICTOR: return DPCM Predictor; + case V4L2_CID_TEST_PATTERN: return Test Pattern; default: return NULL; @@ -841,6 +842,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, case V4L2_CID_EXPOSURE_METERING: case V4L2_CID_SCENE_MODE: case V4L2_CID_DPCM_PREDICTOR: + case V4L2_CID_TEST_PATTERN: *type = V4L2_CTRL_TYPE_MENU; break; case V4L2_CID_LINK_FREQ: diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index ca9fb78..7014c0b 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2005,6 +2005,7 @@ enum v4l2_dpcm_predictor { V4L2_DPCM_PREDICTOR_SIMPLE = 0, V4L2_DPCM_PREDICTOR_ADVANCED= 1, }; +#define V4L2_CID_TEST_PATTERN (V4L2_CID_IMAGE_PROC_CLASS_BASE + 4) /* * T U N I N G -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] media: v4l2-ctrl: add a helper function to add standard control with driver specific menu
On Mon September 24 2012 14:53:40 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com Add helper function v4l2_ctrl_new_std_menu_items(), which adds a standard menu control, with driver specific menu. Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Rob Landley r...@landley.net --- Changes for v5: 1: Fixed some grammatical issues pointed by Hans. Changes for v4: 1: Rather then adding a function to modify the menu, added a helper function, that creates a new standard control with user specific menu. Changes for v3: 1: Fixed style/grammer issues as pointed by Hans. Thanks Hans for providing the description. Changes for v2: 1: Fixed review comments from Hans, to have return type as void, add WARN_ON() for fail conditions, allow this fucntion to modify the menu of custom controls. Documentation/video4linux/v4l2-controls.txt | 24 + drivers/media/v4l2-core/v4l2-ctrls.c| 30 +++ include/media/v4l2-ctrls.h | 23 3 files changed, 77 insertions(+), 0 deletions(-) -- 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 v5] media: v4l2-ctrl: add a helper function to add standard control with driver specific menu
On Monday 24 September 2012 18:23:40 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com Add helper function v4l2_ctrl_new_std_menu_items(), which adds a standard menu control, with driver specific menu. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Rob Landley r...@landley.net Thank you for not giving up despite the many change requests you have received :-) Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: davinci: vpif: display: separate out subdev from output
Hi Hans, On Mon, Sep 24, 2012 at 5:20 PM, Hans Verkuil hverk...@xs4all.nl wrote: On Mon September 24 2012 12:59:11 Hans Verkuil wrote: On Mon September 24 2012 12:44:11 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sekhar Nori nsek...@ti.com Acked-by: Hans Verkuil hans.verk...@cisco.com I'm retracting this Ack. I did see something that wasn't right but I thought it was harmless, but after thinking some more I believe it should be fixed. Luckily, it's easy to fix. See below. Since we need a new version anyway I also added a comment to a few minor issues that can be fixed at the same time. Regards, Hans --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) Changes for V2: 1: Changed v4l2_device_call_until_err() call to v4l2_subdev_call() for s_routing, since this call is for specific subdev, pointed out by Hans. arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 136 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 183 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PIN GPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPONENT_ID, + }, + { + .output = { + .index = 2, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name =
[PATCH RFC] V4L: Add s_rx_buffer subdev video operation
The s_rx_buffer callback allows the host to set buffer for non-image (meta) data at a subdev. This callback can be implemented by an image sensor or a MIPI-CSI receiver, allowing the host to retrieve the frame embedded data from a subdev. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- include/media/v4l2-subdev.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 22ab09e..28067ed 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -274,6 +274,10 @@ struct v4l2_subdev_audio_ops { s_mbus_config: set a certain mediabus configuration. This operation is added for compatibility with soc-camera drivers and should not be used by new software. + + s_rx_buffer: set a host allocated memory buffer for the subdev. The subdev + can adjust @size to a lower value and must not write more data to the + buffer starting at @data than the original value of @size. */ struct v4l2_subdev_video_ops { int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32 config); @@ -327,6 +331,8 @@ struct v4l2_subdev_video_ops { struct v4l2_mbus_config *cfg); int (*s_mbus_config)(struct v4l2_subdev *sd, const struct v4l2_mbus_config *cfg); + int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf, + unsigned int *size); }; /* -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: davinci: vpif: display: separate out subdev from output
On Mon September 24 2012 15:21:44 Prabhakar Lad wrote: Hi Hans, On Mon, Sep 24, 2012 at 5:20 PM, Hans Verkuil hverk...@xs4all.nl wrote: On Mon September 24 2012 12:59:11 Hans Verkuil wrote: On Mon September 24 2012 12:44:11 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sekhar Nori nsek...@ti.com Acked-by: Hans Verkuil hans.verk...@cisco.com I'm retracting this Ack. I did see something that wasn't right but I thought it was harmless, but after thinking some more I believe it should be fixed. Luckily, it's easy to fix. See below. Since we need a new version anyway I also added a comment to a few minor issues that can be fixed at the same time. Regards, Hans --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) Changes for V2: 1: Changed v4l2_device_call_until_err() call to v4l2_subdev_call() for s_routing, since this call is for specific subdev, pointed out by Hans. arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 136 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 183 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PIN GPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPONENT_ID, + }, + {
Re: [PATCH v3] media: v4l2-ctrls: add control for test pattern
On Mon September 24 2012 14:53:41 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com add V4L2_CID_TEST_PATTERN of type menu, which determines the internal test pattern selected by the device. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Acked-by: Sakari Ailus sakari.ai...@iki.fi Cc: Hans Verkuil hans.verk...@cisco.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Rob Landley r...@landley.net --- This patches has one checkpatch warning for line over 80 characters altough it can be avoided I have kept it for consistency. Changes for v3: 1: Removed the menu for test pattern, pointed by Sakari. Changes for v2: 1: Included display devices in the description for test pattern as pointed by Hans. 2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as pointed by Sylwester. 3: Removed the test patterns from menu as the are hardware specific as pointed by Sakari. Documentation/DocBook/media/v4l/controls.xml | 10 ++ drivers/media/v4l2-core/v4l2-ctrls.c |2 ++ include/linux/videodev2.h|1 + 3 files changed, 13 insertions(+), 0 deletions(-) diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index f0fb08d..51e9c4e 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -4313,6 +4313,16 @@ interface and may change in the future./para /tbody /entrytbl /row + row + entry spanname=idconstantV4L2_CID_TEST_PATTERN/constant/entry + entrymenu/entry + /row + row id=v4l2-test-pattern + entry spanname=descr The Capture/Display/Sensors have the capability + to generate internal test patterns and this are hardware specific. This + test patterns are used to test a device is properly working and can generate + the desired waveforms that it supports./entry Some grammar/style fixes: entry spanname=descr Some capture/display/sensor devices have the capability to generate test pattern images. These hardware specific test patterns can be used to test if a device is working properly./entry + /row rowentry/entry/row /tbody /tgroup diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 8f2f40b..41b7732 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -740,6 +740,7 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_LINK_FREQ:return Link Frequency; case V4L2_CID_PIXEL_RATE: return Pixel Rate; case V4L2_CID_DPCM_PREDICTOR: return DPCM Predictor; + case V4L2_CID_TEST_PATTERN: return Test Pattern; default: return NULL; @@ -841,6 +842,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, case V4L2_CID_EXPOSURE_METERING: case V4L2_CID_SCENE_MODE: case V4L2_CID_DPCM_PREDICTOR: + case V4L2_CID_TEST_PATTERN: *type = V4L2_CTRL_TYPE_MENU; break; case V4L2_CID_LINK_FREQ: diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index ca9fb78..7014c0b 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2005,6 +2005,7 @@ enum v4l2_dpcm_predictor { V4L2_DPCM_PREDICTOR_SIMPLE = 0, V4L2_DPCM_PREDICTOR_ADVANCED= 1, }; +#define V4L2_CID_TEST_PATTERN (V4L2_CID_IMAGE_PROC_CLASS_BASE + 4) /* * T U N I N G Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] V4L: Add s_rx_buffer subdev video operation
Hi Sylwester, On Mon, Sep 24, 2012 at 03:26:53PM +0200, Sylwester Nawrocki wrote: The s_rx_buffer callback allows the host to set buffer for non-image (meta) data at a subdev. This callback can be implemented by an image sensor or a MIPI-CSI receiver, allowing the host to retrieve the frame embedded data from a subdev. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- include/media/v4l2-subdev.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 22ab09e..28067ed 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -274,6 +274,10 @@ struct v4l2_subdev_audio_ops { s_mbus_config: set a certain mediabus configuration. This operation is added for compatibility with soc-camera drivers and should not be used by new software. + + s_rx_buffer: set a host allocated memory buffer for the subdev. The subdev + can adjust @size to a lower value and must not write more data to the + buffer starting at @data than the original value of @size. */ struct v4l2_subdev_video_ops { int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32 config); @@ -327,6 +331,8 @@ struct v4l2_subdev_video_ops { struct v4l2_mbus_config *cfg); int (*s_mbus_config)(struct v4l2_subdev *sd, const struct v4l2_mbus_config *cfg); + int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf, +unsigned int *size); }; /* How about useing a separate video buffer queue for the purpose? That would provide a nice way to pass it to the user space where it's needed. It'd also play nicely together with the frame layout descriptors. Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3isp: wrong image after resizer with mt9v034 sensor
Hi Laurent, 2012/9/24 Laurent Pinchart laurent.pinch...@ideasonboard.com: Hi Enric, On Monday 24 September 2012 10:33:42 Enric Balletbò i Serra wrote: Hi everybody, I'm trying to add support for MT9V034 Aptina image sensor to current mainline, as a base of my current work I start using the latest omap3isp-next branch from Laurent's git tree [1]. The MT9V034 image sensor is very similar to MT9V032 sensor, so I modified current driver to accept MT9V034 sensor adding the chip ID. The driver recognizes the sensor and I'm able to capture some frames. I started capturing directly frames using the pipeline Sensor - CCDC ./media-ctl -r ./media-ctl -l 'mt9v032 3-005c:0-OMAP3 ISP CCDC:0[1]' ./media-ctl -l 'OMAP3 ISP CCDC:1-OMAP3 ISP CCDC output:0[1]' ./media-ctl -f 'mt9v032 3-005c:0 [SGRBG10 752x480]' ./media-ctl -f 'OMAP3 ISP CCDC:1 [SGRBG10 752x480]' # Test pattern ./yavta --set-control '0x00981901 1' /dev/v4l-subdev8 # ./yavta -p -f SGRBG10 -s 752x480 -n 4 --capture=3 /dev/video2 --file=img-#.bin To convert to jpg I used bayer2rgb [2] program executing following command, $ convert -size 752x480 GRBG_BAYER:./img-00.bin img-00.jpg And the result image looks like this http://downloads.isee.biz/pub/files/patterns/img-from-sensor.jpg Seems good, so I tried to use following pipeline Sensor - CCDC - Preview - Resizer ./media-ctl -r ./media-ctl -l 'mt9v032 3-005c:0-OMAP3 ISP CCDC:0[1]' ./media-ctl -l 'OMAP3 ISP CCDC:2-OMAP3 ISP preview:0[1]' ./media-ctl -l 'OMAP3 ISP preview:1-OMAP3 ISP resizer:0[1]' ./media-ctl -l 'OMAP3 ISP resizer:1-OMAP3 ISP resizer output:0[1]' ./media-ctl -V 'mt9v032 3-005c:0[SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP CCDC:0 [SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP CCDC:2 [SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP preview:1 [UYVY 752x480]' ./media-ctl -V 'OMAP3 ISP resizer:1 [UYVY 752x480]' # Set Test pattern ./yavta --set-control '0x00981901 1' /dev/v4l-subdev8 ./yavta -f UYVY -s 752x480 --capture=3 --file=img-#.uyvy /dev/video6 I used 'convert' program to pass from UYVY to jpg, $ convert -size 752x480 img-00.uyvy img-00.jpg and the result image looks like this http://downloads.isee.biz/pub/files/patterns/img-from-resizer.jpg As you can see, the image is wrong and I'm not sure if the problem is from the sensor, from the previewer, from the resizer or from my conversion. Anyone have idea where should I look ? Or which is the source of the problem ? Could you please post the output of all the above media-ctl and yavta runs, as well as the captured raw binary frame ? Of course, The log configuring the pipeline Sensor - CCDC is http://pastebin.com/WX8ex5x2 and the raw image can be found http://downloads.isee.biz/pub/files/patterns/img-00.bin And the log configuring the pipeline Sensor - CCDC - Previewer - Resizer is http://pastebin.com/wh5ZJwne and the raw image can be found http://downloads.isee.biz/pub/files/patterns/img-00.uyvy [1] http://git.linuxtv.org/pinchartl/media.git/shortlog/refs/heads/omap3isp- omap3isp-next [2] https://github.com/jdthomas/bayer2rgb -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 4/6] videobuf2-core: fill in length field for multiplanar buffers.
On Fri September 21 2012 18:54:12 Hans Verkuil wrote: On Fri September 21 2012 18:47:54 Sylwester Nawrocki wrote: On 09/21/2012 06:23 PM, Hans Verkuil wrote: On Fri September 21 2012 18:13:20 Sylwester Nawrocki wrote: Hi Hans, On 09/19/2012 04:37 PM, Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com length should be set to num_planes in __fill_v4l2_buffer(). That way the caller knows how many planes there are in the buffer. Signed-off-by: Hans Verkuil hans.verk...@cisco.com I think this would break VIDIOC_CREATE_BUFS. We need per buffer num_planes. Consider a use case where device is streaming with 2-planar pixel format and we invoke VIDIOC_CREATE_BUFS with single-planar format. On a single queue there will be buffers with different number of planes. The number of planes information must be attached to a buffer, otherwise VIDIOC_QUERYBUF won't work. That's a very good point and one I need to meditate on. However, your comment applies to patch 1/6, not to this one. This patch is about whether or not the length field of v4l2_buffer should be filled in with the actual number of planes used by that buffer or not. Yes, right. Sorry, I was editing response to multiple patches from this series and have mixed things a bit. I agree that it is logical and expected to update struct v4l2_buffer for user space. OK, great. That's was actually the main reason for working on this as this unexpected behavior bit me when writing mplane streaming support for v4l2-ctl. I have spent some time on this series, and even prepared a patch for s5p-mfc, as it relies on num_planes being in struct vb2_buffer. But then a realized there could be buffers with distinct number of planes an a single queue. I'll get back to you about this, probably on Monday. I need to think about the implications of this. You are absolutely right about that. It makes my patch a bit more complex since you have to be careful in VIDIOC_DQBUF not to dequeue unless the provided v4l2_buffer struct has enough room to store the plane information. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] V4L: Add s_rx_buffer subdev video operation
On Monday 24 September 2012 16:44:54 Sakari Ailus wrote: On Mon, Sep 24, 2012 at 03:26:53PM +0200, Sylwester Nawrocki wrote: The s_rx_buffer callback allows the host to set buffer for non-image (meta) data at a subdev. This callback can be implemented by an image sensor or a MIPI-CSI receiver, allowing the host to retrieve the frame embedded data from a subdev. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- include/media/v4l2-subdev.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 22ab09e..28067ed 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -274,6 +274,10 @@ struct v4l2_subdev_audio_ops { s_mbus_config: set a certain mediabus configuration. This operation is added for compatibility with soc-camera drivers and should not be used by new software. + + s_rx_buffer: set a host allocated memory buffer for the subdev. The subdev +can adjust @size to a lower value and must not write more data to the +buffer starting at @data than the original value of @size. */ struct v4l2_subdev_video_ops { int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32 config); @@ -327,6 +331,8 @@ struct v4l2_subdev_video_ops { struct v4l2_mbus_config *cfg); int (*s_mbus_config)(struct v4l2_subdev *sd, const struct v4l2_mbus_config *cfg); + int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf, + unsigned int *size); }; /* How about useing a separate video buffer queue for the purpose? That would provide a nice way to pass it to the user space where it's needed. It'd also play nicely together with the frame layout descriptors. Beside, a void *buf wouldn't support DMA. Only subdevs that use PIO to transfer meta data could be supported by this. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC] V4L: Add get/set_frame_desc subdev callbacks
Add subdev callbacks for setting up and retrieving parameters of the frame on media bus that are not exposed to user space directly. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- Hi All, This patch is intended as an initial, mostly a stub, implementation of the media bus frame format descriptors idea outlined in Sakari's RFC [1]. I included in this patch only what is necessary for the s5p-fimc driver to capture JPEG and interleaved JPEG/YUV image data from M-5MOLS and S5C73M3 cameras. The union containing per media bus type structures describing bus specific configuration is not included here, it likely needs much discussions and I would like to get this patch merged for v3.7 if possible. To follow is a patch adding users of these new subdev operations. Comments are welcome. Thanks, Sylwester [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg43530.html --- include/media/v4l2-subdev.h | 42 ++ 1 file changed, 42 insertions(+) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 28067ed..f5d8441 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -21,6 +21,7 @@ #ifndef _V4L2_SUBDEV_H #define _V4L2_SUBDEV_H +#include linux/types.h #include linux/v4l2-subdev.h #include media/media-entity.h #include media/v4l2-common.h @@ -45,6 +46,7 @@ struct v4l2_fh; struct v4l2_subdev; struct v4l2_subdev_fh; struct tuner_setup; +struct v4l2_mbus_frame_desc; /* decode_vbi_line */ struct v4l2_decode_vbi_line { @@ -226,6 +228,36 @@ struct v4l2_subdev_audio_ops { int (*s_stream)(struct v4l2_subdev *sd, int enable); }; +/* Indicates the @length field specifies maximum data length. */ +#define V4L2_MBUS_FRAME_DESC_FL_LEN_MAX(1U 0) +/* Indicates user defined data format, i.e. non standard frame format. */ +#define V4L2_MBUS_FRAME_DESC_FL_BLOB (1U 1) + +/** + * struct v4l2_mbus_frame_desc_entry - media bus frame description structure + * @flags: V4L2_MBUS_FRAME_DESC_FL_* flags + * @pixelcode: media bus pixel code, valid if FRAME_DESC_FL_BLOB is not set + * @length: number of octets per frame, valid for compressed or unspecified + * formats + */ +struct v4l2_mbus_frame_desc_entry { + u16 flags; + u32 pixelcode; + u32 length; +}; + +#define V4L2_FRAME_DESC_ENTRY_MAX 4 + +/** + * struct v4l2_mbus_frame_desc - media bus data frame description + * @entry: frame descriptors array + * @num_entries: number of entries in @entry array + */ +struct v4l2_mbus_frame_desc { + struct v4l2_mbus_frame_desc_entry entry[V4L2_FRAME_DESC_ENTRY_MAX]; + unsigned short num_entries; +}; + /* s_std_output: set v4l2_std_id for video OUTPUT devices. This is ignored by video input devices. @@ -461,6 +493,12 @@ struct v4l2_subdev_ir_ops { struct v4l2_subdev_ir_parameters *params); }; +/** + * struct v4l2_subdev_pad_ops - v4l2-subdev pad level operations + * @get_frame_desc: get the current low level media bus frame parameters. + * @get_frame_desc: set the low level media bus frame parameters, @fd array + * may be adjusted by the subdev driver to device capabilities. + */ struct v4l2_subdev_pad_ops { int (*enum_mbus_code)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, struct v4l2_subdev_mbus_code_enum *code); @@ -489,6 +527,10 @@ struct v4l2_subdev_pad_ops { struct v4l2_subdev_format *source_fmt, struct v4l2_subdev_format *sink_fmt); #endif /* CONFIG_MEDIA_CONTROLLER */ + int (*get_frame_desc)(struct v4l2_subdev *sd, unsigned int pad, + struct v4l2_mbus_frame_desc *fd); + int (*set_frame_desc)(struct v4l2_subdev *sd, unsigned int pad, + struct v4l2_mbus_frame_desc *fd); }; struct v4l2_subdev_ops { -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] gspca_pac7302: add support for green balance adjustment
Hi, On 09/24/2012 03:36 PM, Frank Schäfer wrote: Hi Hans, Am 24.09.2012 12:47, schrieb Hans de Goede: Hi, Sorry for the slow response ... On 09/20/2012 01:54 PM, Frank Schäfer wrote: Hi, Am 20.09.2012 11:08, schrieb Hans de Goede: snip Many webcams have RGB gains, but we don't have standard CID-s for these, instead we've Blue and Red Balance. This has grown historically because of the bttv cards which actually have Blue and Red balance controls in hardware, rather then the usual RGB gain triplet. Various gspca drivers cope with this in different ways. If you look at the pac7302 driver before your latest 4 patches it has a Red and Blue balance control controlling the Red and Blue gain, and a Whitebalance control, which is not White balance at all, but simply controls the green gain... Ok, so if I understand you right, red+green+blue balance = white balance. And because we already have defined red, blue and whitebalance controls for historical reasons, we don't need green balance ? Maybe that matches physics, but I don't think it's a sane approach for a user interface... No what I was trying to say is that the balance controls are for hardware which actually has balance controls and not per color gains (such as the bt87x chips), but they are being abused by many drivers to add support for per color gains. And then you miss one control. And in the case of the pac7302 driver the original route was taken of using whitebalance to control the green gain. Which is wrong IMHO, but it is what the driver does know. A proper fix would be to introduce new controls for all 3 gains, and use those instead of using the balance controls + adding a 3th balance control being discussed in the thread titled: Gain controls in v4l2-ctrl framework. Ok, it seems I'm misunderstanding the meaning of color gain and balance... Anyway, I would say that it makes sense to have per color AND global controls, so a V4L2_CID_GREEN_BALANCE would be missing. Weather it makes sense to use them at the same time is a different question. And why do you think the controls in question are gain controls instead of balance controls ? Balance suggest balancing it against some fixed value, where as once there are 3 for all of r, g and b, there is no fixed value, then the registers are just controlling an amplification factor (which could be less then 1.0) and that is normally called a gain. If you look in most sensor datasheets they talk about color gains not balance controls ... Anyways this is mostly just semantics. We are working on defining standard controls for per color gains, and once we have those we can map them to reg 0x01 - 0x03. The Windows driver calls them balance controls, too (which could of course also be a Windows driver or API issue...). And as said other drivers have similar (albeit usually different) hacks. At a minimum I would like you to rework your patches to: 1) Not add the new Green balance, and instead modify the existing whitebalance to control the new green gain you've found. Keeping things as broken as they are, but not worse; and I prefer waiting for the results of the discussion you are proposing further down. I see in your next mail that you've changed your mind. So I would like to move forward with this by adding your 2 patches + 1 more patch to also make the whitebalance control (which really is the green gain control) use 0x02 rather then 0xc6. To do this we must make sure that 0xc6 has a proper fixed / default setting. So what does the windows driver use for this? 1 like with 0xc5 and 0xc7 ? And can you do a 3th patch to make the whitebalance control control 0x02 rather then 0xc6 like you did for red and blue balance? No, we shouldn't do that. Reg 0xc6 (currently called white balance temperature) definitely works different compared to register 0x02. Whatever it does exactly, it's not green gain or balance adjustment. Will try to figure out next time. The Windows driver doesn't use this register for an (user settable) image control. It just sets its value to 55, which I fixed with one of the patches from my previous patch series, so we have the correct default value now. Ah, interesting! 0xc6 is also different from regs 0xc5 and 0xc7: settable values are 0-255 compared to 0-3. True, I should have noticed that. So let's not touch 0xc6 / white balance until we know its real meaning and just apply the first 2 patches. OK, will do. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] pwc: Add return code check at vb2_queue_init()
Hi, Thanks I've added this to my media tree and it will be included in my next pull-req to Mauro. Regards, Hans On 09/17/2012 03:47 PM, elezegar...@gmail.com wrote: From: Ezequiel Garcia elezegar...@gmail.com This function returns an integer and it's mandatory to check the return code. Cc: Hans de Goede hdego...@redhat.com Signed-off-by: Ezequiel Garcia elezegar...@gmail.com --- drivers/media/usb/pwc/pwc-if.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c index 42e36ba..31d082e 100644 --- a/drivers/media/usb/pwc/pwc-if.c +++ b/drivers/media/usb/pwc/pwc-if.c @@ -1000,7 +1000,9 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id pdev-vb_queue.buf_struct_size = sizeof(struct pwc_frame_buf); pdev-vb_queue.ops = pwc_vb_queue_ops; pdev-vb_queue.mem_ops = vb2_vmalloc_memops; - vb2_queue_init(pdev-vb_queue); + rc = vb2_queue_init(pdev-vb_queue); + if (rc) + goto err_free_mem; /* Init video_device structure */ memcpy(pdev-vdev, pwc_template, sizeof(pwc_template)); -- 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/4] gspca_pac7302: correct register documentation
Hi, On 09/23/2012 03:29 PM, Frank Schäfer wrote: R,G,B balance registers are 0x01-0x03 instead of 0x02-0x04, which lead to the wrong conclusion that values are inverted. Exposure is controlled via page 3 registers and this is already documented. Also fix a whitespace issue. Signed-off-by: Frank Schäfer fschaefer@googlemail.com Thanks, as discussed I've added the first 2 patches to my media tree and they will be included in my next pull-req to Mauro. Regards, Hans --- drivers/media/usb/gspca/pac7302.c | 11 +-- 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/media/usb/gspca/pac7302.c b/drivers/media/usb/gspca/pac7302.c index 2d5c6d83..4894ac1 100644 --- a/drivers/media/usb/gspca/pac7302.c +++ b/drivers/media/usb/gspca/pac7302.c @@ -29,14 +29,13 @@ * Register page 0: * * AddressDescription - * 0x02Red balance control - * 0x03Green balance control - * 0x04Blue balance control - * Valus are inverted (0=max, 255=min). + * 0x01Red balance control + * 0x02Green balance control + * 0x03Blue balance control * The Windows driver uses a quadratic approach to map * the settable values (0-200) on register values: - * min=0x80, default=0x40, max=0x20 - * 0x0f-0x20 Colors, saturation and exposure control + * min=0x20, default=0x40, max=0x80 + * 0x0f-0x20 Color and saturation control * 0xa2-0xab Brightness, contrast and gamma control * 0xb6 Sharpness control (bits 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
[PATCH] media_build: add module_pci_driver to compat.h
This patch corrects this warnings on a Ubuntu 10.04 system with kernel 2.6.32-43: bt87x.c:972: warning: data definition has no type or storage class bt87x.c:972: warning: type defaults to 'int' in declaration of 'module_pci_driver' bt87x.c:972: warning: parameter names (without types) in function declaration bt87x.c:965: warning: 'bt87x_driver' defined but not used core.c:321: warning: data definition has no type or storage class core.c:321: warning: type defaults to 'int' in declaration of 'module_pci_driver' core.c:321: warning: parameter names (without types) in function declaration core.c:314: warning: 'solo_pci_driver' defined but not used Signed-off-by: Gianluca Gennari gennar...@gmail.com --- v4l/compat.h |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/v4l/compat.h b/v4l/compat.h index fdc6d4a..8d5c13a 100644 --- a/v4l/compat.h +++ b/v4l/compat.h @@ -1064,4 +1064,10 @@ static inline int usb_endpoint_maxp(const struct usb_endpoint_descriptor *epd) #define printk_ratelimited printk #endif +#ifndef module_pci_driver +#define module_pci_driver(__pci_driver) \ + module_driver(__pci_driver, pci_register_driver, \ + pci_unregister_driver) +#endif + #endif /* _COMPAT_H */ -- 1.7.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
[PATCH] media-ctl: Fix build error with newer autotools
Rename configure.in to be configure.ac - required for newer versions of autotools (older versions silently handled this, now it's an error) Signed-off-by: Gary Thomas g...@mlbassoc.com --- configure.ac | 93 ++ configure.in | 93 -- 2 files changed, 93 insertions(+), 93 deletions(-) create mode 100644 configure.ac delete mode 100644 configure.in diff --git a/configure.ac b/configure.ac new file mode 100644 index 000..98459d4 --- /dev/null +++ b/configure.ac @@ -0,0 +1,93 @@ +AC_PREREQ([2.61]) +AC_INIT([media-ctl], [0.0.1], [laurent.pinch...@ideasonboard.com]) +AC_CONFIG_SRCDIR([src/main.c]) +AC_CONFIG_AUX_DIR([config]) +AC_CONFIG_HEADERS([config.h]) +AC_CONFIG_MACRO_DIR([m4]) + +AM_INIT_AUTOMAKE([-Wall -Werror foreign]) + +# Checks for programs. +AC_PROG_CC +AM_PROG_CC_C_O +# automake 1.12 seems to require this, but automake 1.11 doesn't recognize it +m4_ifdef([AM_PROG_AR], [AM_PROG_AR]) +AC_PROG_LIBTOOL + +# Checks for libraries. + +AC_ARG_WITH([libudev], +AS_HELP_STRING([--with-libudev], +[Enable libudev to detect a device name])) + +AS_IF([test x$with_libudev = xyes], +[PKG_CHECK_MODULES(libudev, libudev, have_libudev=yes, have_libudev=no)], +[have_libudev=no]) + +AS_IF([test x$have_libudev = xyes], +[ +AC_DEFINE([HAVE_LIBUDEV], [], [Use libudev]) +LIBUDEV_CFLAGS=$libudev_CFLAGS +LIBUDEV_LIBS=$libudev_LIBS +AC_SUBST(LIBUDEV_CFLAGS) +AC_SUBST(LIBUDEV_LIBS) +], +[AS_IF([test x$with_libudev = xyes], +[AC_MSG_ERROR([libudev requested but not found]) +]) +]) + + +# Kernel headers path. +AC_ARG_WITH(kernel-headers, +[AC_HELP_STRING([--with-kernel-headers=DIR], +[specify path of Linux kernel headers [/usr/src/kernel-headers]])], +[case ${withval} in +yes | no) AC_MSG_ERROR([bad value ${withval} for --with-kernel-headers]) ;; +*) KERNEL_HEADERS_DIR=${withval} ;; + esac], +[KERNEL_HEADERS_DIR=/usr/src/kernel-headers]) + +CPPFLAGS=-I$KERNEL_HEADERS_DIR/include + +# Checks for header files. +AC_CHECK_HEADERS([linux/media.h \ + linux/types.h \ + linux/v4l2-mediabus.h \ + linux/v4l2-subdev.h \ + linux/videodev2.h], + [], + [echo ERROR: Kernel header file not found or not usable!; exit 1]) + +AC_CHECK_HEADERS([fcntl.h \ + stdlib.h \ + string.h \ + sys/ioctl.h \ + sys/time.h \ + unistd.h], + [], + [echo ERROR: Header file not found or not usable!; exit 1]) + +# Checks for typedefs, structures, and compiler characteristics. +AC_C_INLINE +AC_TYPE_SIZE_T +AC_CHECK_MEMBERS([struct stat.st_rdev]) + +# Checks for library functions. +AC_HEADER_MAJOR +AS_IF([test x$cross_compiling != xyes], +[ +AC_FUNC_MALLOC +AC_FUNC_REALLOC +]) +AC_CHECK_FUNCS([memset strerror strrchr strtoul]) + +AC_CONFIG_FILES([ + Makefile + src/Makefile + libmediactl.pc + libv4l2subdev.pc +]) + +AC_OUTPUT + diff --git a/configure.in b/configure.in deleted file mode 100644 index 98459d4..000 --- a/configure.in +++ /dev/null @@ -1,93 +0,0 @@ -AC_PREREQ([2.61]) -AC_INIT([media-ctl], [0.0.1], [laurent.pinch...@ideasonboard.com]) -AC_CONFIG_SRCDIR([src/main.c]) -AC_CONFIG_AUX_DIR([config]) -AC_CONFIG_HEADERS([config.h]) -AC_CONFIG_MACRO_DIR([m4]) - -AM_INIT_AUTOMAKE([-Wall -Werror foreign]) - -# Checks for programs. -AC_PROG_CC -AM_PROG_CC_C_O -# automake 1.12 seems to require this, but automake 1.11 doesn't recognize it -m4_ifdef([AM_PROG_AR], [AM_PROG_AR]) -AC_PROG_LIBTOOL - -# Checks for libraries. - -AC_ARG_WITH([libudev], -AS_HELP_STRING([--with-libudev], -[Enable libudev to detect a device name])) - -AS_IF([test x$with_libudev = xyes], -[PKG_CHECK_MODULES(libudev, libudev, have_libudev=yes, have_libudev=no)], -[have_libudev=no]) - -AS_IF([test x$have_libudev = xyes], -[ -AC_DEFINE([HAVE_LIBUDEV], [], [Use libudev]) -LIBUDEV_CFLAGS=$libudev_CFLAGS -LIBUDEV_LIBS=$libudev_LIBS -AC_SUBST(LIBUDEV_CFLAGS) -AC_SUBST(LIBUDEV_LIBS) -], -[AS_IF([test x$with_libudev = xyes], -[AC_MSG_ERROR([libudev requested but not found]) -]) -]) - - -# Kernel headers path. -AC_ARG_WITH(kernel-headers, -[AC_HELP_STRING([--with-kernel-headers=DIR], -[specify path of Linux kernel headers [/usr/src/kernel-headers]])], -[case ${withval} in -yes | no) AC_MSG_ERROR([bad value ${withval} for --with-kernel-headers]) ;; -*) KERNEL_HEADERS_DIR=${withval} ;; - esac], -[KERNEL_HEADERS_DIR=/usr/src/kernel-headers]) - -CPPFLAGS=-I$KERNEL_HEADERS_DIR/include - -# Checks for header files. -AC_CHECK_HEADERS([linux/media.h \ - linux/types.h \ -
Re: Gain controls in v4l2-ctrl framework
Hi Hans, On Mon, Sep 24, 2012 at 4:25 PM, Hans de Goede hdego...@redhat.com wrote: Hi, On 09/23/2012 01:26 PM, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE Not all sensors have separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE, so we will need a separate control for sensors which have one combined gain called simply V4L2_CID_GAIN_GREEN Agreed Also do we really need separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE controls? I know hardware has them, but in my experience that is only done as it is simpler to make the hardware this way (fully symmetric sensor grid), have you ever tried actually using different gain settings for the 2 different green rows ? Never tried it. I've and that always results in an ugly checker board pattern. So I think we can and should only have a V4L2_CID_GAIN_GREEN, and for sensors with 2 green gains have that control both, forcing both to always have the same setting, which is really what you want anyways ... Agreed. Regards, --Prabhakar Lad 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET GAIN_OFFSET that sounds a bit weird... GAIN_OFFSET sounds like it is a number which gets added to the 3/4 gain settings before the gain gets applied, but I assume that you just mean a number which gets added to the value from the pixel, either before or after the gain is applied and I must admit I cannot come up with a better name. I believe (not sure) that some sensors have these per color ... The question is if it makes sense to actually control this per color though, I don't think it does as it is meant to compensate for any fixed measuring errors, which are the same for all 3/4 colors. Note that all the sensor cells are exactly the same, later on a color grid gets added on top of the sensors to turn them into r/g/b cells, but physically they are the same cells, so with the same process and temperature caused measuring errors... Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 0/5] s5p-fimc: Add interleaved image data capture support
Hi All, This patch series adds device/vendor specific media bus pixel code section and defines S5C73MX camera specific media bus pixel code, along with corresponding fourcc. I realize this isn't probably the best possible solution but I don't know how to better handle this without major changes in V4L2 API. The third patch adds support for MIPI-CSI2 Embedded Data capture in Samsung S5P/Exynos MIPI-CSIS device. It depends on patch [PATCH RFC] V4L: Add s_rx_buffer subdev video operation. The fourth patch extends s5p-fimc driver to allow it to support 2-planar V4L2_PIX_FMT_S5C_UYVY_JPG format. More details can be found in the patch summary. The [get/set]_frame_desc subdev callback are used only to retrive from a sensor subdev required buffer size. It depends on patch [PATCH RFC] V4L: Add get/set_frame_desc subdev callbacks The fifth patch adds [get/set]_frame_desc op handlers to the m5mols driver as an example. I prepared also similar patch for S5C73M3 sensor where 2 frame description entries are used, but that driver is not yet mainlined due to a few missing items in V4L2 required to fully control it, so I didn't include that patch in this series. Comments, suggestions welcome. Thanks, Sylwester Sylwester Nawrocki (5): V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition s5p-csis: Add support for non-image data packets capture s5p-fimc: Add support for V4L2_PIX_FMT_S5C_UYVY_JPG fourcc m5mols: Implement .get_frame_desc subdev callback Documentation/DocBook/media/v4l/compat.xml | 4 + Documentation/DocBook/media/v4l/pixfmt.xml | 9 ++ Documentation/DocBook/media/v4l/subdev-formats.xml | 45 drivers/media/i2c/m5mols/m5mols.h | 9 ++ drivers/media/i2c/m5mols/m5mols_capture.c | 3 + drivers/media/i2c/m5mols/m5mols_core.c | 47 drivers/media/i2c/m5mols/m5mols_reg.h | 1 + drivers/media/platform/s5p-fimc/fimc-capture.c | 128 ++--- drivers/media/platform/s5p-fimc/fimc-core.c| 19 ++- drivers/media/platform/s5p-fimc/fimc-core.h| 28 - drivers/media/platform/s5p-fimc/fimc-reg.c | 23 +++- drivers/media/platform/s5p-fimc/fimc-reg.h | 3 +- drivers/media/platform/s5p-fimc/mipi-csis.c| 59 +- include/linux/v4l2-mediabus.h | 5 + include/linux/videodev2.h | 1 + 15 files changed, 351 insertions(+), 33 deletions(-) -- 1.7.11.3 -- 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 RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format
This patch adds media bus pixel code for the interleaved JPEG/UYVY image format used by S5C73MX Samsung cameras. This interleaved image data is transferred on MIPI-CSI2 bus as User Defined Byte-based Data. It also defines an experimental vendor and device specific media bus formats section and adds related DocBook documentation. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- Documentation/DocBook/media/v4l/compat.xml | 4 ++ Documentation/DocBook/media/v4l/subdev-formats.xml | 45 ++ include/linux/v4l2-mediabus.h | 5 +++ 3 files changed, 54 insertions(+) diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index 98e8d08..5d2480b 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2605,6 +2605,10 @@ ioctls./para listitem paraSupport for frequency band enumeration: VIDIOC-ENUM-FREQ-BANDS; ioctl./para /listitem +listitem + paraVendor and device specific media bus pixel formats. + xref linkend=v4l2-mbus-vendor-spec-fmts /./para +/listitem /itemizedlist /section diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml b/Documentation/DocBook/media/v4l/subdev-formats.xml index 49c532e..d7aa870 100644 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml @@ -2565,5 +2565,50 @@ /tgroup /table /section + +section id=v4l2-mbus-vendor-spec-fmts + titleVendor and Device Specific Formats/title + + note + title Experimental /title + paraThis is an link linkend=experimentalexperimental/link +interface and may change in the future./para + /note + + para This section lists complex data formats that are either vendor or + device specific. These formats comprise raw and compressed image data + and optional meta-data within a single frame. + /para + + paraThe following table lists the existing vendor and device specific + formats./para + + table pgwide=0 frame=none id=v4l2-mbus-pixelcode-vendor-specific + titleVendor and device specific formats/title + tgroup cols=3 + colspec colname=id align=left / + colspec colname=code align=left/ + colspec colname=remarks align=left/ + thead + row + entryIdentifier/entry + entryCode/entry + entryComments/entry + /row + /thead + tbody valign=top + row id=V4L2-MBUS-FMT-S5C-UYVY-JPG-1X8 + entryV4L2_MBUS_FMT_S5C_UYVY_JPG_1X8/entry + entry0x8001/entry + entry + Interleaved raw UYVY and JPEG image format with embedded + meta-data, produced by S3C73M3 camera sensors. + /entry + /row + /tbody + /tgroup + /table +/section + /section /section diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h index 5ea7f75..b98c566 100644 --- a/include/linux/v4l2-mediabus.h +++ b/include/linux/v4l2-mediabus.h @@ -92,6 +92,11 @@ enum v4l2_mbus_pixelcode { /* JPEG compressed formats - next is 0x4002 */ V4L2_MBUS_FMT_JPEG_1X8 = 0x4001, + + /* Vendor specific formats - next is 0x8002 */ + + /* S5C73M3 interleaved UYVY and JPEG */ + V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 = 0x8001, }; /** -- 1.7.11.3 -- 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 RFC 2/5] V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition
The V4L2_PIX_FMT_S5C_UYVY_JPG is a two-plane image format used by Samsung S5C73M3 camera. The first plane contains interleaved UYVY and JPEG data, followed by meta-data in form of offsets to the UYVU data blocks. The second plane contains additional meta-data needed for extracting JPEG and UYVY data stream from the first plane, in particular an offset to the first entry of an array of data offsets in the first plane. The offsets are expressed as 4-byte unsigned integers. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- Documentation/DocBook/media/v4l/pixfmt.xml | 9 + include/linux/videodev2.h | 1 + 2 files changed, 10 insertions(+) diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml b/Documentation/DocBook/media/v4l/pixfmt.xml index 1ddbfab..9caed9b 100644 --- a/Documentation/DocBook/media/v4l/pixfmt.xml +++ b/Documentation/DocBook/media/v4l/pixfmt.xml @@ -996,6 +996,15 @@ the other bits are set to 0./entry entryOld 6-bit greyscale format. Only the most significant 6 bits of each byte are used, the other bits are set to 0./entry /row + row id=V4L2-PIX-FMT-JPG-YUYV-S5C + entryconstantV4L2_PIX_FMT_S5C_YUYV_JPG/constant/entry + entry'S5CJ'/entry + entryTwo-planar format used by Samsung S5C73MX cameras.The first +plane contains interleaved JPEG and YUYV data, followed by meta data describing +layout of the YUYV and JPEG data blocks. The second plane contains additional +information about data layout in the first plane, like an offset to the array +of offsets to YUVY data chunks./entry + /row /tbody /tgroup /table diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 4862165..aa6fb4d 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -435,6 +435,7 @@ struct v4l2_pix_format { #define V4L2_PIX_FMT_KONICA420 v4l2_fourcc('K', 'O', 'N', 'I') /* YUV420 planar in blocks of 256 pixels */ #define V4L2_PIX_FMT_JPGL v4l2_fourcc('J', 'P', 'G', 'L') /* JPEG-Lite */ #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */ +#define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'J') /* S5C73M3 interleaved YUYV/JPEG */ /* * F O R M A T E N U M E R A T I O N -- 1.7.11.3 -- 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 RFC 3/5] s5p-csis: Add support for non-image data packets capture
MIPI-CSI has internal memory mapped buffers for the frame embedded (non-image) data. There are two buffers, for even and odd frames which need to be saved after an interrupt is raised. The packet data buffers size is 4 KiB and there is no status register in the hardware where the actual non-image data size can be read from. Hence the driver copies whole packet data buffer into a buffer provided by the FIMC driver. This will form a separate plane in the user buffer. When FIMC DMA engine is stopped by the driver due the to user space not keeping up with buffer de-queuing the MIPI-CSIS will still run, however it must discard data which is not captured by FIMC. Which frames are actually capture by MIPI-CSIS is determined by means of the s_tx_buffer subdev callback. When it is not called after a single embedded data frame has been captured and copied and before next embedded data frame interrupt occurrs, subsequent embedded data frames will be dropped. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/s5p-fimc/mipi-csis.c | 51 + 1 file changed, 51 insertions(+) diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c b/drivers/media/platform/s5p-fimc/mipi-csis.c index c1c5c86..306410f 100644 --- a/drivers/media/platform/s5p-fimc/mipi-csis.c +++ b/drivers/media/platform/s5p-fimc/mipi-csis.c @@ -98,6 +98,11 @@ MODULE_PARM_DESC(debug, Debug level (0-2)); #define CSIS_MAX_PIX_WIDTH 0x #define CSIS_MAX_PIX_HEIGHT0x +/* Non-image packet data buffers */ +#define S5PCSIS_PKTDATA_ODD0x2000 +#define S5PCSIS_PKTDATA_EVEN 0x3000 +#define S5PCSIS_PKTDATA_SIZE SZ_4K + enum { CSIS_CLK_MUX, CSIS_CLK_GATE, @@ -144,6 +149,11 @@ static const struct s5pcsis_event s5pcsis_events[] = { }; #define S5PCSIS_NUM_EVENTS ARRAY_SIZE(s5pcsis_events) +struct csis_pktbuf { + u32 *data; + unsigned int len; +}; + /** * struct csis_state - the driver's internal state data structure * @lock: mutex serializing the subdev and power management operations, @@ -160,6 +170,7 @@ static const struct s5pcsis_event s5pcsis_events[] = { * @csis_fmt: current CSIS pixel format * @format: common media bus format for the source and sink pad * @slock: spinlock protecting structure members below + * @pkt_buf: the frame embedded (non-image) data buffer * @events: MIPI-CSIS event (error) counters */ struct csis_state { @@ -177,6 +188,7 @@ struct csis_state { struct v4l2_mbus_framefmt format; struct spinlock slock; + struct csis_pktbuf pkt_buf; struct s5pcsis_event events[S5PCSIS_NUM_EVENTS]; }; @@ -534,6 +546,21 @@ static int s5pcsis_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, return 0; } +static int s5pcsis_s_rx_buffer(struct v4l2_subdev *sd, void *buf, + unsigned int *size) +{ + struct csis_state *state = sd_to_csis_state(sd); + unsigned long flags; + + spin_lock_irqsave(state-slock, flags); + *size = min_t(unsigned int, *size, S5PCSIS_PKTDATA_SIZE); + state-pkt_buf.data = buf; + state-pkt_buf.len = *size; + spin_unlock_irqrestore(state-slock, flags); + + return 0; +} + static int s5pcsis_log_status(struct v4l2_subdev *sd) { struct csis_state *state = sd_to_csis_state(sd); @@ -571,6 +598,7 @@ static struct v4l2_subdev_pad_ops s5pcsis_pad_ops = { }; static struct v4l2_subdev_video_ops s5pcsis_video_ops = { + .s_rx_buffer = s5pcsis_s_rx_buffer, .s_stream = s5pcsis_s_stream, }; @@ -580,16 +608,39 @@ static struct v4l2_subdev_ops s5pcsis_subdev_ops = { .video = s5pcsis_video_ops, }; +static void s5pcsis_pkt_copy(struct csis_state *state, u32 *buf, +u32 offset, u32 size) +{ + int i; + + for (i = 0; i S5PCSIS_PKTDATA_SIZE; i += 4, buf++) + *buf = s5pcsis_read(state, offset + i); +} + static irqreturn_t s5pcsis_irq_handler(int irq, void *dev_id) { struct csis_state *state = dev_id; + struct csis_pktbuf *pktbuf = state-pkt_buf; unsigned long flags; u32 status; status = s5pcsis_read(state, S5PCSIS_INTSRC); + s5pcsis_write(state, S5PCSIS_INTSRC, status); spin_lock_irqsave(state-slock, flags); + if ((status S5PCSIS_INTSRC_NON_IMAGE_DATA) pktbuf-data) { + u32 offset; + + if (status S5PCSIS_INTSRC_EVEN) + offset = S5PCSIS_PKTDATA_EVEN; + else + offset = S5PCSIS_PKTDATA_ODD; + + s5pcsis_pkt_copy(state, pktbuf-data, offset, pktbuf-len); + pktbuf-data = NULL; + } + /* Update the event/error counters */ if ((status S5PCSIS_INTSRC_ERRORS) || debug) {
[PATCH RFC 4/5] s5p-fimc: Add support for V4L2_PIX_FMT_S5C_UYVY_JPG fourcc
The V4L2_PIX_FMT_S5C_YUYV_JPG image format consists of 2 planes, the first containing interleaved JPEG/YUYV image data followed by meta-data describing the interleaving method and the second plane containing suplementary frame meta data. The image data is transferred with MIPI-CSI User Defined Byte-Based Data 1 type and is captured to memory by FIMC DMA engine. The meta data is transferred using MIPI-CSI2 Embedded 8-bit non Image Data and it is captured in the MIPI-CSI slave device and copied to the bridge provided buffer. To make sure the size of allocated buffers is correct for the subdevs configuration when VIDIOC_STREAMON ioctl is invoked, an additional check is added at the video pipeline validation function. Flag FMT_FLAGS_COMPRESSED indicates the buffer size must be retrieved from a sensor subdev. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/s5p-fimc/fimc-capture.c | 128 + drivers/media/platform/s5p-fimc/fimc-core.c| 19 +++- drivers/media/platform/s5p-fimc/fimc-core.h| 28 +- drivers/media/platform/s5p-fimc/fimc-reg.c | 23 - drivers/media/platform/s5p-fimc/fimc-reg.h | 3 +- drivers/media/platform/s5p-fimc/mipi-csis.c| 8 +- 6 files changed, 176 insertions(+), 33 deletions(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-capture.c b/drivers/media/platform/s5p-fimc/fimc-capture.c index 98d420f..a1f6c3c 100644 --- a/drivers/media/platform/s5p-fimc/fimc-capture.c +++ b/drivers/media/platform/s5p-fimc/fimc-capture.c @@ -177,7 +177,9 @@ static int fimc_capture_config_update(struct fimc_ctx *ctx) void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf) { + struct v4l2_subdev *csis = fimc-pipeline.subdevs[IDX_CSIS]; struct fimc_vid_cap *cap = fimc-vid_cap; + struct fimc_frame *f = cap-ctx-d_frame; struct fimc_vid_buffer *v_buf; struct timeval *tv; struct timespec ts; @@ -216,6 +218,25 @@ void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf) if (++cap-buf_index = FIMC_MAX_OUT_BUFS) cap-buf_index = 0; } + /* +* Set up a buffer at MIPI-CSIS if current image format +* requires the frame embedded data capture. +*/ + if (f-fmt-mdataplanes !list_empty(cap-active_buf_q)) { + unsigned int plane = ffs(f-fmt-mdataplanes) - 1; + unsigned int size = f-payload[plane]; + s32 index = fimc_hw_get_frame_index(fimc); + void *vaddr; + + list_for_each_entry(v_buf, cap-active_buf_q, list) { + if (v_buf-index != index) + continue; + vaddr = vb2_plane_vaddr(v_buf-vb, plane); + v4l2_subdev_call(csis, video, s_rx_buffer, +vaddr, size); + break; + } + } if (cap-active_buf_cnt == 0) { if (deq_buf) @@ -351,6 +372,8 @@ static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *pfmt, unsigned int size = (wh * fmt-depth[i]) / 8; if (pixm) sizes[i] = max(size, pixm-plane_fmt[i].sizeimage); + else if (fimc_fmt_is_user_defined(fmt-color)) + sizes[i] = frame-payload[i]; else sizes[i] = max_t(u32, size, frame-payload[i]); @@ -611,10 +634,10 @@ static struct fimc_fmt *fimc_capture_try_format(struct fimc_ctx *ctx, u32 mask = FMT_FLAGS_CAM; struct fimc_fmt *ffmt; - /* Color conversion from/to JPEG is not supported */ + /* Conversion from/to JPEG or User Defined format is not supported */ if (code ctx-s_frame.fmt pad == FIMC_SD_PAD_SOURCE - fimc_fmt_is_jpeg(ctx-s_frame.fmt-color)) - *code = V4L2_MBUS_FMT_JPEG_1X8; + fimc_fmt_is_user_defined(ctx-s_frame.fmt-color)) + *code = ctx-s_frame.fmt-mbus_code; if (fourcc *fourcc != V4L2_PIX_FMT_JPEG pad != FIMC_SD_PAD_SINK) mask |= FMT_FLAGS_M2M; @@ -628,18 +651,19 @@ static struct fimc_fmt *fimc_capture_try_format(struct fimc_ctx *ctx, *fourcc = ffmt-fourcc; if (pad == FIMC_SD_PAD_SINK) { - max_w = fimc_fmt_is_jpeg(ffmt-color) ? + max_w = fimc_fmt_is_user_defined(ffmt-color) ? pl-scaler_dis_w : pl-scaler_en_w; /* Apply the camera input interface pixel constraints */ v4l_bound_align_image(width, max_t(u32, *width, 32), max_w, 4, height, max_t(u32, *height, 32), FIMC_CAMIF_MAX_HEIGHT, -
[PATCH RFC 5/5] m5mols: Implement .get_frame_desc subdev callback
.get_frame_desc can be used by host interface driver to query properties of captured frames, e.g. required memory buffer size. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/i2c/m5mols/m5mols.h | 9 ++ drivers/media/i2c/m5mols/m5mols_capture.c | 3 ++ drivers/media/i2c/m5mols/m5mols_core.c| 47 +++ drivers/media/i2c/m5mols/m5mols_reg.h | 1 + 4 files changed, 60 insertions(+) diff --git a/drivers/media/i2c/m5mols/m5mols.h b/drivers/media/i2c/m5mols/m5mols.h index 15d3a4f..de3b755 100644 --- a/drivers/media/i2c/m5mols/m5mols.h +++ b/drivers/media/i2c/m5mols/m5mols.h @@ -19,6 +19,13 @@ #include media/v4l2-subdev.h #include m5mols_reg.h + +/* An amount of data transmitted in addition to the value + * determined by CAPP_JPEG_SIZE_MAX register. + */ +#define M5MOLS_JPEG_TAGS_SIZE 0x2 +#define M5MOLS_MAIN_JPEG_SIZE_MAX (5 * SZ_1M) + extern int m5mols_debug; enum m5mols_restype { @@ -67,12 +74,14 @@ struct m5mols_exif { /** * struct m5mols_capture - Structure for the capture capability * @exif: EXIF information + * @buf_size: internal JPEG frame buffer size, in bytes * @main: size in bytes of the main image * @thumb: size in bytes of the thumb image, if it was accompanied * @total: total size in bytes of the produced image */ struct m5mols_capture { struct m5mols_exif exif; + unsigned int buf_size; u32 main; u32 thumb; u32 total; diff --git a/drivers/media/i2c/m5mols/m5mols_capture.c b/drivers/media/i2c/m5mols/m5mols_capture.c index cb243bd..ab34cce 100644 --- a/drivers/media/i2c/m5mols/m5mols_capture.c +++ b/drivers/media/i2c/m5mols/m5mols_capture.c @@ -105,6 +105,7 @@ static int m5mols_capture_info(struct m5mols_info *info) int m5mols_start_capture(struct m5mols_info *info) { + unsigned int framesize = info-cap.buf_size - M5MOLS_JPEG_TAGS_SIZE; struct v4l2_subdev *sd = info-sd; int ret; @@ -121,6 +122,8 @@ int m5mols_start_capture(struct m5mols_info *info) if (!ret) ret = m5mols_write(sd, CAPP_MAIN_IMAGE_SIZE, info-resolution); if (!ret) + ret = m5mols_write(sd, CAPP_JPEG_SIZE_MAX, framesize); + if (!ret) ret = m5mols_set_mode(info, REG_CAPTURE); if (!ret) /* Wait until a frame is captured to ISP internal memory */ diff --git a/drivers/media/i2c/m5mols/m5mols_core.c b/drivers/media/i2c/m5mols/m5mols_core.c index 933014f..c780689 100644 --- a/drivers/media/i2c/m5mols/m5mols_core.c +++ b/drivers/media/i2c/m5mols/m5mols_core.c @@ -599,6 +599,51 @@ static int m5mols_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, return ret; } +static int m5mols_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad, +struct v4l2_mbus_frame_desc *fd) +{ + struct m5mols_info *info = to_m5mols(sd); + + if (pad != 0 || fd == NULL) + return -EINVAL; + + mutex_lock(info-lock); + /* +* .get_frame_desc is only used for compressed formats, +* thus we always return the capture frame parameters here. +*/ + fd-entry[0].length = info-cap.buf_size; + fd-entry[0].pixelcode = info-ffmt[M5MOLS_RESTYPE_CAPTURE].code; + mutex_unlock(info-lock); + + fd-entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX; + fd-num_entries = 1; + + return 0; +} + +static int m5mols_set_frame_desc(struct v4l2_subdev *sd, unsigned int pad, +struct v4l2_mbus_frame_desc *fd) +{ + struct m5mols_info *info = to_m5mols(sd); + struct v4l2_mbus_framefmt *mf = info-ffmt[M5MOLS_RESTYPE_CAPTURE]; + + if (pad != 0 || fd == NULL) + return -EINVAL; + + fd-entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX; + fd-num_entries = 1; + fd-entry[0].length = clamp_t(u32, fd-entry[0].length, + mf-width * mf-height, + M5MOLS_MAIN_JPEG_SIZE_MAX); + mutex_lock(info-lock); + info-cap.buf_size = fd-entry[0].length; + mutex_unlock(info-lock); + + return 0; +} + + static int m5mols_enum_mbus_code(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, struct v4l2_subdev_mbus_code_enum *code) @@ -615,6 +660,8 @@ static struct v4l2_subdev_pad_ops m5mols_pad_ops = { .enum_mbus_code = m5mols_enum_mbus_code, .get_fmt= m5mols_get_fmt, .set_fmt= m5mols_set_fmt, + .get_frame_desc = m5mols_get_frame_desc, + .set_frame_desc = m5mols_set_frame_desc, }; /** diff --git a/drivers/media/i2c/m5mols/m5mols_reg.h b/drivers/media/i2c/m5mols/m5mols_reg.h index 14d4be7..58d8027 100644 --- a/drivers/media/i2c/m5mols/m5mols_reg.h +++
RE: [RFC] Frame format descriptors
Hi Sakari Thank you for sharing your opinion on this subject. I know that for you it is an old topics but I would like to revive this RFC by asking you some questions about your proposal in order to move forward on this subject. Indeed, stream format description is very important and mandatory for every CSI-2 receiver in order to be well configured. I understand from your proposal that frame format descriptor is defined by the sensor/camera and retrieve by the CSI-2 receiver. Since the CSI-2 stream to capture does not have multiple pixel format (one pixel format but multiple embedded data) it does not bring so many complexity. But I have some difficulties to put things together in case of a sensor/camera providing 2 interleaved streams with 2 different pixel format such JPEG interleaved with YUV. In that case, what is the external trigger from the client to warn sensor/camera that the client is requesting an interleaved stream? New mbus pixel code added in the v4l2_mbus_pixelcode enum and defined in the sensor/camera pad? Kind regards, -- Vincent Abriou -- 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 v2 0/5] s5p-fimc: Add interleaved image data capture support
On 09/24/2012 04:55 PM, Sylwester Nawrocki wrote: Hi All, This patch series adds device/vendor specific media bus pixel code section and defines S5C73MX camera specific media bus pixel code, along with corresponding fourcc. I realize this isn't probably the best possible solution but I don't know how to better handle this without major changes in V4L2 API. The third patch adds support for MIPI-CSI2 Embedded Data capture in Samsung S5P/Exynos MIPI-CSIS device. It depends on patch [PATCH RFC] V4L: Add s_rx_buffer subdev video operation. The fourth patch extends s5p-fimc driver to allow it to support 2-planar V4L2_PIX_FMT_S5C_UYVY_JPG format. More details can be found in the patch summary. The [get/set]_frame_desc subdev callback are used only to retrive from a sensor subdev required buffer size. It depends on patch [PATCH RFC] V4L: Add get/set_frame_desc subdev callbacks The fifth patch adds [get/set]_frame_desc op handlers to the m5mols driver as an example. I prepared also similar patch for S5C73M3 sensor where 2 frame description entries are used, but that driver is not yet mainlined due to a few missing items in V4L2 required to fully control it, so I didn't include that patch in this series. I forgot to mention that this patch series with all dependencies can be found in git repository git://git.infradead.org/users/kmpark/linux-samsung v4l-framedesc http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/v4l-framedesc Comments, suggestions welcome. Thanks, Sylwester Sylwester Nawrocki (5): V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition s5p-csis: Add support for non-image data packets capture s5p-fimc: Add support for V4L2_PIX_FMT_S5C_UYVY_JPG fourcc m5mols: Implement .get_frame_desc subdev callback Documentation/DocBook/media/v4l/compat.xml | 4 + Documentation/DocBook/media/v4l/pixfmt.xml | 9 ++ Documentation/DocBook/media/v4l/subdev-formats.xml | 45 drivers/media/i2c/m5mols/m5mols.h | 9 ++ drivers/media/i2c/m5mols/m5mols_capture.c | 3 + drivers/media/i2c/m5mols/m5mols_core.c | 47 drivers/media/i2c/m5mols/m5mols_reg.h | 1 + drivers/media/platform/s5p-fimc/fimc-capture.c | 128 ++--- drivers/media/platform/s5p-fimc/fimc-core.c| 19 ++- drivers/media/platform/s5p-fimc/fimc-core.h| 28 - drivers/media/platform/s5p-fimc/fimc-reg.c | 23 +++- drivers/media/platform/s5p-fimc/fimc-reg.h | 3 +- drivers/media/platform/s5p-fimc/mipi-csis.c| 59 +- include/linux/v4l2-mediabus.h | 5 + include/linux/videodev2.h | 1 + 15 files changed, 351 insertions(+), 33 deletions(-) -- 1.7.11.3 -- 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 v5] of: add display helper (was: of: add videomode helper)
Hi! After the feedback I got on v4, I thought about the current state of the videomode helper and decided to reengineer it. The most confusion seems to stem from the fact, that a videomode does not have ranges. And that is correct. Therefore the description of the display now uses a list of timings. These timings support ranges, as they do in datasheets (min/typ/max). A device driver may choose to grep a display-description from the devicetree and directly work with that (matching parameters according to their range etc.). Or one can use the former struct videomode to convert from the timings to a videomode (at the moment it just grabs the typical-value from every timing-parameter). This videomode on the other hand, can then be converted to a mode the backend wants (drm_mode_info, fb_videomode,...). As of now, this intermediate step is a bit, well, unnecessary. But it provides a way to have a generic videomode and functions to possibly convert back-and-forth. In the end, this version does the same as of_videomode, but I hope makes the whole thing a little clearer. Thanks to everybody who reviewed the previous versions. Feedback is always welcome. Regards, Steffen Steffen Trumtrar (2): of: add helper to parse display specs video: add generic videomode description Documentation/devicetree/bindings/video/display | 208 ++ drivers/of/Kconfig |5 +++ drivers/of/Makefile |1 + drivers/of/of_display.c | 157 + drivers/video/Makefile |1 + drivers/video/videomode.c | 146 +++ include/linux/display.h | 85 ++ include/linux/of_display.h | 15 include/linux/videomode.h | 38 +++ 9 files changed, 656 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display create mode 100644 drivers/of/of_display.c create mode 100644 drivers/video/videomode.c create mode 100644 include/linux/display.h create mode 100644 include/linux/of_display.h create mode 100644 include/linux/videomode.h -- 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 driver support on OMAP3 system
Hi Pete, The next issue to solve is auto-exposure/white balance (AEWB). For this, Laurent has put together a nice example application in a project called omap3-isp-live [1]. This project builds two programs, 'live' and 'snapshot'. For my purposes, I want to have live streaming video. 'live' makes use of a local display and was not able to get this running using an HDMI display (on Beagleboard-xM). I modified the program to write the buffer data to stdout in the same way as yavta does, and use netcat to pipe the live data over a network socket to mplayer. This provides the video live using a network stream. This application wants allocate display buffers that are larger than coded into the kernel, so I had to modify the kernel to increase the buffer size. I'm still working on this and things are in a bit of disarray. I don't have suitable patches just yet, otherwise I would share them. [1] http://git.ideasonboard.org/omap3-isp-live.git -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Laurent Pinchart Sent: Monday, September 24, 2012 7:24 AM To: Guennadi Liakhovetski Cc: P Jackson; linux-media@vger.kernel.org Subject: Re: mt9t031 driver support on OMAP3 system Hi Pete, On Sunday 23 September 2012 00:04:20 Guennadi Liakhovetski wrote: On Sat, 22 Sep 2012, P Jackson wrote: I'm trying to incorporate an MT9T031-based sensor into a commercial small form-factor OMAP3 DM3730-based system which is supplied with sources for a 2.6.37 kernel and is somewhat out-dated.The application I'm working on requires a single image to be acquired from the sensor every once in a while which is then processed off-line by another algorithm on the OMAP3 I'd appreciate any advice from the list as to what the most suitable up to-date compatible kernel I should use would be and what other work I need to do in order to get things sorted. I would certainly advise to use a current kernel (e.g., 3.5). As for how, I know, that Laurent has worked on a similar tasks, you can find his posts in mailing list archives, or maybe he'll be able to advise you more specifically. You can have a look at http://git.linuxtv.org/pinchartl/media.git/shortlog/refs/heads/omap3isp- sensors-board to see how I've modified the ov772x driver to make it usable with the OMAP3 ISP. The patches are not upstreamable as such, I still need to work on them. I've explained the issues in detail on the mailing list. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] of: add helper to parse display specs
Parse a display-node with timings and hardware-specs from devictree. Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de --- Documentation/devicetree/bindings/video/display | 208 +++ drivers/of/Kconfig |5 + drivers/of/Makefile |1 + drivers/of/of_display.c | 157 + include/linux/display.h | 85 + include/linux/of_display.h | 15 ++ 6 files changed, 471 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/display create mode 100644 drivers/of/of_display.c create mode 100644 include/linux/display.h create mode 100644 include/linux/of_display.h diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display new file mode 100644 index 000..722766a --- /dev/null +++ b/Documentation/devicetree/bindings/video/display @@ -0,0 +1,208 @@ +display bindings +== + +display-node + + +required properties: + - none + +optional properties: + - default-timing: the default timing value + - width-mm, height-mm: Display dimensions in mm + - hsync-active-high (bool): Hsync pulse is active high + - vsync-active-high (bool): Vsync pulse is active high + - de-active-high (bool): Data-Enable pulse is active high + - pixelclk-inverted (bool): pixelclock is inverted + - pixel-per-clk + - link-width: number of channels (e.g. LVDS) + - bpp: bits-per-pixel + +timings-subnode +--- + +required properties: +subnodes that specify + - hactive, vactive: Display resolution + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters + in pixels + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in + lines + - clock: displayclock in Hz + +There are different ways of describing a display and its capabilities. The devicetree +representation corresponds to the one commonly found in datasheets for displays. +The description of the display and its timing is split in two parts: first the display +properties like size in mm and (optionally) multiple subnodes with the supported timings. +If a display supports multiple signal timings, the default-timing can be specified. + +Example: + + display@0 { + width-mm = 800; + height-mm = 480; + default-timing = timing0; + timings { + timing0: timing@0 { + /* 1920x1080p24 */ + clock = 5200; + hactive = 1920; + vactive = 1080; + hfront-porch = 25; + hback-porch = 25; + hsync-len = 25; + vback-porch = 2; + vfront-porch = 2; + vsync-len = 2; + hsync-active-high; + }; + }; + }; + +Every property also supports the use of ranges, so the commonly used datasheet +description with min typ max-tuples can be used. + +Example: + + timing1: timing@1 { + /* 1920x1080p24 */ + clock = 14850; + hactive = 1920; + vactive = 1080; + hsync-len = 0 44 60; + hfront-porch = 80 88 95; + hback-porch = 100 148 160; + vfront-porch = 0 4 6; + vback-porch = 0 36 50; + vsync-len = 0 5 6; + }; + +The display-property in a connector-node (e.g. hdmi, ldb,...) is used to connect +the display to that driver. +of_display expects a phandle, that specifies the display-node, in that property. + +Example: + + hdmi@0012 { + status = okay; + display = acme; + }; + +Usage in backend + + +A backend driver may choose to use the display directly and convert the timing +ranges to a suitable mode. Or it may just use the conversion of the display timings +to the required mode via the generic videomode struct. + + dtb +| +| of_get_display +↓ + struct display +| +| videomode_from_timings +↓ + --- struct videomode --- + | | + videomode_to_displaymode | | videomode_to_fb_videomode + ↓ ↓ +drm_display_mode fb_videomode + + +Conversion to videomode
[PATCH 2/2] video: add generic videomode description
Backend-independent videomode. At the moment this is not very different from fb_mode or drm_mode. It is supposed to be a generic description of videomodes and conversion step to the desired backend video mode representation. At the moment, only really makes sense when used with of_display Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de --- drivers/video/Makefile|1 + drivers/video/videomode.c | 146 + include/linux/videomode.h | 38 3 files changed, 185 insertions(+) create mode 100644 drivers/video/videomode.c create mode 100644 include/linux/videomode.h diff --git a/drivers/video/Makefile b/drivers/video/Makefile index ee8dafb..6a02fe0 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -170,3 +170,4 @@ obj-$(CONFIG_FB_VIRTUAL) += vfb.o #video output switch sysfs driver obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o +obj-y += videomode.o diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c new file mode 100644 index 000..b118d00 --- /dev/null +++ b/drivers/video/videomode.c @@ -0,0 +1,146 @@ +/* + * generic videomode helper + * + * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix + * + * This file is released under the GPLv2 + */ +#include linux/of.h +#include linux/fb.h +#include linux/slab.h +#include drm/drm_mode.h +#include linux/display.h +#include linux/videomode.h + +struct videomode *videomode_from_display(struct display *disp, int index) +{ + struct videomode *vm; + struct signal_timing *st; + + vm = kmalloc(sizeof(struct videomode *), GFP_KERNEL); + + if (!vm) + return NULL; + + st = display_get_timing(disp, index); + + vm-pixelclock = signal_timing_get_value(st-pixelclock, 0); + vm-hactive = signal_timing_get_value(st-hactive, 0); + vm-hfront_porch = signal_timing_get_value(st-hfront_porch, 0); + vm-hback_porch = signal_timing_get_value(st-hback_porch, 0); + vm-hsync_len = signal_timing_get_value(st-hsync_len, 0); + + vm-vactive = signal_timing_get_value(st-vactive, 0); + vm-vfront_porch = signal_timing_get_value(st-vfront_porch, 0); + vm-vback_porch = signal_timing_get_value(st-vback_porch, 0); + vm-vsync_len = signal_timing_get_value(st-vsync_len, 0); + + vm-vah = disp-vsync_pol_active_high; + vm-hah = disp-hsync_pol_active_high; + + return vm; +} + +#if defined(CONFIG_DRM) +int videomode_to_display_mode(struct videomode *vm, struct drm_display_mode *dmode) +{ + memset(dmode, 0, sizeof(*dmode)); + + dmode-hdisplay = vm-hactive; + dmode-hsync_start = dmode-hdisplay + vm-hfront_porch; + dmode-hsync_end = dmode-hsync_start + vm-hsync_len; + dmode-htotal = dmode-hsync_end + vm-hback_porch; + + dmode-vdisplay = vm-vactive; + dmode-vsync_start = dmode-vdisplay + vm-vfront_porch; + dmode-vsync_end = dmode-vsync_start + vm-vsync_len; + dmode-vtotal = dmode-vtotal + vm-vback_porch; + + dmode-clock = vm-pixelclock / 1000; + + if (vm-hah) + dmode-flags |= DRM_MODE_FLAG_PHSYNC; + else + dmode-flags |= DRM_MODE_FLAG_NHSYNC; + if (vm-vah) + dmode-flags |= DRM_MODE_FLAG_PVSYNC; + else + dmode-flags |= DRM_MODE_FLAG_NVSYNC; + if (vm-interlaced) + dmode-flags |= DRM_MODE_FLAG_INTERLACE; + if (vm-doublescan) + dmode-flags |= DRM_MODE_FLAG_DBLSCAN; + drm_mode_set_name(dmode); + + return 0; +} +EXPORT_SYMBOL_GPL(videomode_to_display_mode); +#else +int videomode_to_displaymode(struct videomode *vm, struct drm_display_mode *dmode) +{ + return 0; +} +#endif + +int videomode_to_fb_videomode(struct videomode *vm, struct fb_videomode *fbmode) +{ + memset(fbmode, 0, sizeof(*fbmode)); + + fbmode-xres = vm-hactive; + fbmode-left_margin = vm-hback_porch; + fbmode-right_margin = vm-hfront_porch; + fbmode-hsync_len = vm-hsync_len; + + fbmode-yres = vm-vactive; + fbmode-upper_margin = vm-vback_porch; + fbmode-lower_margin = vm-vfront_porch; + fbmode-vsync_len = vm-vsync_len; + + fbmode-pixclock = KHZ2PICOS(vm-pixelclock) / 1000; + + if (vm-hah) + fbmode-sync |= FB_SYNC_HOR_HIGH_ACT; + if (vm-vah) + fbmode-sync |= FB_SYNC_VERT_HIGH_ACT; + if (vm-interlaced) + fbmode-vmode |= FB_VMODE_INTERLACED; + if (vm-doublescan) + fbmode-vmode |= FB_VMODE_DOUBLE; + + return 0; +} +EXPORT_SYMBOL_GPL(videomode_to_fb_videomode); + +int of_get_display_mode(struct device_node *np, struct drm_display_mode *dmode, int index) +{ + struct videomode *vm; + struct display *disp; + + disp = of_get_display(np); + + if (!disp) { + pr_err(%s: no display specified\n, __func__); +
Re: [PATCH RFC] V4L: Add s_rx_buffer subdev video operation
Hi Sakari, On 09/24/2012 03:44 PM, Sakari Ailus wrote: How about useing a separate video buffer queue for the purpose? That would provide a nice way to pass it to the user space where it's needed. It'd also play nicely together with the frame layout descriptors. It's tempting, but doing frame synchronisation in user space in this case would have been painful, if at all possible in reliable manner. It would have significantly complicate applications and the drivers. VIDIOC_STREAMON, VIDIOC_QBUF/DQBUF calls would have been at least roughly synchronized, and applications would have to know somehow which video nodes needs to be opened together. I guess things like that could be abstracted in a library, but what do we really gain for such effort ? And now I can just ask kernel for 2-planar buffers where everything is in place.. Regards, Sylwester -- 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] V4L: Add s_rx_buffer subdev video operation
Hi Laurent, On 09/24/2012 03:58 PM, Laurent Pinchart wrote: How about useing a separate video buffer queue for the purpose? That would provide a nice way to pass it to the user space where it's needed. It'd also play nicely together with the frame layout descriptors. Beside, a void *buf wouldn't support DMA. Only subdevs that use PIO to transfer meta data could be supported by this. I guess most of MIPI-CSI2 receivers out there support data capture from distinct Data Types into separate DMA buffers. But not this one. In case of multi-context DMA engine I guess MIPI-CSI2 receiver driver would expose video node anyway and it wouldn't need such a callback at all ? Perhaps using struct v4l2_subdev_core_ops::ioctl would be more appropriate here ? -- Regards, Sylwester -- 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: Gain controls in v4l2-ctrl framework
On 09/24/2012 07:42 AM, Prabhakar Lad wrote: Hi Hans, On Mon, Sep 24, 2012 at 4:25 PM, Hans de Goede hdego...@redhat.com wrote: Hi, On 09/23/2012 01:26 PM, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE Not all sensors have separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE, so we will need a separate control for sensors which have one combined gain called simply V4L2_CID_GAIN_GREEN Agreed Also do we really need separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE controls? I know hardware has them, but in my experience that is only done as it is simpler to make the hardware this way (fully symmetric sensor grid), have you ever tried actually using different gain settings for the 2 different green rows ? Never tried it. I've and that always results in an ugly checker board pattern. So I think we can and should only have a V4L2_CID_GAIN_GREEN, and for sensors with 2 green gains have that control both, forcing both to always have the same setting, which is really what you want anyways ... Agreed. Please don't do this. I am working with the MT9P031, which has separate gains, and as we are using the color version of the sensor (which we can get much more cheaply) with infrared illumination, we correct for the slightly different response levels of the different color channels by adjusting the individual gain controls. (I have patches to add the controls, but I haven't had time yet to get them into good enough shape to submit - sorry!) It seems to me that for applications that want to set them to the same value (presumably the vast majority), it is not so hard to set both the green_red and green_blue. If you implement a single control, what happens for the (admittedly rare) application that needs to control them separately? Regards, --Prabhakar Lad 4: V4L2_CID_GAIN_BLUE 5: V4L2_CID_GAIN_OFFSET GAIN_OFFSET that sounds a bit weird... GAIN_OFFSET sounds like it is a number which gets added to the 3/4 gain settings before the gain gets applied, but I assume that you just mean a number which gets added to the value from the pixel, either before or after the gain is applied and I must admit I cannot come up with a better name. I believe (not sure) that some sensors have these per color ... The question is if it makes sense to actually control this per color though, I don't think it does as it is meant to compensate for any fixed measuring errors, which are the same for all 3/4 colors. Note that all the sensor cells are exactly the same, later on a color grid gets added on top of the sensors to turn them into r/g/b cells, but physically they are the same cells, so with the same process and temperature caused measuring errors... Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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] V4L: Add s_rx_buffer subdev video operation
Hi Sylwester, On Mon, Sep 24, 2012 at 06:51:41PM +0200, Sylwester Nawrocki wrote: Hi Sakari, On 09/24/2012 03:44 PM, Sakari Ailus wrote: How about useing a separate video buffer queue for the purpose? That would provide a nice way to pass it to the user space where it's needed. It'd also play nicely together with the frame layout descriptors. It's tempting, but doing frame synchronisation in user space in this case would have been painful, if at all possible in reliable manner. It would have significantly complicate applications and the drivers. Let's face it: applications that are interested in this information have to do exactly the same frame number matching with the statistics buffers. Just stitching the data to the same video buffer isn't a generic solution. VIDIOC_STREAMON, VIDIOC_QBUF/DQBUF calls would have been at least roughly synchronized, and applications would have to know somehow which video nodes needs to be opened together. I guess things like that could be abstracted in a library, but what do we really gain for such effort ? And now I can just ask kernel for 2-planar buffers where everything is in place.. That's equally good --- some hardware can only do that after all, but do you need the callback in that case, if there's a single destination buffer anyway? Wouldn't the frame layout descriptor have enough information to do this? Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi, On 09/24/2012 07:17 PM, Chris MacGregor wrote: On 09/24/2012 07:42 AM, Prabhakar Lad wrote: Hi Hans, On Mon, Sep 24, 2012 at 4:25 PM, Hans de Goede hdego...@redhat.com wrote: Hi, On 09/23/2012 01:26 PM, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE Not all sensors have separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE, so we will need a separate control for sensors which have one combined gain called simply V4L2_CID_GAIN_GREEN Agreed Also do we really need separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE controls? I know hardware has them, but in my experience that is only done as it is simpler to make the hardware this way (fully symmetric sensor grid), have you ever tried actually using different gain settings for the 2 different green rows ? Never tried it. I've and that always results in an ugly checker board pattern. So I think we can and should only have a V4L2_CID_GAIN_GREEN, and for sensors with 2 green gains have that control both, forcing both to always have the same setting, which is really what you want anyways ... Agreed. Please don't do this. I am working with the MT9P031, which has separate gains, and as we are using the color version of the sensor (which we can get much more cheaply) with infrared illumination, we correct for the slightly different response levels of the different color channels by adjusting the individual gain controls. Ok, sofar I'm following you, but are you saying that the correction you need to apply for the green pixels on the same row as red pixels, is different then the one for the green pixels on the same row as blue pixels ? I can understand that the green lenses let through a different amount of infrared light then sat the red lenses, but is there any (significant) differences between the green lenses on 2 different rows? (I have patches to add the controls, but I haven't had time yet to get them into good enough shape to submit - sorry!) It seems to me that for applications that want to set them to the same value (presumably the vast majority), it is not so hard to set both the green_red and green_blue. If you implement a single control, what happens for the (admittedly rare) application that needs to control them separately? Well if these are showing up in something like a user oriented control-panel (which they may) then having one slider for both certainly is more userfriendly. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date:Mon Sep 24 19:00:24 CEST 2012 git hash:f9040ef3fab8f6f5f6fced5583203695d08efde3 gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: OK linux-git-arm-eabi-omap: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-sh: ERRORS linux-git-x86_64: WARNINGS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: ERRORS linux-2.6.33-x86_64: ERRORS linux-2.6.34-x86_64: ERRORS linux-2.6.35.3-x86_64: ERRORS linux-2.6.36-x86_64: ERRORS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-x86_64: WARNINGS linux-3.2.1-x86_64: WARNINGS linux-3.3-x86_64: WARNINGS linux-3.4-x86_64: WARNINGS linux-3.5-x86_64: WARNINGS linux-3.6-rc2-x86_64: WARNINGS linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: ERRORS linux-2.6.33-i686: ERRORS linux-2.6.34-i686: ERRORS linux-2.6.35.3-i686: ERRORS linux-2.6.36-i686: ERRORS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-i686: WARNINGS linux-3.2.1-i686: WARNINGS linux-3.3-i686: WARNINGS linux-3.4-i686: WARNINGS linux-3.5-i686: WARNINGS linux-3.6-rc2-i686: WARNINGS apps: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi, Hans. On 09/24/2012 11:46 AM, Hans de Goede wrote: Hi, On 09/24/2012 07:17 PM, Chris MacGregor wrote: On 09/24/2012 07:42 AM, Prabhakar Lad wrote: Hi Hans, On Mon, Sep 24, 2012 at 4:25 PM, Hans de Goede hdego...@redhat.com wrote: Hi, On 09/23/2012 01:26 PM, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE Not all sensors have separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE, so we will need a separate control for sensors which have one combined gain called simply V4L2_CID_GAIN_GREEN Agreed Also do we really need separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE controls? I know hardware has them, but in my experience that is only done as it is simpler to make the hardware this way (fully symmetric sensor grid), have you ever tried actually using different gain settings for the 2 different green rows ? Never tried it. I've and that always results in an ugly checker board pattern. So I think we can and should only have a V4L2_CID_GAIN_GREEN, and for sensors with 2 green gains have that control both, forcing both to always have the same setting, which is really what you want anyways ... Agreed. Please don't do this. I am working with the MT9P031, which has separate gains, and as we are using the color version of the sensor (which we can get much more cheaply) with infrared illumination, we correct for the slightly different response levels of the different color channels by adjusting the individual gain controls. Ok, sofar I'm following you, but are you saying that the correction you need to apply for the green pixels on the same row as red pixels, is different then the one for the green pixels on the same row as blue pixels ? IIRC, when we were calibrating, the two greens were at some times different. The gain settings we're using at the moment are in fact the same for both greens - we had to compromise to avoid getting into higher values that increase the noise more than we like - but I don't know that it would be that way in the future. I can understand that the green lenses let through a different amount of infrared light then sat the red lenses, but is there any (significant) differences between the green lenses on 2 different rows? I don't have time right now to dig out the datasheet and re-read it, but IIRC the auto-BLC (for instance) is row-wise, and consequently the greens could actually need slightly different values. I think the datasheet made it fairly clear that the motivation for including separate green controls was because you might need them in practice, not because the hardware guys were punting the problem over to the software side. (I have patches to add the controls, but I haven't had time yet to get them into good enough shape to submit - sorry!) It seems to me that for applications that want to set them to the same value (presumably the vast majority), it is not so hard to set both the green_red and green_blue. If you implement a single control, what happens for the (admittedly rare) application that needs to control them separately? Well if these are showing up in something like a user oriented control-panel (which they may) then having one slider for both certainly is more userfriendly. Okay, that's a fair point. But an application that wanted to could insulate the user from it fairly easily. I'm not opposed to having a single control, *if* there is some way for apps to control the greens separately when they need to. I don't have a brilliant solution for this offhand, other than just exposing the separate controls. Regards, Hans Thanks, Chris -- 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] [media] media-devnode: Replace printk with pr_*
On Mon, Sep 24, 2012 at 04:56:24PM +0530, Sachin Kamat wrote: Fixes checkpatch warnings related to printk. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Thanks, Sachin! Acked-by: Sakari Ailus sakari.ai...@iki.fi -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi Prabhakar, On Sun, Sep 23, 2012 at 04:56:21PM +0530, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE One more thing: There's an analogue gain control already in the image source class. I think we should explicitly say that the gains are digital (vs. analogue). Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Timestamps and V4L2
Le dimanche 23 septembre 2012 14:43:42, Sakari Ailus a écrit : I think I like this idea best, it's relatively simple (even with adding support for reporting flags in VIDIOC_QUERYBUF) for the purpose. If we ever need the clock selection API I would vote for an IOCTL. The controls API is a bad choice for something such fundamental as type of clock for buffer timestamping IMHO. Let's stop making the controls API a dumping ground for almost everything in V4L2! ;) Why would the control API be worse than an IOCTL for choosing the type of the timestamp? The control API after all has functionality for exactly for this: this is an obvious menu control. What comes to the nature of things that can be configured using controls and what can be done using IOCTLs I see no difference. It's just a mechanism. That's what traditional Unix APIs do in general: provide mechanism, not a policy. Seriously? Timestamp is _not_ a controllable hardware feature like brightness or flash. Controls are meant to build user interface controls for interaction with the user. Timestamp is _not_ something the user should control directly. The application should figure out what it gets and what it needs. Or why do you use STREAMON/STREAMOFF instead of a STREAM boolean control, eh? -- Rémi Denis-Courmont http://www.remlab.net/ -- 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: Gain controls in v4l2-ctrl framework
Hi Chris and Hans, On Mon, Sep 24, 2012 at 12:16:01PM -0700, Chris MacGregor wrote: ... (I have patches to add the controls, but I haven't had time yet to get them into good enough shape to submit - sorry!) It seems to me that for applications that want to set them to the same value (presumably the vast majority), it is not so hard to set both the green_red and green_blue. If you implement a single control, what happens for the (admittedly rare) application that needs to control them separately? Well if these are showing up in something like a user oriented control-panel (which they may) then having one slider for both certainly is more userfriendly. Okay, that's a fair point. But an application that wanted to could insulate the user from it fairly easily. I'm not opposed to having a single control, *if* there is some way for apps to control the greens separately when they need to. I don't have a brilliant solution for this offhand, other than just exposing the separate controls. I do recognise there's a need for developers to fiddle with such low level controls as these but I can hardly see end users using them as such. Either automatic white balance or a white balance control with higher level of abstraction is likely better for that purpose. Some sensors have only a single gain for the greens so these devices should anyway implement just a single green gain (which is neither of the two). Perhaps such abstraction could be performed by libv4l? Just my 0,05 euros. Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
On Monday 24 September 2012 12:16:01 Chris MacGregor wrote: On 09/24/2012 11:46 AM, Hans de Goede wrote: On 09/24/2012 07:17 PM, Chris MacGregor wrote: On 09/24/2012 07:42 AM, Prabhakar Lad wrote: On Mon, Sep 24, 2012 at 4:25 PM, Hans de Goede wrote: On 09/23/2012 01:26 PM, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE Not all sensors have separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE, so we will need a separate control for sensors which have one combined gain called simply V4L2_CID_GAIN_GREEN Agreed Also do we really need separate V4L2_CID_GAIN_GREEN_RED / V4L2_CID_GAIN_GREEN_BLUE controls? I know hardware has them, but in my experience that is only done as it is simpler to make the hardware this way (fully symmetric sensor grid), have you ever tried actually using different gain settings for the 2 different green rows? Never tried it. I've and that always results in an ugly checker board pattern. So I think we can and should only have a V4L2_CID_GAIN_GREEN, and for sensors with 2 green gains have that control both, forcing both to always have the same setting, which is really what you want anyways ... Agreed. Please don't do this. I am working with the MT9P031, which has separate gains, and as we are using the color version of the sensor (which we can get much more cheaply) with infrared illumination, we correct for the slightly different response levels of the different color channels by adjusting the individual gain controls. Ok, sofar I'm following you, but are you saying that the correction you need to apply for the green pixels on the same row as red pixels, is different then the one for the green pixels on the same row as blue pixels ? IIRC, when we were calibrating, the two greens were at some times different. The gain settings we're using at the moment are in fact the same for both greens - we had to compromise to avoid getting into higher values that increase the noise more than we like - but I don't know that it would be that way in the future. I can understand that the green lenses let through a different amount of infrared light then sat the red lenses, but is there any (significant) differences between the green lenses on 2 different rows? I don't have time right now to dig out the datasheet and re-read it, but IIRC the auto-BLC (for instance) is row-wise, and consequently the greens could actually need slightly different values. I think the datasheet made it fairly clear that the motivation for including separate green controls was because you might need them in practice, not because the hardware guys were punting the problem over to the software side. (I have patches to add the controls, but I haven't had time yet to get them into good enough shape to submit - sorry!) It seems to me that for applications that want to set them to the same value (presumably the vast majority), it is not so hard to set both the green_red and green_blue. If you implement a single control, what happens for the (admittedly rare) application that needs to control them separately? Well if these are showing up in something like a user oriented control-panel (which they may) then having one slider for both certainly is more userfriendly. Those gains are low-level controls, they will very likely not appear in a generic panel. Okay, that's a fair point. But an application that wanted to could insulate the user from it fairly easily. I'm not opposed to having a single control, *if* there is some way for apps to control the greens separately when they need to. I don't have a brilliant solution for this offhand, other than just exposing the separate controls. As you have a use case for separate controls I'd vote for having separate controls. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Gain controls in v4l2-ctrl framework
Hi Sakari, On Monday 24 September 2012 23:06:34 Sakari Ailus wrote: On Sun, Sep 23, 2012 at 04:56:21PM +0530, Prabhakar Lad wrote: Hi All, The CCD/Sensors have the capability to adjust the R/ye, Gr/Cy, Gb/G, B/Mg gain values. Since these control can be re-usable I am planning to add the following gain controls as part of the framework: 1: V4L2_CID_GAIN_RED 2: V4L2_CID_GAIN_GREEN_RED 3: V4L2_CID_GAIN_GREEN_BLUE 4: V4L2_CID_GAIN_BLUE One more thing: There's an analogue gain control already in the image source class. I think we should explicitly say that the gains are digital (vs. analogue). Some sensors have per-component analog and digital gains :-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Timestamps and V4L2
On Friday 21 September 2012 11:33:24 Hans Verkuil wrote: On Thu September 20 2012 22:21:22 Sakari Ailus wrote: Hi all, This RFC intends to summarise and further the recent discussion on linux-media regarding the proposed changes of timestamping V4L2 buffers. The problem === The V4L2 has long used realtime timestamps (such as clock_gettime(CLOCK_REALTIME, ...)) to stamp the video buffers before handing them over to the user. This has been found problematic in associating the video buffers with data from other sources: realtime clock may jump around due to daylight saving time, for example, and ALSA (audio-video synchronisation is a common use case) user space API does not provide the user with realtime timestamps, but instead uses monotonic time (i.e. clock_gettime(CLOCK_MONOTONIC, ...)). This is especially an issue in embedded systems where video recording is a common use case. Drivers typically used in such systems have silently switched to use monotonic timestamps. While against the spec, this is necessary for those systems to operate properly. In general, realtime timestamps are seen of little use in other than debugging purposes, but monotonic timestamps are fine for that as well. It's still possible that an application I'm not aware of uses them in a peculiar way that would be adversely affected by changing to monotonic timestamps. Nevertheless, we're not supposed to break the API (or ABI). It'd be also very important for the application to know what kind of timestamps are provided by the device. Requirements, wishes and constraints Now that it seems to be about the time to fix these issues, it's worth looking a little bit to the future to anticipate the coming changes to be able to accommodate them better later on. - The new default should be monotonic. As the monotonic timestamps are seen to be the most useful, they should be made the default. - timeval vs. timespec. The two structs can be used to store timestamp information. They are not compatible with each other. It's a little bit uncertain what's the case with all the architectures but it looks like the timespec fits into the space of timeval in all cases. If timespec is considered to be used somewhere the compatibility must be ensured. Timespec is better than timeval since timespec has more precision and it's the same struct that's used everywhere else in the V4L2 API: timespec does not need conversion to timespec in the user space. struct timespec { __kernel_time_t tv_sec; /* seconds */ longtv_nsec;/* nanoseconds */ }; struct timeval { __kernel_time_t tv_sec; /* seconds */ __kernel_suseconds_ttv_usec;/* microseconds */ }; To be able to use timespec, the user would have to most likely explicitly choose to do that. - Users should know what kind of timestamps the device produces. This includes existing and future kernels. What should be considered are uninformed porting drivers back and forth across kernel versions and out-of-date kernel header files. - Device-dependent timestamps. Some devices such as the uvcvideo ones produce device-dependent timestamps for synchronising video and audio, both produced by the same physical hardware device. For uvcvideo these timestamps are unsigned 32-bit integers. - There's also another clock, Linux-specific raw monotonic clock (as in clock_gettime(CLOCK_RAW_MONOTONIC, ...)) that could be better in some use cases than the regular monotonic clock. The difference is that the raw monotonic clock is free from the NTP adjustments. It would be nice for the user to be able to choose the clock used for timestamps. This is especially important for device-dependent timestamps: not all applications can be expected to be able to use them. - The field adjacent to timestamp, timecode, is 128 bits wide, and not used by a single driver. This field could be re-used. Possible solutions == Not all of the solutions below that have been proposed are mutually exclusive. That's also what's making the choice difficult: the ultimate solution to the issue of timestamping may involve several of these --- or possibly something better that's not on the list. Use of timespec --- If we can conclude timespec will always fit into the size of timeval (or timecode) we could use timespec instead. The solution should still make the use of timespec explicit to the user space. This seems to conflict with the idea of making monotonic timestamps the default: the default can't be anything incompatible with timeval, and at the same time it's the most important that the monotonic timestamps are timespec. We have to keep
Re: [RFC] Timestamps and V4L2
Hi Hans, On Friday 21 September 2012 11:33:24 Hans Verkuil wrote: On Thu September 20 2012 22:21:22 Sakari Ailus wrote: Hi all, This RFC intends to summarise and further the recent discussion on linux-media regarding the proposed changes of timestamping V4L2 buffers. The problem === The V4L2 has long used realtime timestamps (such as clock_gettime(CLOCK_REALTIME, ...)) to stamp the video buffers before handing them over to the user. This has been found problematic in associating the video buffers with data from other sources: realtime clock may jump around due to daylight saving time, for example, and ALSA (audio-video synchronisation is a common use case) user space API does not provide the user with realtime timestamps, but instead uses monotonic time (i.e. clock_gettime(CLOCK_MONOTONIC, ...)). This is especially an issue in embedded systems where video recording is a common use case. Drivers typically used in such systems have silently switched to use monotonic timestamps. While against the spec, this is necessary for those systems to operate properly. In general, realtime timestamps are seen of little use in other than debugging purposes, but monotonic timestamps are fine for that as well. It's still possible that an application I'm not aware of uses them in a peculiar way that would be adversely affected by changing to monotonic timestamps. Nevertheless, we're not supposed to break the API (or ABI). It'd be also very important for the application to know what kind of timestamps are provided by the device. Requirements, wishes and constraints Now that it seems to be about the time to fix these issues, it's worth looking a little bit to the future to anticipate the coming changes to be able to accommodate them better later on. - The new default should be monotonic. As the monotonic timestamps are seen to be the most useful, they should be made the default. - timeval vs. timespec. The two structs can be used to store timestamp information. They are not compatible with each other. It's a little bit uncertain what's the case with all the architectures but it looks like the timespec fits into the space of timeval in all cases. If timespec is considered to be used somewhere the compatibility must be ensured. Timespec is better than timeval since timespec has more precision and it's the same struct that's used everywhere else in the V4L2 API: timespec does not need conversion to timespec in the user space. struct timespec { __kernel_time_t tv_sec; /* seconds */ longtv_nsec;/* nanoseconds */ }; struct timeval { __kernel_time_t tv_sec; /* seconds */ __kernel_suseconds_ttv_usec;/* microseconds */ }; To be able to use timespec, the user would have to most likely explicitly choose to do that. - Users should know what kind of timestamps the device produces. This includes existing and future kernels. What should be considered are uninformed porting drivers back and forth across kernel versions and out-of-date kernel header files. - Device-dependent timestamps. Some devices such as the uvcvideo ones produce device-dependent timestamps for synchronising video and audio, both produced by the same physical hardware device. For uvcvideo these timestamps are unsigned 32-bit integers. - There's also another clock, Linux-specific raw monotonic clock (as in clock_gettime(CLOCK_RAW_MONOTONIC, ...)) that could be better in some use cases than the regular monotonic clock. The difference is that the raw monotonic clock is free from the NTP adjustments. It would be nice for the user to be able to choose the clock used for timestamps. This is especially important for device-dependent timestamps: not all applications can be expected to be able to use them. - The field adjacent to timestamp, timecode, is 128 bits wide, and not used by a single driver. This field could be re-used. Possible solutions == Not all of the solutions below that have been proposed are mutually exclusive. That's also what's making the choice difficult: the ultimate solution to the issue of timestamping may involve several of these --- or possibly something better that's not on the list. Use of timespec --- If we can conclude timespec will always fit into the size of timeval (or timecode) we could use timespec instead. The solution should still make the use of timespec explicit to the user space. This seems to conflict with the idea of making monotonic timestamps the default: the default can't be anything incompatible with timeval, and at the same time it's the most important that the monotonic timestamps are timespec. We have
Re: [RFC] Timestamps and V4L2
Hi Hans, On Sunday 23 September 2012 11:18:45 Hans Verkuil wrote: On Sat September 22 2012 14:38:07 Sakari Ailus wrote: Hans Verkuil wrote: On Thu September 20 2012 22:21:22 Sakari Ailus wrote: Hi all, This RFC intends to summarise and further the recent discussion on linux-media regarding the proposed changes of timestamping V4L2 buffers. The problem === The V4L2 has long used realtime timestamps (such as clock_gettime(CLOCK_REALTIME, ...)) to stamp the video buffers before handing them over to the user. This has been found problematic in associating the video buffers with data from other sources: realtime clock may jump around due to daylight saving time, for example, and ALSA (audio-video synchronisation is a common use case) user space API does not provide the user with realtime timestamps, but instead uses monotonic time (i.e. clock_gettime(CLOCK_MONOTONIC, ...)). This is especially an issue in embedded systems where video recording is a common use case. Drivers typically used in such systems have silently switched to use monotonic timestamps. While against the spec, this is necessary for those systems to operate properly. In general, realtime timestamps are seen of little use in other than debugging purposes, but monotonic timestamps are fine for that as well. It's still possible that an application I'm not aware of uses them in a peculiar way that would be adversely affected by changing to monotonic timestamps. Nevertheless, we're not supposed to break the API (or ABI). It'd be also very important for the application to know what kind of timestamps are provided by the device. Requirements, wishes and constraints Now that it seems to be about the time to fix these issues, it's worth looking a little bit to the future to anticipate the coming changes to be able to accommodate them better later on. - The new default should be monotonic. As the monotonic timestamps are seen to be the most useful, they should be made the default. - timeval vs. timespec. The two structs can be used to store timestamp information. They are not compatible with each other. It's a little bit uncertain what's the case with all the architectures but it looks like the timespec fits into the space of timeval in all cases. If timespec is considered to be used somewhere the compatibility must be ensured. Timespec is better than timeval since timespec has more precision and it's the same struct that's used everywhere else in the V4L2 API: timespec does not need conversion to timespec in the user space. struct timespec { __kernel_time_t tv_sec; /* seconds */ longtv_nsec;/* nanoseconds */ }; struct timeval { __kernel_time_t tv_sec; /* seconds */ __kernel_suseconds_ttv_usec;/* microseconds */ }; To be able to use timespec, the user would have to most likely explicitly choose to do that. - Users should know what kind of timestamps the device produces. This includes existing and future kernels. What should be considered are uninformed porting drivers back and forth across kernel versions and out-of-date kernel header files. - Device-dependent timestamps. Some devices such as the uvcvideo ones produce device-dependent timestamps for synchronising video and audio, both produced by the same physical hardware device. For uvcvideo these timestamps are unsigned 32-bit integers. - There's also another clock, Linux-specific raw monotonic clock (as in clock_gettime(CLOCK_RAW_MONOTONIC, ...)) that could be better in some use cases than the regular monotonic clock. The difference is that the raw monotonic clock is free from the NTP adjustments. It would be nice for the user to be able to choose the clock used for timestamps. This is especially important for device-dependent timestamps: not all applications can be expected to be able to use them. - The field adjacent to timestamp, timecode, is 128 bits wide, and not used by a single driver. This field could be re-used. Possible solutions == Not all of the solutions below that have been proposed are mutually exclusive. That's also what's making the choice difficult: the ultimate solution to the issue of timestamping may involve several of these --- or possibly something better that's not on the list. Use of timespec --- If we can conclude timespec will always fit into the size of timeval (or timecode) we could use timespec instead. The solution should still make the use of timespec explicit to the user space. This seems to conflict with the idea of making monotonic timestamps the
Re: [RFC] Timestamps and V4L2
Hi Sylwester, On Saturday 22 September 2012 19:12:52 Sylwester Nawrocki wrote: On 09/22/2012 02:38 PM, Sakari Ailus wrote: You are missing one other option: Using v4l2_buffer flags to report the clock --- By defining flags like this: V4L2_BUF_FLAG_CLOCK_MASK 0x7000 /* Possible Clocks */ V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x /* system or monotonic, we don't know */ V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 you could tell the application which clock is used. This does allow for more clocks to be added in the future and clock selection would then be done by a control or possibly an ioctl. For now there are no plans to do such things, so this flag should be sufficient. And it can be implemented very efficiently. It works with existing drivers as well, since they will report CLOCK_UNKNOWN. I am very much in favor of this approach. +1 I think I like this idea best, it's relatively simple (even with adding support for reporting flags in VIDIOC_QUERYBUF) for the purpose. If we ever need the clock selection API I would vote for an IOCTL. The controls API is a bad choice for something such fundamental as type of clock for buffer timestamping IMHO. Let's stop making the controls API a dumping ground for almost everything in V4L2! ;) What's wrong in using the control API in this case ? :-) Thanks for adding this. I knew I was forgetting something but didn't remember what --- I swear it was unintentional! :-) If we'd add more clocks without providing an ability to choose the clock from the user space, how would the clock be selected? It certainly isn't the driver's job, nor I think it should be system-specific either (platform data on embedded systems). It's up to the application and its needs. That would suggest we should always provide monotonic timestamps to applications (besides a potential driver-specific timestamp), and for that purpose the capability flag --- I admit I disliked the idea at first --- is enough. What comes to buffer flags, the application would also have to receive the first buffer from the device to even know what kind of timestamps the device uses, or at least call QUERYBUF. And in principle the flag should be checked on every buffer, unless we also specify the flag is the same for all buffers. And at certain point this will stop to make any sense... Good point. Perhaps VIDIOC_QUERYBUF and VIDIOC_DQBUF should be reporting timestamps type only for the time they are being called. Not per buffer, per device. And applications would be checking the flags any time they want to find out what is the buffer timestamp type. Or every time if it don't have full control over the device (S/G_PRIORITY). A capability flag is cleaner solution from this perspective, and it can be amended by a control (or an ioctl) later on: the flag can be disregarded by applications whenever the control is present. If the application doesn't know about the control it can still rely on the flag. (I think this would be less clean than to go for the control right from the beginning, but better IMO.) But with the capability flag we would only be able to report one type of clock, right ? That's correct. The capability flag could mean I support the clock selection API and default to a monotonic timestamp though. Device-dependent timestamp -- Should we agree on selectable timestamps, the existing timestamp field (or a union with another field of different type) could be used for the device-dependent timestamps. No. Device timestamps should get their own field. You want to be able to relate device timestamps with the monotonic timestamps, so you need both. Alternatively we can choose to re-use the existing timecode field. At the moment there's no known use case for passing device-dependent timestamps at the same time with monotonic timestamps. Well, the use case is there, but there is no driver support. The device timestamps should be 64 bits to accomodate things like PTS and DTS from MPEG streams. Since timecode is 128 bits we might want to use two u64 fields or perhaps 4 u32 fields. That should be an union for different kinds (or rather types) of device-dependent timestamps. On uvcvideo I think this is u32, not u64. We should be also able to tell what kind device dependent timestamp there is --- should buffer flags be used for that as well? Timecode has 'type' and 'flags' fields, couldn't it be accommodated for reporting device-dependant timestamps as well ? The timecode field is free for reuse, so we can definitely use it for device- specific timestamps. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Timestamps and V4L2
Hi Sylwester, On Sunday 23 September 2012 20:40:36 Sylwester Nawrocki wrote: On 09/22/2012 10:28 PM, Daniel Glöckner wrote: On Sat, Sep 22, 2012 at 07:12:52PM +0200, Sylwester Nawrocki wrote: If we ever need the clock selection API I would vote for an IOCTL. The controls API is a bad choice for something such fundamental as type of clock for buffer timestamping IMHO. Let's stop making the controls API a dumping ground for almost everything in V4L2! ;) Perhaps VIDIOC_QUERYBUF and VIDIOC_DQBUF should be reporting timestamps type only for the time they are being called. Not per buffer, per device. And applications would be checking the flags any time they want to find out what is the buffer timestamp type. Or every time if it don't have full control over the device (S/G_PRIORITY). I'm all for adding an IOCTL, but if we think about adding a VIDIOC_S_TIMESTAMP_TYPE in the future, we might as well add a VIDIOC_G_TIMESTAMP_TYPE right now. Old drivers will return ENOSYS, so the application knows it will have to guess the type (or take own timestamps). Hmm, would it make sense to design a single ioctl that would allow getting and setting the clock type, e.g. VIDIOC_CLOCK/TIMESTAMP_TYPE ? I can't imagine anything useful coming from an app that has to process timestamps that change their source every now and then and I seriously doubt anyone will go to such an extent that they check the timestamp type on every buffer. If they don't set their priority high enough to prevent others from changing the timestamp type, they also run the risk of someone else changing the image format. It should be enough to forbid changing the timestamp type while I/O is in progress, as it is done for VIDIOC_S_FMT. I agree, but mem-to-mem devices can have multiple logically independent, concurrent streams active. If the clock type is per device it might not be that straightforward... Does the clock type need to be selectable for mem-to-mem devices ? Do device- specific timestamps make sense there ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] media-devnode: Replace printk with pr_*
Hi Sachin, Thanks for the patch. On Monday 24 September 2012 16:56:24 Sachin Kamat wrote: Fixes checkpatch warnings related to printk. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com I've applied the patch to my tree. --- drivers/media/media-devnode.c | 14 -- 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c index f6b52d5..023b2a1 100644 --- a/drivers/media/media-devnode.c +++ b/drivers/media/media-devnode.c @@ -30,6 +30,8 @@ * counting. */ +#define pr_fmt(fmt) KBUILD_MODNAME : fmt + #include linux/errno.h #include linux/init.h #include linux/module.h @@ -215,7 +217,7 @@ int __must_check media_devnode_register(struct media_devnode *mdev) minor = find_next_zero_bit(media_devnode_nums, MEDIA_NUM_DEVICES, 0); if (minor == MEDIA_NUM_DEVICES) { mutex_unlock(media_devnode_lock); - printk(KERN_ERR could not get a free minor\n); + pr_err(could not get a free minor\n); return -ENFILE; } @@ -230,7 +232,7 @@ int __must_check media_devnode_register(struct media_devnode *mdev) ret = cdev_add(mdev-cdev, MKDEV(MAJOR(media_dev_t), mdev-minor), 1); if (ret 0) { - printk(KERN_ERR %s: cdev_add failed\n, __func__); + pr_err(%s: cdev_add failed\n, __func__); goto error; } @@ -243,7 +245,7 @@ int __must_check media_devnode_register(struct media_devnode *mdev) dev_set_name(mdev-dev, media%d, mdev-minor); ret = device_register(mdev-dev); if (ret 0) { - printk(KERN_ERR %s: device_register failed\n, __func__); + pr_err(%s: device_register failed\n, __func__); goto error; } @@ -287,18 +289,18 @@ static int __init media_devnode_init(void) { int ret; - printk(KERN_INFO Linux media interface: v0.10\n); + pr_info(Linux media interface: v0.10\n); ret = alloc_chrdev_region(media_dev_t, 0, MEDIA_NUM_DEVICES, MEDIA_NAME); if (ret 0) { - printk(KERN_WARNING media: unable to allocate major\n); + pr_warn(unable to allocate major\n); return ret; } ret = bus_register(media_bus_type); if (ret 0) { unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES); - printk(KERN_WARNING media: bus_register failed\n); + pr_warn(bus_register failed\n); return -EIO; } -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3isp: wrong image after resizer with mt9v034 sensor
Hi Enric, On Monday 24 September 2012 15:49:01 Enric Balletbò i Serra wrote: 2012/9/24 Laurent Pinchart laurent.pinch...@ideasonboard.com: On Monday 24 September 2012 10:33:42 Enric Balletbò i Serra wrote: Hi everybody, I'm trying to add support for MT9V034 Aptina image sensor to current mainline, as a base of my current work I start using the latest omap3isp-next branch from Laurent's git tree [1]. The MT9V034 image sensor is very similar to MT9V032 sensor, so I modified current driver to accept MT9V034 sensor adding the chip ID. The driver recognizes the sensor and I'm able to capture some frames. I started capturing directly frames using the pipeline Sensor - CCDC ./media-ctl -r ./media-ctl -l 'mt9v032 3-005c:0-OMAP3 ISP CCDC:0[1]' ./media-ctl -l 'OMAP3 ISP CCDC:1-OMAP3 ISP CCDC output:0[1]' ./media-ctl -f 'mt9v032 3-005c:0 [SGRBG10 752x480]' ./media-ctl -f 'OMAP3 ISP CCDC:1 [SGRBG10 752x480]' # Test pattern ./yavta --set-control '0x00981901 1' /dev/v4l-subdev8 # ./yavta -p -f SGRBG10 -s 752x480 -n 4 --capture=3 /dev/video2 --file=img-#.bin To convert to jpg I used bayer2rgb [2] program executing following command, $ convert -size 752x480 GRBG_BAYER:./img-00.bin img-00.jpg And the result image looks like this http://downloads.isee.biz/pub/files/patterns/img-from-sensor.jpg Seems good, so I tried to use following pipeline Sensor - CCDC - Preview - Resizer ./media-ctl -r ./media-ctl -l 'mt9v032 3-005c:0-OMAP3 ISP CCDC:0[1]' ./media-ctl -l 'OMAP3 ISP CCDC:2-OMAP3 ISP preview:0[1]' ./media-ctl -l 'OMAP3 ISP preview:1-OMAP3 ISP resizer:0[1]' ./media-ctl -l 'OMAP3 ISP resizer:1-OMAP3 ISP resizer output:0[1]' ./media-ctl -V 'mt9v032 3-005c:0[SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP CCDC:0 [SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP CCDC:2 [SGRBG10 752x480]' ./media-ctl -V 'OMAP3 ISP preview:1 [UYVY 752x480]' ./media-ctl -V 'OMAP3 ISP resizer:1 [UYVY 752x480]' # Set Test pattern ./yavta --set-control '0x00981901 1' /dev/v4l-subdev8 ./yavta -f UYVY -s 752x480 --capture=3 --file=img-#.uyvy /dev/video6 I used 'convert' program to pass from UYVY to jpg, $ convert -size 752x480 img-00.uyvy img-00.jpg and the result image looks like this http://downloads.isee.biz/pub/files/patterns/img-from-resizer.jpg As you can see, the image is wrong and I'm not sure if the problem is from the sensor, from the previewer, from the resizer or from my conversion. Anyone have idea where should I look ? Or which is the source of the problem ? Could you please post the output of all the above media-ctl and yavta runs, as well as the captured raw binary frame ? Of course, The log configuring the pipeline Sensor - CCDC is http://pastebin.com/WX8ex5x2 and the raw image can be found http://downloads.isee.biz/pub/files/patterns/img-00.bin It looks like D9 and D8 have trouble keeping their high-level. Possible reasons would be conflicts on the signal lines (with something actively driving them to a low-level, a pull-down wouldn't have such an effect), faulty cable/solder joints (but I doubt that), or sampling the data on the wrong edge. The last option should be easy to test, just change the struct isp_v4l2_subdevs_group::bus::parallel::clk_pol field. And the log configuring the pipeline Sensor - CCDC - Previewer - Resizer is http://pastebin.com/wh5ZJwne and the raw image can be found http://downloads.isee.biz/pub/files/patterns/img-00.uyvy [1] http://git.linuxtv.org/pinchartl/media.git/shortlog/refs/heads/omap3isp- omap3isp-next [2] https://github.com/jdthomas/bayer2rgb -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: davinci: vpif: display: separate out subdev from output
Hi Hans, On Mon, Sep 24, 2012 at 7:02 PM, Hans Verkuil hverk...@xs4all.nl wrote: On Mon September 24 2012 15:21:44 Prabhakar Lad wrote: Hi Hans, On Mon, Sep 24, 2012 at 5:20 PM, Hans Verkuil hverk...@xs4all.nl wrote: On Mon September 24 2012 12:59:11 Hans Verkuil wrote: On Mon September 24 2012 12:44:11 Prabhakar wrote: From: Lad, Prabhakar prabhakar@ti.com vpif_display relied on a 1-1 mapping of output and subdev. This is not necessarily the case. Separate the two. So there is a list of subdevs and a list of outputs. Each output refers to a subdev and has routing information. An output does not have to have a subdev. The initial output for each channel is set to the fist output. Currently missing is support for associating multiple subdevs with an output. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sekhar Nori nsek...@ti.com Acked-by: Hans Verkuil hans.verk...@cisco.com I'm retracting this Ack. I did see something that wasn't right but I thought it was harmless, but after thinking some more I believe it should be fixed. Luckily, it's easy to fix. See below. Since we need a new version anyway I also added a comment to a few minor issues that can be fixed at the same time. Regards, Hans --- This patch is dependent on the patch series from Hans (http://www.mail-archive.com/linux-media@vger.kernel.org/msg52270.html) Changes for V2: 1: Changed v4l2_device_call_until_err() call to v4l2_subdev_call() for s_routing, since this call is for specific subdev, pointed out by Hans. arch/arm/mach-davinci/board-da850-evm.c | 29 +- arch/arm/mach-davinci/board-dm646x-evm.c | 39 ++- drivers/media/platform/davinci/vpif_display.c | 136 - include/media/davinci/vpif_types.h| 20 +++- 4 files changed, 183 insertions(+), 41 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 3081ea4..23a7012 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -46,6 +46,7 @@ #include mach/spi.h #include media/tvp514x.h +#include media/adv7343.h #define DA850_EVM_PHY_ID davinci_mdio-0:00 #define DA850_LCD_PWR_PIN GPIO_TO_PIN(2, 8) @@ -1257,16 +1258,34 @@ static struct vpif_subdev_info da850_vpif_subdev[] = { }, }; -static const char const *vpif_output[] = { - Composite, - S-Video, +static const struct vpif_output da850_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = S-Video, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_SVIDEO_ID, + }, }; static struct vpif_display_config da850_vpif_display_config = { .subdevinfo = da850_vpif_subdev, .subdev_count = ARRAY_SIZE(da850_vpif_subdev), - .output = vpif_output, - .output_count = ARRAY_SIZE(vpif_output), + .chan_config[0] = { + .outputs = da850_ch0_outputs, + .output_count = ARRAY_SIZE(da850_ch0_outputs), + }, .card_name= DA850/OMAP-L138 Video Display, }; diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index ad249c7..c206768 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -26,6 +26,7 @@ #include linux/i2c/pcf857x.h #include media/tvp514x.h +#include media/adv7343.h #include linux/mtd/mtd.h #include linux/mtd/nand.h @@ -496,18 +497,44 @@ static struct vpif_subdev_info dm646x_vpif_subdev[] = { }, }; -static const char *output[] = { - Composite, - Component, - S-Video, +static const struct vpif_output dm6467_ch0_outputs[] = { + { + .output = { + .index = 0, + .name = Composite, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, + .subdev_name = adv7343, + .output_route = ADV7343_COMPOSITE_ID, + }, + { + .output = { + .index = 1, + .name = Component, + .type = V4L2_OUTPUT_TYPE_ANALOG, + }, +