| Message ID | 1348786362-28586-1-git-send-email-agust@denx.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1THMwx-0004Ww-J9 for patchwork@linuxtv.org; Fri, 28 Sep 2012 00:52:55 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-4) with esmtp for <patchwork@linuxtv.org> id 1THMww-0000gm-CW; Fri, 28 Sep 2012 00:52:55 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756210Ab2I0Www (ORCPT <rfc822;patchwork@linuxtv.org>); Thu, 27 Sep 2012 18:52:52 -0400 Received: from mail-out.m-online.net ([212.18.0.10]:43730 "EHLO mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756090Ab2I0Wwv (ORCPT <rfc822;linux-media@vger.kernel.org>); Thu, 27 Sep 2012 18:52:51 -0400 Received: from frontend1.mail.m-online.net (frontend1.mail.intern.m-online.net [192.168.8.180]) by mail-out.m-online.net (Postfix) with ESMTP id 3XSWWQ5pl7z3hhXT; Fri, 28 Sep 2012 00:52:50 +0200 (CEST) X-Auth-Info: irFmnjT9gf44w4Weq1VZ8Hv+r0I10w7hEmRJKx3lZOg= Received: from localhost (p4FC462DF.dip.t-dialin.net [79.196.98.223]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.mnet-online.de (Postfix) with ESMTPSA id 3XSWWQ43hczbc60; Fri, 28 Sep 2012 00:52:50 +0200 (CEST) From: Anatolij Gustschin <agust@denx.de> To: linux-media@vger.kernel.org Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>, Mauro Carvalho Chehab <mchehab@infradead.org> Subject: [PATCH] mt9v022: support required register settings in snapshot mode Date: Fri, 28 Sep 2012 00:52:42 +0200 Message-Id: <1348786362-28586-1-git-send-email-agust@denx.de> X-Mailer: git-send-email 1.7.1 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2012.9.27.224520 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODY_SIZE_3000_3999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_MEDIA_BODY 0, __CP_URI_IN_BODY 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS ' |
Commit Message
Anatolij Gustschin
Sept. 27, 2012, 10:52 p.m. UTC
Some camera systems cannot operate mt9v022 in normal mode and use
only the snapshot mode. The TechNote for mt9v022 (TN0960) and mt9v024
(TN-09-225) describes required register settings when configuring the
snapshot operation. The snapshot mode requires that certain automatic
functions of the image sensor should be disabled or set to fixed values.
According to the TechNote bit 2 and bit 9 in the register 0x20 must be
set in snapshot mode and unset for normal operation. This applies for
mt9v022 Rev.3 and mt9v024. Add required reg. 0x20 settings dependent on
sensor chip version.
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
drivers/media/i2c/soc_camera/mt9v022.c | 31 ++++++++++++++++++++++++++++---
1 files changed, 28 insertions(+), 3 deletions(-)
Comments
Hi Anatolij On Fri, 28 Sep 2012, Anatolij Gustschin wrote: > Some camera systems cannot operate mt9v022 in normal mode and use > only the snapshot mode. The TechNote for mt9v022 (TN0960) and mt9v024 > (TN-09-225) describes required register settings when configuring the > snapshot operation. The snapshot mode requires that certain automatic > functions of the image sensor should be disabled or set to fixed values. > > According to the TechNote bit 2 and bit 9 in the register 0x20 must be > set in snapshot mode and unset for normal operation. This applies for > mt9v022 Rev.3 and mt9v024. Add required reg. 0x20 settings dependent on > sensor chip version. > > Signed-off-by: Anatolij Gustschin <agust@denx.de> I'm afraid, I don't understand again > --- > drivers/media/i2c/soc_camera/mt9v022.c | 31 ++++++++++++++++++++++++++++--- > 1 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/soc_camera/mt9v022.c b/drivers/media/i2c/soc_camera/mt9v022.c > index 8feaddc..2abe999 100644 > --- a/drivers/media/i2c/soc_camera/mt9v022.c > +++ b/drivers/media/i2c/soc_camera/mt9v022.c > @@ -51,6 +51,7 @@ MODULE_PARM_DESC(sensor_type, "Sensor type: \"colour\" or \"monochrome\""); > #define MT9V022_PIXEL_OPERATION_MODE 0x0f > #define MT9V022_LED_OUT_CONTROL 0x1b > #define MT9V022_ADC_MODE_CONTROL 0x1c > +#define MT9V022_REG32 0x20 > #define MT9V022_ANALOG_GAIN 0x35 > #define MT9V022_BLACK_LEVEL_CALIB_CTRL 0x47 > #define MT9V022_PIXCLK_FV_LV 0x74 > @@ -79,7 +80,8 @@ MODULE_PARM_DESC(sensor_type, "Sensor type: \"colour\" or \"monochrome\""); > #define MT9V022_VERTICAL_BLANKING_MAX 3000 > #define MT9V022_VERTICAL_BLANKING_DEF 45 > > -#define is_mt9v024(id) (id == 0x1324) > +#define is_mt9v022_rev3(id) (id == 0x1313) > +#define is_mt9v024(id) (id == 0x1324) > > /* MT9V022 has only one fixed colorspace per pixelcode */ > struct mt9v022_datafmt { > @@ -153,6 +155,7 @@ struct mt9v022 { > int num_fmts; > int model; /* V4L2_IDENT_MT9V022* codes from v4l2-chip-ident.h */ > u16 chip_control; > + u16 chip_version; > unsigned short y_skip_top; /* Lines to skip at the top */ > }; > > @@ -235,12 +238,32 @@ static int mt9v022_s_stream(struct v4l2_subdev *sd, int enable) > struct i2c_client *client = v4l2_get_subdevdata(sd); > struct mt9v022 *mt9v022 = to_mt9v022(client); > > - if (enable) > + if (enable) { > /* Switch to master "normal" mode */ > mt9v022->chip_control &= ~0x10; > - else > + if (is_mt9v022_rev3(mt9v022->chip_version) || > + is_mt9v024(mt9v022->chip_version)) { > + /* > + * Unset snapshot mode specific settings: clear bit 9 > + * and bit 2 in reg. 0x20 when in normal mode. > + */ > + if (reg_clear(client, MT9V022_REG32, 0x204)) > + return -EIO; > + } > + } else { > /* Switch to snapshot mode */ > mt9v022->chip_control |= 0x10; > + if (is_mt9v022_rev3(mt9v022->chip_version) || > + is_mt9v024(mt9v022->chip_version)) { > + /* > + * Required settings for snapshot mode: set bit 9 > + * (RST enable) and bit 2 (CR enable) in reg. 0x20 > + * See TechNote TN0960 or TN-09-225. > + */ > + if (reg_set(client, MT9V022_REG32, 0x204)) > + return -EIO; > + } > + } Do I understand it right, that now on mt9v022 rev.3 and mt9v024 you unconditionally added using REG32 for leaving the snapshot mode on streamon and entering it on streamoff. This should be ok in principle, since that's also what we're trying to do, using the CHIP_CONTROL register. But in your comment you say, that on some _systems_ you can only _operate_ in snapshot mode. I.e. the snapshot mode enabled during running streaming, right? Then how does this patch help you with that? Thanks Guennadi > > if (reg_write(client, MT9V022_CHIP_CONTROL, mt9v022->chip_control) < 0) > return -EIO; > @@ -652,6 +675,8 @@ static int mt9v022_video_probe(struct i2c_client *client) > goto ei2c; > } > > + mt9v022->chip_version = data; > + > mt9v022->reg = is_mt9v024(data) ? &mt9v024_register : > &mt9v022_register; > > -- > 1.7.1 > --- 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, On Fri, 28 Sep 2012 14:33:34 +0200 (CEST) Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: ... > > @@ -235,12 +238,32 @@ static int mt9v022_s_stream(struct v4l2_subdev *sd, int enable) > > struct i2c_client *client = v4l2_get_subdevdata(sd); > > struct mt9v022 *mt9v022 = to_mt9v022(client); > > > > - if (enable) > > + if (enable) { > > /* Switch to master "normal" mode */ > > mt9v022->chip_control &= ~0x10; > > - else > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > + is_mt9v024(mt9v022->chip_version)) { > > + /* > > + * Unset snapshot mode specific settings: clear bit 9 > > + * and bit 2 in reg. 0x20 when in normal mode. > > + */ > > + if (reg_clear(client, MT9V022_REG32, 0x204)) > > + return -EIO; > > + } > > + } else { > > /* Switch to snapshot mode */ > > mt9v022->chip_control |= 0x10; > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > + is_mt9v024(mt9v022->chip_version)) { > > + /* > > + * Required settings for snapshot mode: set bit 9 > > + * (RST enable) and bit 2 (CR enable) in reg. 0x20 > > + * See TechNote TN0960 or TN-09-225. > > + */ > > + if (reg_set(client, MT9V022_REG32, 0x204)) > > + return -EIO; > > + } > > + } > > Do I understand it right, that now on mt9v022 rev.3 and mt9v024 you > unconditionally added using REG32 for leaving the snapshot mode on > streamon and entering it on streamoff. This should be ok in principle, > since that's also what we're trying to do, using the CHIP_CONTROL > register. But in your comment you say, that on some _systems_ you can only > _operate_ in snapshot mode. I.e. the snapshot mode enabled during running > streaming, right? Then how does this patch help you with that? Yes. But i.e. the driver calling the sub-device stream control function on streamon knows that the normal mode is not supported and therefore it calls this function with argument enable == 0, effectively setting the snapshot mode. Thanks, Anatolij -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 28 Sep 2012, Anatolij Gustschin wrote: > Hi Guennadi, > > On Fri, 28 Sep 2012 14:33:34 +0200 (CEST) > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > ... > > > @@ -235,12 +238,32 @@ static int mt9v022_s_stream(struct v4l2_subdev *sd, int enable) > > > struct i2c_client *client = v4l2_get_subdevdata(sd); > > > struct mt9v022 *mt9v022 = to_mt9v022(client); > > > > > > - if (enable) > > > + if (enable) { > > > /* Switch to master "normal" mode */ > > > mt9v022->chip_control &= ~0x10; > > > - else > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > + is_mt9v024(mt9v022->chip_version)) { > > > + /* > > > + * Unset snapshot mode specific settings: clear bit 9 > > > + * and bit 2 in reg. 0x20 when in normal mode. > > > + */ > > > + if (reg_clear(client, MT9V022_REG32, 0x204)) > > > + return -EIO; > > > + } > > > + } else { > > > /* Switch to snapshot mode */ > > > mt9v022->chip_control |= 0x10; > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > + is_mt9v024(mt9v022->chip_version)) { > > > + /* > > > + * Required settings for snapshot mode: set bit 9 > > > + * (RST enable) and bit 2 (CR enable) in reg. 0x20 > > > + * See TechNote TN0960 or TN-09-225. > > > + */ > > > + if (reg_set(client, MT9V022_REG32, 0x204)) > > > + return -EIO; > > > + } > > > + } > > > > Do I understand it right, that now on mt9v022 rev.3 and mt9v024 you > > unconditionally added using REG32 for leaving the snapshot mode on > > streamon and entering it on streamoff. This should be ok in principle, > > since that's also what we're trying to do, using the CHIP_CONTROL > > register. But in your comment you say, that on some _systems_ you can only > > _operate_ in snapshot mode. I.e. the snapshot mode enabled during running > > streaming, right? Then how does this patch help you with that? > > Yes. But i.e. the driver calling the sub-device stream control function > on streamon knows that the normal mode is not supported and therefore it > calls this function with argument enable == 0, effectively setting the > snapshot mode. Right, I thought you could be doing that... Well, on the one hand I should be happy, that the problem is solved without driver modifications, OTOH this isn't pretty... In fact this shouldn't work at all. After a stream-off the buffer queue should be stopped too. However, to properly implement this you'd have to add a new V4L2 control to switch into snapshot mode. 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 28 Sep 2012 15:30:33 +0200 (CEST) Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: ... > > Yes. But i.e. the driver calling the sub-device stream control function > > on streamon knows that the normal mode is not supported and therefore it > > calls this function with argument enable == 0, effectively setting the > > snapshot mode. > > Right, I thought you could be doing that... Well, on the one hand I should > be happy, that the problem is solved without driver modifications, OTOH > this isn't pretty... In fact this shouldn't work at all. After a > stream-off the buffer queue should be stopped too. Yes, the capture driver stops its buffer queue on stream-off. Thanks, Anatolij -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, On Fri, 28 Sep 2012 15:30:33 +0200 (CEST) Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Fri, 28 Sep 2012, Anatolij Gustschin wrote: > > > Hi Guennadi, > > > > On Fri, 28 Sep 2012 14:33:34 +0200 (CEST) > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > ... > > > > @@ -235,12 +238,32 @@ static int mt9v022_s_stream(struct v4l2_subdev *sd, int enable) > > > > struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > struct mt9v022 *mt9v022 = to_mt9v022(client); > > > > > > > > - if (enable) > > > > + if (enable) { > > > > /* Switch to master "normal" mode */ > > > > mt9v022->chip_control &= ~0x10; > > > > - else > > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > > + is_mt9v024(mt9v022->chip_version)) { > > > > + /* > > > > + * Unset snapshot mode specific settings: clear bit 9 > > > > + * and bit 2 in reg. 0x20 when in normal mode. > > > > + */ > > > > + if (reg_clear(client, MT9V022_REG32, 0x204)) > > > > + return -EIO; > > > > + } > > > > + } else { > > > > /* Switch to snapshot mode */ > > > > mt9v022->chip_control |= 0x10; > > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > > + is_mt9v024(mt9v022->chip_version)) { > > > > + /* > > > > + * Required settings for snapshot mode: set bit 9 > > > > + * (RST enable) and bit 2 (CR enable) in reg. 0x20 > > > > + * See TechNote TN0960 or TN-09-225. > > > > + */ > > > > + if (reg_set(client, MT9V022_REG32, 0x204)) > > > > + return -EIO; > > > > + } > > > > + } > > > > > > Do I understand it right, that now on mt9v022 rev.3 and mt9v024 you > > > unconditionally added using REG32 for leaving the snapshot mode on > > > streamon and entering it on streamoff. This should be ok in principle, > > > since that's also what we're trying to do, using the CHIP_CONTROL > > > register. But in your comment you say, that on some _systems_ you can only > > > _operate_ in snapshot mode. I.e. the snapshot mode enabled during running > > > streaming, right? Then how does this patch help you with that? > > > > Yes. But i.e. the driver calling the sub-device stream control function > > on streamon knows that the normal mode is not supported and therefore it > > calls this function with argument enable == 0, effectively setting the > > snapshot mode. > > Right, I thought you could be doing that... Well, on the one hand I should > be happy, that the problem is solved without driver modifications, OTOH > this isn't pretty... In fact this shouldn't work at all. After a > stream-off the buffer queue should be stopped too. Why shouldn't it work? The buffer queue is handled by the host driver, not by the sensor driver. And in my case the host driver stops the buffer queue in its streamoff, as it should. It works without issues and doesn't cause any problems for other mt9v022 users. Thanks, Anatolij -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 6 Oct 2012, Anatolij Gustschin wrote: > Hi Guennadi, > > On Fri, 28 Sep 2012 15:30:33 +0200 (CEST) > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > > On Fri, 28 Sep 2012, Anatolij Gustschin wrote: > > > > > Hi Guennadi, > > > > > > On Fri, 28 Sep 2012 14:33:34 +0200 (CEST) > > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > > ... > > > > > @@ -235,12 +238,32 @@ static int mt9v022_s_stream(struct v4l2_subdev *sd, int enable) > > > > > struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > > struct mt9v022 *mt9v022 = to_mt9v022(client); > > > > > > > > > > - if (enable) > > > > > + if (enable) { > > > > > /* Switch to master "normal" mode */ > > > > > mt9v022->chip_control &= ~0x10; > > > > > - else > > > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > > > + is_mt9v024(mt9v022->chip_version)) { > > > > > + /* > > > > > + * Unset snapshot mode specific settings: clear bit 9 > > > > > + * and bit 2 in reg. 0x20 when in normal mode. > > > > > + */ > > > > > + if (reg_clear(client, MT9V022_REG32, 0x204)) > > > > > + return -EIO; > > > > > + } > > > > > + } else { > > > > > /* Switch to snapshot mode */ > > > > > mt9v022->chip_control |= 0x10; > > > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > > > + is_mt9v024(mt9v022->chip_version)) { > > > > > + /* > > > > > + * Required settings for snapshot mode: set bit 9 > > > > > + * (RST enable) and bit 2 (CR enable) in reg. 0x20 > > > > > + * See TechNote TN0960 or TN-09-225. > > > > > + */ > > > > > + if (reg_set(client, MT9V022_REG32, 0x204)) > > > > > + return -EIO; > > > > > + } > > > > > + } > > > > > > > > Do I understand it right, that now on mt9v022 rev.3 and mt9v024 you > > > > unconditionally added using REG32 for leaving the snapshot mode on > > > > streamon and entering it on streamoff. This should be ok in principle, > > > > since that's also what we're trying to do, using the CHIP_CONTROL > > > > register. But in your comment you say, that on some _systems_ you can only > > > > _operate_ in snapshot mode. I.e. the snapshot mode enabled during running > > > > streaming, right? Then how does this patch help you with that? > > > > > > Yes. But i.e. the driver calling the sub-device stream control function > > > on streamon knows that the normal mode is not supported and therefore it > > > calls this function with argument enable == 0, effectively setting the > > > snapshot mode. > > > > Right, I thought you could be doing that... Well, on the one hand I should > > be happy, that the problem is solved without driver modifications, OTOH > > this isn't pretty... In fact this shouldn't work at all. After a > > stream-off the buffer queue should be stopped too. > > Why shouldn't it work? The buffer queue is handled by the host driver, > not by the sensor driver. And in my case the host driver stops the buffer > queue in its streamoff, as it should. It works without issues and doesn't > cause any problems for other mt9v022 users. Because one shouldn't abuse the API by activating streaming on the bridge driver and deactivating it on the sensor. 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 8 Oct 2012 11:22:05 +0200 (CEST) Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Sat, 6 Oct 2012, Anatolij Gustschin wrote: > > > Hi Guennadi, > > > > On Fri, 28 Sep 2012 15:30:33 +0200 (CEST) > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > > > > On Fri, 28 Sep 2012, Anatolij Gustschin wrote: > > > > > > > Hi Guennadi, > > > > > > > > On Fri, 28 Sep 2012 14:33:34 +0200 (CEST) > > > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > > > ... > > > > > > @@ -235,12 +238,32 @@ static int mt9v022_s_stream(struct v4l2_subdev *sd, int enable) > > > > > > struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > > > struct mt9v022 *mt9v022 = to_mt9v022(client); > > > > > > > > > > > > - if (enable) > > > > > > + if (enable) { > > > > > > /* Switch to master "normal" mode */ > > > > > > mt9v022->chip_control &= ~0x10; > > > > > > - else > > > > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > > > > + is_mt9v024(mt9v022->chip_version)) { > > > > > > + /* > > > > > > + * Unset snapshot mode specific settings: clear bit 9 > > > > > > + * and bit 2 in reg. 0x20 when in normal mode. > > > > > > + */ > > > > > > + if (reg_clear(client, MT9V022_REG32, 0x204)) > > > > > > + return -EIO; > > > > > > + } > > > > > > + } else { > > > > > > /* Switch to snapshot mode */ > > > > > > mt9v022->chip_control |= 0x10; > > > > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > > > > + is_mt9v024(mt9v022->chip_version)) { > > > > > > + /* > > > > > > + * Required settings for snapshot mode: set bit 9 > > > > > > + * (RST enable) and bit 2 (CR enable) in reg. 0x20 > > > > > > + * See TechNote TN0960 or TN-09-225. > > > > > > + */ > > > > > > + if (reg_set(client, MT9V022_REG32, 0x204)) > > > > > > + return -EIO; > > > > > > + } > > > > > > + } > > > > > > > > > > Do I understand it right, that now on mt9v022 rev.3 and mt9v024 you > > > > > unconditionally added using REG32 for leaving the snapshot mode on > > > > > streamon and entering it on streamoff. This should be ok in principle, > > > > > since that's also what we're trying to do, using the CHIP_CONTROL > > > > > register. But in your comment you say, that on some _systems_ you can only > > > > > _operate_ in snapshot mode. I.e. the snapshot mode enabled during running > > > > > streaming, right? Then how does this patch help you with that? > > > > > > > > Yes. But i.e. the driver calling the sub-device stream control function > > > > on streamon knows that the normal mode is not supported and therefore it > > > > calls this function with argument enable == 0, effectively setting the > > > > snapshot mode. > > > > > > Right, I thought you could be doing that... Well, on the one hand I should > > > be happy, that the problem is solved without driver modifications, OTOH > > > this isn't pretty... In fact this shouldn't work at all. After a > > > stream-off the buffer queue should be stopped too. > > > > Why shouldn't it work? The buffer queue is handled by the host driver, > > not by the sensor driver. And in my case the host driver stops the buffer > > queue in its streamoff, as it should. It works without issues and doesn't > > cause any problems for other mt9v022 users. > > Because one shouldn't abuse the API by activating streaming on the bridge > driver and deactivating it on the sensor. I don't see how is it an abuse of the API at all. The sensor is just configured to snapshot mode but is still able to stream if it is triggered continuously by external events. It is actually a streaming case. Thanks, Anatolij -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 8 Oct 2012, Anatolij Gustschin wrote: > On Mon, 8 Oct 2012 11:22:05 +0200 (CEST) > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > > On Sat, 6 Oct 2012, Anatolij Gustschin wrote: > > > > > Hi Guennadi, > > > > > > On Fri, 28 Sep 2012 15:30:33 +0200 (CEST) > > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > > > > > > On Fri, 28 Sep 2012, Anatolij Gustschin wrote: > > > > > > > > > Hi Guennadi, > > > > > > > > > > On Fri, 28 Sep 2012 14:33:34 +0200 (CEST) > > > > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > > > > > ... > > > > > > > @@ -235,12 +238,32 @@ static int mt9v022_s_stream(struct v4l2_subdev *sd, int enable) > > > > > > > struct i2c_client *client = v4l2_get_subdevdata(sd); > > > > > > > struct mt9v022 *mt9v022 = to_mt9v022(client); > > > > > > > > > > > > > > - if (enable) > > > > > > > + if (enable) { > > > > > > > /* Switch to master "normal" mode */ > > > > > > > mt9v022->chip_control &= ~0x10; > > > > > > > - else > > > > > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > > > > > + is_mt9v024(mt9v022->chip_version)) { > > > > > > > + /* > > > > > > > + * Unset snapshot mode specific settings: clear bit 9 > > > > > > > + * and bit 2 in reg. 0x20 when in normal mode. > > > > > > > + */ > > > > > > > + if (reg_clear(client, MT9V022_REG32, 0x204)) > > > > > > > + return -EIO; > > > > > > > + } > > > > > > > + } else { > > > > > > > /* Switch to snapshot mode */ > > > > > > > mt9v022->chip_control |= 0x10; > > > > > > > + if (is_mt9v022_rev3(mt9v022->chip_version) || > > > > > > > + is_mt9v024(mt9v022->chip_version)) { > > > > > > > + /* > > > > > > > + * Required settings for snapshot mode: set bit 9 > > > > > > > + * (RST enable) and bit 2 (CR enable) in reg. 0x20 > > > > > > > + * See TechNote TN0960 or TN-09-225. > > > > > > > + */ > > > > > > > + if (reg_set(client, MT9V022_REG32, 0x204)) > > > > > > > + return -EIO; > > > > > > > + } > > > > > > > + } > > > > > > > > > > > > Do I understand it right, that now on mt9v022 rev.3 and mt9v024 you > > > > > > unconditionally added using REG32 for leaving the snapshot mode on > > > > > > streamon and entering it on streamoff. This should be ok in principle, > > > > > > since that's also what we're trying to do, using the CHIP_CONTROL > > > > > > register. But in your comment you say, that on some _systems_ you can only > > > > > > _operate_ in snapshot mode. I.e. the snapshot mode enabled during running > > > > > > streaming, right? Then how does this patch help you with that? > > > > > > > > > > Yes. But i.e. the driver calling the sub-device stream control function > > > > > on streamon knows that the normal mode is not supported and therefore it > > > > > calls this function with argument enable == 0, effectively setting the > > > > > snapshot mode. > > > > > > > > Right, I thought you could be doing that... Well, on the one hand I should > > > > be happy, that the problem is solved without driver modifications, OTOH > > > > this isn't pretty... In fact this shouldn't work at all. After a > > > > stream-off the buffer queue should be stopped too. > > > > > > Why shouldn't it work? The buffer queue is handled by the host driver, > > > not by the sensor driver. And in my case the host driver stops the buffer > > > queue in its streamoff, as it should. It works without issues and doesn't > > > cause any problems for other mt9v022 users. > > > > Because one shouldn't abuse the API by activating streaming on the bridge > > driver and deactivating it on the sensor. > > I don't see how is it an abuse of the API at all. The sensor is just > configured to snapshot mode but is still able to stream if it is > triggered continuously by external events. It is actually a streaming > case. The STREAMOFF API is supposed to stop streaming completely. It is just on mt9v022 driver, that it happens, that no other way has been found to stop video operation but to switch to the snapshot mode. But the API itself prohibits any streaming in this mode. So, any other use of it is an abuse. 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/i2c/soc_camera/mt9v022.c b/drivers/media/i2c/soc_camera/mt9v022.c index 8feaddc..2abe999 100644 --- a/drivers/media/i2c/soc_camera/mt9v022.c +++ b/drivers/media/i2c/soc_camera/mt9v022.c @@ -51,6 +51,7 @@ MODULE_PARM_DESC(sensor_type, "Sensor type: \"colour\" or \"monochrome\""); #define MT9V022_PIXEL_OPERATION_MODE 0x0f #define MT9V022_LED_OUT_CONTROL 0x1b #define MT9V022_ADC_MODE_CONTROL 0x1c +#define MT9V022_REG32 0x20 #define MT9V022_ANALOG_GAIN 0x35 #define MT9V022_BLACK_LEVEL_CALIB_CTRL 0x47 #define MT9V022_PIXCLK_FV_LV 0x74 @@ -79,7 +80,8 @@ MODULE_PARM_DESC(sensor_type, "Sensor type: \"colour\" or \"monochrome\""); #define MT9V022_VERTICAL_BLANKING_MAX 3000 #define MT9V022_VERTICAL_BLANKING_DEF 45 -#define is_mt9v024(id) (id == 0x1324) +#define is_mt9v022_rev3(id) (id == 0x1313) +#define is_mt9v024(id) (id == 0x1324) /* MT9V022 has only one fixed colorspace per pixelcode */ struct mt9v022_datafmt { @@ -153,6 +155,7 @@ struct mt9v022 { int num_fmts; int model; /* V4L2_IDENT_MT9V022* codes from v4l2-chip-ident.h */ u16 chip_control; + u16 chip_version; unsigned short y_skip_top; /* Lines to skip at the top */ }; @@ -235,12 +238,32 @@ static int mt9v022_s_stream(struct v4l2_subdev *sd, int enable) struct i2c_client *client = v4l2_get_subdevdata(sd); struct mt9v022 *mt9v022 = to_mt9v022(client); - if (enable) + if (enable) { /* Switch to master "normal" mode */ mt9v022->chip_control &= ~0x10; - else + if (is_mt9v022_rev3(mt9v022->chip_version) || + is_mt9v024(mt9v022->chip_version)) { + /* + * Unset snapshot mode specific settings: clear bit 9 + * and bit 2 in reg. 0x20 when in normal mode. + */ + if (reg_clear(client, MT9V022_REG32, 0x204)) + return -EIO; + } + } else { /* Switch to snapshot mode */ mt9v022->chip_control |= 0x10; + if (is_mt9v022_rev3(mt9v022->chip_version) || + is_mt9v024(mt9v022->chip_version)) { + /* + * Required settings for snapshot mode: set bit 9 + * (RST enable) and bit 2 (CR enable) in reg. 0x20 + * See TechNote TN0960 or TN-09-225. + */ + if (reg_set(client, MT9V022_REG32, 0x204)) + return -EIO; + } + } if (reg_write(client, MT9V022_CHIP_CONTROL, mt9v022->chip_control) < 0) return -EIO; @@ -652,6 +675,8 @@ static int mt9v022_video_probe(struct i2c_client *client) goto ei2c; } + mt9v022->chip_version = data; + mt9v022->reg = is_mt9v024(data) ? &mt9v024_register : &mt9v022_register;