Re: [ANNOUNCEMENT] (tentative) Android generic V4L2 camera HAL
(+ My gmail address, please start using that address from next week on, since I'm leaving TI) Hi Guennadi, Thanks a lot for sharing these! Nice job. I immediately noticed you have changes on hardware/ti/omap4xxx/ subproject. So, Which devices did you used for testing this? I got confused since you had changes for the Samsung Nexus S, which has an Exynos chip... And you also have this Renesas Mackerel, which seems to use a SuperH 7372. Or maybe you just patched the omap4xxx related file to fix a build :) Regards, Sergio On Mon, Jun 25, 2012 at 8:55 AM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi all It's been a while since I've actually done this work. We have been waiting for various formalities to be resolved to be able to publish this work upstream. There are still a couple of formal issues to sort out before we can begin the submission process, but at least it has been decided to release patches for independent review and testing. For now I've uploaded a development snapshot to http://download.open-technology.de/android/20120625/ In the future we probably will provide git trees at least for the system/media/v4l_camera development. Enjoy:-) Any comments welcome. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] V4L: soc-camera: (cosmetic) use a more explicit name for a host handler
Hi Guennadi, On Tue, May 8, 2012 at 12:00 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Use enum_framesizes instead of enum_fsizes to more precisely follow the name of the respective ioctl(). Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Looks good to me! I'll redo my patch following this ioctl name matching. Reviewed-by: Sergio Aguirre sergio.a.agui...@gmail.com Regards, Sergio --- drivers/media/video/soc_camera.c | 14 +++--- include/media/soc_camera.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index eb25756..b980f99 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -257,13 +257,13 @@ static int soc_camera_g_std(struct file *file, void *priv, v4l2_std_id *a) return v4l2_subdev_call(sd, core, g_std, a); } -static int soc_camera_enum_fsizes(struct file *file, void *fh, +static int soc_camera_enum_framesizes(struct file *file, void *fh, struct v4l2_frmsizeenum *fsize) { struct soc_camera_device *icd = file-private_data; struct soc_camera_host *ici = to_soc_camera_host(icd-parent); - return ici-ops-enum_fsizes(icd, fsize); + return ici-ops-enum_framesizes(icd, fsize); } static int soc_camera_reqbufs(struct file *file, void *priv, @@ -1241,8 +1241,8 @@ static int default_s_parm(struct soc_camera_device *icd, return v4l2_subdev_call(sd, video, s_parm, parm); } -static int default_enum_fsizes(struct soc_camera_device *icd, - struct v4l2_frmsizeenum *fsize) +static int default_enum_framesizes(struct soc_camera_device *icd, + struct v4l2_frmsizeenum *fsize) { int ret; struct v4l2_subdev *sd = soc_camera_to_subdev(icd); @@ -1295,8 +1295,8 @@ int soc_camera_host_register(struct soc_camera_host *ici) ici-ops-set_parm = default_s_parm; if (!ici-ops-get_parm) ici-ops-get_parm = default_g_parm; - if (!ici-ops-enum_fsizes) - ici-ops-enum_fsizes = default_enum_fsizes; + if (!ici-ops-enum_framesizes) + ici-ops-enum_framesizes = default_enum_framesizes; mutex_lock(list_lock); list_for_each_entry(ix, hosts, list) { @@ -1386,7 +1386,7 @@ static const struct v4l2_ioctl_ops soc_camera_ioctl_ops = { .vidioc_s_input = soc_camera_s_input, .vidioc_s_std = soc_camera_s_std, .vidioc_g_std = soc_camera_g_std, - .vidioc_enum_framesizes = soc_camera_enum_fsizes, + .vidioc_enum_framesizes = soc_camera_enum_framesizes, .vidioc_reqbufs = soc_camera_reqbufs, .vidioc_querybuf = soc_camera_querybuf, .vidioc_qbuf = soc_camera_qbuf, diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h index b5c2b6c..00039d8 100644 --- a/include/media/soc_camera.h +++ b/include/media/soc_camera.h @@ -97,7 +97,7 @@ struct soc_camera_host_ops { int (*set_bus_param)(struct soc_camera_device *); int (*get_parm)(struct soc_camera_device *, struct v4l2_streamparm *); int (*set_parm)(struct soc_camera_device *, struct v4l2_streamparm *); - int (*enum_fsizes)(struct soc_camera_device *, struct v4l2_frmsizeenum *); + int (*enum_framesizes)(struct soc_camera_device *, struct v4l2_frmsizeenum *); unsigned int (*poll)(struct file *, poll_table *); }; -- 1.7.2.5 -- 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 05/10] v4l: Add support for ov5640 sensor
Hi Jean-Philippe, On Mon, May 7, 2012 at 2:27 AM, jean-philippe francois jp.franc...@cynove.com wrote: 2012/5/2 Sergio Aguirre saagui...@ti.com: This adds a very limited driver for ov5640, which only supports: - 2592x1944 @ ~7.5 fps - 1920x1080 @ ~15 fps, - 1280x720 @ ~24 fps, - 640x480 @ ~24 fps, - 320x240 @ ~24 fps, All in YUV422i format, using 1 CSI2 datalane @ 333 MHz. There is already a limited driver in mainline for ov5642. How does the 5642 differ from 5640 ? Well, it has several differences, see: - OV5640 product brief: http://www.ovt.com/download_document.php?type=sensorsensorid=93 - OV5642 product brief: http://www.ovt.com/download_document.php?type=sensorsensorid=65 Some of the most notable differences are: - OV5642 has a MIPI input, for using OV ISP with an external sensor. - OV5640 has JPEG compression, OV5642 apparently not... - OV5640 supports anti-shake, OV5640 apparently not... Can a single driver handle both chip ? Maybe, yeah.. Now, for the OV5640 differences above, I'm not enabling much nice features so far, so it might be worth a try to attempt the ov5642 driver on my OV5640 as-is, to see if we can expand it to support both. I'll also see if I can reach people at OmniVision to consult feasibility of register settings sharing. Regards, Sergio Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/Kconfig | 6 + drivers/media/video/Makefile | 1 + drivers/media/video/ov5640.c | 948 ++ include/media/ov5640.h | 10 + 4 files changed, 965 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/ov5640.c create mode 100644 include/media/ov5640.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 4482ac4..cc76652 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -480,6 +480,12 @@ config VIDEO_OV7670 OV7670 VGA camera. It currently only works with the M88ALP01 controller. +config VIDEO_OV5640 + tristate OmniVision OV5640 sensor support + depends on I2C VIDEO_V4L2 + help + This is a ov5640 camera driver + config VIDEO_VS6624 tristate ST VS6624 sensor support depends on VIDEO_V4L2 I2C diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index c95cc0d..da40ab3 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -68,6 +68,7 @@ obj-$(CONFIG_VIDEO_CX25840) += cx25840/ obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o obj-$(CONFIG_VIDEO_OV7670) += ov7670.o +obj-$(CONFIG_VIDEO_OV5640) += ov5640.o obj-$(CONFIG_VIDEO_TCM825X) += tcm825x.o obj-$(CONFIG_VIDEO_TVEEPROM) += tveeprom.o obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o diff --git a/drivers/media/video/ov5640.c b/drivers/media/video/ov5640.c new file mode 100644 index 000..2a64d50 --- /dev/null +++ b/drivers/media/video/ov5640.c @@ -0,0 +1,948 @@ +/* + * OmniVision OV5640 sensor driver + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation version 2. + * + * This program is distributed as is WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include linux/videodev2.h +#include linux/slab.h +#include linux/i2c.h +#include linux/log2.h +#include linux/delay.h +#include linux/module.h + +#include media/v4l2-device.h +#include media/v4l2-subdev.h +#include media/v4l2-ctrls.h + +#include media/ov5640.h + +/* OV5640 has only one fixed colorspace per pixelcode */ +struct ov5640_datafmt { + enum v4l2_mbus_pixelcode code; + enum v4l2_colorspace colorspace; +}; + +struct ov5640_timing_cfg { + u16 x_addr_start; + u16 y_addr_start; + u16 x_addr_end; + u16 y_addr_end; + u16 h_output_size; + u16 v_output_size; + u16 h_total_size; + u16 v_total_size; + u16 isp_h_offset; + u16 isp_v_offset; + u8 h_odd_ss_inc; + u8 h_even_ss_inc; + u8 v_odd_ss_inc; + u8 v_even_ss_inc; +}; + +enum ov5640_size { + OV5640_SIZE_QVGA, + OV5640_SIZE_VGA, + OV5640_SIZE_720P, + OV5640_SIZE_1080P, + OV5640_SIZE_5MP, + OV5640_SIZE_LAST, +}; + +static const struct v4l2_frmsize_discrete ov5640_frmsizes[OV5640_SIZE_LAST] = { + { 320, 240 }, + { 640, 480 }, + { 1280, 720 }, + { 1920, 1080 }, + { 2592, 1944 }, +}; + +/* Find a frame size in an array */ +static int ov5640_find_framesize(u32
Re: Android Support for camera?
Hi Sriram, On Mon, May 7, 2012 at 10:33 AM, Sriram V vshrir...@gmail.com wrote: Hi Sergio, I understand that you are working on providing Android HAL Support for camera on omap4. That's right. Not an active task at the moment, due to some other stuff going on, but yes, I have that task pending to do. Were you able to capture and record? Well, I'm trying to take these patches as a reference: http://review.omapzoom.org/#/q/project:platform/hardware/ti/omap4xxx+topic:usbcamera,n,z Which are implementing V4L2 camera support for the CameraHAL, currently tested with the UVC camera driver only. So, I need to set the IOCTLs to program the omap4iss media controller device, to set a usecase, and start preview. I'll keep you posted. Regards, Sergio -- Regards, Sriram -- 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: Android Support for camera?
Hi Guennadi, On Mon, May 7, 2012 at 4:25 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Sergio On Mon, 7 May 2012, Aguirre, Sergio wrote: Hi Sriram, On Mon, May 7, 2012 at 10:33 AM, Sriram V vshrir...@gmail.com wrote: Hi Sergio, I understand that you are working on providing Android HAL Support for camera on omap4. That's right. Not an active task at the moment, due to some other stuff going on, but yes, I have that task pending to do. Were you able to capture and record? Well, I'm trying to take these patches as a reference: http://review.omapzoom.org/#/q/project:platform/hardware/ti/omap4xxx+topic:usbcamera,n,z Which are implementing V4L2 camera support for the CameraHAL, currently tested with the UVC camera driver only. I've implemented a (pretty basic so far) V4L2 camera HAL for android (ICS), patche submission is pending legal clarifications... I hope to manage to push them into the upstream android, after which they shall become available to all platforms. I've implemented the HAL as a platform-agnostic library in C with a minimal (and naive;-)) C++ glue. I'm sure, those patches will need some improvements, but I'd be happy, if they could be taken as a basis. Ok, good to know. Please share them whenever you get over those legal clarifications :) Maybe we can work out a nice and complete solution all together! Regards, Sergio Regards, Sergio Thanks Guennadi So, I need to set the IOCTLs to program the omap4iss media controller device, to set a usecase, and start preview. I'll keep you posted. Regards, Sergio -- Regards, Sriram --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l: soc-camera: Add support for enum_frameintervals ioctl
Hi Guennadi, No problem. On Fri, May 4, 2012 at 10:05 AM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Sergio Sorry about the delay. On Wed, 18 Apr 2012, Aguirre, Sergio wrote: From: Sergio Aguirre saagui...@ti.com Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/soc_camera.c | 37 + include/media/soc_camera.h | 1 + 2 files changed, 38 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index eb25756..62c8956 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -266,6 +266,15 @@ static int soc_camera_enum_fsizes(struct file *file, void *fh, return ici-ops-enum_fsizes(icd, fsize); } +static int soc_camera_enum_fivals(struct file *file, void *fh, fivals is a bit short for my taste. Yes, I know about the *_enum_fsizes() precedent in soc_camera.c, we should have used a more descriptive name for that too. So, maybe I'll push a patch to change that to enum_frmsizes() or enum_framesizes(). Agreed. But that brings in a larger question, which is also the reason, why I added a couple more people to the CC: the following 3 operations in struct v4l2_subdev_video_ops don't make me particularly happy: int (*enum_framesizes)(struct v4l2_subdev *sd, struct v4l2_frmsizeenum *fsize); int (*enum_frameintervals)(struct v4l2_subdev *sd, struct v4l2_frmivalenum *fival); int (*enum_mbus_fsizes)(struct v4l2_subdev *sd, struct v4l2_frmsizeenum *fsize); The problems are: 1. enum_framesizes and enum_mbus_fsizes seem to be identical (yes, I see my Sob under the latter:-() Yeah, IMHO, the mbus one should go, since there's no mbus specific structure being handed as a parameter. 2. both struct v4l2_frmsizeenum and struct v4l2_frmivalenum are the structs, used in the respective V4L2 ioctl()s, and they both contain a field for a fourcc value, which doesn't make sense to subdevice drivers. So far the only driver combination in the mainline, that I see, that uses these operations is marvell-ccic ov7670. These drivers just ignore the pixel format. Relatively recently enum_mbus_fsizes() has been added to soc-camera, and this patch is adding enum_frameintervals(). Both these implementations abuse the .pixel_format field to pass a media-bus code value in it to subdevice drivers. This sends meaningful information to subdevice drivers, but is really a hack, rather than a proper implementation. True. Any idea how to improve this? Shall we create mediabus clones of those structs with an mbus code instead of fourcc, and drop one of the above enum_framesizes() operations? Well, to add more confusion to this.. :) We have this v4l2-subdev IOCTLs exported to userspace: #define VIDIOC_SUBDEV_ENUM_FRAME_SIZE \ _IOWR('V', 74, struct v4l2_subdev_frame_size_enum) #define VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL \ _IOWR('V', 75, struct v4l2_subdev_frame_interval_enum) Which in drivers/media/video/v4l2-subdev.c, are translated to pad ops: - v4l2_subdev_call(... enum_frame_size ...); - v4l2_subdev_call(... enum_frame_interval ...); respectively. So, this is also another thing that's causing some confusion. Does soc_camera use pad ops? Regards, Sergio Thanks Guennadi + struct v4l2_frmivalenum *fival) +{ + struct soc_camera_device *icd = file-private_data; + struct soc_camera_host *ici = to_soc_camera_host(icd-parent); + + return ici-ops-enum_fivals(icd, fival); +} + static int soc_camera_reqbufs(struct file *file, void *priv, struct v4l2_requestbuffers *p) { @@ -1266,6 +1275,31 @@ static int default_enum_fsizes(struct soc_camera_device *icd, return 0; } +static int default_enum_fivals(struct soc_camera_device *icd, + struct v4l2_frmivalenum *fival) +{ + int ret; + struct v4l2_subdev *sd = soc_camera_to_subdev(icd); + const struct soc_camera_format_xlate *xlate; + __u32 pixfmt = fival-pixel_format; + struct v4l2_frmivalenum fival_sd = *fival; + + xlate = soc_camera_xlate_by_fourcc(icd, pixfmt); + if (!xlate) + return -EINVAL; + /* map xlate-code to pixel_format, sensor only handle xlate-code*/ + fival_sd.pixel_format = xlate-code; + + ret = v4l2_subdev_call(sd, video, enum_frameintervals, fival_sd); + if (ret 0) + return ret; + + *fival = fival_sd; + fival-pixel_format = pixfmt; + + return 0; +} + int soc_camera_host_register(struct soc_camera_host *ici) { struct soc_camera_host *ix; @@ -1297,6 +1331,8 @@ int soc_camera_host_register(struct soc_camera_host *ici) ici-ops-get_parm = default_g_parm; if (!ici-ops-enum_fsizes) ici-ops
Re: [PATCH v3 07/10] arm: omap4430sdp: Add support for omap4iss camera
Hi Sakari, Thanks for reviewing. On Wed, May 2, 2012 at 2:47 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Hi Sergio, Thanks for the patches!! On Wed, May 02, 2012 at 10:15:46AM -0500, Sergio Aguirre wrote: ... +static int sdp4430_ov_cam1_power(struct v4l2_subdev *subdev, int on) +{ + struct device *dev = subdev-v4l2_dev-dev; + int ret; + + if (on) { + if (!regulator_is_enabled(sdp4430_cam2pwr_reg)) { + ret = regulator_enable(sdp4430_cam2pwr_reg); + if (ret) { + dev_err(dev, + Error in enabling sensor power regulator 'cam2pwr'\n); + return ret; + } + + msleep(50); + } + + gpio_set_value(OMAP4430SDP_GPIO_CAM_PDN_B, 1); + msleep(10); + ret = clk_enable(sdp4430_cam1_aux_clk); /* Enable XCLK */ + if (ret) { + dev_err(dev, + Error in clk_enable() in %s(%d)\n, + __func__, on); + gpio_set_value(OMAP4430SDP_GPIO_CAM_PDN_B, 0); + return ret; + } + msleep(10); + } else { + clk_disable(sdp4430_cam1_aux_clk); + msleep(1); + gpio_set_value(OMAP4430SDP_GPIO_CAM_PDN_B, 0); + if (regulator_is_enabled(sdp4430_cam2pwr_reg)) { + ret = regulator_disable(sdp4430_cam2pwr_reg); + if (ret) { + dev_err(dev, + Error in disabling sensor power regulator 'cam2pwr'\n); + return ret; + } + } + } + + return 0; +} Isn't this something that should be part of the sensor driver? There's nothing in the above code that would be board specific, except the names of the clocks, regulators and GPIOs. The sensor driver could hold the names instead; this would be also compatible with the device tree. Agreed. I see what you mean... I'll take care of that. It should be possible to have s_power() callback NULL, too. +static int sdp4430_ov_cam2_power(struct v4l2_subdev *subdev, int on) +{ + struct device *dev = subdev-v4l2_dev-dev; + int ret; + + if (on) { + u8 gpoctl = 0; + + ret = regulator_enable(sdp4430_cam2pwr_reg); + if (ret) { + dev_err(dev, + Error in enabling sensor power regulator 'cam2pwr'\n); + return ret; + } + + msleep(50); + + if (twl_i2c_read_u8(TWL_MODULE_AUDIO_VOICE, gpoctl, + TWL6040_REG_GPOCTL)) + return -ENODEV; + if (twl_i2c_write_u8(TWL_MODULE_AUDIO_VOICE, + gpoctl | TWL6040_GPO3, + TWL6040_REG_GPOCTL)) + return -ENODEV; The above piece of code looks quite interesting. What does it do? Well, this is because the camera adapter board in 4430SDP has 3 sensors actually: - 1 Sony IMX060 12.1 MP sensor - 2 OmniVision OV5650 sensors And there's 3 wideband analog switches, like this: http://www.analog.com/static/imported-files/data_sheets/ADG936_936R.pdf That basically select either IMX060 or OV5650 for CSI2A input. So, this commands are because the TWL6040 chip has a GPO pin to toggle this, instead of an OMAP GPIO (Don't ask me why :) ) Anyways... I see your point, maybe this should be explained better through a comment. Regards, Sergio Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/10] arm: omap4430sdp: Add support for omap4iss camera
Hi Sakari, On Thu, May 3, 2012 at 7:03 AM, Aguirre, Sergio saagui...@ti.com wrote: Hi Sakari, Thanks for reviewing. On Wed, May 2, 2012 at 2:47 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Hi Sergio, Thanks for the patches!! On Wed, May 02, 2012 at 10:15:46AM -0500, Sergio Aguirre wrote: ... +static int sdp4430_ov_cam1_power(struct v4l2_subdev *subdev, int on) +{ + struct device *dev = subdev-v4l2_dev-dev; + int ret; + + if (on) { + if (!regulator_is_enabled(sdp4430_cam2pwr_reg)) { + ret = regulator_enable(sdp4430_cam2pwr_reg); + if (ret) { + dev_err(dev, + Error in enabling sensor power regulator 'cam2pwr'\n); + return ret; + } + + msleep(50); + } + + gpio_set_value(OMAP4430SDP_GPIO_CAM_PDN_B, 1); + msleep(10); + ret = clk_enable(sdp4430_cam1_aux_clk); /* Enable XCLK */ + if (ret) { + dev_err(dev, + Error in clk_enable() in %s(%d)\n, + __func__, on); + gpio_set_value(OMAP4430SDP_GPIO_CAM_PDN_B, 0); + return ret; + } + msleep(10); + } else { + clk_disable(sdp4430_cam1_aux_clk); + msleep(1); + gpio_set_value(OMAP4430SDP_GPIO_CAM_PDN_B, 0); + if (regulator_is_enabled(sdp4430_cam2pwr_reg)) { + ret = regulator_disable(sdp4430_cam2pwr_reg); + if (ret) { + dev_err(dev, + Error in disabling sensor power regulator 'cam2pwr'\n); + return ret; + } + } + } + + return 0; +} Isn't this something that should be part of the sensor driver? There's nothing in the above code that would be board specific, except the names of the clocks, regulators and GPIOs. The sensor driver could hold the names instead; this would be also compatible with the device tree. Agreed. I see what you mean... I'll take care of that. Can you please check out these patches? 1. http://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera/commit/cb6c10d58053180364461e6bc8d30d1ec87e6e22 2. http://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera/commit/6732e0db25c6647b34ef8f01c244a49a1fd6b45d 3. http://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera/commit/d61c4e3142dc9cae972f9128fe73d986838c0ca1 4. http://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera/commit/e83f36001c7f7cbe184ad094d9b0c95c39e5028f I want to see if I got your point properly... Regards, Sergio It should be possible to have s_power() callback NULL, too. +static int sdp4430_ov_cam2_power(struct v4l2_subdev *subdev, int on) +{ + struct device *dev = subdev-v4l2_dev-dev; + int ret; + + if (on) { + u8 gpoctl = 0; + + ret = regulator_enable(sdp4430_cam2pwr_reg); + if (ret) { + dev_err(dev, + Error in enabling sensor power regulator 'cam2pwr'\n); + return ret; + } + + msleep(50); + + if (twl_i2c_read_u8(TWL_MODULE_AUDIO_VOICE, gpoctl, + TWL6040_REG_GPOCTL)) + return -ENODEV; + if (twl_i2c_write_u8(TWL_MODULE_AUDIO_VOICE, + gpoctl | TWL6040_GPO3, + TWL6040_REG_GPOCTL)) + return -ENODEV; The above piece of code looks quite interesting. What does it do? Well, this is because the camera adapter board in 4430SDP has 3 sensors actually: - 1 Sony IMX060 12.1 MP sensor - 2 OmniVision OV5650 sensors And there's 3 wideband analog switches, like this: http://www.analog.com/static/imported-files/data_sheets/ADG936_936R.pdf That basically select either IMX060 or OV5650 for CSI2A input. So, this commands are because the TWL6040 chip has a GPO pin to toggle this, instead of an OMAP GPIO (Don't ask me why :) ) Anyways... I see your point, maybe this should be explained better through a comment. Regards, Sergio Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/10] v4l2: OMAP4 ISS driver + Sensor + Board support
On Wed, May 2, 2012 at 10:15 AM, Sergio Aguirre saagui...@ti.com wrote: Hi everyone, It's been a long time since last version (5 months)! :) This is the third version of the OMAP4 ISS driver, which uses Media Controller and videobuf2 frameworks. This patchset should apply cleanly on top of v3.4-rc5 kernel tag. This driver attempts to provide an fully open source solution to control the OMAP4 Imaging SubSystem (a.k.a. ISS). Starts with just CSI2-A/B interface support, and pretends to be ready for expansion to add support to the many ISS block modules as possible. Please see newly added documentation for more details: Documentation/video4linux/omap4_camera.txt Any comments/complaints are welcome. :) Apologies, forgot to mention this: Tested with these patchsets: - Sakari's patches for N9 and some v4l2 changes: http://www.spinics.net/lists/linux-media/msg45052.html - CMA v24: http://www.spinics.net/lists/linux-media/msg46106.html Both rebased to v3.4-rc5. Regards, Sergio Changes since v2: - Supports CSI2B now! - Add support for RAW8. - Usage of V4L2_CID_PIXEL_RATE, instead of dphy configuration in boardfile (similar to omap3isp) - Removes save/restore support for now, as it is broken. - Attend several comments form Sakari Ailus (Thanks Sakari!) - Populate hw_revision in media_dev struct. - Ported several fixes pushed for omap3isp (Thanks Laurent!) - Use module_platform_driver. - Use proposed generic v4l2_subdev_link_validate. - Move OMAP4_CTRL_MODULE_PAD_CORE_CONTROL_CAMERA_RX handle to omap4iss code, instead of board file. Changes since v1: - Simplification of auxclk handlign in board files - Use of HWMOD declaration for assisted platform_device creation. - Videobuf2 migration (Removal of custom iss_queue buffer handling driver) Regards, Sergio Sergio Aguirre (10): mfd: twl6040: Fix wrong TWL6040_GPO3 bitfield value OMAP4: hwmod: Include CSI2A/B and CSIPHY1/2 memory sections OMAP4: Add base addresses for ISS v4l: Add support for omap4iss driver v4l: Add support for ov5640 sensor v4l: Add support for ov5650 sensor arm: omap4430sdp: Add support for omap4iss camera arm: omap4panda: Add support for omap4iss camera omap2plus: Add support for omap4iss camera arm: Add support for CMA for omap4iss driver Documentation/video4linux/omap4_camera.txt | 64 ++ arch/arm/configs/omap2plus_defconfig | 2 + arch/arm/mach-omap2/Kconfig | 32 + arch/arm/mach-omap2/Makefile | 3 + arch/arm/mach-omap2/board-4430sdp-camera.c | 415 arch/arm/mach-omap2/board-4430sdp.c | 20 + arch/arm/mach-omap2/board-omap4panda-camera.c | 209 arch/arm/mach-omap2/devices.c | 40 + arch/arm/mach-omap2/devices.h | 4 + arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 22 +- drivers/media/video/Kconfig | 25 + drivers/media/video/Makefile | 3 + drivers/media/video/omap4iss/Makefile | 6 + drivers/media/video/omap4iss/iss.c | 1159 + drivers/media/video/omap4iss/iss.h | 121 +++ drivers/media/video/omap4iss/iss_csi2.c | 1368 + drivers/media/video/omap4iss/iss_csi2.h | 155 +++ drivers/media/video/omap4iss/iss_csiphy.c | 281 + drivers/media/video/omap4iss/iss_csiphy.h | 51 + drivers/media/video/omap4iss/iss_regs.h | 244 + drivers/media/video/omap4iss/iss_video.c | 1123 drivers/media/video/omap4iss/iss_video.h | 201 drivers/media/video/ov5640.c | 948 + drivers/media/video/ov5650.c | 733 + include/linux/mfd/twl6040.h | 2 +- include/media/omap4iss.h | 65 ++ include/media/ov5640.h | 10 + include/media/ov5650.h | 10 + 28 files changed, 7314 insertions(+), 2 deletions(-) create mode 100644 Documentation/video4linux/omap4_camera.txt create mode 100644 arch/arm/mach-omap2/board-4430sdp-camera.c create mode 100644 arch/arm/mach-omap2/board-omap4panda-camera.c create mode 100644 drivers/media/video/omap4iss/Makefile create mode 100644 drivers/media/video/omap4iss/iss.c create mode 100644 drivers/media/video/omap4iss/iss.h create mode 100644 drivers/media/video/omap4iss/iss_csi2.c create mode 100644 drivers/media/video/omap4iss/iss_csi2.h create mode 100644 drivers/media/video/omap4iss/iss_csiphy.c create mode 100644 drivers/media/video/omap4iss/iss_csiphy.h create mode 100644 drivers/media/video/omap4iss/iss_regs.h create mode 100644 drivers/media/video/omap4iss/iss_video.c create mode 100644 drivers/media/video/omap4iss/iss_video.h create mode 100644 drivers/media/video/ov5640.c create mode 100644 drivers/media/video/ov5650.c create mode
Re: [Query] About V4L2_CAP_VIDEO_CAPTURE_MPLANE device types
Hi Sylwester, Thanks for the very informative reply! On Tue, May 1, 2012 at 3:08 PM, Sylwester Nawrocki snj...@gmail.com wrote: Hi Sergio, On 05/01/2012 09:08 PM, Aguirre, Sergio wrote: Hi all, I wonder if there's an example app for v4l2 devices with V4L2_CAP_VIDEO_CAPTURE_MPLANE capability? (like capture.c in V4L2 API docs) There isn't at the V4L2 API docs, it probably makes sense to add one though. I'll try to find a time to prepare a patch for that. A very simple application with V4L2_CAP_VIDEO_CAPTURE_MPLANE support can be found here: http://thread.gmane.org/gmane.comp.video.dri.devel/65997 It might be not that straightforward since there is the DMABUF memory type used. Some more applications using multi-plane API are available at: http://git.infradead.org/users/kmpark/public-apps An MMAP memory example can be found here: http://git.infradead.org/users/kmpark/public-apps/blob/9c057b001e8873861a70f7025214003837a0860b:/v4l2-mfc-example/mfc.c Please see mfc_dec_setup_capture function. Thanks. I'll check these links in detail. Also, does it have to be mutually exclusive with a V4L2_CAP_VIDEO_CAPTURE device? No, the driver could support both. But it generally makes sense to implement only _mplane ioctls at drivers. The multi/single-plane conversion should be done in user space. There were some patches for libv4l to support that (http://www.spinics.net/lists/linux-media/msg35080.html), but such conversions are not yet supported in the v4l libraries yet AFAIK. Ok. Got it. I'm also not sure how is V4L2_CAP_VIDEO_CAPTURE_MPLANE handled in standard GStreamer plugins like v4l2src at the moment, having only multi-plane interface at the driver might cause additional problems there. Ok. I guess that's why i was concerned on keeping the non-mplane support coexisting with this. Thanks again for your time! Regards, Sergio -- 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: [media-ctl PATCH] Compare entity name length aswell
Hi Laurent, On Sun, Apr 29, 2012 at 12:40 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Sergio, On Saturday 28 April 2012 10:04:01 Aguirre, Sergio wrote: On Wed, Apr 25, 2012 at 8:57 AM, Sergio Aguirre saagui...@ti.com wrote: Otherwise, some false positives might arise when having 2 subdevices with similar names, like: OMAP4 ISS ISP IPIPEIF OMAP4 ISS ISP IPIPE Before this patch, trying to find OMAP4 ISS ISP IPIPE, resulted in a false entity match, retrieving OMAP4 ISS ISP IPIPEIF information instead. Checking length should ensure such cases are handled well. Any feedback about this? Thanks for the patch, and sorry for the delay. No problem. :) Signed-off-by: Sergio Aguirre saagui...@ti.com --- src/mediactl.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/mediactl.c b/src/mediactl.c index 5b8c587..451a386 100644 --- a/src/mediactl.c +++ b/src/mediactl.c @@ -66,7 +66,8 @@ struct media_entity *media_get_entity_by_name(struct media_device *media, for (i = 0; i media-entities_count; ++i) { struct media_entity *entity = media-entities[i]; - if (strncmp(entity-info.name, name, length) == 0) + if ((strncmp(entity-info.name, name, length) == 0) + (strlen(entity-info.name) == length)) return entity; } Instead of calling strlen() which has a O(n) complexity, what about just checking that the entity name has a '\0' in the length'th position ? Something like the following patch: From 46bec667b675573cf1ce698c68112e3dbd31930e Mon Sep 17 00:00:00 2001 From: Sergio Aguirre saagui...@ti.com Date: Wed, 25 Apr 2012 08:57:13 -0500 Subject: [PATCH] Compare name length to avoid false positives in media_get_entity_by_name If two subdevice have names that only differ by a suffix (such as OMAP4 ISS ISP IPIPE and OMAP4 ISS ISP IPIPEIF) the media_get_entity_by_name function might return a pointer to the entity with the longest name when called with the shortest name. Fix this by verifying that the candidate entity name length is equal to the requested name length. Signed-off-by: Sergio Aguirre saagui...@ti.com Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- src/mediactl.c | 9 - src/tools.h | 1 + 2 files changed, 9 insertions(+), 1 deletions(-) diff --git a/src/mediactl.c b/src/mediactl.c index 5b8c587..bc6a713 100644 --- a/src/mediactl.c +++ b/src/mediactl.c @@ -63,10 +63,17 @@ struct media_entity *media_get_entity_by_name(struct media_device *media, { unsigned int i; + /* A match is impossible if the entity name is longer than the maximum + * size we can get from the kernel. + */ + if (length = FIELD_SIZEOF(struct media_entity_desc, name)) + return NULL; + Good idea. for (i = 0; i media-entities_count; ++i) { struct media_entity *entity = media-entities[i]; - if (strncmp(entity-info.name, name, length) == 0) + if (strncmp(entity-info.name, name, length) == 0 + entity-info.name[length] == '\0') ACK. Your patch is definitely better. IMHO, I think it'll be more fair if you put yourself as the author of this new patch, and me as: Reported-by:. Regards, Sergio return entity; } diff --git a/src/tools.h b/src/tools.h index e56edb2..de06cb3 100644 --- a/src/tools.h +++ b/src/tools.h @@ -23,6 +23,7 @@ #define __TOOLS_H__ #define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0])) +#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)-f)) #endif /* __TOOLS_H__ */ -- 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: [media-ctl PATCH] Compare entity name length aswell
Hi Laurent, On Sun, Apr 29, 2012 at 6:13 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Sergio, On Sunday 29 April 2012 17:36:43 Aguirre, Sergio wrote: On Sun, Apr 29, 2012 at 12:40 PM, Laurent Pinchart wrote: On Saturday 28 April 2012 10:04:01 Aguirre, Sergio wrote: On Wed, Apr 25, 2012 at 8:57 AM, Sergio Aguirre saagui...@ti.com wrote: Otherwise, some false positives might arise when having 2 subdevices with similar names, like: OMAP4 ISS ISP IPIPEIF OMAP4 ISS ISP IPIPE Before this patch, trying to find OMAP4 ISS ISP IPIPE, resulted in a false entity match, retrieving OMAP4 ISS ISP IPIPEIF information instead. Checking length should ensure such cases are handled well. Any feedback about this? Thanks for the patch, and sorry for the delay. No problem. :) Signed-off-by: Sergio Aguirre saagui...@ti.com --- src/mediactl.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/mediactl.c b/src/mediactl.c index 5b8c587..451a386 100644 --- a/src/mediactl.c +++ b/src/mediactl.c @@ -66,7 +66,8 @@ struct media_entity *media_get_entity_by_name(struct media_device *media, for (i = 0; i media-entities_count; ++i) { struct media_entity *entity = media-entities[i]; - if (strncmp(entity-info.name, name, length) == 0) + if ((strncmp(entity-info.name, name, length) == 0) + (strlen(entity-info.name) == length)) return entity; } Instead of calling strlen() which has a O(n) complexity, what about just checking that the entity name has a '\0' in the length'th position ? Something like the following patch: From 46bec667b675573cf1ce698c68112e3dbd31930e Mon Sep 17 00:00:00 2001 From: Sergio Aguirre saagui...@ti.com Date: Wed, 25 Apr 2012 08:57:13 -0500 Subject: [PATCH] Compare name length to avoid false positives in media_get_entity_by_name If two subdevice have names that only differ by a suffix (such as OMAP4 ISS ISP IPIPE and OMAP4 ISS ISP IPIPEIF) the media_get_entity_by_name function might return a pointer to the entity with the longest name when called with the shortest name. Fix this by verifying that the candidate entity name length is equal to the requested name length. Signed-off-by: Sergio Aguirre saagui...@ti.com Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- src/mediactl.c | 9 - src/tools.h | 1 + 2 files changed, 9 insertions(+), 1 deletions(-) diff --git a/src/mediactl.c b/src/mediactl.c index 5b8c587..bc6a713 100644 --- a/src/mediactl.c +++ b/src/mediactl.c @@ -63,10 +63,17 @@ struct media_entity *media_get_entity_by_name(struct media_device *media, { unsigned int i; + /* A match is impossible if the entity name is longer than the maximum + * size we can get from the kernel. + */ + if (length = FIELD_SIZEOF(struct media_entity_desc, name)) + return NULL; + Good idea. for (i = 0; i media-entities_count; ++i) { struct media_entity *entity = media-entities[i]; - if (strncmp(entity-info.name, name, length) == 0) + if (strncmp(entity-info.name, name, length) == 0 + entity-info.name[length] == '\0') ACK. Your patch is definitely better. IMHO, I think it'll be more fair if you put yourself as the author of this new patch, and me as: Reported-by:. It's such a small patch, it's fine :-) Thank you for the proposal nonetheless. I've pushed the patch to the repository. Excellent! Thanks for that :) Regards, Sergio return entity; } diff --git a/src/tools.h b/src/tools.h index e56edb2..de06cb3 100644 --- a/src/tools.h +++ b/src/tools.h @@ -23,6 +23,7 @@ #define __TOOLS_H__ #define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0])) +#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)-f)) #endif /* __TOOLS_H__ */ -- 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: [media-ctl PATCH] Compare entity name length aswell
Hi Laurent On Wed, Apr 25, 2012 at 8:57 AM, Sergio Aguirre saagui...@ti.com wrote: Otherwise, some false positives might arise when having 2 subdevices with similar names, like: OMAP4 ISS ISP IPIPEIF OMAP4 ISS ISP IPIPE Before this patch, trying to find OMAP4 ISS ISP IPIPE, resulted in a false entity match, retrieving OMAP4 ISS ISP IPIPEIF information instead. Checking length should ensure such cases are handled well. Any feedback about this? Regards, Sergio Signed-off-by: Sergio Aguirre saagui...@ti.com --- src/mediactl.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/mediactl.c b/src/mediactl.c index 5b8c587..451a386 100644 --- a/src/mediactl.c +++ b/src/mediactl.c @@ -66,7 +66,8 @@ struct media_entity *media_get_entity_by_name(struct media_device *media, for (i = 0; i media-entities_count; ++i) { struct media_entity *entity = media-entities[i]; - if (strncmp(entity-info.name, name, length) == 0) + if ((strncmp(entity-info.name, name, length) == 0) + (strlen(entity-info.name) == length)) return entity; } -- 1.7.5.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: ics port for camera hal?
Hi Ryan On Sun, Apr 22, 2012 at 6:05 AM, Ryan ryanphilip...@googlemail.com wrote: Hi, Is there an V4L Port in the camera hal implementations on android ice cream sandwich which makes use of panda v4l camera drivers. Me and a couple of colleagues in TI are working on this at the moment, and we hope to have something useful very soon. What we have achieved so far is a PandaBoard bootign ICS with support for UVC cameras, so that Camera Application can start preview (just that for the moment, plenty of things to take care of yet..) But you can check this out here of you are interested on it: http://omappedia.org/wiki/Building_L27.IS.2.M1_for_PandaBoard,_with_USB_camera_support Regards, Sergio Thanks Regards, Ryan -- 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: Mipi csi2 driver for Omap4
Hi Steve, Sakari, On Fri, Apr 20, 2012 at 3:42 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Hi Steve, On Fri, Apr 20, 2012 at 12:11:38PM +0200, Steve Lindell wrote: Hi! I'm developing a mipi csi2 receiver for test purpose and need some help of how to capture the data stream from a camera module. I'm using a phytec board with a Omap4430 processor running Linux kernel 3.0.9. Connected to the MIPI lanes I have a camera module (soled on a flexfilm) The camera follows the Mipi csi2 specs and is controlled via an external I2C controller. I have activated the camera and its now transmitting a test pattern on the Mipi lines (4 line connection). I need to capture the stream and store it as a Raw Bayer snapshot. Is this possible use Omap4430 and does Linux have the necessary drivers to capture the stream. If this driver exists are there any documentation of how to implement the driver? Is it possible to get some help of how to get started? Sergio Aguirre has posted the patches for the Omap 4 Iss to this list some time ago. I believe you'll also be able to find them here: URL:https://gitorious.org/omap4-v4l2-camera/pages/Home Thanks Sakari for the reference. I guess you'd be also better off using a newer kernel than that. Hope this helps... Cc Sergio. Steve, I think it'll be easier for you to consider my code as a reference, and follow as a reference the pandaboard implementation i've been maintaining. Take a look specially at the devel branch, and to these files: # Board file arch/arm/mach-omap2/board-omap4panda-camera.c # OV5650 Sensor file (which is RAW10) drivers/media/video/ov5650.c Please let me know any questions you might have. I can help you out on getting what you need. Regards, Sergio Regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: 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
[PATCH] v4l: soc-camera: Add support for enum_frameintervals ioctl
From: Sergio Aguirre saagui...@ti.com Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/soc_camera.c | 37 + include/media/soc_camera.h |1 + 2 files changed, 38 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index eb25756..62c8956 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -266,6 +266,15 @@ static int soc_camera_enum_fsizes(struct file *file, void *fh, return ici-ops-enum_fsizes(icd, fsize); } +static int soc_camera_enum_fivals(struct file *file, void *fh, + struct v4l2_frmivalenum *fival) +{ + struct soc_camera_device *icd = file-private_data; + struct soc_camera_host *ici = to_soc_camera_host(icd-parent); + + return ici-ops-enum_fivals(icd, fival); +} + static int soc_camera_reqbufs(struct file *file, void *priv, struct v4l2_requestbuffers *p) { @@ -1266,6 +1275,31 @@ static int default_enum_fsizes(struct soc_camera_device *icd, return 0; } +static int default_enum_fivals(struct soc_camera_device *icd, + struct v4l2_frmivalenum *fival) +{ + int ret; + struct v4l2_subdev *sd = soc_camera_to_subdev(icd); + const struct soc_camera_format_xlate *xlate; + __u32 pixfmt = fival-pixel_format; + struct v4l2_frmivalenum fival_sd = *fival; + + xlate = soc_camera_xlate_by_fourcc(icd, pixfmt); + if (!xlate) + return -EINVAL; + /* map xlate-code to pixel_format, sensor only handle xlate-code*/ + fival_sd.pixel_format = xlate-code; + + ret = v4l2_subdev_call(sd, video, enum_frameintervals, fival_sd); + if (ret 0) + return ret; + + *fival = fival_sd; + fival-pixel_format = pixfmt; + + return 0; +} + int soc_camera_host_register(struct soc_camera_host *ici) { struct soc_camera_host *ix; @@ -1297,6 +1331,8 @@ int soc_camera_host_register(struct soc_camera_host *ici) ici-ops-get_parm = default_g_parm; if (!ici-ops-enum_fsizes) ici-ops-enum_fsizes = default_enum_fsizes; + if (!ici-ops-enum_fivals) + ici-ops-enum_fivals = default_enum_fivals; mutex_lock(list_lock); list_for_each_entry(ix, hosts, list) { @@ -1387,6 +1423,7 @@ static const struct v4l2_ioctl_ops soc_camera_ioctl_ops = { .vidioc_s_std= soc_camera_s_std, .vidioc_g_std= soc_camera_g_std, .vidioc_enum_framesizes = soc_camera_enum_fsizes, + .vidioc_enum_frameintervals = soc_camera_enum_fivals, .vidioc_reqbufs = soc_camera_reqbufs, .vidioc_querybuf = soc_camera_querybuf, .vidioc_qbuf = soc_camera_qbuf, diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h index b5c2b6c..0a3ac07 100644 --- a/include/media/soc_camera.h +++ b/include/media/soc_camera.h @@ -98,6 +98,7 @@ struct soc_camera_host_ops { int (*get_parm)(struct soc_camera_device *, struct v4l2_streamparm *); int (*set_parm)(struct soc_camera_device *, struct v4l2_streamparm *); int (*enum_fsizes)(struct soc_camera_device *, struct v4l2_frmsizeenum *); + int (*enum_fivals)(struct soc_camera_device *, struct v4l2_frmivalenum *); unsigned int (*poll)(struct file *, poll_table *); }; -- 1.7.5.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
[Query] About NV12 pixel format support in a subdevice
Hi everyone, I'll like to request for your advice on adding NV12 support for my omap4iss camera driver, which is done after the resizer block in the OMAP4 ISS ISP (Imaging SubSystem Image Signal Processor). So, the problem with that, is that I don't see a match for V4L2_PIX_FMT_NV12 pixel format in enum v4l2_mbus_pixelcode. Now, I wonder what's the best way to describe the format... Is this correct? V4L2_MBUS_FMT_NV12_1X12 Because every pixel is comprised of a 8-bit Y element, and it's UV components are grouped in pairs with the next horizontal pixel, whcih in combination are represented in 8 bits... So it's like that UV component per-pixel is 4-bits. Not exactly, but it's the best representation I could think of to simplify things. I mean, the HW itself writes in memory to 2 contiguous buffers, so there's 2 separate DMA writes. I have to program 2 starting addresses, which, in an internal non-v4l2-subdev implementation, I have been programming like this: paddr = start of 32-byte aligned physical address to store buffer x = width y = height Ysize = (x * y) UVsize = (x / 2) * y Total size = Ysize + UVsize Ystart = paddr UVstart = (paddr + Ysize) But, in the media controller framework, i have a single DMA output pad, that creates a v4l2 capture device node, and i'll be queueing a single buffer. Any advice on how to address this properly? Does anyone has/had a similar need? Regards, Sergio -- 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: [Query] About NV12 pixel format support in a subdevice
Hi Guennadi, Thanks for your reply. On Sat, Apr 7, 2012 at 4:21 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Sergio On Sat, 7 Apr 2012, Aguirre, Sergio wrote: Hi everyone, I'll like to request for your advice on adding NV12 support for my omap4iss camera driver, which is done after the resizer block in the OMAP4 ISS ISP (Imaging SubSystem Image Signal Processor). So, the problem with that, is that I don't see a match for V4L2_PIX_FMT_NV12 pixel format in enum v4l2_mbus_pixelcode. Now, I wonder what's the best way to describe the format... Is this correct? V4L2_MBUS_FMT_NV12_1X12 Because every pixel is comprised of a 8-bit Y element, and it's UV components are grouped in pairs with the next horizontal pixel, whcih in combination are represented in 8 bits... So it's like that UV component per-pixel is 4-bits. Not exactly, but it's the best representation I could think of to simplify things. Do I understand it right, that your resizer is sending the data to the DMA engine interleaved, not Y and UV planes separately, and it's only the DMA engine, that is separating the planes, when writing to buffers? In such a case I'd use a suitable YUV420 V4L2_MBUS_FMT_* format for that and have the DMA engine convert it to NV12, similar to what sh_mobile_ceu_camera does. No, it actually has 2 register sets for specifying the start address for each plane. So, I have one register that I program with Y-start address, and another register that I program with UV-start address. For both cases, you control the byte offset between every begin of each line. So, in theory, you could save it interleaved in memory if you want, or in 2 separate buffers depending on how you program the address/offset pair. Regards, Sergio Thanks Guennadi I mean, the HW itself writes in memory to 2 contiguous buffers, so there's 2 separate DMA writes. I have to program 2 starting addresses, which, in an internal non-v4l2-subdev implementation, I have been programming like this: paddr = start of 32-byte aligned physical address to store buffer x = width y = height Ysize = (x * y) UVsize = (x / 2) * y Total size = Ysize + UVsize Ystart = paddr UVstart = (paddr + Ysize) But, in the media controller framework, i have a single DMA output pad, that creates a v4l2 capture device node, and i'll be queueing a single buffer. Any advice on how to address this properly? Does anyone has/had a similar need? Regards, Sergio -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Query] About NV12 pixel format support in a subdevice
Hi Guennadi, On Sat, Apr 7, 2012 at 4:51 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Sat, 7 Apr 2012, Aguirre, Sergio wrote: Hi Guennadi, Thanks for your reply. On Sat, Apr 7, 2012 at 4:21 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Sergio On Sat, 7 Apr 2012, Aguirre, Sergio wrote: Hi everyone, I'll like to request for your advice on adding NV12 support for my omap4iss camera driver, which is done after the resizer block in the OMAP4 ISS ISP (Imaging SubSystem Image Signal Processor). So, the problem with that, is that I don't see a match for V4L2_PIX_FMT_NV12 pixel format in enum v4l2_mbus_pixelcode. Now, I wonder what's the best way to describe the format... Is this correct? V4L2_MBUS_FMT_NV12_1X12 Because every pixel is comprised of a 8-bit Y element, and it's UV components are grouped in pairs with the next horizontal pixel, whcih in combination are represented in 8 bits... So it's like that UV component per-pixel is 4-bits. Not exactly, but it's the best representation I could think of to simplify things. Do I understand it right, that your resizer is sending the data to the DMA engine interleaved, not Y and UV planes separately, and it's only the DMA engine, that is separating the planes, when writing to buffers? In such a case I'd use a suitable YUV420 V4L2_MBUS_FMT_* format for that and have the DMA engine convert it to NV12, similar to what sh_mobile_ceu_camera does. No, it actually has 2 register sets for specifying the start address for each plane. Sorry, what it? The DMA engine, right? Then it still looks pretty similar to the CEU case to me: it also can either write the data interleaved into RAM and produce a YUV420 format, or convert to NV12. Which one to do is decided by the format, configured on the video device node by the driver. Hmm, ok. I think I know what you mean now, sorry. So you're saying I should really use, say: V4L2_MBUS_FMT_YUYV8_1_5X8 as subdevice format, and let the v4l2 device output node either use: - V4L2_PIX_FMT_NV12 or - V4L2_PIX_FMT_YUV420 depending on how I want the DMA engine to organize the data. Did I got your point correctly? Thanks for your patience. Regards, Sergio Thanks Guennadi So, I have one register that I program with Y-start address, and another register that I program with UV-start address. For both cases, you control the byte offset between every begin of each line. So, in theory, you could save it interleaved in memory if you want, or in 2 separate buffers depending on how you program the address/offset pair. Regards, Sergio Thanks Guennadi I mean, the HW itself writes in memory to 2 contiguous buffers, so there's 2 separate DMA writes. I have to program 2 starting addresses, which, in an internal non-v4l2-subdev implementation, I have been programming like this: paddr = start of 32-byte aligned physical address to store buffer x = width y = height Ysize = (x * y) UVsize = (x / 2) * y Total size = Ysize + UVsize Ystart = paddr UVstart = (paddr + Ysize) But, in the media controller framework, i have a single DMA output pad, that creates a v4l2 capture device node, and i'll be queueing a single buffer. Any advice on how to address this properly? Does anyone has/had a similar need? Regards, Sergio -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add support for YUV420 formats
Hi Laurent, Sorry, forgot to mention that this is for your media-ctl app. Regards, Sergio On Sat, Apr 7, 2012 at 10:54 PM, saagui...@ti.com wrote: From: Sergio Aguirre saagui...@ti.com Signed-off-by: Sergio Aguirre saagui...@ti.com --- src/v4l2subdev.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/v4l2subdev.c b/src/v4l2subdev.c index b886b72..e28ed49 100644 --- a/src/v4l2subdev.c +++ b/src/v4l2subdev.c @@ -498,8 +498,10 @@ static struct { { Y12, V4L2_MBUS_FMT_Y12_1X12 }, { YUYV, V4L2_MBUS_FMT_YUYV8_1X16 }, { YUYV2X8, V4L2_MBUS_FMT_YUYV8_2X8 }, + { YUYV1_5X8, V4L2_MBUS_FMT_YUYV8_1_5X8 }, { UYVY, V4L2_MBUS_FMT_UYVY8_1X16 }, { UYVY2X8, V4L2_MBUS_FMT_UYVY8_2X8 }, + { UYVY1_5X8, V4L2_MBUS_FMT_UYVY8_1_5X8 }, { SBGGR8, V4L2_MBUS_FMT_SBGGR8_1X8 }, { SGBRG8, V4L2_MBUS_FMT_SGBRG8_1X8 }, { SGRBG8, V4L2_MBUS_FMT_SGRBG8_1X8 }, -- 1.7.5.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 06/11] v4l: Add support for omap4iss driver
Hi Sakari, On Thu, Jan 26, 2012 at 3:05 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Hi Sergio, Aguirre, Sergio wrote: On Wed, Nov 30, 2011 at 06:14:55PM -0600, Sergio Aguirre wrote: ... +/* + * iss_save_ctx - Saves ISS context. + * @iss: OMAP4 ISS device + * + * Routine for saving the context of each module in the ISS. + */ +static void iss_save_ctx(struct iss_device *iss) +{ + iss_save_context(iss, iss_reg_list); +} Do you really, really need to save context related data? Couldn't you initialise the device the usual way when e.g. returning from suspended state? I'll actually have to revisit this more carefuly. I haven't really tested Runtime PM on this. I think I'll remove this for now, until i come up with something better. Usually it's best to recreate the configuration the same way the driver did it in the first place. Some registers have side effects so that restoring them requires extra care. +/* + * iss_restore_ctx - Restores ISS context. + * @iss: OMAP4 ISS device + * + * Routine for restoring the context of each module in the ISS. + */ +static void iss_restore_ctx(struct iss_device *iss) +{ + iss_restore_context(iss, iss_reg_list); +} + ... +/* + * csi2_irq_ctx_set - Enables CSI2 Context IRQs. + * @enable: Enable/disable CSI2 Context interrupts + */ +static void csi2_irq_ctx_set(struct iss_csi2_device *csi2, int enable) +{ + u32 reg = CSI2_CTX_IRQ_FE; + int i; + + if (csi2-use_fs_irq) + reg |= CSI2_CTX_IRQ_FS; + + for (i = 0; i 8; i++) { 8 == number of contexts? Yes. This loops from 0 to 7. Are you implying that I need to add a define to this? I think something that tells it is the number of contexts would be nicer. ... + writel(readl(csi2-regs1 + CSI2_SYSCONFIG) | + CSI2_SYSCONFIG_SOFT_RESET, + csi2-regs1 + CSI2_SYSCONFIG); + + do { + reg = readl(csi2-regs1 + CSI2_SYSSTATUS) + CSI2_SYSSTATUS_RESET_DONE; + if (reg == CSI2_SYSSTATUS_RESET_DONE) + break; + soft_reset_retries++; + if (soft_reset_retries 5) + udelay(100); How about usleep_range() here? Or omit such a long busyloop. Hardware typically resets quite fast. I guess i'll try to fine-tune this then. I think it's fine to replace it with usleep_range() for now. Fine optimisations can be left for later if really needed. ... +static void csi2_print_status(struct iss_csi2_device *csi2) +{ + struct iss_device *iss = csi2-iss; + + if (!csi2-available) + return; + + dev_dbg(iss-dev, -CSI2 Register dump-\n); + + CSI2_PRINT_REGISTER(iss, csi2-regs1, SYSCONFIG); + CSI2_PRINT_REGISTER(iss, csi2-regs1, SYSSTATUS); + CSI2_PRINT_REGISTER(iss, csi2-regs1, IRQENABLE); + CSI2_PRINT_REGISTER(iss, csi2-regs1, IRQSTATUS); + CSI2_PRINT_REGISTER(iss, csi2-regs1, CTRL); + CSI2_PRINT_REGISTER(iss, csi2-regs1, DBG_H); + CSI2_PRINT_REGISTER(iss, csi2-regs1, COMPLEXIO_CFG); + CSI2_PRINT_REGISTER(iss, csi2-regs1, COMPLEXIO_IRQSTATUS); + CSI2_PRINT_REGISTER(iss, csi2-regs1, SHORT_PACKET); + CSI2_PRINT_REGISTER(iss, csi2-regs1, COMPLEXIO_IRQENABLE); + CSI2_PRINT_REGISTER(iss, csi2-regs1, DBG_P); + CSI2_PRINT_REGISTER(iss, csi2-regs1, TIMING); + CSI2_PRINT_REGISTER(iss, csi2-regs1, CTX_CTRL1(0)); + CSI2_PRINT_REGISTER(iss, csi2-regs1, CTX_CTRL2(0)); + CSI2_PRINT_REGISTER(iss, csi2-regs1, CTX_DAT_OFST(0)); + CSI2_PRINT_REGISTER(iss, csi2-regs1, CTX_PING_ADDR(0)); + CSI2_PRINT_REGISTER(iss, csi2-regs1, CTX_PONG_ADDR(0)); + CSI2_PRINT_REGISTER(iss, csi2-regs1, CTX_IRQENABLE(0)); + CSI2_PRINT_REGISTER(iss, csi2-regs1, CTX_IRQSTATUS(0)); + CSI2_PRINT_REGISTER(iss, csi2-regs1, CTX_CTRL3(0)); + + dev_dbg(iss-dev, \n); _If_ this is user-triggered, you might want to consider using debugfs for the same. Just FYI. Ok. I'll see what can I do. Just FYI. :-) Thinking about this agian, debugfs might not go well with this since the prints may be triggered by the driver. ... + /* Skip interrupts until we reach the frame skip count. The CSI2 will be + * automatically disabled, as the frame skip count has been programmed + * in the CSI2_CTx_CTRL1::COUNT field, so reenable it. + * + * It would have been nice to rely on the FRAME_NUMBER interrupt instead + * but it turned out that the interrupt is only generated when the CSI2 + * writes to memory (the CSI2_CTx_CTRL1::COUNT field is decreased + * correctly and reaches 0 when data is forwarded to the video port only + * but no interrupt arrives). Maybe a CSI2 hardware bug. + */ + if (csi2-frame_skip) { + csi2-frame_skip--; + if (csi2
Re: Video Capture Issue
Sriram, On Sun, Feb 26, 2012 at 8:54 AM, Sriram V vshrir...@gmail.com wrote: Hi, When I take the dump of the buffer which is pointed by DATA MEM PING ADDRESS. It always shows 0x55. Even if i write 0x00 to the address. I do notice that it quickly changes to 0x55. Under what conditions could this happen? What am i missing here. If you're using yavta for capture, notice that it clears out the buffers before queuing them in: static int video_queue_buffer(struct device *dev, int index, enum buffer_fill_mode fill) { struct v4l2_buffer buf; int ret; ... ... if (dev-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { ... } else { if (fill BUFFER_FILL_FRAME) memset(dev-buffers[buf.index].mem, 0x55, dev-buffers[index].size); if (fill BUFFER_FILL_PADDING) memset(dev-buffers[buf.index].mem + dev-buffers[index].size, 0x55, dev-buffers[index].padding); } ... } So, just make sure this condition is not met. I do notice that the OMAP4 ISS is tested to work with OV5640 (YUV422 Frames) and OV5650 (Raw Data) When you say 422 Frames only. Do you mean 422-8Bit Mode?. Yes. When saving YUV422 to memory, you can only use this mode AFAIK. I havent tried RAW12 which my device gives, Do i have to update only the Data Format Selection register of the ISS for RAW12? Ok, now it makes sense. So, if your CSI2 source is giving, you need to make sure: CSI2_CTX_CTRL2_0.FORMAT[9:0] is: - 0xAC: RAW12 + EXP16 or - 0x2C: RAW12 The difference is that the EXP16 variant, will save to memory in expansion to 2 bytes, instead of 12 bits, so it'll be byte aligned. Can you try attached patch? Regards, Sergio Please advice. On Thu, Feb 23, 2012 at 11:24 PM, Sriram V vshrir...@gmail.com wrote: Hi, 1) An Hexdump of the captured file shows 0x55 at all locations. Is there any buffer location i need to check. 2) I have tried with devel branch. 3) Changing the polarities doesnt help either. 4) The sensor is giving out YUV422 8Bit Mode, Will 0x52001074 = 0x0A1E (UYVY Format) it bypass the ISP and dump directly into memory. On 2/23/12, Aguirre, Sergio saagui...@ti.com wrote: Hi Sriram, On Thu, Feb 23, 2012 at 11:25 AM, Sriram V vshrir...@gmail.com wrote: Hi, 1) I am trying to get a HDMI to CSI Bridge chip working with OMAP4 ISS. The issue is the captured frames are completely green in color. Sounds like the buffer is all zeroes, can you confirm? 2) The Chip is configured to output VGA Color bar sequence with YUV422-8Bit and uses datalane 0 only. 3) The Format on OMAP4 ISS is UYVY (Register 0x52001074 = 0x0A1E) I am trying to directly dump the data into memory without ISP processing. Please advice. Just to be clear on your environment, which branch/commitID are you based on? Regards, Sergio -- Regards, Sriram -- 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 -- Regards, Sriram -- Regards, Sriram diff --git a/drivers/media/video/omap4iss/iss_csi2.c b/drivers/media/video/omap4iss/iss_csi2.c index 04985a0..0e8a069 100644 --- a/drivers/media/video/omap4iss/iss_csi2.c +++ b/drivers/media/video/omap4iss/iss_csi2.c @@ -106,6 +106,10 @@ static const unsigned int csi2_input_fmts[] = { V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SRGGB8_1X8, + V4L2_MBUS_FMT_SBGGR12_1X12, + V4L2_MBUS_FMT_SGBRG12_1X12, + V4L2_MBUS_FMT_SGRBG12_1X12, + V4L2_MBUS_FMT_SRGGB12_1X12, V4L2_MBUS_FMT_UYVY8_1X16, V4L2_MBUS_FMT_YUYV8_1X16, }; @@ -171,6 +175,23 @@ static const u16 __csi2_fmt_map[][2][2] = { 0, }, }, + /* RAW12 formats */ + { + /* Output to memory */ + { + /* No DPCM decompression */ + CSI2_PIX_FMT_RAW12_EXP16, + /* DPCM decompression */ + 0, + }, + /* Output to both */ + { + /* No DPCM decompression */ + CSI2_PIX_FMT_RAW12_VP, + /* DPCM decompression */ + 0, + }, + }, /* YUV422 formats */ { /* Output to memory */ @@ -220,9 +241,15 @@ static u16 csi2_ctx_map_format(struct iss_csi2_device *csi2) case V4L2_MBUS_FMT_SRGGB8_1X8: fmtidx = 2; break; + case V4L2_MBUS_FMT_SBGGR12_1X12: + case V4L2_MBUS_FMT_SGBRG12_1X12: + case V4L2_MBUS_FMT_SGRBG12_1X12: + case V4L2_MBUS_FMT_SRGGB12_1X12: + fmtidx = 3; + break; case V4L2_MBUS_FMT_UYVY8_1X16: case V4L2_MBUS_FMT_YUYV8_1X16: - fmtidx = 3; + fmtidx = 4; break; default: WARN(1, KERN_ERR CSI2: pixel format %08x unsupported!\n, diff --git a/drivers/media/video/omap4iss/iss_csi2.h b/drivers/media/video/omap4iss/iss_csi2.h index aa81971..beed331 100644 --- a/drivers/media/video/omap4iss/iss_csi2.h +++ b/drivers/media/video/omap4iss/iss_csi2.h @@ -32,6 +32,8 @@ enum iss_csi2_pix_formats
Re: Video Capture Issue
Hi Sriram, On Thu, Feb 23, 2012 at 11:25 AM, Sriram V vshrir...@gmail.com wrote: Hi, 1) I am trying to get a HDMI to CSI Bridge chip working with OMAP4 ISS. The issue is the captured frames are completely green in color. Sounds like the buffer is all zeroes, can you confirm? 2) The Chip is configured to output VGA Color bar sequence with YUV422-8Bit and uses datalane 0 only. 3) The Format on OMAP4 ISS is UYVY (Register 0x52001074 = 0x0A1E) I am trying to directly dump the data into memory without ISP processing. Please advice. Just to be clear on your environment, which branch/commitID are you based on? Regards, Sergio -- Regards, Sriram -- 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: omap4 v4l media-ctl usage
H Ryan, On Sun, Feb 19, 2012 at 10:04 AM, Ryan ryanphilip...@googlemail.com wrote: Hi Laurent, It actually blocks at VIDIOC_DQBUF ioctl and not in STREAMON ioctl. Looks like the data is not available. Any way of verifying if OMAP is receiving data. Wonder why i keep interrupts though? Can you try again after cherry-picking these 3 commits?: 7ea3af1 ov5640: Move initial config to registering callback dbe85fd ov5640: Fix s_fmt behaviour 522e30a panda: camera: Fix ddr_freq for OV5640 selection I'll suggest you to try keeping on latest devel branch head, since there's many fixes and cleanups I have been pushing in the tree. Regards, Sergio Is my media-ctl setting up things correctly? Thank you. Regards, Ryan On Sun, Feb 19, 2012 at 7:47 PM, Ryan ryanphilip...@googlemail.com wrote: Hi Laurent, On Sun, Feb 19, 2012 at 2:27 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Ryan, On Saturday 18 February 2012 20:59:57 Ryan wrote: hello, I am using media-ctl on the panda board. The sensor gets detected. But media-ctl doesnt print anything. The kernel is cloned from omap4 v4l git tree: commit id: 3bc023462a68f78bb0273848f5ab08a01b434ffa what could be wrong in here? ~ # ./media-ctl -p Opening media device /dev/media0 Enumerating entities Found 0 entities Enumerating pads and links Device topology What steps i need to follow get output from sensor in terms of arguments to media-ctl and yavta. Could you please first make sure that media-ctl has be compiled against header files from the kernel running on your board ? Thank you, That did the trick, Now i am able to run media-ctl. I am trying to capture YUV422. When i run yavta, It blocks infinitely at STREAM ON, I see that the ISS interrupts keeps incrementing. Am i missing something? #yavta /dev/video0 -n1 -c5 -s640x480 -fUYVY -Ftest.yuv Device /dev/video0 opened: OMAP4 ISS CSI2a output (media). Video format set: width: 640 height: 480 buffer size: 614400 Video format: UYVY (59565955) 640x480 1 buffers requested. length: 614400 offset: 0 Buffer 0 mapped at address 0x4028e000. [ 577.605590] Enable STREAM ON.. After configuring using the media-ctl, This is how my device topology looks like: Is this okay? Device topology - entity 1: OMAP4 ISS CSI2a (2 pads, 2 links) type V4L2 subdev subtype Unknown device node name /dev/v4l-subdev0 pad0: Sink [UYVY 640x480] - ov5640 3-0036:0 [ENABLED,IMMUTABLE] pad1: Source [UYVY 640x480] - OMAP4 ISS CSI2a output:0 [ENABLED] - entity 2: OMAP4 ISS CSI2a output (1 pad, 1 link) type Node subtype V4L device node name /dev/video0 pad0: Sink - OMAP4 ISS CSI2a:1 [ENABLED] - entity 3: ov5640 3-0036 (1 pad, 1 link) type V4L2 subdev subtype Unknown device node name /dev/v4l-subdev1 pad0: Source [UYVY 640x480] - OMAP4 ISS CSI2a:0 [ENABLED,IMMUTABLE] Thank you, Regards, Ryan -- 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
Re: [PATCH v2 27/31] omap3isp: Configure CSI-2 phy based on platform data
Hi Sakari, On Fri, Feb 10, 2012 at 2:32 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Hi Sergio, Thanks for the review! Aguirre, Sergio wrote: On Thu, Feb 2, 2012 at 5:54 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Configure CSI-2 phy based on platform data in the ISP driver. For that, the new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same was configured from the board code. Signed-off-by: Sakari Ailus sakari.ai...@iki.fi --- drivers/media/video/omap3isp/ispcsi2.c | 10 +++- drivers/media/video/omap3isp/ispcsiphy.c | 78 ++ drivers/media/video/omap3isp/ispcsiphy.h | 2 + 3 files changed, 78 insertions(+), 12 deletions(-) diff --git a/drivers/media/video/omap3isp/ispcsi2.c b/drivers/media/video/omap3isp/ispcsi2.c index 9313f7c..e2e3d63 100644 --- a/drivers/media/video/omap3isp/ispcsi2.c +++ b/drivers/media/video/omap3isp/ispcsi2.c @@ -1068,7 +1068,13 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int enable) struct isp_video *video_out = csi2-video_out; switch (enable) { - case ISP_PIPELINE_STREAM_CONTINUOUS: + case ISP_PIPELINE_STREAM_CONTINUOUS: { + int ret; + + ret = omap3isp_csiphy_config(isp, sd); + if (ret 0) + return ret; + if (omap3isp_csiphy_acquire(csi2-phy) 0) return -ENODEV; csi2-use_fs_irq = pipe-do_propagation; @@ -1092,7 +1098,7 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int enable) csi2_if_enable(isp, csi2, 1); isp_video_dmaqueue_flags_clr(video_out); break; - + } case ISP_PIPELINE_STREAM_STOPPED: if (csi2-state == ISP_PIPELINE_STREAM_STOPPED) return 0; diff --git a/drivers/media/video/omap3isp/ispcsiphy.c b/drivers/media/video/omap3isp/ispcsiphy.c index 5be37ce..5d7a6ab 100644 --- a/drivers/media/video/omap3isp/ispcsiphy.c +++ b/drivers/media/video/omap3isp/ispcsiphy.c @@ -28,6 +28,8 @@ #include linux/device.h #include linux/regulator/consumer.h +#include ../../../../arch/arm/mach-omap2/control.h + #include isp.h #include ispreg.h #include ispcsiphy.h @@ -138,15 +140,73 @@ static void csiphy_dphy_config(struct isp_csiphy *phy) isp_reg_writel(phy-isp, reg, phy-phy_regs, ISPCSIPHY_REG1); } -static int csiphy_config(struct isp_csiphy *phy, - struct isp_csiphy_dphy_cfg *dphy, - struct isp_csiphy_lanes_cfg *lanes) +/* + * TCLK values are OK at their reset values + */ +#define TCLK_TERM 0 +#define TCLK_MISS 1 +#define TCLK_SETTLE 14 + +int omap3isp_csiphy_config(struct isp_device *isp, + struct v4l2_subdev *csi2_subdev) { + struct isp_csi2_device *csi2 = v4l2_get_subdevdata(csi2_subdev); + struct isp_pipeline *pipe = to_isp_pipeline(csi2_subdev-entity); + struct isp_v4l2_subdevs_group *subdevs = pipe-external-host_priv; + struct isp_csiphy_dphy_cfg csi2phy; + int csi2_ddrclk_khz; + struct isp_csiphy_lanes_cfg *lanes; unsigned int used_lanes = 0; unsigned int i; + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY1 + || subdevs-interface == ISP_INTERFACE_CCP2B_PHY2) + lanes = subdevs-bus.ccp2.lanecfg; + else + lanes = subdevs-bus.csi2.lanecfg; + + /* FIXME: Do 34xx / 35xx require something here? */ + if (cpu_is_omap3630()) { + u32 cam_phy_ctrl = omap_readl( + OMAP343X_CTRL_BASE + OMAP3630_CONTROL_CAMERA_PHY_CTRL); How about: u32 cam_phy_ctrl = omap_ctrl_readl(OMAP3630_CONTROL_CAMERA_PHY_CTRL); ? Fine for me. + + /* + * SCM.CONTROL_CAMERA_PHY_CTRL + * - bit[4] : CSIPHY1 data sent to CSIB + * - bit [3:2] : CSIPHY1 config: 00 d-phy, 01/10 ccp2 + * - bit [1:0] : CSIPHY2 config: 00 d-phy, 01/10 ccp2 + */ + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY1) + cam_phy_ctrl |= 1 2; + else if (subdevs-interface == ISP_INTERFACE_CSI2C_PHY1) + cam_phy_ctrl = 1 2; Shouldn't this be: cam_phy_ctrl = ~(1 2); ? Probably yes. This has come directly as it was in the original board code and might not be entirely correct. It works on the N9 at least. + + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY2) + cam_phy_ctrl |= 1; + else if (subdevs-interface == ISP_INTERFACE_CSI2A_PHY2) + cam_phy_ctrl = 1; ... and: cam_phy_ctrl = ~1; + + omap_writel(cam_phy_ctrl, + OMAP343X_CTRL_BASE
Re: [PATCH v2 27/31] omap3isp: Configure CSI-2 phy based on platform data
On Sat, Feb 11, 2012 at 11:17 AM, Aguirre, Sergio saagui...@ti.com wrote: Hi Sakari, On Fri, Feb 10, 2012 at 2:32 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Hi Sergio, Thanks for the review! Aguirre, Sergio wrote: On Thu, Feb 2, 2012 at 5:54 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Configure CSI-2 phy based on platform data in the ISP driver. For that, the new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same was configured from the board code. Signed-off-by: Sakari Ailus sakari.ai...@iki.fi --- drivers/media/video/omap3isp/ispcsi2.c | 10 +++- drivers/media/video/omap3isp/ispcsiphy.c | 78 ++ drivers/media/video/omap3isp/ispcsiphy.h | 2 + 3 files changed, 78 insertions(+), 12 deletions(-) diff --git a/drivers/media/video/omap3isp/ispcsi2.c b/drivers/media/video/omap3isp/ispcsi2.c index 9313f7c..e2e3d63 100644 --- a/drivers/media/video/omap3isp/ispcsi2.c +++ b/drivers/media/video/omap3isp/ispcsi2.c @@ -1068,7 +1068,13 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int enable) struct isp_video *video_out = csi2-video_out; switch (enable) { - case ISP_PIPELINE_STREAM_CONTINUOUS: + case ISP_PIPELINE_STREAM_CONTINUOUS: { + int ret; + + ret = omap3isp_csiphy_config(isp, sd); + if (ret 0) + return ret; + if (omap3isp_csiphy_acquire(csi2-phy) 0) return -ENODEV; csi2-use_fs_irq = pipe-do_propagation; @@ -1092,7 +1098,7 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int enable) csi2_if_enable(isp, csi2, 1); isp_video_dmaqueue_flags_clr(video_out); break; - + } case ISP_PIPELINE_STREAM_STOPPED: if (csi2-state == ISP_PIPELINE_STREAM_STOPPED) return 0; diff --git a/drivers/media/video/omap3isp/ispcsiphy.c b/drivers/media/video/omap3isp/ispcsiphy.c index 5be37ce..5d7a6ab 100644 --- a/drivers/media/video/omap3isp/ispcsiphy.c +++ b/drivers/media/video/omap3isp/ispcsiphy.c @@ -28,6 +28,8 @@ #include linux/device.h #include linux/regulator/consumer.h +#include ../../../../arch/arm/mach-omap2/control.h + #include isp.h #include ispreg.h #include ispcsiphy.h @@ -138,15 +140,73 @@ static void csiphy_dphy_config(struct isp_csiphy *phy) isp_reg_writel(phy-isp, reg, phy-phy_regs, ISPCSIPHY_REG1); } -static int csiphy_config(struct isp_csiphy *phy, - struct isp_csiphy_dphy_cfg *dphy, - struct isp_csiphy_lanes_cfg *lanes) +/* + * TCLK values are OK at their reset values + */ +#define TCLK_TERM 0 +#define TCLK_MISS 1 +#define TCLK_SETTLE 14 + +int omap3isp_csiphy_config(struct isp_device *isp, + struct v4l2_subdev *csi2_subdev) { + struct isp_csi2_device *csi2 = v4l2_get_subdevdata(csi2_subdev); + struct isp_pipeline *pipe = to_isp_pipeline(csi2_subdev-entity); + struct isp_v4l2_subdevs_group *subdevs = pipe-external-host_priv; + struct isp_csiphy_dphy_cfg csi2phy; + int csi2_ddrclk_khz; + struct isp_csiphy_lanes_cfg *lanes; unsigned int used_lanes = 0; unsigned int i; + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY1 + || subdevs-interface == ISP_INTERFACE_CCP2B_PHY2) + lanes = subdevs-bus.ccp2.lanecfg; + else + lanes = subdevs-bus.csi2.lanecfg; + + /* FIXME: Do 34xx / 35xx require something here? */ + if (cpu_is_omap3630()) { + u32 cam_phy_ctrl = omap_readl( + OMAP343X_CTRL_BASE + OMAP3630_CONTROL_CAMERA_PHY_CTRL); How about: u32 cam_phy_ctrl = omap_ctrl_readl(OMAP3630_CONTROL_CAMERA_PHY_CTRL); ? Fine for me. + + /* + * SCM.CONTROL_CAMERA_PHY_CTRL + * - bit[4] : CSIPHY1 data sent to CSIB + * - bit [3:2] : CSIPHY1 config: 00 d-phy, 01/10 ccp2 + * - bit [1:0] : CSIPHY2 config: 00 d-phy, 01/10 ccp2 + */ + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY1) + cam_phy_ctrl |= 1 2; + else if (subdevs-interface == ISP_INTERFACE_CSI2C_PHY1) + cam_phy_ctrl = 1 2; Shouldn't this be: cam_phy_ctrl = ~(1 2); ? Probably yes. This has come directly as it was in the original board code and might not be entirely correct. It works on the N9 at least. + + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY2) + cam_phy_ctrl |= 1; + else if (subdevs-interface == ISP_INTERFACE_CSI2A_PHY2) + cam_phy_ctrl = 1; ... and: cam_phy_ctrl = ~1
Re: [PATCH v2 27/31] omap3isp: Configure CSI-2 phy based on platform data
Hi Sakari, On Thu, Feb 2, 2012 at 5:54 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Configure CSI-2 phy based on platform data in the ISP driver. For that, the new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same was configured from the board code. Signed-off-by: Sakari Ailus sakari.ai...@iki.fi --- drivers/media/video/omap3isp/ispcsi2.c | 10 +++- drivers/media/video/omap3isp/ispcsiphy.c | 78 ++ drivers/media/video/omap3isp/ispcsiphy.h | 2 + 3 files changed, 78 insertions(+), 12 deletions(-) diff --git a/drivers/media/video/omap3isp/ispcsi2.c b/drivers/media/video/omap3isp/ispcsi2.c index 9313f7c..e2e3d63 100644 --- a/drivers/media/video/omap3isp/ispcsi2.c +++ b/drivers/media/video/omap3isp/ispcsi2.c @@ -1068,7 +1068,13 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int enable) struct isp_video *video_out = csi2-video_out; switch (enable) { - case ISP_PIPELINE_STREAM_CONTINUOUS: + case ISP_PIPELINE_STREAM_CONTINUOUS: { + int ret; + + ret = omap3isp_csiphy_config(isp, sd); + if (ret 0) + return ret; + if (omap3isp_csiphy_acquire(csi2-phy) 0) return -ENODEV; csi2-use_fs_irq = pipe-do_propagation; @@ -1092,7 +1098,7 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int enable) csi2_if_enable(isp, csi2, 1); isp_video_dmaqueue_flags_clr(video_out); break; - + } case ISP_PIPELINE_STREAM_STOPPED: if (csi2-state == ISP_PIPELINE_STREAM_STOPPED) return 0; diff --git a/drivers/media/video/omap3isp/ispcsiphy.c b/drivers/media/video/omap3isp/ispcsiphy.c index 5be37ce..5d7a6ab 100644 --- a/drivers/media/video/omap3isp/ispcsiphy.c +++ b/drivers/media/video/omap3isp/ispcsiphy.c @@ -28,6 +28,8 @@ #include linux/device.h #include linux/regulator/consumer.h +#include ../../../../arch/arm/mach-omap2/control.h + #include isp.h #include ispreg.h #include ispcsiphy.h @@ -138,15 +140,73 @@ static void csiphy_dphy_config(struct isp_csiphy *phy) isp_reg_writel(phy-isp, reg, phy-phy_regs, ISPCSIPHY_REG1); } -static int csiphy_config(struct isp_csiphy *phy, - struct isp_csiphy_dphy_cfg *dphy, - struct isp_csiphy_lanes_cfg *lanes) +/* + * TCLK values are OK at their reset values + */ +#define TCLK_TERM 0 +#define TCLK_MISS 1 +#define TCLK_SETTLE 14 + +int omap3isp_csiphy_config(struct isp_device *isp, + struct v4l2_subdev *csi2_subdev) { + struct isp_csi2_device *csi2 = v4l2_get_subdevdata(csi2_subdev); + struct isp_pipeline *pipe = to_isp_pipeline(csi2_subdev-entity); + struct isp_v4l2_subdevs_group *subdevs = pipe-external-host_priv; + struct isp_csiphy_dphy_cfg csi2phy; + int csi2_ddrclk_khz; + struct isp_csiphy_lanes_cfg *lanes; unsigned int used_lanes = 0; unsigned int i; + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY1 + || subdevs-interface == ISP_INTERFACE_CCP2B_PHY2) + lanes = subdevs-bus.ccp2.lanecfg; + else + lanes = subdevs-bus.csi2.lanecfg; + + /* FIXME: Do 34xx / 35xx require something here? */ + if (cpu_is_omap3630()) { + u32 cam_phy_ctrl = omap_readl( + OMAP343X_CTRL_BASE + OMAP3630_CONTROL_CAMERA_PHY_CTRL); How about: u32 cam_phy_ctrl = omap_ctrl_readl(OMAP3630_CONTROL_CAMERA_PHY_CTRL); ? + + /* + * SCM.CONTROL_CAMERA_PHY_CTRL + * - bit[4] : CSIPHY1 data sent to CSIB + * - bit [3:2] : CSIPHY1 config: 00 d-phy, 01/10 ccp2 + * - bit [1:0] : CSIPHY2 config: 00 d-phy, 01/10 ccp2 + */ + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY1) + cam_phy_ctrl |= 1 2; + else if (subdevs-interface == ISP_INTERFACE_CSI2C_PHY1) + cam_phy_ctrl = 1 2; Shouldn't this be: cam_phy_ctrl = ~(1 2); ? + + if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY2) + cam_phy_ctrl |= 1; + else if (subdevs-interface == ISP_INTERFACE_CSI2A_PHY2) + cam_phy_ctrl = 1; ... and: cam_phy_ctrl = ~1; + + omap_writel(cam_phy_ctrl, + OMAP343X_CTRL_BASE + + OMAP3630_CONTROL_CAMERA_PHY_CTRL); Again: omap_ctrl_writel(cam_phy_ctrl, OMAP3630_CONTROL_CAMERA_PHY_CTRL); + } + + csi2_ddrclk_khz = pipe-external_rate / 1000 + / (2 * csi2-phy-num_data_lanes) + *
Re: [PATCH v2 03/11] v4l: Introduce sensor operation for getting interface configuration
Hi Stan, On Fri, Dec 2, 2011 at 7:32 AM, Stanimir Varbanov svarba...@mm-sol.com wrote: Hi, Sergio This change in interface is not used from the omap4iss driver. You could drop it from the patch set, if so. Oops, yes... I used to depend on this for my soc_camera implementation before... You're absolutely right, i'll remove it in the next iteration. Merci! On 12/01/2011 02:14 AM, Sergio Aguirre wrote: From: Stanimir Varbanov svarba...@mm-sol.com Introduce g_interface_parms sensor operation for getting sensor interface parameters. These parameters are needed from the host side to determine it's own configuration. Signed-off-by: Stanimir Varbanov svarba...@mm-sol.com --- include/media/v4l2-subdev.h | 42 ++ 1 files changed, 42 insertions(+), 0 deletions(-) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index f0f3358..0d322ed 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -362,6 +362,42 @@ struct v4l2_subdev_vbi_ops { int (*s_sliced_fmt)(struct v4l2_subdev *sd, struct v4l2_sliced_vbi_format *fmt); }; +/* Which interface the sensor use to provide it's image data */ +enum v4l2_subdev_sensor_iface { + V4L2_SUBDEV_SENSOR_PARALLEL, + V4L2_SUBDEV_SENSOR_SERIAL, +}; + +/* Each interface could use the following modes */ +/* Image sensor provides horizontal and vertical sync signals */ +#define V4L2_SUBDEV_SENSOR_MODE_PARALLEL_SYNC 0 +/* BT.656 interface. Embedded sync */ +#define V4L2_SUBDEV_SENSOR_MODE_PARALLEL_ITU 1 +/* MIPI CSI1 */ +#define V4L2_SUBDEV_SENSOR_MODE_SERIAL_CSI1 2 +/* MIPI CSI2 */ +#define V4L2_SUBDEV_SENSOR_MODE_SERIAL_CSI2 3 + +struct v4l2_subdev_sensor_serial_parms { + unsigned char lanes; /* number of lanes used */ + unsigned char channel; /* virtual channel */ + unsigned int phy_rate; /* output rate at CSI phy in bps */ + unsigned int pix_clk; /* pixel clock in Hz */ +}; + +struct v4l2_subdev_sensor_parallel_parms { + unsigned int pix_clk; /* pixel clock in Hz */ +}; + +struct v4l2_subdev_sensor_interface_parms { + enum v4l2_subdev_sensor_iface if_type; + unsigned int if_mode; + union { + struct v4l2_subdev_sensor_serial_parms serial; + struct v4l2_subdev_sensor_parallel_parms parallel; + } parms; +}; + /** * struct v4l2_subdev_sensor_ops - v4l2-subdev sensor operations * @g_skip_top_lines: number of lines at the top of the image to be skipped. @@ -371,10 +407,16 @@ struct v4l2_subdev_vbi_ops { * @g_skip_frames: number of frames to skip at stream start. This is needed for * buggy sensors that generate faulty frames when they are * turned on. + * @g_interface_parms: get sensor interface parameters. The sensor subdev should + * fill this structure with current interface params. These + * interface parameters are needed on host side to configure + * it's own hardware receivers. */ struct v4l2_subdev_sensor_ops { int (*g_skip_top_lines)(struct v4l2_subdev *sd, u32 *lines); int (*g_skip_frames)(struct v4l2_subdev *sd, u32 *frames); + int (*g_interface_parms)(struct v4l2_subdev *sd, + struct v4l2_subdev_sensor_interface_parms *parms); }; /* -- best regards, Stan -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v1 2/7] omap4: build fdif omap device from hwmod
Hi Ming, Thanks for the patches. On Fri, Dec 2, 2011 at 9:02 AM, Ming Lei ming@canonical.com wrote: Signed-off-by: Ming Lei ming@canonical.com --- arch/arm/mach-omap2/devices.c | 33 + 1 files changed, 33 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c index 1166bdc..a392af5 100644 --- a/arch/arm/mach-omap2/devices.c +++ b/arch/arm/mach-omap2/devices.c @@ -728,6 +728,38 @@ void __init omap242x_init_mmc(struct omap_mmc_platform_data **mmc_data) #endif +static struct platform_device* __init omap4_init_fdif(void) +{ + int id = -1; You could remove this , as it is being used only once, and never changed. + struct platform_device *pd; + struct omap_hwmod *oh; + const char *dev_name = fdif; + + oh = omap_hwmod_lookup(fdif); + if (!oh) { + pr_err(Could not look up fdif hwmod\n); + return NULL; + } + + pd = omap_device_build(dev_name, id, oh, NULL, 0, NULL, 0, 0); Just do: pd = omap_device_build(dev_name, -1, oh, NULL, 0, NULL, 0, 0); + WARN(IS_ERR(pd), Can't build omap_device for %s.\n, + dev_name); + return pd; +} + +static void __init omap_init_fdif(void) +{ + if (cpu_is_omap44xx()) { + struct platform_device *pd; + + pd = omap4_init_fdif(); + if (!pd) + return; + + pm_runtime_enable(pd-dev); + } +} IMHO, you could reduce 1 level of indentation here, like this: static void __init omap_init_fdif(void) { struct platform_device *pd; if (!cpu_is_omap44xx()) return; pd = omap4_init_fdif(); if (!pd) return; pm_runtime_enable(pd-dev); } Regards, Sergio + /*-*/ #if defined(CONFIG_HDQ_MASTER_OMAP) || defined(CONFIG_HDQ_MASTER_OMAP_MODULE) @@ -808,6 +840,7 @@ static int __init omap2_init_devices(void) omap_init_sham(); omap_init_aes(); omap_init_vout(); + omap_init_fdif(); return 0; } -- 1.7.5.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 -- 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 04/11] OMAP4: hwmod: Include CSI2A and CSIPHY1 memory sections
Hi Kevin, Thanks for the review. On Fri, Dec 2, 2011 at 4:49 PM, Kevin Hilman khil...@ti.com wrote: +Benoit, Aguirre, Sergio saagui...@ti.com writes: Hi Vaibhav, Thanks for the comments. On Thu, Dec 1, 2011 at 12:34 AM, Hiremath, Vaibhav hvaib...@ti.com wrote: -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Aguirre, Sergio Sent: Thursday, December 01, 2011 5:45 AM To: linux-media@vger.kernel.org Cc: linux-o...@vger.kernel.org; laurent.pinch...@ideasonboard.com; sakari.ai...@iki.fi; Aguirre, Sergio Subject: [PATCH v2 04/11] OMAP4: hwmod: Include CSI2A and CSIPHY1 memory sections Signed-off-by: Sergio Aguirre saagui...@ti.com --- arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 16 +--- 1 files changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach- omap2/omap_hwmod_44xx_data.c index 7695e5d..1b59e2f 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -2623,8 +2623,18 @@ static struct omap_hwmod_ocp_if *omap44xx_iss_masters[] = { static struct omap_hwmod_addr_space omap44xx_iss_addrs[] = { { - .pa_start = 0x5200, - .pa_end = 0x52ff, + .pa_start = OMAP44XX_ISS_TOP_BASE, + .pa_end = OMAP44XX_ISS_TOP_END, + .flags = ADDR_TYPE_RT + }, + { + .pa_start = OMAP44XX_ISS_CSI2_A_REGS1_BASE, + .pa_end = OMAP44XX_ISS_CSI2_A_REGS1_END, + .flags = ADDR_TYPE_RT + }, + { + .pa_start = OMAP44XX_ISS_CAMERARX_CORE1_BASE, + .pa_end = OMAP44XX_ISS_CAMERARX_CORE1_END, .flags = ADDR_TYPE_RT }, This patch will result in build failure, because, the above base addresses are getting defined in the next patch [PATCH v2 05/11] OMAP4: Add base addresses for ISS Agreed. Will revisit git-bisectability of the patch series. Will fix. To fix this, just drop the #defines from the header, and use raw addresses directly. Ok, i'll drop patch #5 in this series. Also, work with Benoit to make sure at the scripts that autogenerate this data are updated to include these two regions. Ok. As a side note, I might need more addresses for the rest of the ISP components later on. I'll enable more subsystems once the CSI2-A only initial version is in an acceptable state. Regards, Sergio Kevin -- 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 05/11] OMAP4: Add base addresses for ISS
Hi Kevin, Thanks for the review. On Fri, Dec 2, 2011 at 4:45 PM, Kevin Hilman khil...@ti.com wrote: Sergio Aguirre saagui...@ti.com writes: NOTE: This isn't the whole list of features that the ISS supports, but the only ones supported at the moment. Signed-off-by: Sergio Aguirre saagui...@ti.com [...] diff --git a/arch/arm/plat-omap/include/plat/omap44xx.h b/arch/arm/plat-omap/include/plat/omap44xx.h index ea2b8a6..31432aa 100644 --- a/arch/arm/plat-omap/include/plat/omap44xx.h +++ b/arch/arm/plat-omap/include/plat/omap44xx.h @@ -49,6 +49,15 @@ #define OMAP44XX_MAILBOX_BASE (L4_44XX_BASE + 0xF4000) #define OMAP44XX_HSUSB_OTG_BASE (L4_44XX_BASE + 0xAB000) +#define OMAP44XX_ISS_BASE 0x5200 +#define OMAP44XX_ISS_TOP_BASE (OMAP44XX_ISS_BASE + 0x0) +#define OMAP44XX_ISS_CSI2_A_REGS1_BASE (OMAP44XX_ISS_BASE + 0x1000) +#define OMAP44XX_ISS_CAMERARX_CORE1_BASE (OMAP44XX_ISS_BASE + 0x1170) + +#define OMAP44XX_ISS_TOP_END (OMAP44XX_ISS_TOP_BASE + 256 - 1) +#define OMAP44XX_ISS_CSI2_A_REGS1_END (OMAP44XX_ISS_CSI2_A_REGS1_BASE + 368 - 1) +#define OMAP44XX_ISS_CAMERARX_CORE1_END (OMAP44XX_ISS_CAMERARX_CORE1_BASE + 32 - 1) + #define OMAP4_MMU1_BASE 0x55082000 #define OMAP4_MMU2_BASE 0x4A066000 Who are the users of thes address ranges? IMO, we shouldn't ned to add anymore based address definitions. These should be done in the hwmod data, and drivers get base addresses using the standard ways of getting resources (DT or platform_get_resource()) I see... I get your point now. Will remove them from this patch series then. Regards, Sergio Kevin -- 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 04/11] OMAP4: hwmod: Include CSI2A and CSIPHY1 memory sections
Hi Vaibhav, Thanks for the comments. On Thu, Dec 1, 2011 at 12:34 AM, Hiremath, Vaibhav hvaib...@ti.com wrote: -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Aguirre, Sergio Sent: Thursday, December 01, 2011 5:45 AM To: linux-media@vger.kernel.org Cc: linux-o...@vger.kernel.org; laurent.pinch...@ideasonboard.com; sakari.ai...@iki.fi; Aguirre, Sergio Subject: [PATCH v2 04/11] OMAP4: hwmod: Include CSI2A and CSIPHY1 memory sections Signed-off-by: Sergio Aguirre saagui...@ti.com --- arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 16 +--- 1 files changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach- omap2/omap_hwmod_44xx_data.c index 7695e5d..1b59e2f 100644 --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c @@ -2623,8 +2623,18 @@ static struct omap_hwmod_ocp_if *omap44xx_iss_masters[] = { static struct omap_hwmod_addr_space omap44xx_iss_addrs[] = { { - .pa_start = 0x5200, - .pa_end = 0x52ff, + .pa_start = OMAP44XX_ISS_TOP_BASE, + .pa_end = OMAP44XX_ISS_TOP_END, + .flags = ADDR_TYPE_RT + }, + { + .pa_start = OMAP44XX_ISS_CSI2_A_REGS1_BASE, + .pa_end = OMAP44XX_ISS_CSI2_A_REGS1_END, + .flags = ADDR_TYPE_RT + }, + { + .pa_start = OMAP44XX_ISS_CAMERARX_CORE1_BASE, + .pa_end = OMAP44XX_ISS_CAMERARX_CORE1_END, .flags = ADDR_TYPE_RT }, This patch will result in build failure, because, the above base addresses are getting defined in the next patch [PATCH v2 05/11] OMAP4: Add base addresses for ISS Agreed. Will revisit git-bisectability of the patch series. Will fix. Regards, Sergio Thanks, Vaibhav { } @@ -5350,7 +5360,7 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = { omap44xx_ipu_c1_hwmod, /* iss class */ -/* omap44xx_iss_hwmod, */ + omap44xx_iss_hwmod, /* iva class */ omap44xx_iva_hwmod, -- 1.7.7.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 -- 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 10/11] arm: omap4panda: Add support for omap4iss camera
Hi Vaibhav, Thanks for the comments. On Thu, Dec 1, 2011 at 12:24 AM, Hiremath, Vaibhav hvaib...@ti.com wrote: -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Aguirre, Sergio Sent: Thursday, December 01, 2011 5:45 AM To: linux-media@vger.kernel.org Cc: linux-o...@vger.kernel.org; laurent.pinch...@ideasonboard.com; sakari.ai...@iki.fi; Aguirre, Sergio Subject: [PATCH v2 10/11] arm: omap4panda: Add support for omap4iss camera This adds support for camera interface with the support for following sensors: - OV5640 - OV5650 Signed-off-by: Sergio Aguirre saagui...@ti.com --- arch/arm/mach-omap2/Kconfig | 27 arch/arm/mach-omap2/Makefile | 1 + arch/arm/mach-omap2/board-omap4panda-camera.c | 198 + 3 files changed, 226 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-omap2/board-omap4panda-camera.c diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index f883abb..0fc5ce9 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -358,6 +358,33 @@ config MACH_OMAP4_PANDA select OMAP_PACKAGE_CBS select REGULATOR_FIXED_VOLTAGE +config MACH_OMAP4_PANDA_CAMERA_SUPPORT + bool OMAP4 Panda Board Camera support + depends on MACH_OMAP4_PANDA + select MEDIA_SUPPORT + select MEDIA_CONTROLLER + select VIDEO_DEV + select VIDEO_V4L2_SUBDEV_API + select VIDEO_OMAP4 + help + Enable Camera HW support for PandaBoard. + This is for using the OMAP4 ISS CSI2A Camera sensor + interface. + +choice + prompt Camera sensor to use + depends on MACH_OMAP4_PANDA_CAMERA_SUPPORT + default MACH_OMAP4_PANDA_CAM_OV5650 + + config MACH_OMAP4_PANDA_CAM_OV5640 + bool Use OmniVision OV5640 Camera + select VIDEO_OV5640 + + config MACH_OMAP4_PANDA_CAM_OV5650 + bool Use OmniVision OV5650 Camera + select VIDEO_OV5650 +endchoice + config OMAP3_EMU bool OMAP3 debugging peripherals depends on ARCH_OMAP3 diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index 8bc446a..e80724d 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -236,6 +236,7 @@ obj-$(CONFIG_MACH_TI8168EVM) += board- ti8168evm.o # Platform specific device init code obj-$(CONFIG_MACH_OMAP_4430SDP_CAMERA_SUPPORT) += board-4430sdp-camera.o +obj-$(CONFIG_MACH_OMAP4_PANDA_CAMERA_SUPPORT) += board-omap4panda- camera.o Can't this be merged into single file? Do we really need separate file for every board here? I am sure you would have thought about this. :) Well, not really, to be honest. I could have a single file, but wouldn't that make it more complex? I mean, every board has it's own HW connection specs, like sensor MCLK source, and different power up sequences. How would you probe for all, without messing with an invalid GPIO on that board? Have you resolved a similar case for other OMAP/Davinci boards? omap-flash-$(CONFIG_MTD_NAND_OMAP2) := board-flash.o omap-flash-$(CONFIG_MTD_ONENAND_OMAP2) := board-flash.o diff --git a/arch/arm/mach-omap2/board-omap4panda-camera.c b/arch/arm/mach-omap2/board-omap4panda-camera.c new file mode 100644 index 000..02ef36e --- /dev/null +++ b/arch/arm/mach-omap2/board-omap4panda-camera.c @@ -0,0 +1,198 @@ +#include linux/gpio.h +#include linux/clk.h +#include linux/delay.h + +#include plat/i2c.h +#include plat/omap-pm.h + +#include asm/mach-types.h + +#include media/ov5640.h +#include media/ov5650.h + +#include devices.h +#include ../../../drivers/media/video/omap4iss/iss.h + I believe this is not good practice to include files directly from drivers folder. You should divide the header file such that, driver specific information stays in driver/... and platform specific goes into include/... Right. Will fix. +#include control.h +#include mux.h + +#define PANDA_GPIO_CAM_PWRDN 45 +#define PANDA_GPIO_CAM_RESET 83 + +static struct clk *panda_cam_aux_clk; + +static int panda_ov5640_power(struct v4l2_subdev *subdev, int on) +{ + struct iss_device *iss = v4l2_dev_to_iss_device(subdev-v4l2_dev); + int ret = 0; You are not using this variable at all, you can get rid of this. Right. Will fix. + struct iss_csiphy_dphy_cfg dphy; + struct iss_csiphy_lanes_cfg lanes; + unsigned int ddr_freq = 480; /* FIXME: Do an actual query for this */ + + memset(lanes, 0, sizeof(lanes)); + memset(dphy, 0, sizeof(dphy)); + + lanes.clk.pos = 1; + lanes.clk.pol = 0; + lanes.data[0].pos = 2; + lanes.data[0].pol = 0; + lanes.data[1].pos = 3; + lanes.data[1].pol = 0; + + dphy.ths_term = 12500 * ddr_freq + 100) / 100) - 1) 0xFF
Re: [PATCH v2 05/11] OMAP4: Add base addresses for ISS
Hi Laurent, Thanks for the review. On Thu, Dec 1, 2011 at 11:24 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Sergio, On Thursday 01 December 2011 01:14:54 Sergio Aguirre wrote: NOTE: This isn't the whole list of features that the ISS supports, but the only ones supported at the moment. Signed-off-by: Sergio Aguirre saagui...@ti.com --- arch/arm/mach-omap2/devices.c | 32 arch/arm/plat-omap/include/plat/omap44xx.h | 9 +++ 2 files changed, 41 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c index c15cfad..b48aeea 100644 --- a/arch/arm/mach-omap2/devices.c +++ b/arch/arm/mach-omap2/devices.c @@ -32,6 +32,7 @@ #include plat/omap_hwmod.h #include plat/omap_device.h #include plat/omap4-keypad.h +#include plat/omap4-iss.h I try to keep headers sorted alphabetically when possible, but that might just be me. No problem. I can change it. #include mux.h #include control.h @@ -217,6 +218,37 @@ int omap3_init_camera(struct isp_platform_data *pdata) return platform_device_register(omap3isp_device); } +int omap4_init_camera(struct iss_platform_data *pdata, struct omap_board_data *bdata) +{ + struct platform_device *pdev; + struct omap_hwmod *oh; + struct iss_platform_data *omap4iss_pdata; + char *oh_name = iss; + char *name = omap4iss; Would const char or static const char help the compiler putting the strings to a read-only section ? Right. Will fix. + unsigned int id = -1; + + oh = omap_hwmod_lookup(oh_name); + if (!oh) { + pr_err(Could not look up %s\n, oh_name); + return -ENODEV; + } + + omap4iss_pdata = pdata; + + pdev = omap_device_build(name, id, oh, omap4iss_pdata, + sizeof(struct iss_platform_data), NULL, 0, 0); This is the only location where id is used, maybe you could pass -1 directly to the function ? Ditto. Thanks and regards, Sergio + + if (IS_ERR(pdev)) { + WARN(1, Can't build omap_device for %s:%s.\n, + name, oh-name); + return PTR_ERR(pdev); + } + + oh-mux = omap_hwmod_mux_init(bdata-pads, bdata-pads_cnt); + + return 0; +} + static inline void omap_init_camera(void) { #if defined(CONFIG_VIDEO_OMAP2) || defined(CONFIG_VIDEO_OMAP2_MODULE) diff --git a/arch/arm/plat-omap/include/plat/omap44xx.h b/arch/arm/plat-omap/include/plat/omap44xx.h index ea2b8a6..31432aa 100644 --- a/arch/arm/plat-omap/include/plat/omap44xx.h +++ b/arch/arm/plat-omap/include/plat/omap44xx.h @@ -49,6 +49,15 @@ #define OMAP44XX_MAILBOX_BASE (L4_44XX_BASE + 0xF4000) #define OMAP44XX_HSUSB_OTG_BASE (L4_44XX_BASE + 0xAB000) +#define OMAP44XX_ISS_BASE 0x5200 +#define OMAP44XX_ISS_TOP_BASE (OMAP44XX_ISS_BASE + 0x0) +#define OMAP44XX_ISS_CSI2_A_REGS1_BASE (OMAP44XX_ISS_BASE + 0x1000) +#define OMAP44XX_ISS_CAMERARX_CORE1_BASE (OMAP44XX_ISS_BASE + 0x1170) + +#define OMAP44XX_ISS_TOP_END (OMAP44XX_ISS_TOP_BASE + 256 - 1) +#define OMAP44XX_ISS_CSI2_A_REGS1_END (OMAP44XX_ISS_CSI2_A_REGS1_BASE + 368 - 1) +#define OMAP44XX_ISS_CAMERARX_CORE1_END (OMAP44XX_ISS_CAMERARX_CORE1_BASE + 32 - 1) + #define OMAP4_MMU1_BASE 0x55082000 #define OMAP4_MMU2_BASE 0x4A066000 -- 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 02/11] mfd: twl6040: Fix wrong TWL6040_GPO3 bitfield value
Hi Laurent, Thanks for the review. On Thu, Dec 1, 2011 at 11:24 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Sergio, On Thursday 01 December 2011 01:14:51 Sergio Aguirre wrote: The define should be the result of 1 Bit number. Bit number for GPOCTL.GPO3 field is 2, which results in 0x4 value. Signed-off-by: Sergio Aguirre saagui...@ti.com --- include/linux/mfd/twl6040.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h index 2463c261..2a7ff16 100644 --- a/include/linux/mfd/twl6040.h +++ b/include/linux/mfd/twl6040.h @@ -142,7 +142,7 @@ #define TWL6040_GPO1 0x01 #define TWL6040_GPO2 0x02 -#define TWL6040_GPO3 0x03 +#define TWL6040_GPO3 0x04 What about defining the fields as (1 x) instead then ? I thought about that, but I guess I just wanted to keep it consistent with the rest of the file. Maybe I can create a separate patch for changing all these bitwise flags to use BIT() macros instead. Thanks and Regards, Sergio /* ACCCTL (0x2D) fields */ -- 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 00/11] v4l2: OMAP4 ISS driver + Sensor + Board support
Hi Sakari, On Thu, Dec 1, 2011 at 10:14 AM, Sakari Ailus sakari.ai...@iki.fi wrote: Hi Sergio, Thanks for the patchset!! And thanks for your attention :) On Wed, Nov 30, 2011 at 06:14:49PM -0600, Sergio Aguirre wrote: Hi everyone, This is the second version of the OMAP4 ISS driver, now ported to the Media Controller framework AND supporting videobuf2 framework. This patchset should apply cleanly on top of v3.2-rc3 kernel tag. This driver attempts to provide an fully open source solution to control the OMAP4 Imaging SubSystem (a.k.a. ISS). Starts with just CSI2-A interface support, and pretends to be ready for expansion to add support to the many ISS block modules as possible. Please see newly added documentation for more details: Documentation/video4linux/omap4_camera.txt I propose s/omap4_camera/omap4iss/, according to the path name in the drivers/media/video directory. Makes sense. Will fix. Any comments/complaints are welcome. :) Changes since v1: - Simplification of auxclk handling in board files. (Pointed out by: Roger Quadros) - Cleanup of Camera support enablement for 4430sdp panda. (Pointed out by: Roger Quadros) - Use of HWMOD declaration for assisted platform_device creation. (Pointed out by: Felipe Balbi) - Videobuf2 migration (Removal of custom iss_queue buffer handling driver) I'm happy to see it's using videobuf2! Yeah, I'll definitely need it for multi-planar buffer handling for NV12 buffer capturing. Resizer can color convert from YUV422-YUV420 NV12 now, and expects 2 pointers (1 for Y, and 1 for UV 2x2 sampled) to be programmed in HW. I have no other comments quite yet. :-) Ok, let me know if you find something eye-popping ugly in there. I'll be happy to fix it. :) Thanks and Regards, Sergio Cheers, -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] v4l2: OMAP4 ISS driver + Sensor + Board support
Hi Laurent, On Thu, Dec 1, 2011 at 11:26 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Sergio, On Thursday 01 December 2011 01:14:49 Sergio Aguirre wrote: Hi everyone, This is the second version of the OMAP4 ISS driver, now ported to the Media Controller framework AND supporting videobuf2 framework. Thanks a lot for working on this. And thanks to you for the patience and interest. I hope to make faster updates from now on :) This patchset should apply cleanly on top of v3.2-rc3 kernel tag. This driver attempts to provide an fully open source solution to control the OMAP4 Imaging SubSystem (a.k.a. ISS). Starts with just CSI2-A interface support, and pretends to be ready for expansion to add support to the many ISS block modules as possible. Please see newly added documentation for more details: Documentation/video4linux/omap4_camera.txt Any comments/complaints are welcome. :) I've started reviewing the patches, but it might take some time as I got lots on my plate at the moment. I will concentrate on the ISS patch (06/11) first. The sensor drivers are needed now for testing purpose, but can get their share of love later. Brilliant! That's fine with me. Thanks for your time. I really appreciate it. Regards, Sergio Changes since v1: - Simplification of auxclk handling in board files. (Pointed out by: Roger Quadros) - Cleanup of Camera support enablement for 4430sdp panda. (Pointed out by: Roger Quadros) - Use of HWMOD declaration for assisted platform_device creation. (Pointed out by: Felipe Balbi) - Videobuf2 migration (Removal of custom iss_queue buffer handling driver) - Proper GPO3 handling for CAM_SEL in 4430sdp. Sergio Aguirre (10): TWL6030: Add mapping for auxiliary regs mfd: twl6040: Fix wrong TWL6040_GPO3 bitfield value OMAP4: hwmod: Include CSI2A and CSIPHY1 memory sections OMAP4: Add base addresses for ISS v4l: Add support for omap4iss driver v4l: Add support for ov5640 sensor v4l: Add support for ov5650 sensor arm: omap4430sdp: Add support for omap4iss camera arm: omap4panda: Add support for omap4iss camera arm: Add support for CMA for omap4iss driver Stanimir Varbanov (1): v4l: Introduce sensor operation for getting interface configuration Documentation/video4linux/omap4_camera.txt | 60 ++ arch/arm/mach-omap2/Kconfig | 54 + arch/arm/mach-omap2/Makefile | 3 + arch/arm/mach-omap2/board-4430sdp-camera.c | 221 arch/arm/mach-omap2/board-omap4panda-camera.c | 198 arch/arm/mach-omap2/devices.c | 40 + arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 16 +- arch/arm/plat-omap/include/plat/omap4-iss.h | 42 + arch/arm/plat-omap/include/plat/omap44xx.h | 9 + drivers/media/video/Kconfig | 25 + drivers/media/video/Makefile | 3 + drivers/media/video/omap4iss/Makefile | 6 + drivers/media/video/omap4iss/iss.c | 1179 ++ drivers/media/video/omap4iss/iss.h | 133 +++ drivers/media/video/omap4iss/iss_csi2.c | 1324 + drivers/media/video/omap4iss/iss_csi2.h | 166 +++ drivers/media/video/omap4iss/iss_csiphy.c | 215 drivers/media/video/omap4iss/iss_csiphy.h | 69 ++ drivers/media/video/omap4iss/iss_regs.h | 238 + drivers/media/video/omap4iss/iss_video.c | 1192 ++ drivers/media/video/omap4iss/iss_video.h | 205 drivers/media/video/ov5640.c | 972 ++ drivers/media/video/ov5650.c | 524 ++ drivers/mfd/twl-core.c | 2 +- include/linux/mfd/twl6040.h | 2 +- include/media/ov5640.h | 10 + include/media/ov5650.h | 10 + include/media/v4l2-chip-ident.h | 2 + include/media/v4l2-subdev.h | 42 + 29 files changed, 6957 insertions(+), 5 deletions(-) create mode 100644 Documentation/video4linux/omap4_camera.txt create mode 100644 arch/arm/mach-omap2/board-4430sdp-camera.c create mode 100644 arch/arm/mach-omap2/board-omap4panda-camera.c create mode 100644 arch/arm/plat-omap/include/plat/omap4-iss.h create mode 100644 drivers/media/video/omap4iss/Makefile create mode 100644 drivers/media/video/omap4iss/iss.c create mode 100644 drivers/media/video/omap4iss/iss.h create mode 100644 drivers/media/video/omap4iss/iss_csi2.c create mode 100644 drivers/media/video/omap4iss/iss_csi2.h create mode 100644 drivers/media/video/omap4iss/iss_csiphy.c create mode 100644 drivers/media/video/omap4iss/iss_csiphy.h create mode 100644 drivers/media/video/omap4iss/iss_regs.h create mode 100644 drivers/media/video/omap4iss/iss_video.c create mode 100644 drivers/media/video/omap4iss/iss_video.h create mode 100644
Re: [PATCH v2 01/11] TWL6030: Add mapping for auxiliary regs
Hi Balaji, Thanks for the review. On Thu, Dec 1, 2011 at 9:58 AM, T Krishnamoorthy, Balaji balaj...@ti.com wrote: On Thu, Dec 1, 2011 at 5:44 AM, Sergio Aguirre saagui...@ti.com wrote: Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/mfd/twl-core.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c index bfbd660..e26b564 100644 --- a/drivers/mfd/twl-core.c +++ b/drivers/mfd/twl-core.c @@ -323,7 +323,7 @@ static struct twl_mapping twl6030_map[] = { { SUB_CHIP_ID0, TWL6030_BASEADD_ZERO }, { SUB_CHIP_ID1, TWL6030_BASEADD_ZERO }, - { SUB_CHIP_ID2, TWL6030_BASEADD_ZERO }, + { SUB_CHIP_ID1, TWL6030_BASEADD_AUX }, Instead you can use TWL6030_MODULE_ID1, with base address as zero for all registers in auxiliaries register map. Ok. I'm actually thinking about this, and in the process on reviewing the need to access those registers. I should probably be using the regulator framework to control VAUX3 instead... Thanks for your inputs. Regards, Sergio { SUB_CHIP_ID2, TWL6030_BASEADD_ZERO }, { SUB_CHIP_ID2, TWL6030_BASEADD_RSV }, { SUB_CHIP_ID2, TWL6030_BASEADD_RSV }, -- 1.7.7.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap 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 v2 00/11] v4l2: OMAP4 ISS driver + Sensor + Board support
Hi again, On Wed, Nov 30, 2011 at 6:14 PM, Sergio Aguirre saagui...@ti.com wrote: Hi everyone, This is the second version of the OMAP4 ISS driver, now ported to the Media Controller framework AND supporting videobuf2 framework. This patchset should apply cleanly on top of v3.2-rc3 kernel tag. This driver attempts to provide an fully open source solution to control the OMAP4 Imaging SubSystem (a.k.a. ISS). Starts with just CSI2-A interface support, and pretends to be ready for expansion to add support to the many ISS block modules as possible. Please see newly added documentation for more details: Documentation/video4linux/omap4_camera.txt Any comments/complaints are welcome. :) Changes since v1: - Simplification of auxclk handling in board files. (Pointed out by: Roger Quadros) - Cleanup of Camera support enablement for 4430sdp panda. (Pointed out by: Roger Quadros) - Use of HWMOD declaration for assisted platform_device creation. (Pointed out by: Felipe Balbi) - Videobuf2 migration (Removal of custom iss_queue buffer handling driver) - Proper GPO3 handling for CAM_SEL in 4430sdp. Also, you can pull this as a branch in my git tree here: Web URL: http://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera git URL: git://gitorious.org/omap4-v4l2-camera/omap4-v4l2-camera.git Branch: for3.2-rc3 Regards, Sergio Sergio Aguirre (10): TWL6030: Add mapping for auxiliary regs mfd: twl6040: Fix wrong TWL6040_GPO3 bitfield value OMAP4: hwmod: Include CSI2A and CSIPHY1 memory sections OMAP4: Add base addresses for ISS v4l: Add support for omap4iss driver v4l: Add support for ov5640 sensor v4l: Add support for ov5650 sensor arm: omap4430sdp: Add support for omap4iss camera arm: omap4panda: Add support for omap4iss camera arm: Add support for CMA for omap4iss driver Stanimir Varbanov (1): v4l: Introduce sensor operation for getting interface configuration Documentation/video4linux/omap4_camera.txt | 60 ++ arch/arm/mach-omap2/Kconfig | 54 + arch/arm/mach-omap2/Makefile | 3 + arch/arm/mach-omap2/board-4430sdp-camera.c | 221 arch/arm/mach-omap2/board-omap4panda-camera.c | 198 arch/arm/mach-omap2/devices.c | 40 + arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 16 +- arch/arm/plat-omap/include/plat/omap4-iss.h | 42 + arch/arm/plat-omap/include/plat/omap44xx.h | 9 + drivers/media/video/Kconfig | 25 + drivers/media/video/Makefile | 3 + drivers/media/video/omap4iss/Makefile | 6 + drivers/media/video/omap4iss/iss.c | 1179 ++ drivers/media/video/omap4iss/iss.h | 133 +++ drivers/media/video/omap4iss/iss_csi2.c | 1324 + drivers/media/video/omap4iss/iss_csi2.h | 166 +++ drivers/media/video/omap4iss/iss_csiphy.c | 215 drivers/media/video/omap4iss/iss_csiphy.h | 69 ++ drivers/media/video/omap4iss/iss_regs.h | 238 + drivers/media/video/omap4iss/iss_video.c | 1192 ++ drivers/media/video/omap4iss/iss_video.h | 205 drivers/media/video/ov5640.c | 972 ++ drivers/media/video/ov5650.c | 524 ++ drivers/mfd/twl-core.c | 2 +- include/linux/mfd/twl6040.h | 2 +- include/media/ov5640.h | 10 + include/media/ov5650.h | 10 + include/media/v4l2-chip-ident.h | 2 + include/media/v4l2-subdev.h | 42 + 29 files changed, 6957 insertions(+), 5 deletions(-) create mode 100644 Documentation/video4linux/omap4_camera.txt create mode 100644 arch/arm/mach-omap2/board-4430sdp-camera.c create mode 100644 arch/arm/mach-omap2/board-omap4panda-camera.c create mode 100644 arch/arm/plat-omap/include/plat/omap4-iss.h create mode 100644 drivers/media/video/omap4iss/Makefile create mode 100644 drivers/media/video/omap4iss/iss.c create mode 100644 drivers/media/video/omap4iss/iss.h create mode 100644 drivers/media/video/omap4iss/iss_csi2.c create mode 100644 drivers/media/video/omap4iss/iss_csi2.h create mode 100644 drivers/media/video/omap4iss/iss_csiphy.c create mode 100644 drivers/media/video/omap4iss/iss_csiphy.h create mode 100644 drivers/media/video/omap4iss/iss_regs.h create mode 100644 drivers/media/video/omap4iss/iss_video.c create mode 100644 drivers/media/video/omap4iss/iss_video.h create mode 100644 drivers/media/video/ov5640.c create mode 100644 drivers/media/video/ov5650.c create mode 100644 include/media/ov5640.h create mode 100644 include/media/ov5650.h -- 1.7.7.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
Re: [RFC][PATCH 2/2] v4l2: Add lv8093 lens driver
Hi Mauro/Laurent, On Sun, Sep 18, 2011 at 7:53 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi, On Sunday 18 September 2011 14:25:50 Mauro Carvalho Chehab wrote: Em 13-07-2010 20:40, Sergio Aguirre escreveu: This adds LV8093 Piezo Actuator Lens driver. This is currently found in tandem with IMX046 sensor. Due to patchwork.kernel.org long outage, I'm working on setting a new patchwork instance somewhere else. So, I'm importing the old patches and double-check if something were missed. In this proccess, I noticed that, for two times, a driver for lv8093 were proposed, one as part of the OMAPZOOM series of patches, back in Feb/2009, and a second submission, on this series. However, it seems that this were never applied, even not having any single comment on the last time you've submitted it, as a RFC. It seems that the omap3 maintainer lost those patches, or am I missing something? If you're talking about the OMAP3 ISP maintainer, indeed, I've never even noticed the patches :-) This patches are so old, that I've forgotten completely about them :P I'll actually need to dust off my old Zoom3 HW I have around to retest this. :) And BTW, Dominic doesn't work at TI since last year, so I'm unlooping his e-mail address. Review inlined. Ok. Thanks. Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/Kconfig | 8 + drivers/media/video/Makefile | 1 + drivers/media/video/lv8093.c | 614 drivers/media/video/lv8093_regs.h | 76 + include/media/lv8093.h | 40 +++ include/media/v4l2-chip-ident.h | 3 + 6 files changed, 742 insertions(+), 0 deletions(-) create mode 100644 drivers/media/video/lv8093.c create mode 100644 drivers/media/video/lv8093_regs.h create mode 100644 include/media/lv8093.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 10cd7b3..b62adce 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -344,6 +344,14 @@ config VIDEO_IMX046 IMX046 camera. It is currently working with the TI OMAP3 camera controller. +config VIDEO_LV8093 + tristate Piezo Actuator Lens driver for LV8093 + depends on I2C VIDEO_V4L2 + ---help--- + This is a Video4Linux2 lens driver for the Sanyo LV8093. + It is currently working with the TI OMAP3 camera controller + and Sony IMX046 sensor. + You will need to implement the media controller API to work with the TI OMAP3 ISP. Yeah. I think I've implemented this driver when Media Controller wasn't still upstreamed, and under review still. config VIDEO_SAA7110 tristate Philips SAA7110 video decoder depends on VIDEO_V4L2 I2C diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index 00341cb..50f528c 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_VIDEO_TDA9840) += tda9840.o obj-$(CONFIG_VIDEO_TEA6415C) += tea6415c.o obj-$(CONFIG_VIDEO_TEA6420) += tea6420.o obj-$(CONFIG_VIDEO_IMX046) += imx046.o +obj-$(CONFIG_VIDEO_LV8093) += lv8093.o obj-$(CONFIG_VIDEO_SAA7110) += saa7110.o obj-$(CONFIG_VIDEO_SAA711X) += saa7115.o obj-$(CONFIG_VIDEO_SAA717X) += saa717x.o diff --git a/drivers/media/video/lv8093.c b/drivers/media/video/lv8093.c new file mode 100644 index 000..b0b0fcf --- /dev/null +++ b/drivers/media/video/lv8093.c @@ -0,0 +1,614 @@ +/* + * drivers/media/video/lv8093.c + * + * LV8093 Piezo Motor (LENS) driver + * + * Copyright (C) 2008-2009 Texas Instruments. + * Copyright (C) 2009 Hewlett-Packard. + * + * This package 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. + * + * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED + * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE. + */ + +#include linux/mutex.h +#include linux/i2c.h +#include linux/delay.h +#include linux/platform_device.h +#include linux/cdev.h +#include linux/device.h + +#include media/lv8093.h +#include media/v4l2-chip-ident.h +#include media/v4l2-device.h +#include media/v4l2-subdev.h + +#include lv8093_regs.h + +static struct vcontrol { + struct v4l2_queryctrl qc; +} video_control[] = { + { + { + .id = V4L2_CID_FOCUS_RELATIVE, + .type = V4L2_CTRL_TYPE_INTEGER, + .name = Lens Relative Position, + .minimum = 0, + .maximum = 0, + .step = LV8093_MAX_RELATIVE_STEP, + .default_value = 0, + } + ,} +}; + +static struct i2c_driver lv8093_i2c_driver; + +static struct
Re: [PATCH 8/8] ARM: S5PV210: example of CMA private area for FIMC device on Goni board
Hi Marek/Kyungmin, On Fri, Aug 19, 2011 at 04:27:44PM +0200, Marek Szyprowski wrote: This patch is an example how device private CMA area can be activated. It creates one CMA region and assigns it to the first s5p-fimc device on Samsung Goni S5PC110 board. Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- arch/arm/mach-s5pv210/mach-goni.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-s5pv210/mach-goni.c b/arch/arm/mach-s5pv210/mach-goni.c index 14578f5..f766c45 100644 --- a/arch/arm/mach-s5pv210/mach-goni.c +++ b/arch/arm/mach-s5pv210/mach-goni.c @@ -26,6 +26,7 @@ #include linux/input.h #include linux/gpio.h #include linux/interrupt.h +#include linux/dma-contiguous.h #include asm/mach/arch.h #include asm/mach/map.h @@ -857,6 +858,9 @@ static void __init goni_map_io(void) static void __init goni_reserve(void) { s5p_mfc_reserve_mem(0x4300, 8 20, 0x5100, 8 20); + + /* Create private 16MiB contiguous memory area for s5p-fimc.0 device */ + dma_declare_contiguous(s5p_device_fimc0.dev, 16*SZ_1M, 0); This is broken, since according to patch #0006, dma_declare_contiguous requires a 4th param (limit) which you're not providing here. Regards, Sergio } static void __init goni_machine_init(void) -- 1.7.1.569.g6f426 -- 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 media bus configuration subdev operations
Hi Guennadi, Thanks for the patch. On Mon, Jun 6, 2011 at 7:31 AM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Add media bus configuration types and two subdev operations to get supported mediabus configurations and to set a specific configuration. Subdevs can support several configurations, e.g., they can send video data on 1 or several lanes, can be configured to use a specific CSI-2 channel, in such cases subdevice drivers return bitmasks with all respective bits set. When a set-configuration operation is called, it has to specify a non-ambiguous configuration. Signed-off-by: Stanimir Varbanov svarba...@mm-sol.com Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- This change would allow a re-use of soc-camera and standard subdev drivers. It is a modified and extended version of http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/29408 therefore the original Sob. After this we only would have to switch to the control framework:) Please, comment. diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 971c7fa..0983b7b 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -13,6 +13,76 @@ #include linux/v4l2-mediabus.h +/* Parallel flags */ +/* Can the client run in master or in slave mode */ +#define V4L2_MBUS_MASTER (1 0) +#define V4L2_MBUS_SLAVE (1 1) +/* Which signal polarities it supports */ +#define V4L2_MBUS_HSYNC_ACTIVE_HIGH (1 2) +#define V4L2_MBUS_HSYNC_ACTIVE_LOW (1 3) +#define V4L2_MBUS_VSYNC_ACTIVE_HIGH (1 4) +#define V4L2_MBUS_VSYNC_ACTIVE_LOW (1 5) +#define V4L2_MBUS_PCLK_SAMPLE_RISING (1 6) +#define V4L2_MBUS_PCLK_SAMPLE_FALLING (1 7) +#define V4L2_MBUS_DATA_ACTIVE_HIGH (1 8) +#define V4L2_MBUS_DATA_ACTIVE_LOW (1 9) +/* Which datawidths are supported */ +#define V4L2_MBUS_DATAWIDTH_4 (1 10) +#define V4L2_MBUS_DATAWIDTH_8 (1 11) +#define V4L2_MBUS_DATAWIDTH_9 (1 12) +#define V4L2_MBUS_DATAWIDTH_10 (1 13) +#define V4L2_MBUS_DATAWIDTH_15 (1 14) +#define V4L2_MBUS_DATAWIDTH_16 (1 15) + +#define V4L2_MBUS_DATAWIDTH_MASK (V4L2_MBUS_DATAWIDTH_4 | V4L2_MBUS_DATAWIDTH_8 | \ + V4L2_MBUS_DATAWIDTH_9 | V4L2_MBUS_DATAWIDTH_10 | \ + V4L2_MBUS_DATAWIDTH_15 | V4L2_MBUS_DATAWIDTH_16) + +/* Serial flags */ +/* How many lanes the client can use */ +#define V4L2_MBUS_CSI2_1_LANE (1 0) +#define V4L2_MBUS_CSI2_2_LANE (1 1) +#define V4L2_MBUS_CSI2_3_LANE (1 2) +#define V4L2_MBUS_CSI2_4_LANE (1 3) +/* On which channels it can send video data */ +#define V4L2_MBUS_CSI2_CHANNEL_0 (1 4) +#define V4L2_MBUS_CSI2_CHANNEL_1 (1 5) +#define V4L2_MBUS_CSI2_CHANNEL_2 (1 6) +#define V4L2_MBUS_CSI2_CHANNEL_3 (1 7) +/* Does it support only continuous or also non-contimuous clock mode */ Typo: non-continuous +#define V4L2_MBUS_CSI2_CONTINUOUS_CLOCK (1 8) Doesn't having above bit disabled, imply a non-continuous clock already? +#define V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK (1 9) Regards, Sergio + +#define V4L2_MBUS_CSI2_LANES (V4L2_MBUS_CSI2_1_LANE | V4L2_MBUS_CSI2_2_LANE | \ + V4L2_MBUS_CSI2_3_LANE | V4L2_MBUS_CSI2_4_LANE) +#define V4L2_MBUS_CSI2_CHANNELS (V4L2_MBUS_CSI2_CHANNEL_0 | V4L2_MBUS_CSI2_CHANNEL_1 | \ + V4L2_MBUS_CSI2_CHANNEL_2 | V4L2_MBUS_CSI2_CHANNEL_3) + +/** + * v4l2_mbus_type - media bus type + * @V4L2_MBUS_PARALLEL: parallel interface with hsync and vsync + * @V4L2_MBUS_BT656: parallel interface with embedded synchronisation + * @V4L2_MBUS_CSI2: MIPI CSI-2 serial interface + */ +enum v4l2_mbus_type { + V4L2_MBUS_PARALLEL, + V4L2_MBUS_BT656, + V4L2_MBUS_CSI2, +}; + +/** + * v4l2_mbus_config - media bus configuration + * @type: interface type + * @flags: configuration flags, depending on @type + * @clk: output clock, the bridge driver can try to use clk_set_parent() + * to specify the master clock to the client + */ +struct v4l2_mbus_config { + enum v4l2_mbus_type type; + unsigned long flags; + struct clk *clk; +}; + static inline void v4l2_fill_pix_format(struct v4l2_pix_format *pix_fmt, const struct v4l2_mbus_framefmt *mbus_fmt) { diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 1562c4f..6ea25f4 100644 --- a/include/media/v4l2-subdev.h +++
Re: [ANNOUNCE] New OMAP4 V4L2 Camera Project started
On Fri, May 13, 2011 at 6:01 PM, Sakari Ailus sakari.ai...@iki.fi wrote: Aguirre, Sergio wrote: Hi all, Hi, Sergio! Hi Sakari, Just to let you know that I've just officially registered for a new project in the Pandaboard.org portal for OMAP4 v4l2 camera support. You can find it here: http://omiio.org/content/omap4-v4l2-camera And also, you can find the actual Gitorious project with the code here: https://www.gitorious.org/omap4-v4l2-camera If anyone is interested in contributing for this project, please let me know, so I can add you as a contributor to the project. I'm very, very happy to see a project to start implementing V4L2 support for the OMAP 4 ISS!! Thanks, Sergio! I'm pretty excited about it too :) Thanks. A few comments: - The driver is using videobuf. I wonder if the driver would benefit more from videobuf2. Sure, that's definitely one of the first targets. I'll migrate to it. - As far as I understand, the OMAP 4 ISS is partially similar to the OMAP 3 one in design --- it has a hardware pipeline, that is. Fitting the bus receivers and the ISP under an Media controller graph looks relatively straightforward. The same might apply to SIMCOP, but then the question is: what kind of interface should the SIMCOP have? That's a big question :) And I'm yet not sure on that. I'll certainly need to think about it, and probably start planning for RFCs. BTW, this driver is just implementing a super simple CSI2-A Rx - MEM pipeline so far. I started with this because i wanted to avoid wasting my time on developing somethign huge, and having to do heavy rework after reviews take place. Being familiar with the history of the OMAP 3 ISP driver, I know this is not a small project. Still, starting to use the Media controller in an early phase would benefit the project in long run since the conversion can be avoided later. Agreed. Which parts of the ISS require regular attention from the M3s? Is it the whole ISS or just the SIMCOP, for example? In theory, the whole ISS, which includes SIMCOP, cna be driven from either A9 or M3 cores. Architecturally, it's better to keep the dedicated M3 cores for driving ISS, and to save some considerable cycles from A9. Problem is, we have to deal with IPC communication channels, and that might make the driver much more complex and requiring much more software layers to be in place for that. The long term vision about this is that, it might be good ot see how easy is to keep a Media Controller device, which sends the pipeline and subdevice configuration to M3 software, and just keep the Board specific and Usecases in A9 side. Immediate problems are how to approach this with purely open source tools. (As far as I know, it's so far only possible with TI proprietary compilers) So, it might definitely take this discussion to a much more complex level and more complete analysis. Hopefully we can have a good discussion about the long term future of this. So far, I'm just starting with the simple stuff, ISS CSI2 Rx interface :) Kiitos! Thanks for the interest. Regards, Sergio Kind regards, Sakari Ailus Ps. I have nothing against SoC camera, but when I look at the ISS overview diagram (section 8.1 in my TRM) I can't avoid thinking that this is exactly what the Media controller was created for. :-) The main reason why it started as a soc_camera is because I started this driver in a 2.6.35 android kernel, and refused to backport all the Media Controller patches to it :) But now being based in mainline, that's a different story. ;) -- Sakari Ailus sakari.ai...@iki.fi -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] New OMAP4 V4L2 Camera Project started
Hi all, Just to let you know that I've just officially registered for a new project in the Pandaboard.org portal for OMAP4 v4l2 camera support. You can find it here: http://omiio.org/content/omap4-v4l2-camera And also, you can find the actual Gitorious project with the code here: https://www.gitorious.org/omap4-v4l2-camera If anyone is interested in contributing for this project, please let me know, so I can add you as a contributor to the project. Regards, Sergio -- 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
[Query] Anyone here working with the mainline omap3isp driver?
Hi Everyone, I'll just like to know if there's someone working with the mainline version of the omap3isp driver. Ohad (in CC) has some omap3 iommu changes which might affect the omap3isp driver. I have been a bit away of the omap3 driver these days, so, if there's someone else that can try some iommu changes on top, that'll be great. Regards, Sergio -- 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] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c
On Mon, Apr 11, 2011 at 4:52 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: Dnia poniedziałek 11 kwiecień 2011 o 22:05:35 Aguirre, Sergio napisał(a): Please find below a refreshed patch, which should be based on mainline commit: Hi, This version works for me, and fixes the regression. Ok, Thanks for testing. Guennadi, It's up to you if you want to take your patch or mine. I guess I just wanted to be more complete in my patch, but strictly speaking about a regression fix (And based on what the current popular apps do), both do the job. This was just my proposal :). Regards, Sergio Thanks, Janusz From f767059c12c755ebe79c4b74de17c23a257007c7 Mon Sep 17 00:00:00 2001 From: Sergio Aguirre saagui...@ti.com Date: Mon, 11 Apr 2011 11:14:33 -0500 Subject: [PATCH] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c A recent patch has given individual soc-camera host drivers a possibility to calculate .sizeimage and .bytesperline pixel format fields internally, however, some drivers relied on the core calculating these values for them, following a default algorithm. This patch restores the default calculation for such drivers. Based on initial patch by Guennadi Liakhovetski, found here: http://www.spinics.net/lists/linux-media/msg31282.html Except that this covers try_fmt aswell. Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/soc_camera.c | 48 + 1 files changed, 42 insertions(+), 6 deletions(-) diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index 4628448..dcc6623 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -136,11 +136,50 @@ unsigned long soc_camera_apply_sensor_flags(struct soc_camera_link *icl, } EXPORT_SYMBOL(soc_camera_apply_sensor_flags); +#define pixfmtstr(x) (x) 0xff, ((x) 8) 0xff, ((x) 16) 0xff, \ + ((x) 24) 0xff + +static int soc_camera_try_fmt(struct soc_camera_device *icd, + struct v4l2_format *f) +{ + struct soc_camera_host *ici = to_soc_camera_host(icd-dev.parent); + struct v4l2_pix_format *pix = f-fmt.pix; + int ret; + + dev_dbg(icd-dev, TRY_FMT(%c%c%c%c, %ux%u)\n, + pixfmtstr(pix-pixelformat), pix-width, pix-height); + + pix-bytesperline = 0; + pix-sizeimage = 0; + + ret = ici-ops-try_fmt(icd, f); + if (ret 0) + return ret; + + if (!pix-sizeimage) { + if (!pix-bytesperline) { + const struct soc_camera_format_xlate *xlate; + + xlate = soc_camera_xlate_by_fourcc(icd, pix-pixelformat); + if (!xlate) + return -EINVAL; + + ret = soc_mbus_bytes_per_line(pix-width, + xlate-host_fmt); + if (ret 0) + pix-bytesperline = ret; + } + if (pix-bytesperline) + pix-sizeimage = pix-bytesperline * pix-height; + } + + return 0; +} + static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f) { struct soc_camera_device *icd = file-private_data; - struct soc_camera_host *ici = to_soc_camera_host(icd-dev.parent); WARN_ON(priv != file-private_data); @@ -149,7 +188,7 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv, return -EINVAL; /* limit format to hardware capabilities */ - return ici-ops-try_fmt(icd, f); + return soc_camera_try_fmt(icd, f); } static int soc_camera_enum_input(struct file *file, void *priv, @@ -362,9 +401,6 @@ static void soc_camera_free_user_formats(struct soc_camera_device *icd) icd-user_formats = NULL; } -#define pixfmtstr(x) (x) 0xff, ((x) 8) 0xff, ((x) 16) 0xff, \ - ((x) 24) 0xff - /* Called with .vb_lock held, or from the first open(2), see comment there */ static int soc_camera_set_fmt(struct soc_camera_device *icd, struct v4l2_format *f) @@ -377,7 +413,7 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd, pixfmtstr(pix-pixelformat), pix-width, pix-height); /* We always call try_fmt() before set_fmt() or set_crop() */ - ret = ici-ops-try_fmt(icd, f); + ret = soc_camera_try_fmt(icd, f); if (ret 0) return ret; -- 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] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c
2011/4/12 Janusz Krzysztofik jkrzy...@tis.icnet.pl: Dnia wtorek 12 kwiecień 2011 o 17:39:35 Aguirre, Sergio napisał(a): On Mon, Apr 11, 2011 at 4:52 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: Dnia poniedziałek 11 kwiecień 2011 o 22:05:35 Aguirre, Sergio napisał(a): Please find below a refreshed patch, which should be based on mainline commit: Hi, This version works for me, and fixes the regression. Ok, Thanks for testing. I forgot to mention: the patch didn't apply cleanly, I had to unwrap a few lines manually, so you may want to resend it unwrapped. Hmm, I think that's a problem with my mail servers :/ Anyways, I'm attaching it now. Hopefully that's unwrapped. Regards, Sergio Thanks, Janusz From f767059c12c755ebe79c4b74de17c23a257007c7 Mon Sep 17 00:00:00 2001 From: Sergio Aguirre saagui...@ti.com Date: Mon, 11 Apr 2011 11:14:33 -0500 Subject: [PATCH] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c A recent patch has given individual soc-camera host drivers a possibility to calculate .sizeimage and .bytesperline pixel format fields internally, however, some drivers relied on the core calculating these values for them, following a default algorithm. This patch restores the default calculation for such drivers. Based on initial patch by Guennadi Liakhovetski, found here: http://www.spinics.net/lists/linux-media/msg31282.html Except that this covers try_fmt aswell. Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/soc_camera.c | 48 + 1 files changed, 42 insertions(+), 6 deletions(-) diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index 4628448..dcc6623 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -136,11 +136,50 @@ unsigned long soc_camera_apply_sensor_flags(struct soc_camera_link *icl, } EXPORT_SYMBOL(soc_camera_apply_sensor_flags); +#define pixfmtstr(x) (x) 0xff, ((x) 8) 0xff, ((x) 16) 0xff, \ + ((x) 24) 0xff + +static int soc_camera_try_fmt(struct soc_camera_device *icd, + struct v4l2_format *f) +{ + struct soc_camera_host *ici = to_soc_camera_host(icd-dev.parent); + struct v4l2_pix_format *pix = f-fmt.pix; + int ret; + + dev_dbg(icd-dev, TRY_FMT(%c%c%c%c, %ux%u)\n, + pixfmtstr(pix-pixelformat), pix-width, pix-height); + + pix-bytesperline = 0; + pix-sizeimage = 0; + + ret = ici-ops-try_fmt(icd, f); + if (ret 0) + return ret; + + if (!pix-sizeimage) { + if (!pix-bytesperline) { + const struct soc_camera_format_xlate *xlate; + + xlate = soc_camera_xlate_by_fourcc(icd, pix-pixelformat); + if (!xlate) +return -EINVAL; + + ret = soc_mbus_bytes_per_line(pix-width, + xlate-host_fmt); + if (ret 0) +pix-bytesperline = ret; + } + if (pix-bytesperline) + pix-sizeimage = pix-bytesperline * pix-height; + } + + return 0; +} + static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f) { struct soc_camera_device *icd = file-private_data; - struct soc_camera_host *ici = to_soc_camera_host(icd-dev.parent); WARN_ON(priv != file-private_data); @@ -149,7 +188,7 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv, return -EINVAL; /* limit format to hardware capabilities */ - return ici-ops-try_fmt(icd, f); + return soc_camera_try_fmt(icd, f); } static int soc_camera_enum_input(struct file *file, void *priv, @@ -362,9 +401,6 @@ static void soc_camera_free_user_formats(struct soc_camera_device *icd) icd-user_formats = NULL; } -#define pixfmtstr(x) (x) 0xff, ((x) 8) 0xff, ((x) 16) 0xff, \ - ((x) 24) 0xff - /* Called with .vb_lock held, or from the first open(2), see comment there */ static int soc_camera_set_fmt(struct soc_camera_device *icd, struct v4l2_format *f) @@ -377,7 +413,7 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd, pixfmtstr(pix-pixelformat), pix-width, pix-height); /* We always call try_fmt() before set_fmt() or set_crop() */ - ret = ici-ops-try_fmt(icd, f); + ret = soc_camera_try_fmt(icd, f); if (ret 0) return ret; -- 1.7.0.4
[soc_camera] Dynamic crop window change while streaming (Zoom case)?
Hi Guennadi, I was wondering what's the stand on allowing soc_camera host drivers to have internal scalers... It looks possible, but however I see one important blocker for being able to change window rectangle while streaming (for example, when attempting to do zoom in/out during streaming). See here: in soc_camera.c::soc_camera_s_crop() ... /* If get_crop fails, we'll let host and / or client drivers decide */ ret = ici-ops-get_crop(icd, current_crop); /* Prohibit window size change with initialised buffers */ if (ret 0) { dev_err(icd-dev, S_CROP denied: getting current crop failed\n); } else if (icd-vb_vidq.bufs[0] (a-c.width != current_crop.c.width || a-c.height != current_crop.c.height)) { dev_err(icd-dev, S_CROP denied: queue initialised and sizes differ\n); ret = -EBUSY; } else { ret = ici-ops-set_crop(icd, a); } ... Now, I don't want to move just yet to a Media Controller implementation in my omap4 camera driver, since I don't intend yet to exploit the full HW, which is easly 2x more complicated than omap3. But I want to start with a simplistic driver which Pandaboard community can take (which don't need any advanced features, just being able to receive frames.) and i just want to know how complicated is to just offer the scaler functionality still. Regards, Sergio -- 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] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c
Hi Guennadi, On Mon, Apr 11, 2011 at 8:23 AM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Mon, 11 Apr 2011, Aguirre, Sergio wrote: Hi Guennadi, On Mon, Apr 11, 2011 at 3:58 AM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: A recent patch has given individual soc-camera host drivers a possibility to calculate .sizeimage and .bytesperline pixel format fields internally, however, some drivers relied on the core calculating these values for them, following a default algorithm. This patch restores the default calculation for such drivers. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index 4628448..0918c48 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -376,6 +376,9 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd, dev_dbg(icd-dev, S_FMT(%c%c%c%c, %ux%u)\n, pixfmtstr(pix-pixelformat), pix-width, pix-height); + pix-bytesperline = 0; + pix-sizeimage = 0; + /* We always call try_fmt() before set_fmt() or set_crop() */ ret = ici-ops-try_fmt(icd, f); if (ret 0) @@ -391,6 +394,17 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd, return -EINVAL; } + if (!pix-sizeimage) { + if (!pix-bytesperline) { + ret = soc_mbus_bytes_per_line(pix-width, + icd-current_fmt-host_fmt); + if (ret 0) + pix-bytesperline = ret; + } + if (pix-bytesperline) + pix-sizeimage = pix-bytesperline * pix-height; + } + Shouldn't all this be better done in try_fmt? Not _better_. We might choose to additionally do it for try_fmt too. But 1. we didn't do it before, applications don't seem to care. 2. you cannot store / reuse those .sizeimage .bytesperline values - this is just a try 3. it would be a bit difficult to realise - we need a struct soc_mbus_pixelfmt to call soc_mbus_bytes_per_line(), which we don't have, so, we'd need to obtain it using soc_camera_xlate_by_fourcc(). This all would work, but in any case it would be an enhancement, but not a regression fix. Ok. And how about the attached patch? Would that work? Regards, Sergio Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ From 5442074c7760429c06f62917503c1d8a2f79cd21 Mon Sep 17 00:00:00 2001 From: Sergio Aguirre saagui...@ti.com Date: Mon, 11 Apr 2011 11:14:33 -0500 Subject: [PATCH] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c A recent patch has given individual soc-camera host drivers a possibility to calculate .sizeimage and .bytesperline pixel format fields internally, however, some drivers relied on the core calculating these values for them, following a default algorithm. This patch restores the default calculation for such drivers. Based on initial patch by Guennadi Liakhovetski, found here: http://www.spinics.net/lists/linux-media/msg31282.html Except that this covers try_fmt aswell. Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/soc_camera.c | 41 - 1 files changed, 39 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index 4628448..875842a 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -136,6 +136,43 @@ unsigned long soc_camera_apply_sensor_flags(struct soc_camera_link *icl, } EXPORT_SYMBOL(soc_camera_apply_sensor_flags); +static int soc_camera_try_fmt(struct soc_camera_device *icd, + struct v4l2_format *f) +{ + struct soc_camera_host *ici = to_soc_camera_host(icd-dev.parent); + struct v4l2_pix_format *pix = f-fmt.pix; + int ret; + + dev_dbg(icd-dev, TRY_FMT(%c%c%c%c, %ux%u)\n, + pixfmtstr(pix-pixelformat), pix-width, pix-height); + + pix-bytesperline = 0; + pix-sizeimage = 0; + + ret = ici-ops-try_fmt(icd, f); + if (ret 0) + return ret; + + if (!pix-sizeimage) { + if (!pix-bytesperline) { + const struct soc_camera_format_xlate *xlate; + + xlate = soc_camera_xlate_by_fourcc(icd, pix-pixelformat); + if (!xlate) +return -EINVAL; + + ret = soc_mbus_bytes_per_line(pix-width, + xlate-host_fmt); + if (ret 0) +pix-bytesperline = ret; + } + if (pix-bytesperline) + pix-sizeimage = pix-bytesperline * pix-height; + } + + return 0; +} + static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f) { @@ -149,7 +186,7 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv, return -EINVAL; /* limit format to hardware capabilities */ - return ici
Re: [PATCH] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c
On Mon, Apr 11, 2011 at 11:58 AM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Mon, 11 Apr 2011, Aguirre, Sergio wrote: Hi Guennadi, On Mon, Apr 11, 2011 at 8:23 AM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Mon, 11 Apr 2011, Aguirre, Sergio wrote: Hi Guennadi, On Mon, Apr 11, 2011 at 3:58 AM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: A recent patch has given individual soc-camera host drivers a possibility to calculate .sizeimage and .bytesperline pixel format fields internally, however, some drivers relied on the core calculating these values for them, following a default algorithm. This patch restores the default calculation for such drivers. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index 4628448..0918c48 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -376,6 +376,9 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd, dev_dbg(icd-dev, S_FMT(%c%c%c%c, %ux%u)\n, pixfmtstr(pix-pixelformat), pix-width, pix-height); + pix-bytesperline = 0; + pix-sizeimage = 0; + /* We always call try_fmt() before set_fmt() or set_crop() */ ret = ici-ops-try_fmt(icd, f); if (ret 0) @@ -391,6 +394,17 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd, return -EINVAL; } + if (!pix-sizeimage) { + if (!pix-bytesperline) { + ret = soc_mbus_bytes_per_line(pix-width, + icd-current_fmt-host_fmt); + if (ret 0) + pix-bytesperline = ret; + } + if (pix-bytesperline) + pix-sizeimage = pix-bytesperline * pix-height; + } + Shouldn't all this be better done in try_fmt? Not _better_. We might choose to additionally do it for try_fmt too. But 1. we didn't do it before, applications don't seem to care. 2. you cannot store / reuse those .sizeimage .bytesperline values - this is just a try 3. it would be a bit difficult to realise - we need a struct soc_mbus_pixelfmt to call soc_mbus_bytes_per_line(), which we don't have, so, we'd need to obtain it using soc_camera_xlate_by_fourcc(). This all would work, but in any case it would be an enhancement, but not a regression fix. Ok. And how about the attached patch? Would that work? Please, post patches inline. Ok, sorry. Yes, I think, ot would work too, only the call to soc_camera_xlate_by_fourcc() in the S_FMT case is superfluous, after ici-ops-set_fmt() we already have it in icd-current_fmt-host_fmt. Well, the difference is that, after my patch, this is called after ici-ops-try_fmt, so icd-current_fmt-host_fmt doesn't match the intended format to try, as it's not stored as a current_fmt. Unless i'm getting confused with something... Regards, Sergio Otherwise - yes, we could do it this way too. Janusz, could you test, please? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c
On Mon, Apr 11, 2011 at 1:40 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: Dnia poniedziałek 11 kwiecień 2011 o 18:58:51 Guennadi Liakhovetski napisał(a): On Mon, 11 Apr 2011, Aguirre, Sergio wrote: Ok. And how about the attached patch? Would that work? Yes, I think, ot would work too, only the call to soc_camera_xlate_by_fourcc() in the S_FMT case is superfluous, after ici-ops-set_fmt() we already have it in icd-current_fmt-host_fmt. Otherwise - yes, we could do it this way too. Janusz, could you test, please? Looks like not based on the current mainline (-rc2) tree: CHECK drivers/media/video/soc_camera.c drivers/media/video/soc_camera.c:146:9: error: undefined identifier 'pixfmtstr' CC drivers/media/video/soc_camera.o drivers/media/video/soc_camera.c: In function 'soc_camera_try_fmt': drivers/media/video/soc_camera.c:146: error: implicit declaration of function 'pixfmtstr' drivers/media/video/soc_camera.c:146: warning: too few arguments for format drivers/media/video/soc_camera.c: In function 'soc_camera_try_fmt_vid_cap': drivers/media/video/soc_camera.c:180: warning: unused variable 'ici' Oops, my bad. Please find below a refreshed patch, which should be based on mainline commit: b42282e pci: fix PCI bus allocation alignment handling Thanks, Janusz From f767059c12c755ebe79c4b74de17c23a257007c7 Mon Sep 17 00:00:00 2001 From: Sergio Aguirre saagui...@ti.com Date: Mon, 11 Apr 2011 11:14:33 -0500 Subject: [PATCH] V4L: soc-camera: regression fix: calculate .sizeimage in soc_camera.c A recent patch has given individual soc-camera host drivers a possibility to calculate .sizeimage and .bytesperline pixel format fields internally, however, some drivers relied on the core calculating these values for them, following a default algorithm. This patch restores the default calculation for such drivers. Based on initial patch by Guennadi Liakhovetski, found here: http://www.spinics.net/lists/linux-media/msg31282.html Except that this covers try_fmt aswell. Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/soc_camera.c | 48 + 1 files changed, 42 insertions(+), 6 deletions(-) diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index 4628448..dcc6623 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -136,11 +136,50 @@ unsigned long soc_camera_apply_sensor_flags(struct soc_camera_link *icl, } EXPORT_SYMBOL(soc_camera_apply_sensor_flags); +#define pixfmtstr(x) (x) 0xff, ((x) 8) 0xff, ((x) 16) 0xff, \ + ((x) 24) 0xff + +static int soc_camera_try_fmt(struct soc_camera_device *icd, + struct v4l2_format *f) +{ + struct soc_camera_host *ici = to_soc_camera_host(icd-dev.parent); + struct v4l2_pix_format *pix = f-fmt.pix; + int ret; + + dev_dbg(icd-dev, TRY_FMT(%c%c%c%c, %ux%u)\n, + pixfmtstr(pix-pixelformat), pix-width, pix-height); + + pix-bytesperline = 0; + pix-sizeimage = 0; + + ret = ici-ops-try_fmt(icd, f); + if (ret 0) + return ret; + + if (!pix-sizeimage) { + if (!pix-bytesperline) { + const struct soc_camera_format_xlate *xlate; + + xlate = soc_camera_xlate_by_fourcc(icd, pix-pixelformat); + if (!xlate) + return -EINVAL; + + ret = soc_mbus_bytes_per_line(pix-width, + xlate-host_fmt); + if (ret 0) + pix-bytesperline = ret; + } + if (pix-bytesperline) + pix-sizeimage = pix-bytesperline * pix-height; + } + + return 0; +} + static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f) { struct soc_camera_device *icd = file-private_data; - struct soc_camera_host *ici = to_soc_camera_host(icd-dev.parent); WARN_ON(priv != file-private_data); @@ -149,7 +188,7 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv, return -EINVAL; /* limit format to hardware capabilities */ - return ici-ops-try_fmt(icd, f); + return soc_camera_try_fmt(icd, f); } static int soc_camera_enum_input(struct file *file, void *priv, @@ -362,9 +401,6 @@ static void soc_camera_free_user_formats(struct soc_camera_device *icd) icd-user_formats = NULL; } -#define pixfmtstr(x) (x) 0xff, ((x) 8) 0xff, ((x) 16) 0xff, \ - ((x) 24) 0xff - /* Called with .vb_lock held, or from the first open(2), see comment there */ static int soc_camera_set_fmt(struct soc_camera_device *icd, struct v4l2_format *f) @@ -377,7 +413,7 @@ static int soc_camera_set_fmt(struct
RE: [RFC/PATCH 0/1] New subdev sensor operation g_interface_parms
-Original Message- From: Hans Verkuil [mailto:hansv...@cisco.com] Sent: Wednesday, February 23, 2011 3:32 AM To: Hans Verkuil Cc: Sylwester Nawrocki; Guennadi Liakhovetski; Stan; linux- me...@vger.kernel.org; Laurent Pinchart; Aguirre, Sergio Subject: Re: [RFC/PATCH 0/1] New subdev sensor operation g_interface_parms On Wednesday, February 23, 2011 09:10:42 Hans Verkuil wrote: Unfortunately, if a subdev is set to 'sample at rising edge', then that does not necessarily mean that the host should sample at the same edge. Depending on the clock line routing and the integrity of the clock signal the host may actually have to sample on the other edge. And yes, I've seen this. It might be useful to give some background information regarding the sampling edge problems. There are two main reasons why the sampling edge can be hardware dependent. The first is if the data lines go through an amplifier or something similar that will slightly delay the data lines compared to the clock signal. This can shift the edge at which you have to sample. Actually, this may even be dependent on the clock frequency. I have not seen that in real life yet, but it might happen. This will complicate things even more since in that case you need to make a callback function in the board code that determines the sampling edge based on the clock frequency. I think we can ignore that for now, but we do need to keep it in mind. The other is the waveform of the clock. For relatively low frequencies this will resemble a symmetrical square wave. But for higher frequencies this more resembles the bottom waveform in this picture: http://myweb.msoe.edu/williamstm/Images/Divider2.jpg This is asymmetric so depending on the slopes the sampling edge can make quite a difference. The higher the clock frequency, the more asymmetric the waveform will look. I think this a very specific HW problem (Low slew rate), and it's something That should be fixed in HW, or avoided in SW after proper experimentation is done. One example on how to fix this is described in this document: http://www.actel.com/documents/SchmittTrigger_AN.pdf Regards, Sergio 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: [RFC/PATCH 0/1] New subdev sensor operation g_interface_parms
Hi, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Wednesday, February 23, 2011 10:15 AM To: Hans Verkuil Cc: Aguirre, Sergio; Guennadi Liakhovetski; Hans Verkuil; Sylwester Nawrocki; Stan; linux-media@vger.kernel.org Subject: Re: [RFC/PATCH 0/1] New subdev sensor operation g_interface_parms On Wednesday 23 February 2011 17:02:57 Hans Verkuil wrote: On Wednesday, February 23, 2011 16:30:42 Laurent Pinchart wrote: On Wednesday 23 February 2011 15:06:49 Aguirre, Sergio wrote: snip The only static data I am concerned about are those that affect signal integrity. After thinking carefully about this I realized that there is really only one setting that is relevant to that: the sampling edge. The polarities do not matter in this. I respectfully disagree. So do I. Sampling edge is related to polarities, so you need to take both into account. When you switch polarity for data/field/hsync/vsync signals on a simple bus you just invert whether a 1 bit is output as high or low voltage. So you just change the meaning of the bit. This does not matter for signal integrity, since you obviously have to be able to sample both low and high voltages. It is *when* you sample that can have a major effect. When you switch the polarity you will likely have to sample on the opposite edge. If, for signal integrity reasons, you can only sample on a given edge, you will want to use a fixed polarity and not negotiate it. I guess this should be reason enough to decide this in platform data in the board file. Given the very small number of parameters that are negotiated by soc- camera at the moment, I'm very much in favour of hardcoding all of them in platform data and just adding a g_interface_parms subdev operation. I'll second that. Negotiating all this adds unnecessary complexity, and just makes the code More prone to errors, and even probably causing hardware damage due to misconfiguration. It's better to keep this static and make the board config fully conscious of it. Regards, Sergio This might be different for differential clocks. I have no experience with this, so I can't say anything sensible about that. -- 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/PATCH 0/1] New subdev sensor operation g_interface_parms
Hi Guennadi, -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Wednesday, February 23, 2011 10:46 AM To: Aguirre, Sergio Cc: Laurent Pinchart; Hans Verkuil; Sylwester Nawrocki; Stan; linux- me...@vger.kernel.org Subject: RE: [RFC/PATCH 0/1] New subdev sensor operation g_interface_parms On Wed, 23 Feb 2011, Aguirre, Sergio wrote: Hi, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Wednesday, February 23, 2011 10:15 AM To: Hans Verkuil Cc: Aguirre, Sergio; Guennadi Liakhovetski; Hans Verkuil; Sylwester Nawrocki; Stan; linux-media@vger.kernel.org Subject: Re: [RFC/PATCH 0/1] New subdev sensor operation g_interface_parms On Wednesday 23 February 2011 17:02:57 Hans Verkuil wrote: On Wednesday, February 23, 2011 16:30:42 Laurent Pinchart wrote: On Wednesday 23 February 2011 15:06:49 Aguirre, Sergio wrote: snip The only static data I am concerned about are those that affect signal integrity. After thinking carefully about this I realized that there is really only one setting that is relevant to that: the sampling edge. The polarities do not matter in this. I respectfully disagree. So do I. Sampling edge is related to polarities, so you need to take both into account. When you switch polarity for data/field/hsync/vsync signals on a simple bus you just invert whether a 1 bit is output as high or low voltage. So you just change the meaning of the bit. This does not matter for signal integrity, since you obviously have to be able to sample both low and high voltages. It is *when* you sample that can have a major effect. When you switch the polarity you will likely have to sample on the opposite edge. If, for signal integrity reasons, you can only sample on a given edge, you will want to use a fixed polarity and not negotiate it. I guess this should be reason enough to decide this in platform data in the board file. Given the very small number of parameters that are negotiated by soc- camera at the moment, I'm very much in favour of hardcoding all of them in platform data and just adding a g_interface_parms subdev operation. I'll second that. Negotiating all this adds unnecessary complexity, and just makes the code More prone to errors, and even probably causing hardware damage due to misconfiguration. It's better to keep this static and make the board config fully conscious of it. Sorry, I accept different opinions, and in the end only one of the two possibilities will be implemented, and either way it'll all work in the end, but, I don't buy either of these arguments. Complexity - the code is already there, it is working, it is simple, it has not broken since it has been implemented. I had it hard-coded in the beginning and I went over to negotiation and never regretted it. First of all, it seems that this discussion is heavily parallel i/f oriented, and soc_camera focused, and it's just not like that. Now, _just_ for soc_camera framework, yeah... it works and it's there, but still not providing a solution for other v4l2_subdev users (like Media Controller). Complexity comes only when trying to make this truly generic, and avoid fragmentation of solutions (1 for soc, 1 for MC), plus adding support for serial (MIPI) interfaces Now, also, the patch originally proposed by Stan doesn't actually deal with putting polarities as part of the interface parameters, which is something you're currently negotiating in soc_camera framework, again, just for parallel interfaces. Now, just for the sake of clarifying my understanding, I guess what you're saying is to make sensor driver expose all possible polarities and physical details configurable, and make the platform data limit the actual options due to the physical layout. For example, if in my board A, I have: - OV5640 sensor driver, which supports both Parallel and CSI2 Interfaces (with up to 2 datalanes) - Rx subdev (or host) driver(s) which support Parallel, CSI2-A and CSI2-B interfaces (with 2 and 1 datalanes respectively). I should specify in my boardfile integration details, such as the sensor is actually wired to the CSI2-B I/f, so make the sensor negotiate with the other side of the bus and enable CSI2 i/f with given details, like just use 1 datalane, and match datalane position/polarity. Am I understanding right? Hardware damage - if this were the case, I'd probably be surrounded only by bricks. How configuring a wrong hsync polarity can damage your hardware? Ok, I'll regret my statement on this one. I guess I was a bit too dramatic to point out consequences of HW mismatches. Nevermind this. Regards, Sergio Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance
RE: [RFC/PATCH 0/1] New subdev sensor operation g_interface_parms
Hi, -Original Message- From: Hans Verkuil [mailto:hansv...@cisco.com] Sent: Tuesday, February 22, 2011 7:33 AM To: Guennadi Liakhovetski Cc: Stanimir Varbanov; linux-media@vger.kernel.org; laurent.pinch...@ideasonboard.com; Aguirre, Sergio Subject: Re: [RFC/PATCH 0/1] New subdev sensor operation g_interface_parms On Tuesday, February 22, 2011 12:40:32 Guennadi Liakhovetski wrote: On Tue, 22 Feb 2011, Stanimir Varbanov wrote: This RFC patch adds a new subdev sensor operation named g_interface_parms. It is planned as a not mandatory operation and it is driver's developer decision to use it or not. Please share your opinions and ideas. Stanimir, thanks for the RFC. I think it is time that we create a good solution for this. This is currently the last remaining issue preventing soc- camera subdevs from being used generally. (Control handling is also still special, but this is being worked on.) Yes, I like the idea in principle (/me pulling his bullet-proof vest on), :-) as some of you might guess, because I feel it's going away from the idea, that I've been hard pressed to accept of hard-coding the media-bus configuration and in the direction of direct communication of bus-parameters between the (sub-)devices, e.g., a camera host and a camera device in soc-camera terminology. But before reviewing the patch as such, I'd like to discuss the strategy, that we want to pursue here - what exactly do we want to hard-code and what we want to configure dynamically? As explained before, my preference would be to only specify the absolute minimum in the platform data, i.e., parameters that either are ambiguous or special for this platform. So, once again, my approach to configure interface parameters like signal polarities and edge sensitivity is: 1. if at least one side has a fixed value of the specific parameter, usually no need to specify it in platform data. Example: sensor only supports HSYNC active high, host supports both, normally high should be selected. 2. as above, but there's an inverter on the board in the signal path. The invert parameter must be specified in the platform data and the host will configure itself to low and send high confirmed to the sensor. 3. both are configurable. In this case the platform data has to specify, which polarity shall be used. This is simple, it is implemented, it has worked that way with no problem for several years now. The configuration procedure in this case looks like: 1. host requests supported interface configurations from the client (sensor) 2. host matches returned parameters against platform data and its own capabilities 3. if no suitable configuration possible - error out 4. the single possible configuration is identified and sent to the sensor back for its configuration This way we need one more method: s_interface_parms. Shortly talking to Laurent earlier today privately, he mentioned, that one of the reasons for this move is to support dynamic bus reconfiguration, e.g., the number of used CSI lanes. The same is useful for parallel interfaces. E.g., I had to hack the omap3spi driver to capture only 8 (parallel) data lanes from the sensor, connected with all its 10 lanes to get a format, easily supported by user-space applications. Ideally you don't want to change anything in the code for this. If the user is requesting the 10-bit format, all 10 lanes are used, if only 8 - the interface is reconfigured accordingly. I have no problems with dynamic bus reconfiguration as such. So if the host driver wants to do lane reconfiguration, then that's fine by me. When it comes to signal integrity (polarity, rising/falling edge), then I remain convinced that this should be set via platform data. This is not something that should be negotiated since this depends not only on the sensor and host devices, but also on the routing of the lines between them on the actual board, how much noise there is on those lines, the quality of the clock signal, etc. Not really an issue with PAL/NTSC type signals, but when you get to 1080p60 and up, then such things become much more important. So these settings should not be negotiated, but set explicitly. It actually doesn't have to be done through platform data (although that makes the most sense), as long as it is explicitly set based on board-specific data. My 2 cents here is that I think this consists in 2 parts, and should be divided properly. 1. You know required # of lanes clockspeed after you had set the format. For example: - VGA @ 30fps DDR Clk: 330 MHz Number of Datalanes: 1 - VGA @ 60fps DDR Clk: 330 MHz Number of Datalanes: 2 - 12MPix @ 10fps DDR Clk: 480 MHz Number of Datalanes: 2 This usually is something you know from the sensor config selected based on your params. So, I think this is something that the host
[Query] Soc_camera: Passing MIPI bus physical link details
Hi Guennadi, I have been working internally in a private 2.6.35.7 kernel with the TI OMAP4 platform, and as I have a very simple camera support driver (It just enables a CSI2 Rx Interface), i decided to go for a soc-camera host implementation. Now, I see that there is a set_bus_param callback function for host drivers, which if i understand correctly, it tries to negotiate the bus parameters between the host and the client. But what i notice is that this seems to be mostly oriented towards a parallel interface, as most of the things won't make much sense in MIPI CSI2 spec (HSYNC_ACTIVE_HIGH, DATAWIDTH_x, etc.) I was wondering what's the best way to be able to tell to the host driver MIPI specific details such as, how many datalanes the interface is using, and the MIPI Clk speed in which the sensor will be transmitting the data. Does it make sense to expand this inside the [query/set]_bus_param APIs? or Will it be better to implement a new v4l2_subdev_sensor_ops entry, something like g_mipi_params? Regards, Sergio -- 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: [Query][videobuf-dma-sg] Pages in Highmem handling
Hi Mauro, -Original Message- From: Mauro Carvalho Chehab [mailto:mche...@redhat.com] Sent: Monday, December 20, 2010 5:23 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org; Warren, Christina; Boateng, Akwasi; Russell King Subject: Re: [Query][videobuf-dma-sg] Pages in Highmem handling Hi Sergio, Em 27-08-2010 11:57, Aguirre, Sergio escreveu: Hi, I see that in current videobuf library, for DMA SG code, these functions fail when Detecting a page in Highmem region: - videobuf_pages_to_sg - videobuf_vmalloc_to_sg Now, what's the real reason to not allow handling of Highmem pages? Is it an assumption that _always_ HighMem is not reachable by DMA? I guess my point is, OMAP platform (and maybe other platforms) can handle Highmem pages without any problem. I commented these validations: 65 static struct scatterlist *videobuf_vmalloc_to_sg(unsigned char *virt, 66 int nr_pages) 67 { ... 77 for (i = 0; i nr_pages; i++, virt += PAGE_SIZE) { 78 pg = vmalloc_to_page(virt); 79 if (NULL == pg) 80 goto err; 81 /* BUG_ON(PageHighMem(pg)); */ ... 96 static struct scatterlist *videobuf_pages_to_sg(struct page **pages, 97 int nr_pages, int offset) 98 { ... 109 /* if (PageHighMem(pages[0])) */ 110 /* DMA to highmem pages might not work */ 111 /* goto highmem; */ 112 sg_set_page(sglist[0], pages[0], PAGE_SIZE - offset, offset); 113 for (i = 1; i nr_pages; i++) { 114 if (NULL == pages[i]) 115 goto nopage; 116 /* if (PageHighMem(pages[i])) 117 goto highmem; */ 118 sg_set_page(sglist[i], pages[i], PAGE_SIZE, 0); 119 } Can somebody shed any light on this? Sorry for taking so long to answer you. Hey, no worries. That's fine. Basically, videobuf code were written at Linux 2.4 days, to be used by bttv driver (and later used by cx88 and saa7134). At that time, there where a hack for the usage of highmem (I think it was called bigmem or something like that). As I was not maintaining the code on that time, I'm not really sure what where the issues, but I suspect that this were an arch-implementation limit related to DMA transfers at highmem, on that time, due to x86 intrinsic limits. I'm not sure about the current limits of newer x86 chips, on 32 and on 64 bits mode, but i think that this limit doesn't exist anymore. So, I suspect that just converting it to a call to dma_capable() should be enough to fix the issue. Ok, sounds reasonable. Yet, as videobuf2 is almost ready for merge, maybe the best is to take some efforts on testing it, and to be sure that it doesn't contain any arch-specific limits inside its code. So, do you want me to make a patch for this, or is this already taken care on videobuf2? Regards, Sergio Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [omap3isp][PATCH v2 8/9] omap3isp: ccp2: Make SYSCONFIG fields consistent
Hi, -Original Message- From: David Cohen [mailto:david.co...@nokia.com] Sent: Saturday, November 20, 2010 4:49 AM To: ext Laurent Pinchart Cc: Aguirre, Sergio; linux-media@vger.kernel.org Subject: Re: [omap3isp][PATCH v2 8/9] omap3isp: ccp2: Make SYSCONFIG fields consistent On Sat, Nov 20, 2010 at 11:34:04AM +0100, ext Laurent Pinchart wrote: Hi, On Saturday 20 November 2010 11:00:30 David Cohen wrote: On Sat, Nov 20, 2010 at 12:10:11AM +0100, ext Aguirre, Sergio wrote: On Friday, November 19, 2010 9:44 AM Aguirre, Sergio wrote: On Friday, November 19, 2010 4:06 AM David Cohen wrote: On Mon, Nov 15, 2010 at 03:30:00PM +0100, ext Sergio Aguirre wrote: Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/ispccp2.c |3 +-- drivers/media/video/isp/ispreg.h | 14 -- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/media/video/isp/ispccp2.c b/drivers/media/video/isp/ispccp2.c index fa23394..3127a74 100644 --- a/drivers/media/video/isp/ispccp2.c +++ b/drivers/media/video/isp/ispccp2.c @@ -419,8 +419,7 @@ static void ispccp2_mem_configure(struct isp_ccp2_device *ccp2, config-src_ofst = 0; } - isp_reg_writel(isp, (ISPCSI1_MIDLEMODE_SMARTSTANDBY -ISPCSI1_MIDLEMODE_SHIFT), + isp_reg_writel(isp, ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SMART, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_SYSCONFIG); To make your cleanup even better, you could change isp_reg_clr_set() instead. If CCP2 MSTANDY_MODE is set to NOSTANDY, the result is going to be an invalid 0x3 value. Despite it cannot happen with the current code, it's still better to avoid such situations for future versions. :) Hmm you're right, I guess I didn't do any real functional changes, just defines renaming. I can repost an updated patch with this suggestion. No problem. David, Geez I guess I wasn't paying much attention to this either. Sorry. The case you mention about it potentially become 0x3 is not possible, because, I'm basically overwriting the whole register with (0x2 12) Notice the isp_reg_write, there's no OR operation... That's correct. My mistake. IMO ISP power settings still need a better way to be setup, but it definitively does not belong to your cleanup patch. I'm fine with your changes. Now, this means that we have been implicitly setting other fields as Zeroes (SOFT_RESET = 0, and AUTO_IDLE = 0) aswell. Has AUTOIDLE in CCP2 been tried in N900? I don't have a CCP2 sensor to test with :(. I have no idea. Indeed, AUTOIDLE in ISP doesn't seem to be much reliable so far. It should be set to 0 until somebody proves it can be set to 1 :) To summarize things, we're only setting the CCP2 AUTOIDLE bit on ES 1.0 devices, and we're clearing it anyway ispccp2_mem_configure(). For the sake of correctness we should replace isp_reg_writel() by isp_reg_clr_set() in ispccp2_mem_configure(), which will only make a difference on ES 1.0 devices that have basically no users. Is that OK with both of you ? Yes, It's fine with me. I'll say that if this doesn't change things for N900, which appears to be the only community available device (I know of) that uses CCP2 cameras, then we shouldn't care. If somebody comes up with an ES1.0 in the future, and he does see problems, then we will address that. But that would be a very weird case. In fact, it will be most probably someone from TI with a hidden board buried in some box of scraps, which happens to have hacked the board for a CCP2 camera... The more I think about it, the more I'm getting convinced this shouldn't be a worry. :) Regards, Sergio I'm fine with both solutions. IMO power settings could be improved and not be hardcoded. It could come from platform data. But that's another case. :) Br, David -- 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
RE: [omap3isp RFC][PATCH 1/4] omap3isp: Abstract isp subdevs clock control
-Original Message- From: David Cohen [mailto:david.co...@nokia.com] Sent: Saturday, November 20, 2010 5:27 AM To: Aguirre, Sergio Cc: Laurent Pinchart; linux-media@vger.kernel.org Subject: Re: [omap3isp RFC][PATCH 1/4] omap3isp: Abstract isp subdevs clock control Hi Sergio, Hi David, Thanks for the patch. :) Thanks for the review. On Sat, Nov 20, 2010 at 12:23:48AM +0100, ext Sergio Aguirre wrote: Submodules shouldn't be aware of global register bit structure, specially if the submodules are shared in the future with other TI architectures (Davinci, future OMAPs, etc) Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/isp.c| 54 ++ drivers/media/video/isp/isp.h| 12 +++ drivers/media/video/isp/ispccdc.c|6 +-- drivers/media/video/isp/isppreview.c |6 +-- drivers/media/video/isp/ispresizer.c |6 +-- 5 files changed, 72 insertions(+), 12 deletions(-) diff --git a/drivers/media/video/isp/isp.c b/drivers/media/video/isp/isp.c index 30bdc48..2e5030f 100644 --- a/drivers/media/video/isp/isp.c +++ b/drivers/media/video/isp/isp.c @@ -991,6 +991,60 @@ void isp_sbl_disable(struct isp_device *isp, enum isp_sbl_resource res) * Clock management */ +void isp_subclk_enable(struct isp_device *isp, enum isp_subclk_resource res) +{ + u32 clk = 0; + + isp-subclk_resources |= res; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_H3A) + clk |= ISPCTRL_H3A_CLK_EN; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_HIST) + clk |= ISPCTRL_HIST_CLK_EN; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_RESIZER) + clk |= ISPCTRL_RSZ_CLK_EN; + + /* NOTE: For CCDC Preview submodules, we need to affect internal +* RAM aswell. +*/ + if (isp-subclk_resources OMAP3_ISP_SUBCLK_CCDC) + clk |= ISPCTRL_CCDC_CLK_EN | ISPCTRL_CCDC_RAM_EN; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_PREVIEW) + clk |= ISPCTRL_PREV_CLK_EN | ISPCTRL_PREV_RAM_EN; + + isp_reg_set(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL, clk); +} To decrease the lines of code, you can create a new static function called e.g. isp_subclk_update(). It can be a copy of isp_subclk_enable() but removing the line where it changes 'isp-subclk_resources' and replacing 'isp_reg_set(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL, clk)' by 'isp_reg_clr_set(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL, clocks mask, clk)'. Then, isp_subclk_enable() can update isp-subclk_resources, call isp_subclk_update() and finish. Sounds nicer. Sure, I can redesign it that way. Thanks for the suggestion! + +void isp_subclk_disable(struct isp_device *isp, enum isp_subclk_resource res) +{ + u32 clk = 0; + + isp-subclk_resources = ~res; Same here. This function can call isp_subclk_update() and end now. Agreed. + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_H3A)) + clk |= ISPCTRL_H3A_CLK_EN; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_HIST)) + clk |= ISPCTRL_HIST_CLK_EN; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_RESIZER)) + clk |= ISPCTRL_RSZ_CLK_EN; + + /* NOTE: For CCDC Preview submodules, we need to affect internal +* RAM aswell. +*/ + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_CCDC)) + clk |= ISPCTRL_CCDC_CLK_EN | ISPCTRL_CCDC_RAM_EN; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_PREVIEW)) + clk |= ISPCTRL_PREV_CLK_EN | ISPCTRL_PREV_RAM_EN; + + isp_reg_clr(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL, clk); +} + /* * isp_enable_clocks - Enable ISP clocks * @isp: OMAP3 ISP device diff --git a/drivers/media/video/isp/isp.h b/drivers/media/video/isp/isp.h index b8f63e2..1260e9f 100644 --- a/drivers/media/video/isp/isp.h +++ b/drivers/media/video/isp/isp.h @@ -85,6 +85,14 @@ enum isp_sbl_resource { OMAP3_ISP_SBL_RESIZER_WRITE = 0x200, }; +enum isp_subclk_resource { + OMAP3_ISP_SUBCLK_CCDC = 0x1, + OMAP3_ISP_SUBCLK_H3A= 0x2, + OMAP3_ISP_SUBCLK_HIST = 0x4, + OMAP3_ISP_SUBCLK_PREVIEW= 0x8, + OMAP3_ISP_SUBCLK_RESIZER= 0x10, I'm fine with this way, but (1 0), (1 1), ... has a better readability. I guess I just replicated above enum for consistency, but I agree that your suggestion looks better. I'll change it. Obrigado, Sergio Br, David Cohen +}; + enum isp_interface_type { ISP_INTERFACE_PARALLEL, ISP_INTERFACE_CSI2A_PHY2, @@ -262,6 +270,7 @@ struct isp_device { struct isp_csiphy isp_csiphy2; unsigned int sbl_resources; + unsigned int subclk_resources; struct iommu *iommu; }; @@ -294,6 +303,9 @@ void isp_print_status(struct isp_device *isp); void isp_sbl_enable(struct isp_device *isp, enum
RE: [omap3isp RFC][PATCH 2/4] omap3isp: Move CCDC LSC prefetch wait to main isp code
Hi David and Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Saturday, November 20, 2010 11:29 AM To: David Cohen Cc: Aguirre, Sergio; linux-media@vger.kernel.org Subject: Re: [omap3isp RFC][PATCH 2/4] omap3isp: Move CCDC LSC prefetch wait to main isp code Hi David, On Saturday 20 November 2010 12:39:56 David Cohen wrote: On Sat, Nov 20, 2010 at 12:23:49AM +0100, ext Sergio Aguirre wrote: Since this sequence strictly touches ISP global registers, it's not really part of the same register address space than the CCDC. Do this check in main isp code instead. Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/isp.c | 24 drivers/media/video/isp/isp.h |2 ++ drivers/media/video/isp/ispccdc.c | 26 +- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/drivers/media/video/isp/isp.c b/drivers/media/video/isp/isp.c index 2e5030f..ee45eb6 100644 --- a/drivers/media/video/isp/isp.c +++ b/drivers/media/video/isp/isp.c @@ -339,6 +339,30 @@ void isphist_dma_done(struct isp_device *isp) } } +int ispccdc_lsc_wait_prefetch(struct isp_device *isp) This is up to you, but to ensure this function now belongs to ISP core and not CCDC anymore, I would change the function name to something like isp_ccdc_lsc_wait_prefetch(). isp_* is prefix for ISP core and ispccdc_* is prefix for CCDC driver. I know we have the isphist_dma_done() inside ISP core, but changing it to isp_hist_dma_done could be a good cleanup as well. But this is my opinion only. :) I agree. I plan to submit a patch at some point that will rename all non- static functions to use the omap3isp_ prefix instead of the isp_ prefix. Static functions should use the module name as prefix (ccdc_, preview_, ...). Sounds good. It will be a simple patch but will conflict with pretty much everything, so I'll try to push at at a quiet time (or at least quiet enough) to minimize disturbances. I will also see if I can use spatch [1] to generate it. Ok, so, sounds to me then this leaves my patch as-is, and we'll wait for Laurent's coccinelle-assisted changes on top in the future. Is that correct? Regards, Sergio [1] http://coccinelle.lip6.fr/ -- 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 RFC][PATCH 0/4] Improve inter subdev interaction
-Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Saturday, November 20, 2010 11:31 AM To: David Cohen Cc: Aguirre, Sergio; linux-media@vger.kernel.org Subject: Re: [omap3isp RFC][PATCH 0/4] Improve inter subdev interaction Hi David, On Saturday 20 November 2010 12:44:27 David Cohen wrote: On Sat, Nov 20, 2010 at 12:23:47AM +0100, ext Sergio Aguirre wrote: Hi, These are some patches to make these operations more generic: - Clock control is being controlled in a very crude manner by subdevices, it should be centralized in isp.c. - LSC prefetch wait check is reading a main ISP register, so move it to isp.c - Abstract SBL busy check: we don't want a submodule thinkering with main ISP registers. That should be done in the main isp.c Also, remove main ISP register dump from CSI2 specific dump. We should be using isp_print_status if we'll like to know main ISP regdump. Comments are welcome. More cleanups for better subdevice isolation are on the way. Your patches are fine for me. I sent you some comments, but they are opitional and it's up to you to decide what to do. :) You can copy linux-omap@ as well in future patches. David and Laurent, I appreciate a lot your review time and comments. I will try to submit the next version of the omap3isp driver for upstream review, either at the end of the weekend or on Monday. I will cross-post the driver to linux-media, linux-omap and LKML. Let's be ready to defend the media controller and the omap3isp driver :-) Yeah! :) Until then let's not spam linux-omap with patches for a driver they don't know about. Agreed. Regards, Sergio -- 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 RFC][PATCH 2/4] omap3isp: Move CCDC LSC prefetch wait to main isp code
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Monday, November 22, 2010 9:35 AM To: Aguirre, Sergio Cc: David Cohen; linux-media@vger.kernel.org Subject: Re: [omap3isp RFC][PATCH 2/4] omap3isp: Move CCDC LSC prefetch wait to main isp code Hi Sergio, On Monday 22 November 2010 16:29:46 Aguirre, Sergio wrote: On Saturday, November 20, 2010 11:29 AM Aguirre, Sergio wrote: On Saturday 20 November 2010 12:39:56 David Cohen wrote: On Sat, Nov 20, 2010 at 12:23:49AM +0100, ext Sergio Aguirre wrote: Since this sequence strictly touches ISP global registers, it's not really part of the same register address space than the CCDC. Do this check in main isp code instead. Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/isp.c | 24 drivers/media/video/isp/isp.h |2 ++ drivers/media/video/isp/ispccdc.c | 26 +--- -- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/drivers/media/video/isp/isp.c b/drivers/media/video/isp/isp.c index 2e5030f..ee45eb6 100644 --- a/drivers/media/video/isp/isp.c +++ b/drivers/media/video/isp/isp.c @@ -339,6 +339,30 @@ void isphist_dma_done(struct isp_device *isp) } } +int ispccdc_lsc_wait_prefetch(struct isp_device *isp) This is up to you, but to ensure this function now belongs to ISP core and not CCDC anymore, I would change the function name to something like isp_ccdc_lsc_wait_prefetch(). isp_* is prefix for ISP core and ispccdc_* is prefix for CCDC driver. I know we have the isphist_dma_done() inside ISP core, but changing it to isp_hist_dma_done could be a good cleanup as well. But this is my opinion only. :) I agree. I plan to submit a patch at some point that will rename all non- static functions to use the omap3isp_ prefix instead of the isp_ prefix. Static functions should use the module name as prefix (ccdc_, preview_, ...). Sounds good. It will be a simple patch but will conflict with pretty much everything, so I'll try to push at at a quiet time (or at least quiet enough) to minimize disturbances. I will also see if I can use spatch [1] to generate it. Ok, so, sounds to me then this leaves my patch as-is, and we'll wait for Laurent's coccinelle-assisted changes on top in the future. Is that correct? As you need to resubmit the serie anyway, I'd appreciate if you could already make the change. Ok, sure. Not a problem for me. Will add that change in v2 of this patch series. Thanks for the clarification. Regards, Sergio -- 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
Please ignore... (RE: [omap3isp RFC][PATCH v2 0/4] Improve inter subdev interaction)
-Original Message- From: Aguirre, Sergio Sent: Monday, November 22, 2010 1:48 PM To: Laurent Pinchart Cc: linux-media@vger.kernel.org; David Cohen; Aguirre, Sergio Subject: [omap3isp RFC][PATCH v2 0/4] Improve inter subdev interaction Hi, These are some patches to make these operations more generic: - Clock control is being controlled in a very crude manner by subdevices, it should be centralized in isp.c. - LSC prefetch wait check is reading a main ISP register, so move it to isp.c - Abstract SBL busy check: we don't want a submodule thinkering with main ISP registers. That should be done in the main isp.c Also, remove main ISP register dump from CSI2 specific dump. We should be using isp_print_status if we'll like to know main ISP regdump. Comments are welcome. More cleanups for better subdevice isolation are on the way. Regards, Sergio Oops, I resent the same patches with just an updated cover letter... I'm resending the correct ones now... Please ignore this patch series. I'm very sorry for the spamming. Regards, Sergio Changelog: v2: - Improved logic in isp subdevs clock control (Thanks David Cohen) - Renamed ispccdc_lsc_wait_prefetch - isp_ccdc_* to be clear on function declaration new location (isp.c) (Thanks David and Laurent) v1: - Initial version Sergio Aguirre (4): omap3isp: Abstract isp subdevs clock control omap3isp: Move CCDC LSC prefetch wait to main isp code omap3isp: sbl: Abstract SBL busy check omap3isp: csi2: Don't dump ISP main registers drivers/media/video/isp/isp.c| 95 ++ drivers/media/video/isp/isp.h| 16 ++ drivers/media/video/isp/ispccdc.c| 42 ++- drivers/media/video/isp/ispcsi2.c|7 --- drivers/media/video/isp/isppreview.c |6 +-- drivers/media/video/isp/ispresizer.c |6 +-- 6 files changed, 119 insertions(+), 53 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: [omap3isp][PATCH v2 8/9] omap3isp: ccp2: Make SYSCONFIG fields consistent
-Original Message- From: David Cohen [mailto:david.co...@nokia.com] Sent: Friday, November 19, 2010 4:06 AM To: Aguirre, Sergio Cc: Laurent Pinchart; linux-media@vger.kernel.org Subject: Re: [omap3isp][PATCH v2 8/9] omap3isp: ccp2: Make SYSCONFIG fields consistent Hi Sergio, Hi David, Thanks for the review. I've few comments below. On Mon, Nov 15, 2010 at 03:30:00PM +0100, ext Sergio Aguirre wrote: Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/ispccp2.c |3 +-- drivers/media/video/isp/ispreg.h | 14 -- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/media/video/isp/ispccp2.c b/drivers/media/video/isp/ispccp2.c index fa23394..3127a74 100644 --- a/drivers/media/video/isp/ispccp2.c +++ b/drivers/media/video/isp/ispccp2.c @@ -419,8 +419,7 @@ static void ispccp2_mem_configure(struct isp_ccp2_device *ccp2, config-src_ofst = 0; } - isp_reg_writel(isp, (ISPCSI1_MIDLEMODE_SMARTSTANDBY - ISPCSI1_MIDLEMODE_SHIFT), + isp_reg_writel(isp, ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SMART, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_SYSCONFIG); To make your cleanup even better, you could change isp_reg_clr_set() instead. If CCP2 MSTANDY_MODE is set to NOSTANDY, the result is going to be an invalid 0x3 value. Despite it cannot happen with the current code, it's still better to avoid such situations for future versions. :) Hmm you're right, I guess I didn't do any real functional changes, just defines renaming. I can repost an updated patch with this suggestion. No problem. /* Hsize, Skip */ diff --git a/drivers/media/video/isp/ispreg.h b/drivers/media/video/isp/ispreg.h index d885541..9b0d3ad 100644 --- a/drivers/media/video/isp/ispreg.h +++ b/drivers/media/video/isp/ispreg.h @@ -141,6 +141,14 @@ #define ISPCCP2_REVISION (0x000) #define ISPCCP2_SYSCONFIG (0x004) #define ISPCCP2_SYSCONFIG_SOFT_RESET (1 1) +#define ISPCCP2_SYSCONFIG_AUTO_IDLE0x1 +#define ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SHIFT 12 +#define ISPCCP2_SYSCONFIG_MSTANDBY_MODE_FORCE \ + (0x0 ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SHIFT) +#define ISPCCP2_SYSCONFIG_MSTANDBY_MODE_NO \ + (0x1 ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SHIFT) +#define ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SMART \ + (0x2 ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SHIFT) You can add some ISPCCP2_SYSCONFIG_MSTANDY_MODE_MASK, as 2 bits must be always set together. Sure, will do. Thanks and Regards, Sergio Regards, David Cohen #define ISPCCP2_SYSSTATUS (0x008) #define ISPCCP2_SYSSTATUS_RESET_DONE (1 0) #define ISPCCP2_LC01_IRQENABLE (0x00C) @@ -1309,12 +1317,6 @@ #define ISPMMU_SIDLEMODE_SMARTIDLE 2 #define ISPMMU_SIDLEMODE_SHIFT 3 -#define ISPCSI1_AUTOIDLE 0x1 -#define ISPCSI1_MIDLEMODE_SHIFT12 -#define ISPCSI1_MIDLEMODE_FORCESTANDBY 0x0 -#define ISPCSI1_MIDLEMODE_NOSTANDBY0x1 -#define ISPCSI1_MIDLEMODE_SMARTSTANDBY 0x2 - /* - * CSI2 receiver registers (ES2.0) */ -- 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 -- 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][PATCH v2 7/9] omap3isp: Cleanup isp_power_settings
-Original Message- From: David Cohen [mailto:david.co...@nokia.com] Sent: Friday, November 19, 2010 4:18 AM To: Aguirre, Sergio Cc: Laurent Pinchart; linux-media@vger.kernel.org Subject: Re: [omap3isp][PATCH v2 7/9] omap3isp: Cleanup isp_power_settings Hi Sergio, Thanks for the patch. Hi David, Thanks for the comments. On Mon, Nov 15, 2010 at 03:29:59PM +0100, ext Sergio Aguirre wrote: 1. Get rid of CSI2 / CCP2 power settings, as they are controlled in the receivers code anyways. CCP2 is not correctly handling this. It's setting SMART STANDBY mode one when reading from memory. You should fix it before remove such code from ISP core driver. Ok, agreed. I'll generate a new patch before this to compensate that. Not a problem. 2. Avoid code duplication. Agree. But only after considering the comment above. Ok. Thanks and Regards, Sergio Regards, David Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/isp.c | 49 ++ --- 1 files changed, 7 insertions(+), 42 deletions(-) diff --git a/drivers/media/video/isp/isp.c b/drivers/media/video/isp/isp.c index de9352b..30bdc48 100644 --- a/drivers/media/video/isp/isp.c +++ b/drivers/media/video/isp/isp.c @@ -254,48 +254,13 @@ EXPORT_SYMBOL(isp_set_xclk); */ static void isp_power_settings(struct isp_device *isp, int idle) { - if (idle) { - isp_reg_writel(isp, - (ISP_SYSCONFIG_MIDLEMODE_SMARTSTANDBY - ISP_SYSCONFIG_MIDLEMODE_SHIFT), - OMAP3_ISP_IOMEM_MAIN, ISP_SYSCONFIG); - if (omap_rev() == OMAP3430_REV_ES1_0) { - isp_reg_writel(isp, ISPCSI1_AUTOIDLE | - (ISPCSI1_MIDLEMODE_SMARTSTANDBY - ISPCSI1_MIDLEMODE_SHIFT), - OMAP3_ISP_IOMEM_CSI2A_REGS1, - ISPCSI2_SYSCONFIG); - isp_reg_writel(isp, ISPCSI1_AUTOIDLE | - (ISPCSI1_MIDLEMODE_SMARTSTANDBY - ISPCSI1_MIDLEMODE_SHIFT), - OMAP3_ISP_IOMEM_CCP2, - ISPCCP2_SYSCONFIG); - } - isp_reg_writel(isp, ISPCTRL_SBL_AUTOIDLE, OMAP3_ISP_IOMEM_MAIN, - ISP_CTRL); - - } else { - isp_reg_writel(isp, - (ISP_SYSCONFIG_MIDLEMODE_FORCESTANDBY - ISP_SYSCONFIG_MIDLEMODE_SHIFT), - OMAP3_ISP_IOMEM_MAIN, ISP_SYSCONFIG); - if (omap_rev() == OMAP3430_REV_ES1_0) { - isp_reg_writel(isp, ISPCSI1_AUTOIDLE | - (ISPCSI1_MIDLEMODE_FORCESTANDBY - ISPCSI1_MIDLEMODE_SHIFT), - OMAP3_ISP_IOMEM_CSI2A_REGS1, - ISPCSI2_SYSCONFIG); - - isp_reg_writel(isp, ISPCSI1_AUTOIDLE | - (ISPCSI1_MIDLEMODE_FORCESTANDBY - ISPCSI1_MIDLEMODE_SHIFT), - OMAP3_ISP_IOMEM_CCP2, - ISPCCP2_SYSCONFIG); - } - - isp_reg_writel(isp, ISPCTRL_SBL_AUTOIDLE, OMAP3_ISP_IOMEM_MAIN, - ISP_CTRL); - } + isp_reg_writel(isp, + ((idle ? ISP_SYSCONFIG_MIDLEMODE_SMARTSTANDBY : + ISP_SYSCONFIG_MIDLEMODE_FORCESTANDBY) + ISP_SYSCONFIG_MIDLEMODE_SHIFT), + OMAP3_ISP_IOMEM_MAIN, ISP_SYSCONFIG); + isp_reg_writel(isp, ISPCTRL_SBL_AUTOIDLE, OMAP3_ISP_IOMEM_MAIN, + ISP_CTRL); } /* -- 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 -- 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][PATCH v2 5/9] omap3isp: Remove unused CBUFF register access
Hi Laurent and David, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, November 19, 2010 4:33 AM To: David Cohen Cc: Aguirre, Sergio; linux-media@vger.kernel.org Subject: Re: [omap3isp][PATCH v2 5/9] omap3isp: Remove unused CBUFF register access Hi David, On Friday 19 November 2010 11:19:44 David Cohen wrote: On Mon, Nov 15, 2010 at 03:29:57PM +0100, ext Sergio Aguirre wrote: [snip] @@ -244,26 +239,6 @@ #define ISP_CSIB_SYSCONFIG ISPCCP2_SYSCONFIG #define ISP_CSIA_SYSCONFIG ISPCSI2_SYSCONFIG -/* ISP_CBUFF Registers */ - -#define ISP_CBUFF_SYSCONFIG (0x010) -#define ISP_CBUFF_IRQENABLE (0x01C) - -#define ISP_CBUFF0_CTRL (0x020) -#define ISP_CBUFF1_CTRL (0x024) - -#define ISP_CBUFF0_START (0x040) -#define ISP_CBUFF1_START (0x044) - -#define ISP_CBUFF0_END (0x050) -#define ISP_CBUFF1_END (0x054) - -#define ISP_CBUFF0_WINDOWSIZE(0x060) -#define ISP_CBUFF1_WINDOWSIZE(0x064) - -#define ISP_CBUFF0_THRESHOLD (0x070) -#define ISP_CBUFF1_THRESHOLD (0x074) - No need to remove the registers from header file. We're not using them on current version, but it doesn't mean we won't use ever. :) I would have made the same comment for other registers, but we're not using the CBUFF module at all here, with no plans to use it in the future. It might not be worth it keeping the register definitions. I have no strong feeling about it, I'm fine with both choices. David, IMO, we should not introduce dead code/unusued defines in the first omap3isp upstream version. I think it's already quite hard to review for somebody outside the omap3isp development team. Having all this just in case will most probably end up in bulk, as we might never implement the CBUFF HW block, as Laurent mentions. I'll be more biased on all this being re-added if we end up implementing a ispcbuff submodule. Regards, Sergio -- 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
[omap3isp] Prefered patch base for latest code? (was: RE: Translation faults with OMAP ISP)
Hi David and Laurent, snip Don't forget that Lane is using an older version of the OMAP3 ISP driver. The bug might have been fixed in the latest code. Hm. We did fix some iommu faults. Maybe it's better to test a newer version instead. If you still see that bug using an up-to-date version, please report it and I can try to help you. :) How close is this tree from the latest internal version you guys work with? http://meego.gitorious.com/maemo-multimedia/omap3isp-rx51/commits/devel I have been basing my patches on top of this tree: http://git.linuxtv.org/pinchartl/media.git?h=refs/heads/media-0004-omap3isp Would it be better to be based on the gitorious tree instead? What do you think? Regards, Sergio Regards, David -- 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
RE: [omap3isp] Prefered patch base for latest code? (was: RE: Translation faults with OMAP ISP)
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, November 19, 2010 10:16 AM To: Aguirre, Sergio Cc: David Cohen; ext Lane Brooks; linux-media@vger.kernel.org Subject: Re: [omap3isp] Prefered patch base for latest code? (was: RE: Translation faults with OMAP ISP) Hi Sergio, On Friday 19 November 2010 17:07:09 Aguirre, Sergio wrote: Hi David and Laurent, snip Don't forget that Lane is using an older version of the OMAP3 ISP driver. The bug might have been fixed in the latest code. Hm. We did fix some iommu faults. Maybe it's better to test a newer version instead. If you still see that bug using an up-to-date version, please report it and I can try to help you. :) How close is this tree from the latest internal version you guys work with? http://meego.gitorious.com/maemo-multimedia/omap3isp-rx51/commits/devel There's less differences between gitorious and our internal tree than between linuxtv and our internal tree. Ok, I guess I can treat above tree as an omap3isp-next tree then, to have a sneak preview of what's coming ;) I have been basing my patches on top of this tree: http://git.linuxtv.org/pinchartl/media.git?h=refs/heads/media-0004- omap3isp Would it be better to be based on the gitorious tree instead? What do you think? I will push a big patch that ports all the changes made to the MC and V4L2 core, as well as omap3-isp driver, during public review to our internal tree early next week (should be on Monday). The trees will then get in sync. The gitorious tree will be updated as well. Unless you want your patches to be applied before Monday, which won't happen anyway, please base them on the linuxtv tree :-) Ok, sure. It's better for me, since the gitorious devel tree includes rx51 support which I don't even have a device to try it out :P I'll continue to be based on media-0004-omap3isp branch then, and just watch devel branch to know if I'm not doing something conflicting. Regards, Sergio -- 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] Prefered patch base for latest code? (was: RE: Translation faults with OMAP ISP)
Hi Laurent, -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Laurent Pinchart Sent: Friday, November 19, 2010 10:32 AM To: Aguirre, Sergio Cc: David Cohen; ext Lane Brooks; linux-media@vger.kernel.org Subject: Re: [omap3isp] Prefered patch base for latest code? (was: RE: Translation faults with OMAP ISP) Hi Sergio, On Friday 19 November 2010 17:23:45 Aguirre, Sergio wrote: On Friday, November 19, 2010 10:16 AM Aguirre, Sergio wrote: On Friday 19 November 2010 17:07:09 Aguirre, Sergio wrote: [snip] How close is this tree from the latest internal version you guys work with? http://meego.gitorious.com/maemo-multimedia/omap3isp- rx51/commits/devel There's less differences between gitorious and our internal tree than between linuxtv and our internal tree. Ok, I guess I can treat above tree as an omap3isp-next tree then, to have a sneak preview of what's coming ;) I haven't expressed myself clearly enough. The gitorious tree is currently more in sync with our internal tree that the linuxtv is for a simple reason: both our internal tree and the gitorious tree are missing modifications made during the public review process. Ok. Sorry, I think I didn't quite understood that. Patches published from our internal tree are always pushed to linuxtv and gitorious at the same time (or mostly). Please don't use the gitorious tree for anything else than trying the driver on the N900. I see. So I probably won't worry about this tree at all, since I don't have an N900. I'm trying this in my Zoom board w/OMAP3630, and I have a Beagleboard xM handy aswell (OMAP3730), so In my tree I'll try to keep support for both of These. Also, I'm working on trying to bring this in a very different chip, but that's a secret ;) That's why I'm working in doing cleanups. Regards, Sergio -- 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
RE: [omap3isp][PATCH v2 7/9] omap3isp: Cleanup isp_power_settings
-Original Message- From: Aguirre, Sergio Sent: Friday, November 19, 2010 9:46 AM To: 'David Cohen' Cc: Laurent Pinchart; linux-media@vger.kernel.org Subject: RE: [omap3isp][PATCH v2 7/9] omap3isp: Cleanup isp_power_settings -Original Message- From: David Cohen [mailto:david.co...@nokia.com] Sent: Friday, November 19, 2010 4:18 AM To: Aguirre, Sergio Cc: Laurent Pinchart; linux-media@vger.kernel.org Subject: Re: [omap3isp][PATCH v2 7/9] omap3isp: Cleanup isp_power_settings Hi Sergio, Thanks for the patch. Hi David, Thanks for the comments. On Mon, Nov 15, 2010 at 03:29:59PM +0100, ext Sergio Aguirre wrote: 1. Get rid of CSI2 / CCP2 power settings, as they are controlled in the receivers code anyways. CCP2 is not correctly handling this. It's setting SMART STANDBY mode one when reading from memory. You should fix it before remove such code from ISP core driver. Ok, agreed. I'll generate a new patch before this to compensate that. Not a problem. Is N900 seeing any functional difference w/ this patch? Actually, I reanalyzed the patch, and this code should be unexecuted, since it is conditioned to 3430 ES1.0 chip (omap_rev() == OMAP3430_REV_ES1_0), which I don't think much people has access. And not definitely any production quality device. Even the N900 is using ES3.1 or something like that AFAIK. So, It should not make any functional difference, unless you have an ES1.0. Regards, Sergio 2. Avoid code duplication. Agree. But only after considering the comment above. Ok. Thanks and Regards, Sergio Regards, David Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/isp.c | 49 ++-- -- --- 1 files changed, 7 insertions(+), 42 deletions(-) diff --git a/drivers/media/video/isp/isp.c b/drivers/media/video/isp/isp.c index de9352b..30bdc48 100644 --- a/drivers/media/video/isp/isp.c +++ b/drivers/media/video/isp/isp.c @@ -254,48 +254,13 @@ EXPORT_SYMBOL(isp_set_xclk); */ static void isp_power_settings(struct isp_device *isp, int idle) { - if (idle) { - isp_reg_writel(isp, -(ISP_SYSCONFIG_MIDLEMODE_SMARTSTANDBY - ISP_SYSCONFIG_MIDLEMODE_SHIFT), -OMAP3_ISP_IOMEM_MAIN, ISP_SYSCONFIG); - if (omap_rev() == OMAP3430_REV_ES1_0) { - isp_reg_writel(isp, ISPCSI1_AUTOIDLE | -(ISPCSI1_MIDLEMODE_SMARTSTANDBY - ISPCSI1_MIDLEMODE_SHIFT), -OMAP3_ISP_IOMEM_CSI2A_REGS1, -ISPCSI2_SYSCONFIG); - isp_reg_writel(isp, ISPCSI1_AUTOIDLE | -(ISPCSI1_MIDLEMODE_SMARTSTANDBY - ISPCSI1_MIDLEMODE_SHIFT), -OMAP3_ISP_IOMEM_CCP2, -ISPCCP2_SYSCONFIG); - } - isp_reg_writel(isp, ISPCTRL_SBL_AUTOIDLE, OMAP3_ISP_IOMEM_MAIN, -ISP_CTRL); - - } else { - isp_reg_writel(isp, -(ISP_SYSCONFIG_MIDLEMODE_FORCESTANDBY - ISP_SYSCONFIG_MIDLEMODE_SHIFT), -OMAP3_ISP_IOMEM_MAIN, ISP_SYSCONFIG); - if (omap_rev() == OMAP3430_REV_ES1_0) { - isp_reg_writel(isp, ISPCSI1_AUTOIDLE | -(ISPCSI1_MIDLEMODE_FORCESTANDBY - ISPCSI1_MIDLEMODE_SHIFT), -OMAP3_ISP_IOMEM_CSI2A_REGS1, -ISPCSI2_SYSCONFIG); - - isp_reg_writel(isp, ISPCSI1_AUTOIDLE | -(ISPCSI1_MIDLEMODE_FORCESTANDBY - ISPCSI1_MIDLEMODE_SHIFT), -OMAP3_ISP_IOMEM_CCP2, -ISPCCP2_SYSCONFIG); - } - - isp_reg_writel(isp, ISPCTRL_SBL_AUTOIDLE, OMAP3_ISP_IOMEM_MAIN, -ISP_CTRL); - } + isp_reg_writel(isp, +((idle ? ISP_SYSCONFIG_MIDLEMODE_SMARTSTANDBY : + ISP_SYSCONFIG_MIDLEMODE_FORCESTANDBY) + ISP_SYSCONFIG_MIDLEMODE_SHIFT), +OMAP3_ISP_IOMEM_MAIN, ISP_SYSCONFIG); + isp_reg_writel(isp, ISPCTRL_SBL_AUTOIDLE, OMAP3_ISP_IOMEM_MAIN, +ISP_CTRL); } /* -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body
RE: [omap3isp][PATCH v2 8/9] omap3isp: ccp2: Make SYSCONFIG fields consistent
-Original Message- From: Aguirre, Sergio Sent: Friday, November 19, 2010 9:44 AM To: 'David Cohen' Cc: Laurent Pinchart; linux-media@vger.kernel.org Subject: RE: [omap3isp][PATCH v2 8/9] omap3isp: ccp2: Make SYSCONFIG fields consistent -Original Message- From: David Cohen [mailto:david.co...@nokia.com] Sent: Friday, November 19, 2010 4:06 AM To: Aguirre, Sergio Cc: Laurent Pinchart; linux-media@vger.kernel.org Subject: Re: [omap3isp][PATCH v2 8/9] omap3isp: ccp2: Make SYSCONFIG fields consistent Hi Sergio, Hi David, Thanks for the review. I've few comments below. On Mon, Nov 15, 2010 at 03:30:00PM +0100, ext Sergio Aguirre wrote: Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/ispccp2.c |3 +-- drivers/media/video/isp/ispreg.h | 14 -- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/media/video/isp/ispccp2.c b/drivers/media/video/isp/ispccp2.c index fa23394..3127a74 100644 --- a/drivers/media/video/isp/ispccp2.c +++ b/drivers/media/video/isp/ispccp2.c @@ -419,8 +419,7 @@ static void ispccp2_mem_configure(struct isp_ccp2_device *ccp2, config-src_ofst = 0; } - isp_reg_writel(isp, (ISPCSI1_MIDLEMODE_SMARTSTANDBY -ISPCSI1_MIDLEMODE_SHIFT), + isp_reg_writel(isp, ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SMART, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_SYSCONFIG); To make your cleanup even better, you could change isp_reg_clr_set() instead. If CCP2 MSTANDY_MODE is set to NOSTANDY, the result is going to be an invalid 0x3 value. Despite it cannot happen with the current code, it's still better to avoid such situations for future versions. :) Hmm you're right, I guess I didn't do any real functional changes, just defines renaming. I can repost an updated patch with this suggestion. No problem. David, Geez I guess I wasn't paying much attention to this either. Sorry. The case you mention about it potentially become 0x3 is not possible, because, I'm basically overwriting the whole register with (0x2 12) Notice the isp_reg_write, there's no OR operation... Now, this means that we have been implicitly setting other fields as Zeroes (SOFT_RESET = 0, and AUTO_IDLE = 0) aswell. Has AUTOIDLE in CCP2 been tried in N900? I don't have a CCP2 sensor to test with :(. Regards, Sergio /* Hsize, Skip */ diff --git a/drivers/media/video/isp/ispreg.h b/drivers/media/video/isp/ispreg.h index d885541..9b0d3ad 100644 --- a/drivers/media/video/isp/ispreg.h +++ b/drivers/media/video/isp/ispreg.h @@ -141,6 +141,14 @@ #define ISPCCP2_REVISION (0x000) #define ISPCCP2_SYSCONFIG(0x004) #define ISPCCP2_SYSCONFIG_SOFT_RESET (1 1) +#define ISPCCP2_SYSCONFIG_AUTO_IDLE 0x1 +#define ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SHIFT12 +#define ISPCCP2_SYSCONFIG_MSTANDBY_MODE_FORCE\ + (0x0 ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SHIFT) +#define ISPCCP2_SYSCONFIG_MSTANDBY_MODE_NO \ + (0x1 ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SHIFT) +#define ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SMART\ + (0x2 ISPCCP2_SYSCONFIG_MSTANDBY_MODE_SHIFT) You can add some ISPCCP2_SYSCONFIG_MSTANDY_MODE_MASK, as 2 bits must be always set together. Sure, will do. Thanks and Regards, Sergio Regards, David Cohen #define ISPCCP2_SYSSTATUS(0x008) #define ISPCCP2_SYSSTATUS_RESET_DONE (1 0) #define ISPCCP2_LC01_IRQENABLE (0x00C) @@ -1309,12 +1317,6 @@ #define ISPMMU_SIDLEMODE_SMARTIDLE 2 #define ISPMMU_SIDLEMODE_SHIFT 3 -#define ISPCSI1_AUTOIDLE 0x1 -#define ISPCSI1_MIDLEMODE_SHIFT 12 -#define ISPCSI1_MIDLEMODE_FORCESTANDBY 0x0 -#define ISPCSI1_MIDLEMODE_NOSTANDBY 0x1 -#define ISPCSI1_MIDLEMODE_SMARTSTANDBY 0x2 - /* -- -- - * CSI2 receiver registers (ES2.0) */ -- 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 -- 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 RFC][PATCH 1/4] omap3isp: Abstract isp subdevs clock control
-Original Message- From: Aguirre, Sergio Sent: Friday, November 19, 2010 5:24 PM To: Laurent Pinchart Cc: linux-media@vger.kernel.org; Aguirre, Sergio Subject: [omap3isp RFC][PATCH 1/4] omap3isp: Abstract isp subdevs clock control Submodules shouldn't be aware of global register bit structure, specially if the submodules are shared in the future with other TI architectures (Davinci, future OMAPs, etc) Oops, I just noticed a bug in this patch in clock disabling... Will resend the updated version. Regards, Sergio Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/isp.c| 54 ++ drivers/media/video/isp/isp.h| 12 +++ drivers/media/video/isp/ispccdc.c|6 +-- drivers/media/video/isp/isppreview.c |6 +-- drivers/media/video/isp/ispresizer.c |6 +-- 5 files changed, 72 insertions(+), 12 deletions(-) diff --git a/drivers/media/video/isp/isp.c b/drivers/media/video/isp/isp.c index 30bdc48..2e5030f 100644 --- a/drivers/media/video/isp/isp.c +++ b/drivers/media/video/isp/isp.c @@ -991,6 +991,60 @@ void isp_sbl_disable(struct isp_device *isp, enum isp_sbl_resource res) * Clock management */ +void isp_subclk_enable(struct isp_device *isp, enum isp_subclk_resource res) +{ + u32 clk = 0; + + isp-subclk_resources |= res; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_H3A) + clk |= ISPCTRL_H3A_CLK_EN; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_HIST) + clk |= ISPCTRL_HIST_CLK_EN; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_RESIZER) + clk |= ISPCTRL_RSZ_CLK_EN; + + /* NOTE: For CCDC Preview submodules, we need to affect internal + * RAM aswell. + */ + if (isp-subclk_resources OMAP3_ISP_SUBCLK_CCDC) + clk |= ISPCTRL_CCDC_CLK_EN | ISPCTRL_CCDC_RAM_EN; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_PREVIEW) + clk |= ISPCTRL_PREV_CLK_EN | ISPCTRL_PREV_RAM_EN; + + isp_reg_set(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL, clk); +} + +void isp_subclk_disable(struct isp_device *isp, enum isp_subclk_resource res) +{ + u32 clk = 0; + + isp-subclk_resources = ~res; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_H3A)) + clk |= ISPCTRL_H3A_CLK_EN; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_HIST)) + clk |= ISPCTRL_HIST_CLK_EN; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_RESIZER)) + clk |= ISPCTRL_RSZ_CLK_EN; + + /* NOTE: For CCDC Preview submodules, we need to affect internal + * RAM aswell. + */ + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_CCDC)) + clk |= ISPCTRL_CCDC_CLK_EN | ISPCTRL_CCDC_RAM_EN; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_PREVIEW)) + clk |= ISPCTRL_PREV_CLK_EN | ISPCTRL_PREV_RAM_EN; + + isp_reg_clr(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL, clk); +} + /* * isp_enable_clocks - Enable ISP clocks * @isp: OMAP3 ISP device diff --git a/drivers/media/video/isp/isp.h b/drivers/media/video/isp/isp.h index b8f63e2..1260e9f 100644 --- a/drivers/media/video/isp/isp.h +++ b/drivers/media/video/isp/isp.h @@ -85,6 +85,14 @@ enum isp_sbl_resource { OMAP3_ISP_SBL_RESIZER_WRITE = 0x200, }; +enum isp_subclk_resource { + OMAP3_ISP_SUBCLK_CCDC = 0x1, + OMAP3_ISP_SUBCLK_H3A= 0x2, + OMAP3_ISP_SUBCLK_HIST = 0x4, + OMAP3_ISP_SUBCLK_PREVIEW= 0x8, + OMAP3_ISP_SUBCLK_RESIZER= 0x10, +}; + enum isp_interface_type { ISP_INTERFACE_PARALLEL, ISP_INTERFACE_CSI2A_PHY2, @@ -262,6 +270,7 @@ struct isp_device { struct isp_csiphy isp_csiphy2; unsigned int sbl_resources; + unsigned int subclk_resources; struct iommu *iommu; }; @@ -294,6 +303,9 @@ void isp_print_status(struct isp_device *isp); void isp_sbl_enable(struct isp_device *isp, enum isp_sbl_resource res); void isp_sbl_disable(struct isp_device *isp, enum isp_sbl_resource res); +void isp_subclk_enable(struct isp_device *isp, enum isp_subclk_resource res); +void isp_subclk_disable(struct isp_device *isp, enum isp_subclk_resource res); + int omap3isp_register_entities(struct platform_device *pdev, struct v4l2_device *v4l2_dev); void omap3isp_unregister_entities(struct platform_device *pdev); diff --git a/drivers/media/video/isp/ispccdc.c b/drivers/media/video/isp/ispccdc.c index c3d1d7a..4244edf 100644 --- a/drivers/media/video/isp/ispccdc.c +++ b/drivers/media/video/isp/ispccdc.c @@ -1687,8 +1687,7 @@ static int ccdc_set_stream(struct v4l2_subdev *sd, int enable) if (enable == ISP_PIPELINE_STREAM_STOPPED) return 0; - isp_reg_set(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL, - ISPCTRL_CCDC_RAM_EN
RE: [omap3isp RFC][PATCH 1/4] omap3isp: Abstract isp subdevs clock control
-Original Message- From: Aguirre, Sergio Sent: Friday, November 19, 2010 5:27 PM To: Aguirre, Sergio; Laurent Pinchart Cc: linux-media@vger.kernel.org Subject: RE: [omap3isp RFC][PATCH 1/4] omap3isp: Abstract isp subdevs clock control -Original Message- From: Aguirre, Sergio Sent: Friday, November 19, 2010 5:24 PM To: Laurent Pinchart Cc: linux-media@vger.kernel.org; Aguirre, Sergio Subject: [omap3isp RFC][PATCH 1/4] omap3isp: Abstract isp subdevs clock control Submodules shouldn't be aware of global register bit structure, specially if the submodules are shared in the future with other TI architectures (Davinci, future OMAPs, etc) Oops, I just noticed a bug in this patch in clock disabling... Will resend the updated version. Geez, I'm apologize again. The patch was indeed correct.. Too much coffee. ;P Regards, Sergio Regards, Sergio Signed-off-by: Sergio Aguirre saagui...@ti.com --- drivers/media/video/isp/isp.c| 54 ++ drivers/media/video/isp/isp.h| 12 +++ drivers/media/video/isp/ispccdc.c|6 +-- drivers/media/video/isp/isppreview.c |6 +-- drivers/media/video/isp/ispresizer.c |6 +-- 5 files changed, 72 insertions(+), 12 deletions(-) diff --git a/drivers/media/video/isp/isp.c b/drivers/media/video/isp/isp.c index 30bdc48..2e5030f 100644 --- a/drivers/media/video/isp/isp.c +++ b/drivers/media/video/isp/isp.c @@ -991,6 +991,60 @@ void isp_sbl_disable(struct isp_device *isp, enum isp_sbl_resource res) * Clock management */ +void isp_subclk_enable(struct isp_device *isp, enum isp_subclk_resource res) +{ + u32 clk = 0; + + isp-subclk_resources |= res; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_H3A) + clk |= ISPCTRL_H3A_CLK_EN; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_HIST) + clk |= ISPCTRL_HIST_CLK_EN; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_RESIZER) + clk |= ISPCTRL_RSZ_CLK_EN; + + /* NOTE: For CCDC Preview submodules, we need to affect internal +* RAM aswell. +*/ + if (isp-subclk_resources OMAP3_ISP_SUBCLK_CCDC) + clk |= ISPCTRL_CCDC_CLK_EN | ISPCTRL_CCDC_RAM_EN; + + if (isp-subclk_resources OMAP3_ISP_SUBCLK_PREVIEW) + clk |= ISPCTRL_PREV_CLK_EN | ISPCTRL_PREV_RAM_EN; + + isp_reg_set(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL, clk); +} + +void isp_subclk_disable(struct isp_device *isp, enum isp_subclk_resource res) +{ + u32 clk = 0; + + isp-subclk_resources = ~res; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_H3A)) + clk |= ISPCTRL_H3A_CLK_EN; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_HIST)) + clk |= ISPCTRL_HIST_CLK_EN; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_RESIZER)) + clk |= ISPCTRL_RSZ_CLK_EN; + + /* NOTE: For CCDC Preview submodules, we need to affect internal +* RAM aswell. +*/ + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_CCDC)) + clk |= ISPCTRL_CCDC_CLK_EN | ISPCTRL_CCDC_RAM_EN; + + if (!(isp-subclk_resources OMAP3_ISP_SUBCLK_PREVIEW)) + clk |= ISPCTRL_PREV_CLK_EN | ISPCTRL_PREV_RAM_EN; + + isp_reg_clr(isp, OMAP3_ISP_IOMEM_MAIN, ISP_CTRL, clk); +} + /* * isp_enable_clocks - Enable ISP clocks * @isp: OMAP3 ISP device diff --git a/drivers/media/video/isp/isp.h b/drivers/media/video/isp/isp.h index b8f63e2..1260e9f 100644 --- a/drivers/media/video/isp/isp.h +++ b/drivers/media/video/isp/isp.h @@ -85,6 +85,14 @@ enum isp_sbl_resource { OMAP3_ISP_SBL_RESIZER_WRITE = 0x200, }; +enum isp_subclk_resource { + OMAP3_ISP_SUBCLK_CCDC = 0x1, + OMAP3_ISP_SUBCLK_H3A= 0x2, + OMAP3_ISP_SUBCLK_HIST = 0x4, + OMAP3_ISP_SUBCLK_PREVIEW= 0x8, + OMAP3_ISP_SUBCLK_RESIZER= 0x10, +}; + enum isp_interface_type { ISP_INTERFACE_PARALLEL, ISP_INTERFACE_CSI2A_PHY2, @@ -262,6 +270,7 @@ struct isp_device { struct isp_csiphy isp_csiphy2; unsigned int sbl_resources; + unsigned int subclk_resources; struct iommu *iommu; }; @@ -294,6 +303,9 @@ void isp_print_status(struct isp_device *isp); void isp_sbl_enable(struct isp_device *isp, enum isp_sbl_resource res); void isp_sbl_disable(struct isp_device *isp, enum isp_sbl_resource res); +void isp_subclk_enable(struct isp_device *isp, enum isp_subclk_resource res); +void isp_subclk_disable(struct isp_device *isp, enum isp_subclk_resource res); + int omap3isp_register_entities(struct platform_device *pdev, struct v4l2_device *v4l2_dev); void omap3isp_unregister_entities(struct platform_device *pdev); diff --git a/drivers/media/video/isp/ispccdc.c b/drivers/media/video/isp/ispccdc.c
RE: [omap3isp][PATCH v2 0/9] YUV support for CCDC + cleanups
-Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Tuesday, November 16, 2010 5:45 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org Subject: Re: [omap3isp][PATCH v2 0/9] YUV support for CCDC + cleanups Hi Sergio, Hi Laurent, Thanks for the patches. You're welcome. I've reviewed and tested them and found no issue. They will be a small delay as I need to sync our internal tree with the public tree before applying them. Not a problem. Thanks for your time. Regards, Sergio -- 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/PATCH v2 5/6] omap3: Export omap3isp platform device structure
Hi Laurent, -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Laurent Pinchart Sent: Tuesday, October 05, 2010 8:19 AM To: linux-media@vger.kernel.org Cc: sakari.ai...@maxwell.research.nokia.com Subject: [RFC/PATCH v2 5/6] omap3: Export omap3isp platform device structure From: Stanimir Varbanov svarba...@mm-sol.com The omap3isp platform device requires platform data. As the data can be provided by a kernel module, the device can't be registered during arch initialization. Remove the omap3isp platform device registration from omap_init_camera(), and export the platform device structure to let board code register/unregister it. This patch needs to go through linux-omap ML. Regards, Sergio Signed-off-by: Stanimir Varbanov svarba...@mm-sol.com Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- arch/arm/mach-omap2/devices.c | 18 -- arch/arm/mach-omap2/devices.h | 17 + 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 arch/arm/mach-omap2/devices.h diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c index ade8db0..f9bc507 100644 --- a/arch/arm/mach-omap2/devices.c +++ b/arch/arm/mach-omap2/devices.c @@ -31,6 +31,8 @@ #include mux.h +#include devices.h + #if defined(CONFIG_VIDEO_OMAP2) || defined(CONFIG_VIDEO_OMAP2_MODULE) static struct resource cam_resources[] = { @@ -141,16 +143,28 @@ static struct resource omap3isp_resources[] = { } }; -static struct platform_device omap3isp_device = { +static void omap3isp_release(struct device *dev) +{ + /* Zero the device structure to avoid re-initialization complaints from + * kobject when the device will be re-registered. + */ + memset(dev, 0, sizeof(*dev)); + dev-release = omap3isp_release; +} + +struct platform_device omap3isp_device = { .name = omap3isp, .id = -1, .num_resources = ARRAY_SIZE(omap3isp_resources), .resource = omap3isp_resources, + .dev = { + .release= omap3isp_release, + }, }; +EXPORT_SYMBOL_GPL(omap3isp_device); static inline void omap_init_camera(void) { - platform_device_register(omap3isp_device); } #else static inline void omap_init_camera(void) diff --git a/arch/arm/mach-omap2/devices.h b/arch/arm/mach-omap2/devices.h new file mode 100644 index 000..f312d49 --- /dev/null +++ b/arch/arm/mach-omap2/devices.h @@ -0,0 +1,17 @@ +/* + * arch/arm/mach-omap2/devices.h + * + * OMAP2 platform device setup/initialization + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#ifndef __ARCH_ARM_MACH_OMAP_DEVICES_H +#define __ARCH_ARM_MACH_OMAP_DEVICES_H + +extern struct platform_device omap3isp_device; + +#endif -- 1.7.2.2 -- 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
[Query] Is there a spec to request video sensor information?
Hi, I was wondering if there exists a current standard way to query a Imaging sensor driver for knowing things like the signal vert/horz blanking time. In an old TI custom driver, we used to have a private IOCTL in the sensor Driver we interfaced with the omap3 ISP, which was basically reporting: - Active resolution (Actual image size) - Full resolution (Above size + dummy pixel columns/rows representing blanking times) However I resist to keep importing that custom interface, since I think its Something that could be already part of an standard API. Any pointers will be much appreciated. Regards, Sergio -- 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
[Query][videobuf-dma-sg] Pages in Highmem handling
Hi, I see that in current videobuf library, for DMA SG code, these functions fail when Detecting a page in Highmem region: - videobuf_pages_to_sg - videobuf_vmalloc_to_sg Now, what's the real reason to not allow handling of Highmem pages? Is it an assumption that _always_ HighMem is not reachable by DMA? I guess my point is, OMAP platform (and maybe other platforms) can handle Highmem pages without any problem. I commented these validations: 65 static struct scatterlist *videobuf_vmalloc_to_sg(unsigned char *virt, 66 int nr_pages) 67 { ... 77 for (i = 0; i nr_pages; i++, virt += PAGE_SIZE) { 78 pg = vmalloc_to_page(virt); 79 if (NULL == pg) 80 goto err; 81 /* BUG_ON(PageHighMem(pg)); */ ... 96 static struct scatterlist *videobuf_pages_to_sg(struct page **pages, 97 int nr_pages, int offset) 98 { ... 109 /* if (PageHighMem(pages[0])) */ 110 /* DMA to highmem pages might not work */ 111 /* goto highmem; */ 112 sg_set_page(sglist[0], pages[0], PAGE_SIZE - offset, offset); 113 for (i = 1; i nr_pages; i++) { 114 if (NULL == pages[i]) 115 goto nopage; 116 /* if (PageHighMem(pages[i])) 117 goto highmem; */ 118 sg_set_page(sglist[i], pages[i], PAGE_SIZE, 0); 119 } Can somebody shed any light on this? Regards, Sergio -- 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: [omap3camera] How does a lens subdevice get powered up?
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Thursday, August 26, 2010 2:31 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org; Nataraju, Kiran Subject: Re: [omap3camera] How does a lens subdevice get powered up? Hi Sergio, On Wednesday 25 August 2010 22:15:36 Aguirre, Sergio wrote: Hi Laurent, I see that usually a sensor is powered up on attempting a VIDIOC_STREAMON at the capture endpoint of the pipeline, in which the sensor is linked. Now, what I don't understand quite well is, the Lens driver is a separate subdevice, BUT it's obviously not linked to the sensor, nor the pipeline. How would the lens driver know when to power up? At the moment a userspace application needs to keep the lens subdev open to power-up the lens controller. I see... So in that case, does it make sense to consider it as a media entity? I mean, there's no link, nor pad operations involved, so it doesn't really add any value... What do you think? Regards, Sergio -- 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: [omap3camera] How does a lens subdevice get powered up?
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Thursday, August 26, 2010 10:04 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org; Nataraju, Kiran Subject: Re: [omap3camera] How does a lens subdevice get powered up? Hi Sergio, On Thursday 26 August 2010 16:54:37 Aguirre, Sergio wrote: On Thursday, August 26, 2010 2:31 AM Laurent Pinchart wrote: On Wednesday 25 August 2010 22:15:36 Aguirre, Sergio wrote: I see that usually a sensor is powered up on attempting a VIDIOC_STREAMON at the capture endpoint of the pipeline, in which the sensor is linked. Now, what I don't understand quite well is, the Lens driver is a separate subdevice, BUT it's obviously not linked to the sensor, nor the pipeline. How would the lens driver know when to power up? At the moment a userspace application needs to keep the lens subdev open to power-up the lens controller. I see... So in that case, does it make sense to consider it as a media entity? I mean, there's no link, nor pad operations involved, so it doesn't really add any value... What do you think? Even if not part of the image pipeline, the lens controller is still part of the media device. I think it makes sense to expose it as an entity and a V4L2 subdevice. Hmm... I don't know what I was thinking... you're right. :) Now that I rethink what I just said vs your answer, I think you have a point, so I'll drop that thought... However, I think still there's something that could be done here... Imagine a scenario in which you have 2 sensors, each one with a different Coil motor driver to control each sensor's lens position. Should we have a way to register some sort of association between Sensor and lens subdevices? That way, by querying the media device, an app can know which lens is associated with what sensor, without any hardcoding. That would be very similar to the case in which you would want to associate an audio capturing subdev with a video capturing subdev. They are not technically sharing the same data, but they are related. Is this kind of association considered in the Media Controller framework implementation currently? Regards, Sergio -- 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: [omap3camera] How does a lens subdevice get powered up?
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Thursday, August 26, 2010 10:36 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org; Nataraju, Kiran Subject: Re: [omap3camera] How does a lens subdevice get powered up? Hi Sergio, On Thursday 26 August 2010 17:33:28 Aguirre, Sergio wrote: On Thursday, August 26, 2010 10:04 AM Laurent Pinchart wrote: Even if not part of the image pipeline, the lens controller is still part of the media device. I think it makes sense to expose it as an entity and a V4L2 subdevice. Hmm... I don't know what I was thinking... you're right. :) Now that I rethink what I just said vs your answer, I think you have a point, so I'll drop that thought... However, I think still there's something that could be done here... Imagine a scenario in which you have 2 sensors, each one with a different Coil motor driver to control each sensor's lens position. Should we have a way to register some sort of association between Sensor and lens subdevices? That way, by querying the media device, an app can know which lens is associated with what sensor, without any hardcoding. That would be very similar to the case in which you would want to associate an audio capturing subdev with a video capturing subdev. They are not technically sharing the same data, but they are related. Is this kind of association considered in the Media Controller framework implementation currently? It's implemented in the latest media controller RFC :-) Entities now have a group ID that the driver can set to report associations such as sensor-coil-flash or video-audio. Oh ok, I see. Perfect! Thanks for clarifying that. Regards, Sergio -- 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
[RESEND][omap3camera] How does a lens subdevice get powered up?
(Resending as plain text, as I sent it originally as HTML and got rejected by ML security policies) Hi Laurent, I see that usually a sensor is powered up on attempting a VIDIOC_STREAMON at the capture endpoint of the pipeline, in which the sensor is linked. Now, what I don't understand quite well is, the Lens driver is a separate subdevice, BUT it's obviously not linked to the sensor, nor the pipeline. How would the lens driver know when to power up? Regards, Sergio -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC/PATCH v3 00/10] Media controller (core and V4L2)
Hi Laurent, -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Laurent Pinchart Sent: Thursday, July 29, 2010 11:07 AM To: linux-media@vger.kernel.org Cc: sakari.ai...@maxwell.research.nokia.com Subject: [RFC/PATCH v3 00/10] Media controller (core and V4L2) Hi everybody, Here's the third version of the media controller patches. All comments received on the first and second versions have (hopefully) been incorporated. The rebased V4L2 API additions and OMAP3 ISP patches will follow. Once again please consider them as sample code only. Laurent Pinchart (8): media: Media device node support media: Media device media: Entities, pads and links media: Entities, pads and links enumeration media: Links setup v4l: Add a media_device pointer to the v4l2_device structure v4l: Make video_device inherit from media_entity v4l: Make v4l2_subdev inherit from media_entity This patch (#0010) doesn't apply to mainline, after this commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b74c0aac357e5c71ee6de98b9887fe478bc73cf4 Am I missing something here? Regards, Sergio Sakari Ailus (2): media: Entity graph traversal media: Reference count and power handling Documentation/media-framework.txt| 481 Documentation/video4linux/v4l2-framework.txt | 71 +++- drivers/media/Makefile |8 +- drivers/media/media-device.c | 327 ++ drivers/media/media-devnode.c| 326 ++ drivers/media/media-entity.c | 613 ++ drivers/media/video/v4l2-dev.c | 35 ++- drivers/media/video/v4l2-device.c| 45 ++- drivers/media/video/v4l2-subdev.c| 27 ++- include/linux/media.h| 78 include/media/media-device.h | 70 +++ include/media/media-devnode.h| 84 include/media/media-entity.h | 107 + include/media/v4l2-dev.h |6 + include/media/v4l2-device.h |2 + include/media/v4l2-subdev.h |7 + 16 files changed, 2265 insertions(+), 22 deletions(-) create mode 100644 Documentation/media-framework.txt create mode 100644 drivers/media/media-device.c create mode 100644 drivers/media/media-devnode.c create mode 100644 drivers/media/media-entity.c create mode 100644 include/linux/media.h create mode 100644 include/media/media-device.h create mode 100644 include/media/media-devnode.h create mode 100644 include/media/media-entity.h -- 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
RE: [RFC/PATCH v3 00/10] Media controller (core and V4L2)
-Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Thursday, August 19, 2010 2:12 PM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org; sakari.ai...@maxwell.research.nokia.com Subject: Re: [RFC/PATCH v3 00/10] Media controller (core and V4L2) Hi Sergio, On Thursday 19 August 2010 21:09:30 Aguirre, Sergio wrote: -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Laurent Pinchart Sent: Thursday, July 29, 2010 11:07 AM To: linux-media@vger.kernel.org Cc: sakari.ai...@maxwell.research.nokia.com Subject: [RFC/PATCH v3 00/10] Media controller (core and V4L2) Hi everybody, Here's the third version of the media controller patches. All comments received on the first and second versions have (hopefully) been incorporated. The rebased V4L2 API additions and OMAP3 ISP patches will follow. Once again please consider them as sample code only. Laurent Pinchart (8): media: Media device node support media: Media device media: Entities, pads and links media: Entities, pads and links enumeration media: Links setup v4l: Add a media_device pointer to the v4l2_device structure v4l: Make video_device inherit from media_entity v4l: Make v4l2_subdev inherit from media_entity This patch (#0010) doesn't apply to mainline, after this commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux- 2.6.git;a=commit;h =b74c0aac357e5c71ee6de98b9887fe478bc73cf4 Am I missing something here? Yes, you're missing the next version of the patches :-) I'll probably send them tomorrow. Ohh ok, :) Thanks! Regards, Sergio -- 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] Fixes field names that changed
Hi Henrique, Just some minor comments about the patch description below: -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Henrique Camargo Sent: Tuesday, August 17, 2010 9:35 AM To: Mauro Carvalho Chehab Cc: Guennadi Liakhovetski; Karicheri, Muralidharan; linux- me...@vger.kernel.org; linux-ker...@vger.kernel.org Subject: [PATCH] Fixes field names that changed Add missing subject prefix to quickly describe the affected driver: Subject: [PATCH] mt9t032: Fixes field names that changed If CONFIG_VIDEO_ADV_DEBUG was set, the driver failed to compile because the fields get_register and set_register changed names to s_register and s_register in the struct v4l2_subdev_core_ops. Please break down this comment to 70 chars max. Also, you said s_register twice. Regards, Sergio Signed-off-by: Henrique Camargo henri...@henriquecamargo.com --- drivers/media/video/mt9t031.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/mt9t031.c b/drivers/media/video/mt9t031.c index 716fea6..f3d1995 100644 --- a/drivers/media/video/mt9t031.c +++ b/drivers/media/video/mt9t031.c @@ -499,8 +499,8 @@ static const struct v4l2_subdev_core_ops mt9t031_core_ops = { .g_ctrl = mt9t031_get_control, .s_ctrl = mt9t031_set_control, #ifdef CONFIG_VIDEO_ADV_DEBUG - .get_register = mt9t031_get_register, - .set_register = mt9t031_set_register, + .g_register = mt9t031_get_register, + .s_register = mt9t031_set_register, #endif }; -- 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 -- 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] Fixes field names that changed
Oops, fixing my own typo below: -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Aguirre, Sergio Sent: Wednesday, August 18, 2010 7:16 PM To: henri...@henriquecamargo.com; Mauro Carvalho Chehab Cc: Guennadi Liakhovetski; Karicheri, Muralidharan; linux- me...@vger.kernel.org; linux-ker...@vger.kernel.org Subject: RE: [PATCH] Fixes field names that changed Hi Henrique, Just some minor comments about the patch description below: -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Henrique Camargo Sent: Tuesday, August 17, 2010 9:35 AM To: Mauro Carvalho Chehab Cc: Guennadi Liakhovetski; Karicheri, Muralidharan; linux- me...@vger.kernel.org; linux-ker...@vger.kernel.org Subject: [PATCH] Fixes field names that changed Add missing subject prefix to quickly describe the affected driver: Subject: [PATCH] mt9t032: Fixes field names that changed mt9t032 - mt9t031. Sorry... Regards, Sergio If CONFIG_VIDEO_ADV_DEBUG was set, the driver failed to compile because the fields get_register and set_register changed names to s_register and s_register in the struct v4l2_subdev_core_ops. Please break down this comment to 70 chars max. Also, you said s_register twice. Regards, Sergio Signed-off-by: Henrique Camargo henri...@henriquecamargo.com --- drivers/media/video/mt9t031.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/mt9t031.c b/drivers/media/video/mt9t031.c index 716fea6..f3d1995 100644 --- a/drivers/media/video/mt9t031.c +++ b/drivers/media/video/mt9t031.c @@ -499,8 +499,8 @@ static const struct v4l2_subdev_core_ops mt9t031_core_ops = { .g_ctrl = mt9t031_get_control, .s_ctrl = mt9t031_set_control, #ifdef CONFIG_VIDEO_ADV_DEBUG - .get_register = mt9t031_get_register, - .set_register = mt9t031_set_register, + .g_register = mt9t031_get_register, + .s_register = mt9t031_set_register, #endif }; -- 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 -- 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
[ANNOUNCE] OMAP3 Camera ISP driver wiki in omappedia.org
Hi all, I'll like to announce a new wikipage entry in http://omappedia.org site for OMAP3 Camera-ISP progress and details. Please find it here. http://omappedia.org/wiki/Camera-ISP_Driver The intention is to provide an online documentation of the work and progress of the patches submission, and its dependencies (Media Controller Framework, specifically). If you're interested to contribute/experiment with the driver, this is intended to be a good start point for that, and also feel free to update it aswell. The more people trying the driver, the better, so we can have as much platforms running with it as possible, and we can claim that the driver is flexible enough for all. Have a great weekend! Regards, Sergio -- 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: [media-ctl PATCH 2/3] Just include kernel headers
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, July 30, 2010 9:24 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org Subject: Re: [media-ctl PATCH 2/3] Just include kernel headers Hi Sergio, On Friday 30 July 2010 16:10:08 Aguirre, Sergio wrote: On Friday 30 July 2010 8:45 AM Laurent Pinchart wrote: On Wednesday 14 July 2010 18:17:25 Sergio Aguirre wrote: We shouldn't require full kernel source for this. That's right in theory, but I then get $ make KDIR=/home/laurent/src/arm/kernel/ arm-none-linux-gnueabi-gcc -O2 -Wall -fpic -I. - I/home/laurent/src/arm/kernel//include-c -o media.o media.c In file included from /opt/cs/arm-2009q1/bin/../arm-none-linux- gnueabi/libc/usr/include/asm/types.h:4, from /home/laurent/src/arm/kernel//include/linux/types.h:4, from /home/laurent/src/arm/kernel//include/linux/videodev2.h:66, from media.c:31: /home/laurent/src/arm/kernel//include/asm-generic/int-ll64.h:11:29: error: asm/bitsperlong.h: No such file or directory make: *** [media.o] Error 1 when building against a kernel tree. KDIR doesn't exist anymore. By the result of your log, I don't see how that value got passed into the makefile... Are you sure you applied the patch correctly? I haven't, I've just removed the arch include dir from KDIR in the Makefile. The end result is the same. Hmm.. I think this is expected, since the kernel headers folder generated with make ARCH=arm INSTALL_HDR_PATH=your-fs-root headers_install Is not the same as just reading the kernel source include folder. Some #ifdef get resolved depending on the arch, and the headers are rebuilt. See : Documentation/make/headers_install.txt So, I guess it's not as simple as just removing the arch include folder. Regards, Sergio -- 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: [media-ctl PATCH 2/3] Just include kernel headers
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, July 30, 2010 10:40 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org Subject: Re: [media-ctl PATCH 2/3] Just include kernel headers Hi Sergio, snip Ideally the application should be built against installed kernel headers, bug given the early stage of development of the media controller, I expect most people to build it against a kernel tree. I would like to keep the Makefile as-is for now, and change it when the media controller patches will reach the mainline kernel. Ok, understood. Not a problem. Regards, Sergio -- 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: [media-ctl] [omap3camera:devel] How to use the app?
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Tuesday, June 29, 2010 5:23 AM To: Aguirre, Sergio Cc: Sakari Ailus; linux-media@vger.kernel.org Subject: Re: [media-ctl] [omap3camera:devel] How to use the app? snip You will find a set of patches that remove the legacy video nodes attached to this e-mail. They haven't been applied to the omap3camera tree yet, as we still haven't fixed all userspace components yet, but they should get there in a few weeks hopefully. You should probably apply them to your tree to make sure you don't start using the legacy video nodes by mistake. They also remove a lot of code, which is always good, and remove the hardcoded number of sensors. By any chance, do you keep rebasing these patches in a branch somewhere? I tried rebasing them against latest 'devel' branch, but they fail on omap34xxcam.c, because the removed content is not the same. The delta is just an addition you did to reset all non-legacy links during init. Is it ok to still remove this completely? Or is this going to be rellocated somewhere? The patch I'm talking about is named: omap34xxcam: Reset all links before setting up the pipeline. Regards, Sergio -- 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/PATCH 02/10] media: Media device
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, July 16, 2010 3:56 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org; sakari.ai...@maxwell.research.nokia.com Subject: Re: [RFC/PATCH 02/10] media: Media device Hi Sergio, On Thursday 15 July 2010 16:22:06 Aguirre, Sergio wrote: On Wednesday 14 July 2010 08:30:00 Laurent Pinchart wrote: snip diff --git a/drivers/media/media-device.c b/drivers/media/media- device.c new file mode 100644 index 000..a4d3db5 --- /dev/null +++ b/drivers/media/media-device.c @@ -0,0 +1,77 @@ snip +/** + * media_device_register - register a media device + * @mdev:The media device + * + * The caller is responsible for initializing the media device before + * registration. The following fields must be set: + * + * - dev should point to the parent device. The field can be NULL when no + * parent device is available (for instance with ISA devices). + * - name should be set to the device name. If the name is empty a parent + * device must be set. In that case the name will be set to the parent + * device driver name followed by a space and the parent device name. + */ +int __must_check media_device_register(struct media_device *mdev) +{ + /* If dev == NULL, then name must be filled in by the caller */ + if (mdev-dev == NULL WARN_ON(!mdev-name[0])) If mdev == NULL, you'll have a kernel panic here. That's why drivers must not call media_device_register with a NULL pointer :-) It's not a valid use case, unlike kfree(NULL) for instance. Right. I know. I guess I was thinking more in terms of not compromising the system because of a buggy driver, and exit gracefully instead... But I guess it's also ok, as a driver developer is usually updating the full kernel anyways. Regards, Sergio -- 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/PATCH 03/10] media: Entities, pads and links
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, July 16, 2010 4:10 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org; sakari.ai...@maxwell.research.nokia.com Subject: Re: [RFC/PATCH 03/10] media: Entities, pads and links Hi Sergio, Thanks for the review. On Thursday 15 July 2010 16:35:20 Aguirre, Sergio wrote: On Wednesday 14 July 2010 08:30:00 Laurent Pinchart wrote: [snip] diff --git a/drivers/media/media-device.c b/drivers/media/media- device.c index a4d3db5..6361367 100644 --- a/drivers/media/media-device.c +++ b/drivers/media/media-device.c [snip] @@ -72,6 +77,54 @@ EXPORT_SYMBOL_GPL(media_device_register); [snip] + +/** + * media_device_register_entity - Register an entity with a media device + * @mdev:The media device + * @entity: The entity + */ +int __must_check media_device_register_entity(struct media_device *mdev, + struct media_entity *entity) +{ What if mdev == NULL OR entity == NULL? It's not a valid use case. Drivers must not try to register a NULL entity, or an entity to a NULL media device. Ok. + /* Warn if we apparently re-register an entity */ + WARN_ON(entity-parent != NULL); Shouldn't we exit with -EBUSY here instead? Or is there a usecase In which this is a valid scenario? entity-parent != NULL isn't a valid scenario. It's a driver bug, and WARN_ON will result in a backtrace being printed, notifying the driver developer that something is seriously wrong. Ok. + entity-parent = mdev; + + spin_lock(mdev-lock); + entity-id = mdev-entity_id++; + list_add_tail(entity-list, mdev-entities); + spin_unlock(mdev-lock); + + return 0; +} +EXPORT_SYMBOL_GPL(media_device_register_entity); + +/** + * media_device_unregister_entity - Unregister an entity + * @entity: The entity + * + * If the entity has never been registered this function will return + * immediately. + */ +void media_device_unregister_entity(struct media_entity *entity) +{ + struct media_device *mdev = entity-parent; What if entity == NULL? Still not a valid use case, don't unregister NULL. Ok. + + if (mdev == NULL) + return; + + spin_lock(mdev-lock); + list_del(entity-list); + spin_unlock(mdev-lock); + entity-parent = NULL; +} +EXPORT_SYMBOL_GPL(media_device_unregister_entity); diff --git a/drivers/media/media-entity.c b/drivers/media/media- entity.c new file mode 100644 index 000..d5a4b4c --- /dev/null +++ b/drivers/media/media-entity.c @@ -0,0 +1,145 @@ +/* + * Media Entity support + * + * Copyright (C) 2009 Laurent Pinchart laurent.pinch...@ideasonboard.com 2010? Oops, yes, will fix. [snip] +int +media_entity_create_link(struct media_entity *source, u8 source_pad, + struct media_entity *sink, u8 sink_pad, u32 flags) +{ + struct media_entity_link *link; + struct media_entity_link *backlink; + + BUG_ON(source == NULL || sink == NULL); + BUG_ON(source_pad = source-num_pads); + BUG_ON(sink_pad = sink-num_pads); Isn't this too dramatic? :) I mean, does the entire system needs to be halted because you won't be able to link your video subdevices? If source or sink is NULL, the second or third BUG_ON will result in a crash. If the source or sink pad numbers are out of bounds, undefined memory will be dereferenced later. Both conditions will likely result in a crash, so it's better to have a predictable, easy to understand crash. Once again this should never happen. It's a driver bug if it does, and should not happen randomly at runtime. Ok. [snip] diff --git a/include/media/media-entity.h b/include/media/media- entity.h new file mode 100644 index 000..0929a90 --- /dev/null +++ b/include/media/media-entity.h @@ -0,0 +1,79 @@ +#ifndef _MEDIA_ENTITY_H +#define _MEDIA_ENTITY_H + +#include linux/list.h + +#define MEDIA_ENTITY_TYPE_NODE 1 +#define MEDIA_ENTITY_TYPE_SUBDEV 2 + +#define MEDIA_NODE_TYPE_V4L 1 +#define MEDIA_NODE_TYPE_FB 2 +#define MEDIA_NODE_TYPE_ALSA 3 +#define MEDIA_NODE_TYPE_DVB 4 + +#define MEDIA_SUBDEV_TYPE_VID_DECODER1 +#define MEDIA_SUBDEV_TYPE_VID_ENCODER2 +#define MEDIA_SUBDEV_TYPE_MISC 3 + +#define MEDIA_LINK_FLAG_ACTIVE (1 0) +#define MEDIA_LINK_FLAG_IMMUTABLE(1 1) + +#define MEDIA_PAD_TYPE_INPUT 1 +#define MEDIA_PAD_TYPE_OUTPUT2 Shouldn't all the above be enums? (except of course the MEDIA_LINK_FLAG_* defines) They can be enums
RE: [RFC/PATCH 02/10] media: Media device
-Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, July 16, 2010 3:53 AM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org; sakari.ai...@maxwell.research.nokia.com Subject: Re: [RFC/PATCH 02/10] media: Media device Hi Sergio, Thanks for the review. On Thursday 15 July 2010 16:16:44 Aguirre, Sergio wrote: On Wednesday 14 July 2010 08:30:00 Laurent Pinchart wrote: snip diff --git a/include/media/media-device.h b/include/media/media- device.h new file mode 100644 index 000..6c1fc4a --- /dev/null +++ b/include/media/media-device.h @@ -0,0 +1,53 @@ [snip] +#ifndef _MEDIA_DEVICE_H +#define _MEDIA_DEVICE_H + +#include linux/device.h +#include linux/list.h + +#include media/media-devnode.h + +/* Each instance of a media device should create the media_device struct, + * either stand-alone or embedded in a larger struct. + * + * It allows easy access to sub-devices (see v4l2-subdev.h) and provides + * basic media device-level support. + */ + +#define MEDIA_DEVICE_NAME_SIZE (20 + 16) Where does above numbers come from ?? Copied from v4l2-device.h. It was originally BUS_ID_SIZE (the constant is now gone) + 16. I can replace that by a hardcoded 32. Ok. Regards, Sergio -- 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/PATCH 02/10] media: Media device
Hi Laurent, Very minor comment below. -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Laurent Pinchart Sent: Wednesday, July 14, 2010 8:30 AM To: linux-media@vger.kernel.org Cc: sakari.ai...@maxwell.research.nokia.com Subject: [RFC/PATCH 02/10] media: Media device The media_device structure abstracts functions common to all kind of media devices (v4l2, dvb, alsa, ...). It manages media entities and offers a userspace API to discover and configure the media device internal topology. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- Documentation/media-framework.txt | 68 drivers/media/Makefile|2 +- drivers/media/media-device.c | 77 + include/media/media-device.h | 53 + 4 files changed, 199 insertions(+), 1 deletions(-) create mode 100644 Documentation/media-framework.txt create mode 100644 drivers/media/media-device.c create mode 100644 include/media/media-device.h snip diff --git a/include/media/media-device.h b/include/media/media-device.h new file mode 100644 index 000..6c1fc4a --- /dev/null +++ b/include/media/media-device.h @@ -0,0 +1,53 @@ +/* + * Media device support header. + * + * Copyright (C) 2010 Laurent Pinchart laurent.pinch...@ideasonboard.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef _MEDIA_DEVICE_H +#define _MEDIA_DEVICE_H + +#include linux/device.h +#include linux/list.h + +#include media/media-devnode.h + +/* Each instance of a media device should create the media_device struct, + * either stand-alone or embedded in a larger struct. + * + * It allows easy access to sub-devices (see v4l2-subdev.h) and provides + * basic media device-level support. + */ + +#define MEDIA_DEVICE_NAME_SIZE (20 + 16) Where does above numbers come from ?? Regards, Sergio + +struct media_device { + /* dev-driver_data points to this struct. + * Note: dev might be NULL if there is no parent device + * as is the case with e.g. ISA devices. + */ + struct device *dev; + struct media_devnode devnode; + + /* unique device name, by default the driver name + bus ID */ + char name[MEDIA_DEVICE_NAME_SIZE]; +}; + +int __must_check media_device_register(struct media_device *mdev); +void media_device_unregister(struct media_device *mdev); + +#endif -- 1.7.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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC/PATCH 02/10] media: Media device
Hi Laurent, Other comment I missed to mention... -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Laurent Pinchart Sent: Wednesday, July 14, 2010 8:30 AM To: linux-media@vger.kernel.org Cc: sakari.ai...@maxwell.research.nokia.com Subject: [RFC/PATCH 02/10] media: Media device The media_device structure abstracts functions common to all kind of media devices (v4l2, dvb, alsa, ...). It manages media entities and offers a userspace API to discover and configure the media device internal topology. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- Documentation/media-framework.txt | 68 drivers/media/Makefile|2 +- drivers/media/media-device.c | 77 + include/media/media-device.h | 53 + 4 files changed, 199 insertions(+), 1 deletions(-) create mode 100644 Documentation/media-framework.txt create mode 100644 drivers/media/media-device.c create mode 100644 include/media/media-device.h snip diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c new file mode 100644 index 000..a4d3db5 --- /dev/null +++ b/drivers/media/media-device.c @@ -0,0 +1,77 @@ +/* + * Media device support. + * + * Copyright (C) 2010 Laurent Pinchart laurent.pinch...@ideasonboard.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include linux/types.h +#include linux/ioctl.h + +#include media/media-device.h +#include media/media-devnode.h + +static const struct media_file_operations media_device_fops = { + .owner = THIS_MODULE, +}; + +static void media_device_release(struct media_devnode *mdev) +{ +} + +/** + * media_device_register - register a media device + * @mdev:The media device + * + * The caller is responsible for initializing the media device before + * registration. The following fields must be set: + * + * - dev should point to the parent device. The field can be NULL when no + * parent device is available (for instance with ISA devices). + * - name should be set to the device name. If the name is empty a parent + * device must be set. In that case the name will be set to the parent + * device driver name followed by a space and the parent device name. + */ +int __must_check media_device_register(struct media_device *mdev) +{ + /* If dev == NULL, then name must be filled in by the caller */ + if (mdev-dev == NULL WARN_ON(!mdev-name[0])) If mdev == NULL, you'll have a kernel panic here. Regards, Sergio -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC/PATCH 03/10] media: Entities, pads and links
Hi Laurent, -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Laurent Pinchart Sent: Wednesday, July 14, 2010 8:30 AM To: linux-media@vger.kernel.org Cc: sakari.ai...@maxwell.research.nokia.com Subject: [RFC/PATCH 03/10] media: Entities, pads and links As video hardware pipelines become increasingly complex and configurable, the current hardware description through v4l2 subdevices reaches its limits. In addition to enumerating and configuring subdevices, video camera drivers need a way to discover and modify at runtime how those subdevices are connected. This is done through new elements called entities, pads and links. An entity is a basic media hardware building block. It can correspond to a large variety of logical blocks such as physical hardware devices (CMOS sensor for instance), logical hardware devices (a building block in a System-on-Chip image processing pipeline), DMA channels or physical connectors. A pad is a connection endpoint through which an entity can interact with other entities. Data (not restricted to video) produced by an entity flows from the entity's output to one or more entity inputs. Pads should not be confused with physical pins at chip boundaries. A link is a point-to-point oriented connection between two pads, either on the same entity or on different entities. Data flows from a source pad to a sink pad. Links are stored in the source entity. To make backwards graph walk faster, a copy of all links is also stored in the sink entity. The copy is known as a backlink and is only used to help graph traversal. The entity API is made of three functions: - media_entity_init() initializes an entity. The caller must provide an array of pads as well as an estimated number of links. The links array is allocated dynamically and will be reallocated if it grows beyond the initial estimate. - media_entity_cleanup() frees resources allocated for an entity. It must be called during the cleanup phase after unregistering the entity and before freeing it. - media_entity_create_link() creates a link between two entities. An entry in the link array of each entity is allocated and stores pointers to source and sink pads. When a media device is unregistered, all its entities are unregistered automatically. The code is based on Hans Verkuil hverk...@xs4all.nl initial work. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Sakari Ailus sakari.ai...@maxwell.research.nokia.com --- Documentation/media-framework.txt | 125 drivers/media/Makefile|2 +- drivers/media/media-device.c | 53 ++ drivers/media/media-entity.c | 145 + include/media/media-device.h | 16 include/media/media-entity.h | 79 6 files changed, 419 insertions(+), 1 deletions(-) create mode 100644 drivers/media/media-entity.c create mode 100644 include/media/media-entity.h diff --git a/Documentation/media-framework.txt b/Documentation/media- framework.txt index b942c8f..4a8f379 100644 --- a/Documentation/media-framework.txt +++ b/Documentation/media-framework.txt @@ -35,6 +35,30 @@ belong to userspace. The media kernel API aims at solving those problems. +Abstract media device model +--- + +Discovering a device internal topology, and configuring it at runtime, is one +of the goals of the media framework. To achieve this, hardware devices are +modeled as an oriented graph of building blocks called entities connected +through pads. + +An entity is a basic media hardware building block. It can correspond to +a large variety of logical blocks such as physical hardware devices +(CMOS sensor for instance), logical hardware devices (a building block +in a System-on-Chip image processing pipeline), DMA channels or physical +connectors. + +A pad is a connection endpoint through which an entity can interact with +other entities. Data (not restricted to video) produced by an entity +flows from the entity's output to one or more entity inputs. Pads should +not be confused with physical pins at chip boundaries. + +A link is a point-to-point oriented connection between two pads, either +on the same entity or on different entities. Data flows from a source +pad to a sink pad. + + Media device @@ -66,3 +90,104 @@ Drivers unregister media device instances by calling Unregistering a media device that hasn't been registered is *NOT* safe. + +Entities, pads and links + + +- Entities + +Entities are represented by a struct media_entity instance, defined in +include/media/media-entity.h. The structure is usually embedded into a +higher-level structure, such as a v4l2_subdev or video_device instance, +although drivers can
[Query] How to preserve soc_camera and still use a sensor for media controller?
Hi all, Is it possible to keep a sensor chip driver compatible with 2 interfaces? I'm particularly interested in mt9t112 sensor. Has this been done before with other driver? Thanks for your attention. Regards, Sergio -- 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: [Query] How to preserve soc_camera and still use a sensor for media controller?
Hi Guennadi, -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Wednesday, June 30, 2010 1:53 PM To: Aguirre, Sergio Cc: linux-media@vger.kernel.org Subject: Re: [Query] How to preserve soc_camera and still use a sensor for media controller? Hi Sergio On Wed, 30 Jun 2010, Aguirre, Sergio wrote: Hi all, Is it possible to keep a sensor chip driver compatible with 2 interfaces? I'm particularly interested in mt9t112 sensor. Has this been done before with other driver? You can try looking at this thread: http://thread.gmane.org/gmane.linux.drivers.video-input- infrastructure/16311 Very interesting thread :) and remembering our discussions at the mini-summit;) You can also look at Hans' report of summit's results. A few people know, what has to be done, but noone has done it yet... The world is still looking for a hero to do the job:) OK, thanks for the info :) I'll think about that... Regards, Sergio Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [media-ctl] [omap3camera:devel] How to use the app?
Hi Laurent, -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Tuesday, June 29, 2010 5:23 AM To: Aguirre, Sergio Cc: Sakari Ailus; linux-media@vger.kernel.org Subject: Re: [media-ctl] [omap3camera:devel] How to use the app? Hi Sergio, On Tuesday 29 June 2010 01:34:01 Aguirre, Sergio wrote: Hi Laurent/Sakari, I have been attempting to migrate my IMX046 sensor driver, that I had working on my Zoom3(OMAP3630 ES1.1) with older codebase, to work with the latest omap3camera tree, 'devel' branch: http://gitorious.org/omap3camera/mainline/commits/devel And for that, I'm trying to also understand how to use your test tool: media-ctl: http://git.ideasonboard.org/?p=media-ctl.git;a=summary Now, the thing is that, I don't see any guide to learn how to write the Proper format for some of the parameters, like to build links in interactive mode (-i), or to set formats (-f). Can you please detail about a typical usage for this tool? (example on how to build a link, set link format, etc.) I've pushed a new patch to the media-ctl repository that makes the help message a bit more verbose when running media-ctl -h. It describes the links and formats as follows: Links and formats are defined as link= pad, '-', pad, '[', flags, ']' ; format = pad, '[', fcc, ' ', size, [ ' ', crop ], ']' ; pad = entity, ':', pad number ; entity = entity number | ( '\', entity name, '\' ) ; size= width, 'x', height ; crop= left, ',', top, '/', size ; where the fields are entity number Entity numeric identifier entity name Entify name (string) pad number Pad numeric identifier flags Link flags (0: inactive, 1: active) fcc Format FourCC width Image width in pixels height Image height in pixels For instance, to set the CCDC to preview link as active, the link specifier would be 'OMAP3 ISP CCDC:2 - OMAP3 ISP preview:0 [1]' To set the format on the preview output pad to YUYV 1280x720, the format specifier would be 'OMAP3 ISP preview:2 [YUYV 1280x720]' Spaces are optional. Thanks a lot for the explanation, and for the verbose help patch! That's much better :) So far, my progress is pushed into this branch: http://dev.omapzoom.org/?p=saaguirre/linux-omap- camera.git;a=shortlog;h=ref s/heads/mc_migration_wip Thanks for the link. And with that, after I boot, I get the following topology: [snip] You will find a set of patches that remove the legacy video nodes attached to this e-mail. They haven't been applied to the omap3camera tree yet, as we still haven't fixed all userspace components yet, but they should get there in a few weeks hopefully. You should probably apply them to your tree to make sure you don't start using the legacy video nodes by mistake. They also remove a lot of code, which is always good, and remove the hardcoded number of sensors. I had following compilation error: drivers/media/video/isp/ispvideo.c: In function 'isp_video_streamon': drivers/media/video/isp/ispvideo.c:780: error: 'const struct isp_video_operations' has no member named 'stream_off' drivers/media/video/isp/ispvideo.c:781: error: 'const struct isp_video_operations' has no member named 'stream_off' make[4]: *** [drivers/media/video/isp/ispvideo.o] Error 1 make[3]: *** [drivers/media/video/isp] Error 2 make[3]: *** Waiting for unfinished jobs make[2]: *** [drivers/media/video] Error 2 make[1]: *** [drivers/media] Error 2 make: *** [drivers] Error 2 Which I solved with the attached patch. You might want to squash it with your patch omap3isp: video: Remove the init, cleanup and stream_off operations I'll continue trying to bring up my sensor, and let you know if I have other query. Thanks for your time! Regards, Sergio -- Regards, Laurent Pinchart 0001-SQUASH-omap3isp-video-Remove-the-init-cleanup-and-st.patch Description: 0001-SQUASH-omap3isp-video-Remove-the-init-cleanup-and-st.patch
RE: [omap3camera] Kernel broken when building rx51 with camera
-Original Message- From: Aguirre, Sergio Sent: Friday, June 25, 2010 9:03 PM To: Laurent Pinchart; Sakari Ailus Cc: linux-media@vger.kernel.org Subject: [omap3camera] Kernel broken when building rx51 with camera Hi Laurent/Sakari, Not sure what is exactly happening, but I can't get the kernel to build with latest omap3camera gitorious devel branch. Ahh, I just noticed a patch already for it on latest linux-omap tree. Its an IOMMU bug when building as built-in. Here's the patch: http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commit;h=7d9609c6f0feaf045d5b45fd3e1669837700c152 commit 7d9609c6f0feaf045d5b45fd3e1669837700c152 Author: Hiroshi DOYU hiroshi.d...@nokia.com Date: Wed Jun 16 19:05:52 2010 +0300 omap iommu: fix: Make omap-iommu.o built-in This also fixes the wrong overwritten for obj-y in the previous commit. Signed-off-by: Hiroshi DOYU hiroshi.d...@nokia.com Signed-off-by: Tony Lindgren t...@atomide.com Sorry for the noise. Regards, Sergio snip -- 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
[media-ctl] [omap3camera:devel] How to use the app?
Hi Laurent/Sakari, I have been attempting to migrate my IMX046 sensor driver, that I had working on my Zoom3(OMAP3630 ES1.1) with older codebase, to work with the latest omap3camera tree, 'devel' branch: http://gitorious.org/omap3camera/mainline/commits/devel And for that, I'm trying to also understand how to use your test tool: media-ctl: http://git.ideasonboard.org/?p=media-ctl.git;a=summary Now, the thing is that, I don't see any guide to learn how to write the Proper format for some of the parameters, like to build links in interactive mode (-i), or to set formats (-f). Can you please detail about a typical usage for this tool? (example on how to build a link, set link format, etc.) So far, my progress is pushed into this branch: http://dev.omapzoom.org/?p=saaguirre/linux-omap-camera.git;a=shortlog;h=refs/heads/mc_migration_wip And with that, after I boot, I get the following topology: # /media-ctl -d /dev/media0 -p Opening media device /dev/media0 Enumerating entities Found 18 entities Enumerating pads and links Device topology - entity 1: imx046 2-001a (1 pad, 1 link) type V4L2 subdev subtype Unknown device node name /dev/subdev0 pad0: Output [SGRBG10 3280x2464] - 'OMAP3 ISP CSI2a':pad0 [IMMUTABLE,ACTIVE] - entity 2: lv8093 2-0072 (0 pad, 0 link) type V4L2 subdev subtype Unknown device node name /dev/subdev1 - entity 3: omap3/imx046 2-001a/lv8093 2-00 (1 pad, 0 link) type Node subtype V4L device node name /dev/video0 pad0: Input - entity 4: OMAP3 ISP CCP2 (2 pads, 1 link) type V4L2 subdev subtype Unknown device node name /dev/subdev2 pad0: Input [unknown 0x0] pad1: Output [unknown 0x0] - 'OMAP3 ISP CCDC':pad0 [] - entity 5: OMAP3 ISP CCP2 input (1 pad, 1 link) type Node subtype V4L device node name /dev/video1 pad0: Output - 'OMAP3 ISP CCP2':pad0 [] - entity 6: OMAP3 ISP CSI2a (2 pads, 2 links) type V4L2 subdev subtype Unknown device node name /dev/subdev3 pad0: Input [SGRBG10 0x0] pad1: Output [SGRBG10 0x0] - 'OMAP3 ISP CSI2a output':pad0 [] - 'OMAP3 ISP CCDC':pad0 [] - entity 7: OMAP3 ISP CSI2a output (1 pad, 0 link) type Node subtype V4L device node name /dev/video2 pad0: Input - entity 8: OMAP3 ISP CCDC (3 pads, 7 links) type V4L2 subdev subtype Unknown device node name /dev/subdev4 pad0: Input [unknown 0x0] pad1: Output [unknown 0x0] - 'OMAP3 ISP CCDC output':pad0 [] - 'OMAP3 ISP resizer':pad0 [] - 'omap3/imx046 2-001a/lv8093 2-00':pad0 [] pad2: Output [unknown 0x0] - 'OMAP3 ISP preview':pad0 [] - 'OMAP3 ISP AEWB':pad0 [] - 'OMAP3 ISP AF':pad0 [] - 'OMAP3 ISP histogram':pad0 [] - entity 9: OMAP3 ISP CCDC output (1 pad, 0 link) type Node subtype V4L device node name /dev/video3 pad0: Input - entity 10: OMAP3 ISP preview (2 pads, 2 links) type V4L2 subdev subtype Unknown device node name /dev/subdev5 pad0: Input [unknown 0x0] pad1: Output [unknown 0x0] - 'OMAP3 ISP preview output':pad0 [] - 'OMAP3 ISP resizer':pad0 [] - entity 11: OMAP3 ISP preview input (1 pad, 1 link) type Node subtype V4L device node name /dev/video4 pad0: Output - 'OMAP3 ISP preview':pad0 [] - entity 12: OMAP3 ISP preview output (1 pad, 0 link) type Node subtype V4L device node name /dev/video5 pad0: Input - entity 13: OMAP3 ISP resizer (2 pads, 2 links) type V4L2 subdev subtype Unknown device node name /dev/subdev6 pad0: Input [unknown 0x0] pad1: Output [unknown 0x0] - 'OMAP3 ISP resizer output':pad0 [] - 'omap3/imx046 2-001a/lv8093 2-00':pad0 [] - entity 14: OMAP3 ISP resizer input (1 pad, 1 link) type Node subtype V4L device node name /dev/video6 pad0: Output - 'OMAP3 ISP resizer':pad0 [] - entity 15: OMAP3 ISP resizer output (1 pad, 0 link) type Node subtype V4L device node name /dev/video7 pad0: Input - entity 16: OMAP3 ISP AEWB (1 pad, 0 link) type V4L2 subdev subtype Unknown device node name /dev/subdev7 pad0: Input - entity 17: OMAP3 ISP AF (1 pad, 0 link) type V4L2 subdev subtype Unknown device node name /dev/subdev8 pad0: Input - entity 18: OMAP3 ISP histogram (1 pad, 0 link) type V4L2 subdev subtype Unknown device node name /dev/subdev9 pad0: Input Can you help me understand how to