linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mt9v022: support required register settings in snapshot mode
@ 2012-09-27 22:52 Anatolij Gustschin
       [not found] ` <Pine.LNX.4.64.1209281428490.5428@axis700.grange>
  0 siblings, 1 reply; 7+ messages in thread
From: Anatolij Gustschin @ 2012-09-27 22:52 UTC (permalink / raw)
  To: linux-media; +Cc: Guennadi Liakhovetski, Mauro Carvalho Chehab

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(-)

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;
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] mt9v022: support required register settings in snapshot mode
       [not found]   ` <20120928151004.7741efce@wker>
@ 2012-09-28 13:30     ` Guennadi Liakhovetski
  2012-09-28 13:42       ` Anatolij Gustschin
  2012-10-06 10:57       ` Anatolij Gustschin
  0 siblings, 2 replies; 7+ messages in thread
From: Guennadi Liakhovetski @ 2012-09-28 13:30 UTC (permalink / raw)
  To: Anatolij Gustschin; +Cc: linux-media, Mauro Carvalho Chehab

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/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mt9v022: support required register settings in snapshot mode
  2012-09-28 13:30     ` Guennadi Liakhovetski
@ 2012-09-28 13:42       ` Anatolij Gustschin
  2012-10-06 10:57       ` Anatolij Gustschin
  1 sibling, 0 replies; 7+ messages in thread
From: Anatolij Gustschin @ 2012-09-28 13:42 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media, Mauro Carvalho Chehab

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mt9v022: support required register settings in snapshot mode
  2012-09-28 13:30     ` Guennadi Liakhovetski
  2012-09-28 13:42       ` Anatolij Gustschin
@ 2012-10-06 10:57       ` Anatolij Gustschin
  2012-10-08  9:22         ` Guennadi Liakhovetski
  1 sibling, 1 reply; 7+ messages in thread
From: Anatolij Gustschin @ 2012-10-06 10:57 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media, Mauro Carvalho Chehab

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mt9v022: support required register settings in snapshot mode
  2012-10-06 10:57       ` Anatolij Gustschin
@ 2012-10-08  9:22         ` Guennadi Liakhovetski
  2012-10-08  9:47           ` Anatolij Gustschin
  0 siblings, 1 reply; 7+ messages in thread
From: Guennadi Liakhovetski @ 2012-10-08  9:22 UTC (permalink / raw)
  To: Anatolij Gustschin; +Cc: linux-media, Mauro Carvalho Chehab

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/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mt9v022: support required register settings in snapshot mode
  2012-10-08  9:22         ` Guennadi Liakhovetski
@ 2012-10-08  9:47           ` Anatolij Gustschin
  2012-10-08  9:55             ` Guennadi Liakhovetski
  0 siblings, 1 reply; 7+ messages in thread
From: Anatolij Gustschin @ 2012-10-08  9:47 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media, Mauro Carvalho Chehab, Detlev Zundel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mt9v022: support required register settings in snapshot mode
  2012-10-08  9:47           ` Anatolij Gustschin
@ 2012-10-08  9:55             ` Guennadi Liakhovetski
  0 siblings, 0 replies; 7+ messages in thread
From: Guennadi Liakhovetski @ 2012-10-08  9:55 UTC (permalink / raw)
  To: Anatolij Gustschin; +Cc: linux-media, Mauro Carvalho Chehab, Detlev Zundel

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/

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-10-08  9:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-27 22:52 [PATCH] mt9v022: support required register settings in snapshot mode Anatolij Gustschin
     [not found] ` <Pine.LNX.4.64.1209281428490.5428@axis700.grange>
     [not found]   ` <20120928151004.7741efce@wker>
2012-09-28 13:30     ` Guennadi Liakhovetski
2012-09-28 13:42       ` Anatolij Gustschin
2012-10-06 10:57       ` Anatolij Gustschin
2012-10-08  9:22         ` Guennadi Liakhovetski
2012-10-08  9:47           ` Anatolij Gustschin
2012-10-08  9:55             ` Guennadi Liakhovetski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).