Re: Improving ov7670 sensor driver.
Hi Mauro, thank you for your interest. On 13 September 2012 15:00, Mauro Carvalho Chehab mche...@redhat.com wrote: Hi Javier, I'm not too familiar with soc_camera and ov7670 drivers, so my comments reflects my understanding of the question, without taking into account drivers specifics. Em 13-09-2012 06:48, javier Martin escreveu: Hi, our new i.MX27 based platform (Visstrim-SM20) uses an ov7675 sensor attached to the CSI interface. Apparently, this sensor is fully compatible with the old ov7670. For this reason, it seems rather sensible that they should share the same driver: ov7670.c One of the challenges we have to face is that capture video support for our platform is mx2_camera.c, which is a soc-camera host driver; while ov7670.c was developed for being used as part of a more complex video card. Here is the list of current users of ov7670: http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/ov519.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/sn9c20x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/vc032x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/via-camera.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/marvell-ccic/mcam-core.c In order to avoid breakages on those drivers, we need to be sure that none of the changes will alter the register settings used there. (C/C Hans de Goede, as he is the gspca maintainer) These are basically the improvements we need to make to this driver in order to satisfy our needs: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... 2.- Add the possibility to bypass PLL and clkrc preescaler. 3.- Adjust vstart/vstop in order to remove an horizontal green line. 4.- Disable pixclk during horizontal blanking. 5.- min_height, min_width should be respected in try_fmt(). 6.- Pass platform data when used with a soc-camera host driver. 7.- Add V4L2_CID_POWER_LINE_FREQUENCY ctrl. Doing one patch per change helps to review the changes individually. I suspect that it will needed to be tested with the above drivers, anyway. I will try to summarize below why we need to accomplish each of the previous tasks and what solution we propose for them: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... Why? Because soc-camera needs to inherit v4l2 subdevice controls in order to expose them to user space. How? Something like the following, incomplete, patch: --- @@ -190,6 +196,7 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; struct ov7670_format_struct *fmt; /* Current format */ unsigned char sat; /* Saturation value */ int hue;/* Hue value */ @@ -1480,10 +1518,14 @@ static int ov7670_s_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *r /* --- */ +static const struct v4l2_ctrl_ops ov7670_ctrl_ops = { + .s_ctrl = ov7670_s_ctrl, +}; + static const struct v4l2_subdev_core_ops ov7670_core_ops = { .g_chip_ident = ov7670_g_chip_ident, - .g_ctrl = ov7670_g_ctrl, - .s_ctrl = ov7670_s_ctrl, + .g_ctrl = v4l2_subdev_g_ctrl, + .s_ctrl = v4l2_subdev_s_ctrl, .queryctrl = ov7670_queryctrl, .reset = ov7670_reset, .init = ov7670_init, @@ -1551,6 +1600,16 @@ static int ov7670_probe(struct i2c_client *client, v4l_info(client, chip found @ 0x%02x (%s)\n, client-addr 1, client-adapter-name); + v4l2_ctrl_handler_init(info-hdl, 1); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); ... ... + sd-ctrl_handler = info-hdl; + if (info-hdl.error) { + v4l2_ctrl_handler_free(info-hdl); + kfree(info); + return info-hdl.error; + } + v4l2_ctrl_handler_setup(info-hdl); + --- Tests are required here, but I don't think this would break anything. 2.- Add the possibility to bypass PLL and clkrc preescaler. Why? The formula to get the desired frame rate in this chip in YUV is the following: fps = fpclk / (2 * 510 * 784) This means that for a desired fps = 30 we need fpclk = 24MHz. For that reason we have a clean 24MHz xvclk input that comes from an oscillator. If we enable the PLL it internally transforms the 24MHz in 22MHz and thus fps is not 30 but 27. In order to get 30fps we need to bypass the PLL. How? Defining a platform flag 'direct_clk' or similar that allows xvclk being used directly as the pixel clock. As this should be a new platform data, provided that the old behavior is to use the old formula, this shouldn't break anything. 3.- Adjust vstart/vstop in order to remove an horizontal
Re: Improving ov7670 sensor driver.
Hi Javier, On Thursday 13 September 2012 11:48:17 javier Martin wrote: Hi, our new i.MX27 based platform (Visstrim-SM20) uses an ov7675 sensor attached to the CSI interface. Apparently, this sensor is fully compatible with the old ov7670. For this reason, it seems rather sensible that they should share the same driver: ov7670.c One of the challenges we have to face is that capture video support for our platform is mx2_camera.c, which is a soc-camera host driver; while ov7670.c was developed for being used as part of a more complex video card. Here is the list of current users of ov7670: http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/ov519.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/sn9c20x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/vc032x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/via-camera.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/marvell-ccic/mcam-core .c These are basically the improvements we need to make to this driver in order to satisfy our needs: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... 2.- Add the possibility to bypass PLL and clkrc preescaler. 3.- Adjust vstart/vstop in order to remove an horizontal green line. 4.- Disable pixclk during horizontal blanking. 5.- min_height, min_width should be respected in try_fmt(). 6.- Pass platform data when used with a soc-camera host driver. 7.- Add V4L2_CID_POWER_LINE_FREQUENCY ctrl. 8. Make sure it still works when used with a non soc-camera bridge. That's the tricky part. I've spent time working on this for the ov772x driver (albeit in the other direction, making it usable with a non soc-camera bridge). You can find work-in-progress hacks at http://git.linuxtv.org/pinchartl/media.git/shortlog/refs/heads/omap3isp- sensors-board The basic idea is that the soc-camera platform data structure needs to be split into a generic device part (currently called soc_camera_pdtata, which should be renamed to something not related to soc-camera), and a soc-camera specific structure. The device should only see the device part, and the soc- camera framework will get the host part. That's currently not implemented. -- 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: Improving ov7670 sensor driver.
On Thu 13 September 2012 11:48:17 javier Martin wrote: Hi, our new i.MX27 based platform (Visstrim-SM20) uses an ov7675 sensor attached to the CSI interface. Apparently, this sensor is fully compatible with the old ov7670. For this reason, it seems rather sensible that they should share the same driver: ov7670.c One of the challenges we have to face is that capture video support for our platform is mx2_camera.c, which is a soc-camera host driver; while ov7670.c was developed for being used as part of a more complex video card. Here is the list of current users of ov7670: http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/ov519.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/sn9c20x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/vc032x.c These do not actually use the ov7670 driver. They program it themselves. It would be nice if the gspca driver would get support for subdevs, but that's a separate topic. http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/via-camera.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/marvell-ccic/mcam-core.c These are basically the improvements we need to make to this driver in order to satisfy our needs: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... 2.- Add the possibility to bypass PLL and clkrc preescaler. 3.- Adjust vstart/vstop in order to remove an horizontal green line. 4.- Disable pixclk during horizontal blanking. 5.- min_height, min_width should be respected in try_fmt(). 6.- Pass platform data when used with a soc-camera host driver. 7.- Add V4L2_CID_POWER_LINE_FREQUENCY ctrl. I will try to summarize below why we need to accomplish each of the previous tasks and what solution we propose for them: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... Why? Because soc-camera needs to inherit v4l2 subdevice controls in order to expose them to user space. How? Something like the following, incomplete, patch: Luckily you didn't do too much work on this. I have old patches for this in this tree: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/cafe-ctrl The main reason why I never continued with this was that at the time I wrote this I realized that the control framework needed proper support for what's now called auto-clusters (i.e. how to handle autofoo/foo controls like autogain and gain correctly). I intended to pick this up at some time, but never got around to it. I think these patches will still apply with some work, but it needs to be converted to use the autocluster support that's now in the control framework. --- @@ -190,6 +196,7 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; struct ov7670_format_struct *fmt; /* Current format */ unsigned char sat; /* Saturation value */ int hue;/* Hue value */ @@ -1480,10 +1518,14 @@ static int ov7670_s_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *r /* --- */ +static const struct v4l2_ctrl_ops ov7670_ctrl_ops = { + .s_ctrl = ov7670_s_ctrl, +}; + static const struct v4l2_subdev_core_ops ov7670_core_ops = { .g_chip_ident = ov7670_g_chip_ident, - .g_ctrl = ov7670_g_ctrl, - .s_ctrl = ov7670_s_ctrl, + .g_ctrl = v4l2_subdev_g_ctrl, + .s_ctrl = v4l2_subdev_s_ctrl, .queryctrl = ov7670_queryctrl, .reset = ov7670_reset, .init = ov7670_init, @@ -1551,6 +1600,16 @@ static int ov7670_probe(struct i2c_client *client, v4l_info(client, chip found @ 0x%02x (%s)\n, client-addr 1, client-adapter-name); + v4l2_ctrl_handler_init(info-hdl, 1); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); ... ... + sd-ctrl_handler = info-hdl; + if (info-hdl.error) { + v4l2_ctrl_handler_free(info-hdl); + kfree(info); + return info-hdl.error; + } + v4l2_ctrl_handler_setup(info-hdl); + --- 2.- Add the possibility to bypass PLL and clkrc preescaler. Why? The formula to get the desired frame rate in this chip in YUV is the following: fps = fpclk / (2 * 510 * 784) This means that for a desired fps = 30 we need fpclk = 24MHz. For that reason we have a clean 24MHz xvclk input that comes from an oscillator. If we enable the PLL it internally transforms the 24MHz in 22MHz and thus fps is not 30 but 27. In order to get 30fps we need to bypass the PLL. How? Defining a platform flag 'direct_clk' or similar that allows xvclk being used directly as the pixel clock. 3.- Adjust vstart/vstop in order to remove an horizontal
Re: Improving ov7670 sensor driver.
Hi Hans, thank you for your response. On 13 September 2012 12:07, Hans Verkuil hverk...@xs4all.nl wrote: On Thu 13 September 2012 11:48:17 javier Martin wrote: Hi, our new i.MX27 based platform (Visstrim-SM20) uses an ov7675 sensor attached to the CSI interface. Apparently, this sensor is fully compatible with the old ov7670. For this reason, it seems rather sensible that they should share the same driver: ov7670.c One of the challenges we have to face is that capture video support for our platform is mx2_camera.c, which is a soc-camera host driver; while ov7670.c was developed for being used as part of a more complex video card. Here is the list of current users of ov7670: http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/ov519.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/sn9c20x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/vc032x.c These do not actually use the ov7670 driver. They program it themselves. It would be nice if the gspca driver would get support for subdevs, but that's a separate topic. http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/via-camera.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/marvell-ccic/mcam-core.c These are basically the improvements we need to make to this driver in order to satisfy our needs: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... 2.- Add the possibility to bypass PLL and clkrc preescaler. 3.- Adjust vstart/vstop in order to remove an horizontal green line. 4.- Disable pixclk during horizontal blanking. 5.- min_height, min_width should be respected in try_fmt(). 6.- Pass platform data when used with a soc-camera host driver. 7.- Add V4L2_CID_POWER_LINE_FREQUENCY ctrl. I will try to summarize below why we need to accomplish each of the previous tasks and what solution we propose for them: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... Why? Because soc-camera needs to inherit v4l2 subdevice controls in order to expose them to user space. How? Something like the following, incomplete, patch: Luckily you didn't do too much work on this. I have old patches for this in this tree: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/cafe-ctrl Great. This is the reason why I like to always ask first. The main reason why I never continued with this was that at the time I wrote this I realized that the control framework needed proper support for what's now called auto-clusters (i.e. how to handle autofoo/foo controls like autogain and gain correctly). I intended to pick this up at some time, but never got around to it. I think these patches will still apply with some work, but it needs to be converted to use the autocluster support that's now in the control framework. --- @@ -190,6 +196,7 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; struct ov7670_format_struct *fmt; /* Current format */ unsigned char sat; /* Saturation value */ int hue;/* Hue value */ @@ -1480,10 +1518,14 @@ static int ov7670_s_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *r /* --- */ +static const struct v4l2_ctrl_ops ov7670_ctrl_ops = { + .s_ctrl = ov7670_s_ctrl, +}; + static const struct v4l2_subdev_core_ops ov7670_core_ops = { .g_chip_ident = ov7670_g_chip_ident, - .g_ctrl = ov7670_g_ctrl, - .s_ctrl = ov7670_s_ctrl, + .g_ctrl = v4l2_subdev_g_ctrl, + .s_ctrl = v4l2_subdev_s_ctrl, .queryctrl = ov7670_queryctrl, .reset = ov7670_reset, .init = ov7670_init, @@ -1551,6 +1600,16 @@ static int ov7670_probe(struct i2c_client *client, v4l_info(client, chip found @ 0x%02x (%s)\n, client-addr 1, client-adapter-name); + v4l2_ctrl_handler_init(info-hdl, 1); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); ... ... + sd-ctrl_handler = info-hdl; + if (info-hdl.error) { + v4l2_ctrl_handler_free(info-hdl); + kfree(info); + return info-hdl.error; + } + v4l2_ctrl_handler_setup(info-hdl); + --- 2.- Add the possibility to bypass PLL and clkrc preescaler. Why? The formula to get the desired frame rate in this chip in YUV is the following: fps = fpclk / (2 * 510 * 784) This means that for a desired fps = 30 we need fpclk = 24MHz. For that reason we have a clean 24MHz xvclk input that comes from an oscillator. If we enable the PLL it internally transforms the 24MHz in 22MHz and thus fps is not 30 but 27. In order to get 30fps we need to bypass the PLL. How?
Re: Improving ov7670 sensor driver.
On Thu 13 September 2012 12:47:53 javier Martin wrote: Hi Hans, thank you for your response. On 13 September 2012 12:07, Hans Verkuil hverk...@xs4all.nl wrote: On Thu 13 September 2012 11:48:17 javier Martin wrote: Hi, our new i.MX27 based platform (Visstrim-SM20) uses an ov7675 sensor attached to the CSI interface. Apparently, this sensor is fully compatible with the old ov7670. For this reason, it seems rather sensible that they should share the same driver: ov7670.c One of the challenges we have to face is that capture video support for our platform is mx2_camera.c, which is a soc-camera host driver; while ov7670.c was developed for being used as part of a more complex video card. Here is the list of current users of ov7670: http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/ov519.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/sn9c20x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/vc032x.c These do not actually use the ov7670 driver. They program it themselves. It would be nice if the gspca driver would get support for subdevs, but that's a separate topic. http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/via-camera.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/marvell-ccic/mcam-core.c These are basically the improvements we need to make to this driver in order to satisfy our needs: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... 2.- Add the possibility to bypass PLL and clkrc preescaler. 3.- Adjust vstart/vstop in order to remove an horizontal green line. 4.- Disable pixclk during horizontal blanking. 5.- min_height, min_width should be respected in try_fmt(). 6.- Pass platform data when used with a soc-camera host driver. 7.- Add V4L2_CID_POWER_LINE_FREQUENCY ctrl. I will try to summarize below why we need to accomplish each of the previous tasks and what solution we propose for them: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... Why? Because soc-camera needs to inherit v4l2 subdevice controls in order to expose them to user space. How? Something like the following, incomplete, patch: Luckily you didn't do too much work on this. I have old patches for this in this tree: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/cafe-ctrl Great. This is the reason why I like to always ask first. The main reason why I never continued with this was that at the time I wrote this I realized that the control framework needed proper support for what's now called auto-clusters (i.e. how to handle autofoo/foo controls like autogain and gain correctly). I intended to pick this up at some time, but never got around to it. I think these patches will still apply with some work, but it needs to be converted to use the autocluster support that's now in the control framework. --- @@ -190,6 +196,7 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; struct ov7670_format_struct *fmt; /* Current format */ unsigned char sat; /* Saturation value */ int hue;/* Hue value */ @@ -1480,10 +1518,14 @@ static int ov7670_s_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *r /* --- */ +static const struct v4l2_ctrl_ops ov7670_ctrl_ops = { + .s_ctrl = ov7670_s_ctrl, +}; + static const struct v4l2_subdev_core_ops ov7670_core_ops = { .g_chip_ident = ov7670_g_chip_ident, - .g_ctrl = ov7670_g_ctrl, - .s_ctrl = ov7670_s_ctrl, + .g_ctrl = v4l2_subdev_g_ctrl, + .s_ctrl = v4l2_subdev_s_ctrl, .queryctrl = ov7670_queryctrl, .reset = ov7670_reset, .init = ov7670_init, @@ -1551,6 +1600,16 @@ static int ov7670_probe(struct i2c_client *client, v4l_info(client, chip found @ 0x%02x (%s)\n, client-addr 1, client-adapter-name); + v4l2_ctrl_handler_init(info-hdl, 1); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); ... ... + sd-ctrl_handler = info-hdl; + if (info-hdl.error) { + v4l2_ctrl_handler_free(info-hdl); + kfree(info); + return info-hdl.error; + } + v4l2_ctrl_handler_setup(info-hdl); + --- 2.- Add the possibility to bypass PLL and clkrc preescaler. Why? The formula to get the desired frame rate in this chip in YUV is the following: fps = fpclk / (2 * 510 * 784) This means that for a desired fps = 30 we need fpclk = 24MHz. For that reason we have a clean 24MHz xvclk input
Re: Improving ov7670 sensor driver.
On 13 September 2012 13:00, Hans Verkuil hverk...@xs4all.nl wrote: On Thu 13 September 2012 12:47:53 javier Martin wrote: Hi Hans, thank you for your response. On 13 September 2012 12:07, Hans Verkuil hverk...@xs4all.nl wrote: On Thu 13 September 2012 11:48:17 javier Martin wrote: Hi, our new i.MX27 based platform (Visstrim-SM20) uses an ov7675 sensor attached to the CSI interface. Apparently, this sensor is fully compatible with the old ov7670. For this reason, it seems rather sensible that they should share the same driver: ov7670.c One of the challenges we have to face is that capture video support for our platform is mx2_camera.c, which is a soc-camera host driver; while ov7670.c was developed for being used as part of a more complex video card. Here is the list of current users of ov7670: http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/ov519.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/sn9c20x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/vc032x.c These do not actually use the ov7670 driver. They program it themselves. It would be nice if the gspca driver would get support for subdevs, but that's a separate topic. http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/via-camera.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/marvell-ccic/mcam-core.c These are basically the improvements we need to make to this driver in order to satisfy our needs: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... 2.- Add the possibility to bypass PLL and clkrc preescaler. 3.- Adjust vstart/vstop in order to remove an horizontal green line. 4.- Disable pixclk during horizontal blanking. 5.- min_height, min_width should be respected in try_fmt(). 6.- Pass platform data when used with a soc-camera host driver. 7.- Add V4L2_CID_POWER_LINE_FREQUENCY ctrl. I will try to summarize below why we need to accomplish each of the previous tasks and what solution we propose for them: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... Why? Because soc-camera needs to inherit v4l2 subdevice controls in order to expose them to user space. How? Something like the following, incomplete, patch: Luckily you didn't do too much work on this. I have old patches for this in this tree: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/cafe-ctrl Great. This is the reason why I like to always ask first. The main reason why I never continued with this was that at the time I wrote this I realized that the control framework needed proper support for what's now called auto-clusters (i.e. how to handle autofoo/foo controls like autogain and gain correctly). I intended to pick this up at some time, but never got around to it. I think these patches will still apply with some work, but it needs to be converted to use the autocluster support that's now in the control framework. --- @@ -190,6 +196,7 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; struct ov7670_format_struct *fmt; /* Current format */ unsigned char sat; /* Saturation value */ int hue;/* Hue value */ @@ -1480,10 +1518,14 @@ static int ov7670_s_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *r /* --- */ +static const struct v4l2_ctrl_ops ov7670_ctrl_ops = { + .s_ctrl = ov7670_s_ctrl, +}; + static const struct v4l2_subdev_core_ops ov7670_core_ops = { .g_chip_ident = ov7670_g_chip_ident, - .g_ctrl = ov7670_g_ctrl, - .s_ctrl = ov7670_s_ctrl, + .g_ctrl = v4l2_subdev_g_ctrl, + .s_ctrl = v4l2_subdev_s_ctrl, .queryctrl = ov7670_queryctrl, .reset = ov7670_reset, .init = ov7670_init, @@ -1551,6 +1600,16 @@ static int ov7670_probe(struct i2c_client *client, v4l_info(client, chip found @ 0x%02x (%s)\n, client-addr 1, client-adapter-name); + v4l2_ctrl_handler_init(info-hdl, 1); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); ... ... + sd-ctrl_handler = info-hdl; + if (info-hdl.error) { + v4l2_ctrl_handler_free(info-hdl); + kfree(info); + return info-hdl.error; + } + v4l2_ctrl_handler_setup(info-hdl); + --- 2.- Add the possibility to bypass PLL and clkrc preescaler. Why? The formula to get the desired frame rate in this chip in YUV is the following: fps = fpclk / (2 * 510 * 784) This means that for a desired fps = 30 we need
Re: Improving ov7670 sensor driver.
On Thu 13 September 2012 13:19:14 javier Martin wrote: On 13 September 2012 13:00, Hans Verkuil hverk...@xs4all.nl wrote: On Thu 13 September 2012 12:47:53 javier Martin wrote: 3.- Adjust vstart/vstop in order to remove an horizontal green line. Why? Currently, in the driver, for VGA, vstart = 10 and vstop = 490. From our tests we found out that vstart = 14, vstop = 494 in order to remove a disgusting horizontal green line in ov7675. How? It seems these sensor aren't provided with a version register or anything similar so I can't think of a clean solution for this yet. Suggestions will be much appreciated. Using platform_data for this is what springs to mind. I had thought about it too but, there Unfinished sentence? Yes. Sorry :) I meant that I had thought about it too but there are one pair of vstart,vstop values for each supported resolution: VGA, QVGA, CIF, QCIF. I could add 'vstart_vga', 'vstop_vga' as platform_data but in the future someone could want to add the same values for the other ones and I don't know if that would be acceptable. Another solution I just came up with would be adding a flag 'version' where we could indicate if we are dealing with an ov7670 or an ov7675 and change those 'vstart', 'vstop' values internally based on this. This could be useful for some other issues in the future. You can actually add support for a ov7675 to the ov7670 driver itself by adding a ov7675 entry to the ov7670_id table. See for example the i2c/saa7127.c driver on how to do that. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Improving ov7670 sensor driver.
Hi Javier, I'm not too familiar with soc_camera and ov7670 drivers, so my comments reflects my understanding of the question, without taking into account drivers specifics. Em 13-09-2012 06:48, javier Martin escreveu: Hi, our new i.MX27 based platform (Visstrim-SM20) uses an ov7675 sensor attached to the CSI interface. Apparently, this sensor is fully compatible with the old ov7670. For this reason, it seems rather sensible that they should share the same driver: ov7670.c One of the challenges we have to face is that capture video support for our platform is mx2_camera.c, which is a soc-camera host driver; while ov7670.c was developed for being used as part of a more complex video card. Here is the list of current users of ov7670: http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/ov519.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/sn9c20x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/gspca/vc032x.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/via-camera.c http://lxr.linux.no/#linux+v3.5.3/drivers/media/video/marvell-ccic/mcam-core.c In order to avoid breakages on those drivers, we need to be sure that none of the changes will alter the register settings used there. (C/C Hans de Goede, as he is the gspca maintainer) These are basically the improvements we need to make to this driver in order to satisfy our needs: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... 2.- Add the possibility to bypass PLL and clkrc preescaler. 3.- Adjust vstart/vstop in order to remove an horizontal green line. 4.- Disable pixclk during horizontal blanking. 5.- min_height, min_width should be respected in try_fmt(). 6.- Pass platform data when used with a soc-camera host driver. 7.- Add V4L2_CID_POWER_LINE_FREQUENCY ctrl. Doing one patch per change helps to review the changes individually. I suspect that it will needed to be tested with the above drivers, anyway. I will try to summarize below why we need to accomplish each of the previous tasks and what solution we propose for them: 1.- Adapt v4l2 controls to the subvevice control framework, with a proper ctrl handler, etc... Why? Because soc-camera needs to inherit v4l2 subdevice controls in order to expose them to user space. How? Something like the following, incomplete, patch: --- @@ -190,6 +196,7 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); struct ov7670_format_struct; /* coming later */ struct ov7670_info { struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; struct ov7670_format_struct *fmt; /* Current format */ unsigned char sat; /* Saturation value */ int hue;/* Hue value */ @@ -1480,10 +1518,14 @@ static int ov7670_s_register(struct v4l2_subdev *sd, struct v4l2_dbg_register *r /* --- */ +static const struct v4l2_ctrl_ops ov7670_ctrl_ops = { + .s_ctrl = ov7670_s_ctrl, +}; + static const struct v4l2_subdev_core_ops ov7670_core_ops = { .g_chip_ident = ov7670_g_chip_ident, - .g_ctrl = ov7670_g_ctrl, - .s_ctrl = ov7670_s_ctrl, + .g_ctrl = v4l2_subdev_g_ctrl, + .s_ctrl = v4l2_subdev_s_ctrl, .queryctrl = ov7670_queryctrl, .reset = ov7670_reset, .init = ov7670_init, @@ -1551,6 +1600,16 @@ static int ov7670_probe(struct i2c_client *client, v4l_info(client, chip found @ 0x%02x (%s)\n, client-addr 1, client-adapter-name); + v4l2_ctrl_handler_init(info-hdl, 1); + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); ... ... + sd-ctrl_handler = info-hdl; + if (info-hdl.error) { + v4l2_ctrl_handler_free(info-hdl); + kfree(info); + return info-hdl.error; + } + v4l2_ctrl_handler_setup(info-hdl); + --- Tests are required here, but I don't think this would break anything. 2.- Add the possibility to bypass PLL and clkrc preescaler. Why? The formula to get the desired frame rate in this chip in YUV is the following: fps = fpclk / (2 * 510 * 784) This means that for a desired fps = 30 we need fpclk = 24MHz. For that reason we have a clean 24MHz xvclk input that comes from an oscillator. If we enable the PLL it internally transforms the 24MHz in 22MHz and thus fps is not 30 but 27. In order to get 30fps we need to bypass the PLL. How? Defining a platform flag 'direct_clk' or similar that allows xvclk being used directly as the pixel clock. As this should be a new platform data, provided that the old behavior is to use the old formula, this shouldn't break anything. 3.- Adjust vstart/vstop in order to remove an horizontal green line. Why? Currently, in the driver, for VGA, vstart = 10 and vstop = 490. From our tests we found out that