[PATCH 00/20] MT9M111/MT9M131

[PATCH 00/20] MT9M111/MT9M131

[Michael Grzeschik]

Hi everyone,

the following patchseries was created in a rewrite process of the
mt9m111 camera driver while it was tested for support of the very
similar silicon mt9m121. Some patches add functionality like panning or
test pattern generation or adjust rectengular positioning while others
do some restructuring. It has been tested as functional. Comments on
this are very welcome.

Michael Grzeschik (19):
  mt9m111: init chip after read CHIP_VERSION
  mt9m111: register cleanup hex to dec bitoffset
  mt9m111: added new bit offset defines
  mt9m111: added default row/col/width/height values
  mt9m111: changed MAX_{HEIGHT,WIDTH} values to silicon pixelcount
  mt9m111: changed MIN_DARK_COLS to MT9M131 spec count
  mt9m111: cropcap use defined default rect properties in defrect
  mt9m111: cropcap check if type is CAPTURE
  mt9m111: rewrite make_rect for positioning in debug
  mt9m111: added mt9m111 format structure
  mt9m111: s_crop add calculation of output size
  mt9m111: s_crop check for VIDEO_CAPTURE type
  mt9m111: added reg_mask function
  mt9m111: rewrite setup_rect, added soft_crop for smooth panning
  mt9m111: added more supported BE colour formats
  mt9m111: rewrite set_pixfmt
  mt9m111: make use of testpattern in debug mode
  mt9m111: try_fmt rewrite to use preset values
  mt9m111: s_fmt make use of try_fmt

Philipp Wiesner (1):
  mt9m111: Added indication that MT9M131 is supported by this driver

 drivers/media/video/Kconfig   |    5 +-
 drivers/media/video/mt9m111.c |  596 ++++++++++++++++++++++++++---------------
 2 files changed, 377 insertions(+), 224 deletions(-)

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 01/20] mt9m111: Added indication that MT9M131 is supported by this driver

[Michael Grzeschik]

From: Philipp Wiesner <p.wiesner@phytec.de>

Added this info to Kconfig and mt9m111.c, some comment cleanup,
replaced 'mt9m11x'-statements by clarifications or driver name.
Driver is fully compatible to mt9m131 which has only additional functions
compared to mt9m111. Those aren't used anyway at the moment.

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
---
 drivers/media/video/Kconfig   |    5 +++--
 drivers/media/video/mt9m111.c |   37 +++++++++++++++++++++++--------------
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index cdbbbe4..0e8cf24 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -871,10 +871,11 @@ config SOC_CAMERA_MT9M001
 	  and colour models.
 
 config SOC_CAMERA_MT9M111
-	tristate "mt9m111 and mt9m112 support"
+	tristate "mt9m111, mt9m112 and mt9m131 support"
 	depends on SOC_CAMERA && I2C
 	help
-	  This driver supports MT9M111 and MT9M112 cameras from Micron
+	  This driver supports MT9M111, MT9M112 and MT9M131 cameras from
+	  Micron/Aptina
 
 config SOC_CAMERA_MT9T031
 	tristate "mt9t031 support"
diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index d35f536..e934559 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -1,5 +1,5 @@
 /*
- * Driver for MT9M111/MT9M112 CMOS Image Sensor from Micron
+ * Driver for MT9M111/MT9M112/MT9M131 CMOS Image Sensor from Micron/Aptina
  *
  * Copyright (C) 2008, Robert Jarzmik <robert.jarzmik@free.fr>
  *
@@ -19,11 +19,14 @@
 #include <media/soc_camera.h>
 
 /*
- * mt9m111 and mt9m112 i2c address is 0x5d or 0x48 (depending on SAddr pin)
+ * MT9M111, MT9M112 and MT9M131:
+ * i2c address is 0x48 or 0x5d (depending on SADDR pin)
  * The platform has to define i2c_board_info and call i2c_register_board_info()
  */
 
-/* mt9m111: Sensor register addresses */
+/*
+ * Sensor core register addresses (0x000..0x0ff)
+ */
 #define MT9M111_CHIP_VERSION		0x000
 #define MT9M111_ROW_START		0x001
 #define MT9M111_COLUMN_START		0x002
@@ -72,8 +75,9 @@
 #define MT9M111_CTXT_CTRL_LED_FLASH_EN	(1 << 2)
 #define MT9M111_CTXT_CTRL_VBLANK_SEL_B	(1 << 1)
 #define MT9M111_CTXT_CTRL_HBLANK_SEL_B	(1 << 0)
+
 /*
- * mt9m111: Colorpipe register addresses (0x100..0x1ff)
+ * Colorpipe register addresses (0x100..0x1ff)
  */
 #define MT9M111_OPER_MODE_CTRL		0x106
 #define MT9M111_OUTPUT_FORMAT_CTRL	0x108
@@ -109,8 +113,9 @@
 #define MT9M111_OUTFMT_SWAP_YCbCr_C_Y	(1 << 1)
 #define MT9M111_OUTFMT_SWAP_RGB_EVEN	(1 << 1)
 #define MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr	(1 << 0)
+
 /*
- * mt9m111: Camera control register addresses (0x200..0x2ff not implemented)
+ * Camera control register addresses (0x200..0x2ff not implemented)
  */
 
 #define reg_read(reg) mt9m111_reg_read(client, MT9M111_##reg)
@@ -160,7 +165,8 @@ enum mt9m111_context {
 
 struct mt9m111 {
 	struct v4l2_subdev subdev;
-	int model;	/* V4L2_IDENT_MT9M11x* codes from v4l2-chip-ident.h */
+	int model;	/* V4L2_IDENT_MT9M111 or V4L2_IDENT_MT9M112 code */
+			/* from v4l2-chip-ident.h */
 	enum mt9m111_context context;
 	struct v4l2_rect rect;
 	const struct mt9m111_datafmt *fmt;
@@ -934,7 +940,7 @@ static int mt9m111_init(struct i2c_client *client)
 	if (!ret)
 		ret = mt9m111_set_autoexposure(client, mt9m111->autoexposure);
 	if (ret)
-		dev_err(&client->dev, "mt9m11x init failed: %d\n", ret);
+		dev_err(&client->dev, "mt9m111 init failed: %d\n", ret);
 	return ret;
 }
 
@@ -970,21 +976,24 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
-	case 0x143a: /* MT9M111 */
+	case 0x143a: /* MT9M111 or MT9M131 */
 		mt9m111->model = V4L2_IDENT_MT9M111;
+		dev_info(&client->dev,
+			"Detected a MT9M111/MT9M131 chip ID %x\n", data);
 		break;
 	case 0x148c: /* MT9M112 */
 		mt9m111->model = V4L2_IDENT_MT9M112;
+		dev_info(&client->dev, "Detected a MT9M112 chip ID %x\n", data);
 		break;
 	default:
 		ret = -ENODEV;
 		dev_err(&client->dev,
-			"No MT9M11x chip detected, register read %x\n", data);
+			"No MT9M111/MT9M112/MT9M131 chip detected, "
+			"register read %x\n",
+			data);
 		goto ei2c;
 	}
 
-	dev_info(&client->dev, "Detected a MT9M11x chip ID %x\n", data);
-
 ei2c:
 	return ret;
 }
@@ -1034,13 +1043,13 @@ static int mt9m111_probe(struct i2c_client *client,
 	int ret;
 
 	if (!icd) {
-		dev_err(&client->dev, "MT9M11x: missing soc-camera data!\n");
+		dev_err(&client->dev, "mt9m111: soc-camera data missing!\n");
 		return -EINVAL;
 	}
 
 	icl = to_soc_camera_link(icd);
 	if (!icl) {
-		dev_err(&client->dev, "MT9M11x driver needs platform data\n");
+		dev_err(&client->dev, "mt9m111: driver needs platform data\n");
 		return -EINVAL;
 	}
 
@@ -1116,6 +1125,6 @@ static void __exit mt9m111_mod_exit(void)
 module_init(mt9m111_mod_init);
 module_exit(mt9m111_mod_exit);
 
-MODULE_DESCRIPTION("Micron MT9M111/MT9M112 Camera driver");
+MODULE_DESCRIPTION("Micron/Aptina MT9M111/MT9M112/MT9M131 Camera driver");
 MODULE_AUTHOR("Robert Jarzmik");
 MODULE_LICENSE("GPL");
-- 
1.7.1

Re: [PATCH 01/20] mt9m111: Added indication that MT9M131 is supported by this driver

[Guennadi Liakhovetski]

On Fri, 30 Jul 2010, Michael Grzeschik wrote:

> From: Philipp Wiesner <p.wiesner@phytec.de>
> 
> Added this info to Kconfig and mt9m111.c, some comment cleanup,
> replaced 'mt9m11x'-statements by clarifications or driver name.
> Driver is fully compatible to mt9m131 which has only additional functions
> compared to mt9m111. Those aren't used anyway at the moment.
> 
> Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
> ---
>  drivers/media/video/Kconfig   |    5 +++--
>  drivers/media/video/mt9m111.c |   37 +++++++++++++++++++++++--------------
>  2 files changed, 26 insertions(+), 16 deletions(-)
> 

[snip]

> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index d35f536..e934559 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c

[snip]

> @@ -970,21 +976,24 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
>  	data = reg_read(CHIP_VERSION);
>  
>  	switch (data) {
> -	case 0x143a: /* MT9M111 */
> +	case 0x143a: /* MT9M111 or MT9M131 */
>  		mt9m111->model = V4L2_IDENT_MT9M111;
> +		dev_info(&client->dev,
> +			"Detected a MT9M111/MT9M131 chip ID %x\n", data);
>  		break;
>  	case 0x148c: /* MT9M112 */
>  		mt9m111->model = V4L2_IDENT_MT9M112;
> +		dev_info(&client->dev, "Detected a MT9M112 chip ID %x\n", data);
>  		break;
>  	default:
>  		ret = -ENODEV;
>  		dev_err(&client->dev,
> -			"No MT9M11x chip detected, register read %x\n", data);
> +			"No MT9M111/MT9M112/MT9M131 chip detected, "
> +			"register read %x\n",

Please, join the strings onto one line. Don't worry about > 80 characters.

> +			data);
>  		goto ei2c;
>  	}
>  
> -	dev_info(&client->dev, "Detected a MT9M11x chip ID %x\n", data);
> -
>  ei2c:
>  	return ret;
>  }

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 01/20] mt9m111: Added indication that MT9M131 is supported by this driver

[Robert Jarzmik]

Michael Grzeschik <m.grzeschik@pengutronix.de> writes:

> From: Philipp Wiesner <p.wiesner@phytec.de>
>
> Added this info to Kconfig and mt9m111.c, some comment cleanup,
> replaced 'mt9m11x'-statements by clarifications or driver name.
> Driver is fully compatible to mt9m131 which has only additional functions
> compared to mt9m111. Those aren't used anyway at the moment.

<zip>
>  
> -	dev_info(&client->dev, "Detected a MT9M11x chip ID %x\n", data);
> -

Why this one ? It signals a sensor was successfully detected. Maybe a
replacement from MT9M11x to MT9M1xx would be better ? Or if your real intention
is to remove the message, then transform it to dev_dbg(), and say why you did it
in the commit message.

Cheers.

--
Robert

Re: [PATCH 01/20] mt9m111: Added indication that MT9M131 is supported by this driver

[Guennadi Liakhovetski]

On Sat, 31 Jul 2010, Robert Jarzmik wrote:

> Michael Grzeschik <m.grzeschik@pengutronix.de> writes:
> 
> > From: Philipp Wiesner <p.wiesner@phytec.de>
> >
> > Added this info to Kconfig and mt9m111.c, some comment cleanup,
> > replaced 'mt9m11x'-statements by clarifications or driver name.
> > Driver is fully compatible to mt9m131 which has only additional functions
> > compared to mt9m111. Those aren't used anyway at the moment.
> 
> <zip>
> >  
> > -	dev_info(&client->dev, "Detected a MT9M11x chip ID %x\n", data);
> > -
> 
> Why this one ? It signals a sensor was successfully detected. Maybe a
> replacement from MT9M11x to MT9M1xx would be better ? Or if your real intention
> is to remove the message, then transform it to dev_dbg(), and say why you did it
> in the commit message.

Robert, the message is not removed, it is moved into two chip ID switch 
cases.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 01/20] mt9m111: Added indication that MT9M131 is supported by this driver

[Robert Jarzmik]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

>> Why this one ? It signals a sensor was successfully detected. Maybe a
>> replacement from MT9M11x to MT9M1xx would be better ? Or if your real intention
>> is to remove the message, then transform it to dev_dbg(), and say why you did it
>> in the commit message.
>
> Robert, the message is not removed, it is moved into two chip ID switch 
> cases.

Damn, you're right.

I have no other comments on that one, looks good to me.

Cheers.

-- 
Robert

[PATCH 02/20] mt9m111: init chip after read CHIP_VERSION

[Michael Grzeschik]

Moved mt9m111_init after the chip version detection passage: I
don't like the idea of writing on a device we haven't identified
yet.

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index e934559..39dff7c 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -969,10 +969,6 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
 	mt9m111->swap_rgb_even_odd = 1;
 	mt9m111->swap_rgb_red_blue = 1;
 
-	ret = mt9m111_init(client);
-	if (ret)
-		goto ei2c;
-
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
@@ -994,6 +990,8 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
 		goto ei2c;
 	}
 
+	ret = mt9m111_init(client);
+
 ei2c:
 	return ret;
 }
-- 
1.7.1

Re: [PATCH 02/20] mt9m111: init chip after read CHIP_VERSION

[Guennadi Liakhovetski]

On Fri, 30 Jul 2010, Michael Grzeschik wrote:

> Moved mt9m111_init after the chip version detection passage: I
> don't like the idea of writing on a device we haven't identified
> yet.

In principle it's correct, but what do you do, if a chip cannot be probed, 
before it is initialised / enabled? Actually, this shouldn't be the case, 
devices should be available for probing without any initialisation. So, we 
have to ask the original author, whether this really was necessary, 
Robert?

Thanks
Guennadi

> 
> Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/media/video/mt9m111.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index e934559..39dff7c 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -969,10 +969,6 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
>  	mt9m111->swap_rgb_even_odd = 1;
>  	mt9m111->swap_rgb_red_blue = 1;
>  
> -	ret = mt9m111_init(client);
> -	if (ret)
> -		goto ei2c;
> -
>  	data = reg_read(CHIP_VERSION);
>  
>  	switch (data) {
> @@ -994,6 +990,8 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
>  		goto ei2c;
>  	}
>  
> +	ret = mt9m111_init(client);
> +
>  ei2c:
>  	return ret;
>  }
> -- 
> 1.7.1
> 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 02/20] mt9m111: init chip after read CHIP_VERSION

[Robert Jarzmik]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> On Fri, 30 Jul 2010, Michael Grzeschik wrote:
>
>> Moved mt9m111_init after the chip version detection passage: I
>> don't like the idea of writing on a device we haven't identified
>> yet.
>
> In principle it's correct, but what do you do, if a chip cannot be probed, 
> before it is initialised / enabled? Actually, this shouldn't be the case, 
> devices should be available for probing without any initialisation. So, we 
> have to ask the original author, whether this really was necessary, 
> Robert?

Michael is right I think.
According to the specification, even before the reset, the control registers can
be read, and they'll return their current values, which can be weird before
reset, excepting the CHIP_VERSION which is hard coded.

Therefore I think Michael is right by reading chip version before doing the
reset, and I ack this patch.

Cheers.

-- 
Robert

[PATCH 03/20] mt9m111: register cleanup hex to dec bitoffset

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 39dff7c..c72c4b0 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -100,14 +100,14 @@
 #define MT9M111_OUTFMT_BYPASS_IFP	(1 << 10)
 #define MT9M111_OUTFMT_INV_PIX_CLOCK	(1 << 9)
 #define MT9M111_OUTFMT_RGB		(1 << 8)
-#define MT9M111_OUTFMT_RGB565		(0x0 << 6)
-#define MT9M111_OUTFMT_RGB555		(0x1 << 6)
-#define MT9M111_OUTFMT_RGB444x		(0x2 << 6)
-#define MT9M111_OUTFMT_RGBx444		(0x3 << 6)
-#define MT9M111_OUTFMT_TST_RAMP_OFF	(0x0 << 4)
-#define MT9M111_OUTFMT_TST_RAMP_COL	(0x1 << 4)
-#define MT9M111_OUTFMT_TST_RAMP_ROW	(0x2 << 4)
-#define MT9M111_OUTFMT_TST_RAMP_FRAME	(0x3 << 4)
+#define MT9M111_OUTFMT_RGB565		(0 << 6)
+#define MT9M111_OUTFMT_RGB555		(1 << 6)
+#define MT9M111_OUTFMT_RGB444x		(2 << 6)
+#define MT9M111_OUTFMT_RGBx444		(3 << 6)
+#define MT9M111_OUTFMT_TST_RAMP_OFF	(0 << 4)
+#define MT9M111_OUTFMT_TST_RAMP_COL	(1 << 4)
+#define MT9M111_OUTFMT_TST_RAMP_ROW	(2 << 4)
+#define MT9M111_OUTFMT_TST_RAMP_FRAME	(3 << 4)
 #define MT9M111_OUTFMT_SHIFT_3_UP	(1 << 3)
 #define MT9M111_OUTFMT_AVG_CHROMA	(1 << 2)
 #define MT9M111_OUTFMT_SWAP_YCbCr_C_Y	(1 << 1)
-- 
1.7.1

Re: [PATCH 03/20] mt9m111: register cleanup hex to dec bitoffset

[Robert Jarzmik]

Michael Grzeschik <m.grzeschik@pengutronix.de> writes:

> Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

OK for me (the formal ack will be once we finish the review).

Cheers.

--
Robert

[PATCH 04/20] mt9m111: added new bit offset defines

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index c72c4b0..aeb2241 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -63,6 +63,12 @@
 #define MT9M111_RESET_RESTART_FRAME	(1 << 1)
 #define MT9M111_RESET_RESET_MODE	(1 << 0)
 
+#define MT9M111_RM_FULL_POWER_RD	(0 << 10)
+#define MT9M111_RM_LOW_POWER_RD		(1 << 10)
+#define MT9M111_RM_COL_SKIP_4X		(1 << 5)
+#define MT9M111_RM_ROW_SKIP_4X		(1 << 4)
+#define MT9M111_RM_COL_SKIP_2X		(1 << 3)
+#define MT9M111_RM_ROW_SKIP_2X		(1 << 2)
 #define MT9M111_RMB_MIRROR_COLS		(1 << 1)
 #define MT9M111_RMB_MIRROR_ROWS		(1 << 0)
 #define MT9M111_CTXT_CTRL_RESTART	(1 << 15)
-- 
1.7.1

Re: [PATCH 04/20] mt9m111: added new bit offset defines

[Robert Jarzmik]

Michael Grzeschik <m.grzeschik@pengutronix.de> writes:

> Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
OK for me.

Cheers.

--
Robert

[PATCH 05/20] mt9m111: added default row/col/width/height values

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index aeb2241..5f0c55e 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -133,6 +133,10 @@
 #define MT9M111_MIN_DARK_COLS	24
 #define MT9M111_MAX_HEIGHT	1024
 #define MT9M111_MAX_WIDTH	1280
+#define MT9M111_DEF_DARK_ROWS	12
+#define MT9M111_DEF_DARK_COLS	30
+#define MT9M111_DEF_HEIGHT	1024
+#define MT9M111_DEF_WIDTH	1280
 
 /* MT9M111 has only one fixed colorspace per pixelcode */
 struct mt9m111_datafmt {
-- 
1.7.1

Re: [PATCH 05/20] mt9m111: added default row/col/width/height values

[Guennadi Liakhovetski]

On Fri, 30 Jul 2010, Michael Grzeschik wrote:

> Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/media/video/mt9m111.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index aeb2241..5f0c55e 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -133,6 +133,10 @@
>  #define MT9M111_MIN_DARK_COLS	24
>  #define MT9M111_MAX_HEIGHT	1024
>  #define MT9M111_MAX_WIDTH	1280
> +#define MT9M111_DEF_DARK_ROWS	12
> +#define MT9M111_DEF_DARK_COLS	30
> +#define MT9M111_DEF_HEIGHT	1024
> +#define MT9M111_DEF_WIDTH	1280

Don't think this split makes sense. Please, call them "DEFAUL": "DEF" is 
too ambiguous, and unite with patch 08/20. In general, you're exaggerating 
splitting og patches. Many of them make little sense with this kind of a 
split and have to be merged.

>  
>  /* MT9M111 has only one fixed colorspace per pixelcode */
>  struct mt9m111_datafmt {
> -- 
> 1.7.1

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 06/20] mt9m111: changed MAX_{HEIGHT,WIDTH} values to silicon pixelcount

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 5f0c55e..2080615 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -131,8 +131,8 @@
 
 #define MT9M111_MIN_DARK_ROWS	8
 #define MT9M111_MIN_DARK_COLS	24
-#define MT9M111_MAX_HEIGHT	1024
-#define MT9M111_MAX_WIDTH	1280
+#define MT9M111_MAX_HEIGHT	1032
+#define MT9M111_MAX_WIDTH	1288
 #define MT9M111_DEF_DARK_ROWS	12
 #define MT9M111_DEF_DARK_COLS	30
 #define MT9M111_DEF_HEIGHT	1024
-- 
1.7.1

Re: [PATCH 06/20] mt9m111: changed MAX_{HEIGHT,WIDTH} values to silicon pixelcount

[Robert Jarzmik]

Michael Grzeschik <m.grzeschik@pengutronix.de> writes:

> Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/media/video/mt9m111.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index 5f0c55e..2080615 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -131,8 +131,8 @@
>  
>  #define MT9M111_MIN_DARK_ROWS	8
>  #define MT9M111_MIN_DARK_COLS	24
> -#define MT9M111_MAX_HEIGHT	1024
> -#define MT9M111_MAX_WIDTH	1280
> +#define MT9M111_MAX_HEIGHT	1032
> +#define MT9M111_MAX_WIDTH	1288

If we're going down that path, my specification says in chapter "Pixel Data
Format/Pixel Array Structure" that there are :
 - 1289 optical active pixels in width
 - 1033 optical active pixels in height

So why don't we use the real values here ?

Cheers.

--
Robert

[PATCH 07/20] mt9m111: changed MIN_DARK_COLS to MT9M131 spec count

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 2080615..f024cc5 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -130,7 +130,7 @@
 #define reg_clear(reg, val) mt9m111_reg_clear(client, MT9M111_##reg, (val))
 
 #define MT9M111_MIN_DARK_ROWS	8
-#define MT9M111_MIN_DARK_COLS	24
+#define MT9M111_MIN_DARK_COLS	26
 #define MT9M111_MAX_HEIGHT	1032
 #define MT9M111_MAX_WIDTH	1288
 #define MT9M111_DEF_DARK_ROWS	12
-- 
1.7.1

[PATCH 08/20] mt9m111: cropcap use defined default rect properties in defrect

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index f024cc5..3b19274 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -480,7 +480,10 @@ static int mt9m111_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
 	a->bounds.top			= MT9M111_MIN_DARK_ROWS;
 	a->bounds.width			= MT9M111_MAX_WIDTH;
 	a->bounds.height		= MT9M111_MAX_HEIGHT;
-	a->defrect			= a->bounds;
+	a->defrect.left			= MT9M111_DEF_DARK_COLS;
+	a->defrect.top			= MT9M111_DEF_DARK_ROWS;
+	a->defrect.width		= MT9M111_DEF_WIDTH;
+	a->defrect.height		= MT9M111_DEF_HEIGHT;
 	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	a->pixelaspect.numerator	= 1;
 	a->pixelaspect.denominator	= 1;
-- 
1.7.1

[PATCH 09/20] mt9m111: cropcap check if type is CAPTURE

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 3b19274..e8d8e9b 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -476,6 +476,9 @@ static int mt9m111_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 
 static int mt9m111_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
 {
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
 	a->bounds.left			= MT9M111_MIN_DARK_COLS;
 	a->bounds.top			= MT9M111_MIN_DARK_ROWS;
 	a->bounds.width			= MT9M111_MAX_WIDTH;
@@ -484,7 +487,6 @@ static int mt9m111_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
 	a->defrect.top			= MT9M111_DEF_DARK_ROWS;
 	a->defrect.width		= MT9M111_DEF_WIDTH;
 	a->defrect.height		= MT9M111_DEF_HEIGHT;
-	a->type				= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	a->pixelaspect.numerator	= 1;
 	a->pixelaspect.denominator	= 1;
 
-- 
1.7.1

[PATCH 10/20] mt9m111: rewrite make_rect for positioning in debug

[Michael Grzeschik]

If DEBUG is defined it is possible to set upper left corner

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index e8d8e9b..db5ac32 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -428,14 +428,7 @@ static int mt9m111_make_rect(struct i2c_client *client,
 			     struct v4l2_rect *rect)
 {
 	struct mt9m111 *mt9m111 = to_mt9m111(client);
-
-	if (mt9m111->fmt->code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
-	    mt9m111->fmt->code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE) {
-		/* Bayer format - even size lengths */
-		rect->width	= ALIGN(rect->width, 2);
-		rect->height	= ALIGN(rect->height, 2);
-		/* Let the user play with the starting pixel */
-	}
+	enum v4l2_mbus_pixelcode code = mt9m111->fmt->code;
 
 	/* FIXME: the datasheet doesn't specify minimum sizes */
 	soc_camera_limit_side(&rect->left, &rect->width,
@@ -444,6 +437,28 @@ static int mt9m111_make_rect(struct i2c_client *client,
 	soc_camera_limit_side(&rect->top, &rect->height,
 		     MT9M111_MIN_DARK_ROWS, 2, MT9M111_MAX_HEIGHT);
 
+	switch (code) {
+	case V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE:
+		/* unprocessed Bayer pattern format, IFP is bypassed */
+#ifndef DEBUG
+		/* assure that Bayer sequence is BGGR */
+		/* in debug mode, let user play with starting pixel */
+		rect->left	= ALIGN(rect->left, 2);
+		rect->top	= ALIGN(rect->top, 2) + 1;
+#endif
+	case V4L2_MBUS_FMT_SBGGR8_1X8:
+		/* processed Bayer pattern format, sequence is fixed */
+		/* assure even side lengths for both Bayer modes */
+		rect->width	= ALIGN(rect->width, 2);
+		rect->height	= ALIGN(rect->height, 2);
+	default:
+		/* needed to avoid compiler warnings */;
+	}
+
+	dev_dbg(&client->dev, "%s: rect: left=%d top=%d width=%d height=%d "
+		"mf: pixelcode=%d\n", __func__, rect->left, rect->top,
+		rect->width, rect->height, code);
+
 	return mt9m111_setup_rect(client, rect);
 }
 
-- 
1.7.1

Re: [PATCH 10/20] mt9m111: rewrite make_rect for positioning in debug

[Guennadi Liakhovetski]

On Fri, 30 Jul 2010, Michael Grzeschik wrote:

> If DEBUG is defined it is possible to set upper left corner
> 
> Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/media/video/mt9m111.c |   31 +++++++++++++++++++++++--------
>  1 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index e8d8e9b..db5ac32 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -428,14 +428,7 @@ static int mt9m111_make_rect(struct i2c_client *client,
>  			     struct v4l2_rect *rect)
>  {
>  	struct mt9m111 *mt9m111 = to_mt9m111(client);
> -
> -	if (mt9m111->fmt->code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
> -	    mt9m111->fmt->code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE) {
> -		/* Bayer format - even size lengths */
> -		rect->width	= ALIGN(rect->width, 2);
> -		rect->height	= ALIGN(rect->height, 2);
> -		/* Let the user play with the starting pixel */
> -	}
> +	enum v4l2_mbus_pixelcode code = mt9m111->fmt->code;
>  
>  	/* FIXME: the datasheet doesn't specify minimum sizes */
>  	soc_camera_limit_side(&rect->left, &rect->width,
> @@ -444,6 +437,28 @@ static int mt9m111_make_rect(struct i2c_client *client,
>  	soc_camera_limit_side(&rect->top, &rect->height,
>  		     MT9M111_MIN_DARK_ROWS, 2, MT9M111_MAX_HEIGHT);
>  
> +	switch (code) {
> +	case V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE:
> +		/* unprocessed Bayer pattern format, IFP is bypassed */
> +#ifndef DEBUG
> +		/* assure that Bayer sequence is BGGR */
> +		/* in debug mode, let user play with starting pixel */
> +		rect->left	= ALIGN(rect->left, 2);
> +		rect->top	= ALIGN(rect->top, 2) + 1;
> +#endif
> +	case V4L2_MBUS_FMT_SBGGR8_1X8:
> +		/* processed Bayer pattern format, sequence is fixed */
> +		/* assure even side lengths for both Bayer modes */
> +		rect->width	= ALIGN(rect->width, 2);
> +		rect->height	= ALIGN(rect->height, 2);
> +	default:

hm, don't think I like it. First, why do you only enable it for SBGGR10 
and not for SBGGR8? This choice has nothing to do with how many bytes per 
pixel this format has. It allows you to select the starting _pixel_, not 
byte. And I wouldn't bind this to DEBUG. Either enable or disable 
completely... If you want to disable it, you would have to check for 
regressions. I would keep it - just to make sure we don't break anything.

> +		/* needed to avoid compiler warnings */;
> +	}
> +
> +	dev_dbg(&client->dev, "%s: rect: left=%d top=%d width=%d height=%d "
> +		"mf: pixelcode=%d\n", __func__, rect->left, rect->top,
> +		rect->width, rect->height, code);
> +
>  	return mt9m111_setup_rect(client, rect);
>  }
>  
> -- 
> 1.7.1

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 11/20] mt9m111: added mt9m111 format structure

[Michael Grzeschik]

removed unused rect and fmt structs from mt9m111 struct

set default values for mf.colorspace and mf.code to the common raw
format V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE.

rewrote following functions to make use the new format structure:

* restore_state,
* g_fmt,
* s_fmt,
* g_crop,
* s_crop,
* setup_rect

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |   99 ++++++++++++++++++++++-------------------
 1 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index db5ac32..cc0f996 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -173,13 +173,17 @@ enum mt9m111_context {
 	LOWPOWER,
 };
 
+struct mt9m111_format {
+     struct v4l2_rect rect;
+     struct v4l2_mbus_framefmt mf;
+};
+
 struct mt9m111 {
 	struct v4l2_subdev subdev;
 	int model;	/* V4L2_IDENT_MT9M111 or V4L2_IDENT_MT9M112 code */
 			/* from v4l2-chip-ident.h */
 	enum mt9m111_context context;
-	struct v4l2_rect rect;
-	const struct mt9m111_datafmt *fmt;
+	struct mt9m111_format format;
 	unsigned int gain;
 	unsigned char autoexposure;
 	unsigned char datawidth;
@@ -278,15 +282,15 @@ static int mt9m111_set_context(struct i2c_client *client,
 }
 
 static int mt9m111_setup_rect(struct i2c_client *client,
-			      struct v4l2_rect *rect)
+			      struct mt9m111_format *format)
 {
-	struct mt9m111 *mt9m111 = to_mt9m111(client);
+	struct v4l2_rect *rect = &format->rect;
 	int ret, is_raw_format;
 	int width = rect->width;
 	int height = rect->height;
 
-	if (mt9m111->fmt->code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
-	    mt9m111->fmt->code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE)
+	if (format->mf.code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
+	    format->mf.code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE)
 		is_raw_format = 1;
 	else
 		is_raw_format = 0;
@@ -425,10 +429,10 @@ static int mt9m111_set_bus_param(struct soc_camera_device *icd, unsigned long f)
 }
 
 static int mt9m111_make_rect(struct i2c_client *client,
-			     struct v4l2_rect *rect)
+			     struct mt9m111_format *format)
 {
-	struct mt9m111 *mt9m111 = to_mt9m111(client);
-	enum v4l2_mbus_pixelcode code = mt9m111->fmt->code;
+	struct v4l2_rect *rect = &format->rect;
+	enum v4l2_mbus_pixelcode code = format->mf.code;
 
 	/* FIXME: the datasheet doesn't specify minimum sizes */
 	soc_camera_limit_side(&rect->left, &rect->width,
@@ -459,22 +463,30 @@ static int mt9m111_make_rect(struct i2c_client *client,
 		"mf: pixelcode=%d\n", __func__, rect->left, rect->top,
 		rect->width, rect->height, code);
 
-	return mt9m111_setup_rect(client, rect);
+	return mt9m111_setup_rect(client, format);
 }
 
 static int mt9m111_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 {
-	struct v4l2_rect rect = a->c;
 	struct i2c_client *client = sd->priv;
 	struct mt9m111 *mt9m111 = to_mt9m111(client);
+	struct mt9m111_format format;
+	struct v4l2_mbus_framefmt *mf = &format.mf;
 	int ret;
 
-	dev_dbg(&client->dev, "%s left=%d, top=%d, width=%d, height=%d\n",
-		__func__, rect.left, rect.top, rect.width, rect.height);
+	format.rect	= a->c;
+	format.mf	= mt9m111->format.mf;
+
+	dev_dbg(&client->dev, "%s: rect: left=%d top=%d width=%d height=%d\n",
+		__func__, a->c.left, a->c.top, a->c.width, a->c.height);
+	dev_dbg(&client->dev, "%s: mf: width=%d height=%d pixelcode=%d "
+		"field=%x colorspace=%x\n", __func__, mf->width, mf->height,
+		mf->code, mf->field, mf->colorspace);
 
-	ret = mt9m111_make_rect(client, &rect);
+	ret = mt9m111_make_rect(client, &format);
 	if (!ret)
-		mt9m111->rect = rect;
+		mt9m111->format = format;
+
 	return ret;
 }
 
@@ -483,7 +495,7 @@ static int mt9m111_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 	struct i2c_client *client = sd->priv;
 	struct mt9m111 *mt9m111 = to_mt9m111(client);
 
-	a->c	= mt9m111->rect;
+	a->c	= mt9m111->format.rect;
 	a->type	= V4L2_BUF_TYPE_VIDEO_CAPTURE;
 
 	return 0;
@@ -514,10 +526,7 @@ static int mt9m111_g_fmt(struct v4l2_subdev *sd,
 	struct i2c_client *client = sd->priv;
 	struct mt9m111 *mt9m111 = to_mt9m111(client);
 
-	mf->width	= mt9m111->rect.width;
-	mf->height	= mt9m111->rect.height;
-	mf->code	= mt9m111->fmt->code;
-	mf->field	= V4L2_FIELD_NONE;
+	*mf = mt9m111->format.mf;
 
 	return 0;
 }
@@ -576,12 +585,8 @@ static int mt9m111_s_fmt(struct v4l2_subdev *sd,
 	struct i2c_client *client = sd->priv;
 	const struct mt9m111_datafmt *fmt;
 	struct mt9m111 *mt9m111 = to_mt9m111(client);
-	struct v4l2_rect rect = {
-		.left	= mt9m111->rect.left,
-		.top	= mt9m111->rect.top,
-		.width	= mf->width,
-		.height	= mf->height,
-	};
+	struct v4l2_rect *rect;
+	struct mt9m111_format format;
 	int ret;
 
 	fmt = mt9m111_find_datafmt(mf->code, mt9m111_colour_fmts,
@@ -589,18 +594,19 @@ static int mt9m111_s_fmt(struct v4l2_subdev *sd,
 	if (!fmt)
 		return -EINVAL;
 
+	format.rect	= mt9m111->format.rect;
+	format.mf	= *mf;
+	rect		= &format.rect;
+
 	dev_dbg(&client->dev,
 		"%s code=%x left=%d, top=%d, width=%d, height=%d\n", __func__,
-		mf->code, rect.left, rect.top, rect.width, rect.height);
+		mf->code, rect->left, rect->top, rect->width, rect->height);
 
-	ret = mt9m111_make_rect(client, &rect);
+	ret = mt9m111_make_rect(client, &format);
 	if (!ret)
-		ret = mt9m111_set_pixfmt(client, mf->code);
-	if (!ret) {
-		mt9m111->rect	= rect;
-		mt9m111->fmt	= fmt;
-		mf->colorspace	= fmt->colorspace;
-	}
+		ret = mt9m111_set_pixfmt(client, format.mf.code);
+	if (!ret)
+		mt9m111->format = format;
 
 	return ret;
 }
@@ -609,17 +615,14 @@ static int mt9m111_try_fmt(struct v4l2_subdev *sd,
 			   struct v4l2_mbus_framefmt *mf)
 {
 	struct i2c_client *client = sd->priv;
-	struct mt9m111 *mt9m111 = to_mt9m111(client);
 	const struct mt9m111_datafmt *fmt;
 	bool bayer = mf->code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
 		mf->code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE;
 
 	fmt = mt9m111_find_datafmt(mf->code, mt9m111_colour_fmts,
 				   ARRAY_SIZE(mt9m111_colour_fmts));
-	if (!fmt) {
-		fmt = mt9m111->fmt;
-		mf->code = fmt->code;
-	}
+	if (!fmt)
+		return -EINVAL;
 
 	/*
 	 * With Bayer format enforce even side lengths, but let the user play
@@ -930,8 +933,8 @@ static int mt9m111_restore_state(struct i2c_client *client)
 	struct mt9m111 *mt9m111 = to_mt9m111(client);
 
 	mt9m111_set_context(client, mt9m111->context);
-	mt9m111_set_pixfmt(client, mt9m111->fmt->code);
-	mt9m111_setup_rect(client, &mt9m111->rect);
+	mt9m111_set_pixfmt(client, mt9m111->format.mf.code);
+	mt9m111_setup_rect(client, &mt9m111->format);
 	mt9m111_set_flip(client, mt9m111->hflip, MT9M111_RMB_MIRROR_COLS);
 	mt9m111_set_flip(client, mt9m111->vflip, MT9M111_RMB_MIRROR_ROWS);
 	mt9m111_set_global_gain(client, mt9m111->gain);
@@ -1096,11 +1099,15 @@ static int mt9m111_probe(struct i2c_client *client,
 	/* Second stage probe - when a capture adapter is there */
 	icd->ops		= &mt9m111_ops;
 
-	mt9m111->rect.left	= MT9M111_MIN_DARK_COLS;
-	mt9m111->rect.top	= MT9M111_MIN_DARK_ROWS;
-	mt9m111->rect.width	= MT9M111_MAX_WIDTH;
-	mt9m111->rect.height	= MT9M111_MAX_HEIGHT;
-	mt9m111->fmt		= &mt9m111_colour_fmts[0];
+	mt9m111->format.rect.left       = MT9M111_DEF_DARK_COLS;
+	mt9m111->format.rect.top        = MT9M111_DEF_DARK_ROWS;
+	mt9m111->format.rect.width      = MT9M111_DEF_WIDTH;
+	mt9m111->format.rect.height     = MT9M111_DEF_HEIGHT;
+	mt9m111->format.mf.width        = MT9M111_DEF_WIDTH;
+	mt9m111->format.mf.height       = MT9M111_DEF_HEIGHT;
+	mt9m111->format.mf.code         = mt9m111_colour_fmts[2].code;
+	mt9m111->format.mf.field        = V4L2_FIELD_NONE;
+	mt9m111->format.mf.colorspace   = mt9m111_colour_fmts[2].colorspace;
 
 	ret = mt9m111_video_probe(icd, client);
 	if (ret) {
-- 
1.7.1

Re: [PATCH 11/20] mt9m111: added mt9m111 format structure

[Guennadi Liakhovetski]

On Fri, 30 Jul 2010, Michael Grzeschik wrote:

> removed unused rect and fmt structs from mt9m111 struct

Don't understand. Both rect and fmt do seem to be used to me. If they were 
unused, you could have _just_ removed them. Instead you add a new struct 
mt9m111_format. Why? So, I don't understand this patch. If you find some 
unused data / code - it is ok to remove it, this is one patch. If you want 
to change default data format:

> set default values for mf.colorspace and mf.code to the common raw
> format V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE.

This is a separate patch too. From this patch description I don't 
understand how these two changes are connected and why you need them and 
why you put them in one patch.

Thanks
Guennadi

> 
> rewrote following functions to make use the new format structure:
> 
> * restore_state,
> * g_fmt,
> * s_fmt,
> * g_crop,
> * s_crop,
> * setup_rect
> 
> Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/media/video/mt9m111.c |   99 ++++++++++++++++++++++-------------------
>  1 files changed, 53 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index db5ac32..cc0f996 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -173,13 +173,17 @@ enum mt9m111_context {
>  	LOWPOWER,
>  };
>  
> +struct mt9m111_format {
> +     struct v4l2_rect rect;
> +     struct v4l2_mbus_framefmt mf;
> +};
> +
>  struct mt9m111 {
>  	struct v4l2_subdev subdev;
>  	int model;	/* V4L2_IDENT_MT9M111 or V4L2_IDENT_MT9M112 code */
>  			/* from v4l2-chip-ident.h */
>  	enum mt9m111_context context;
> -	struct v4l2_rect rect;
> -	const struct mt9m111_datafmt *fmt;
> +	struct mt9m111_format format;
>  	unsigned int gain;
>  	unsigned char autoexposure;
>  	unsigned char datawidth;
> @@ -278,15 +282,15 @@ static int mt9m111_set_context(struct i2c_client *client,
>  }
>  
>  static int mt9m111_setup_rect(struct i2c_client *client,
> -			      struct v4l2_rect *rect)
> +			      struct mt9m111_format *format)
>  {
> -	struct mt9m111 *mt9m111 = to_mt9m111(client);
> +	struct v4l2_rect *rect = &format->rect;
>  	int ret, is_raw_format;
>  	int width = rect->width;
>  	int height = rect->height;
>  
> -	if (mt9m111->fmt->code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
> -	    mt9m111->fmt->code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE)
> +	if (format->mf.code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
> +	    format->mf.code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE)
>  		is_raw_format = 1;
>  	else
>  		is_raw_format = 0;
> @@ -425,10 +429,10 @@ static int mt9m111_set_bus_param(struct soc_camera_device *icd, unsigned long f)
>  }
>  
>  static int mt9m111_make_rect(struct i2c_client *client,
> -			     struct v4l2_rect *rect)
> +			     struct mt9m111_format *format)
>  {
> -	struct mt9m111 *mt9m111 = to_mt9m111(client);
> -	enum v4l2_mbus_pixelcode code = mt9m111->fmt->code;
> +	struct v4l2_rect *rect = &format->rect;
> +	enum v4l2_mbus_pixelcode code = format->mf.code;
>  
>  	/* FIXME: the datasheet doesn't specify minimum sizes */
>  	soc_camera_limit_side(&rect->left, &rect->width,
> @@ -459,22 +463,30 @@ static int mt9m111_make_rect(struct i2c_client *client,
>  		"mf: pixelcode=%d\n", __func__, rect->left, rect->top,
>  		rect->width, rect->height, code);
>  
> -	return mt9m111_setup_rect(client, rect);
> +	return mt9m111_setup_rect(client, format);
>  }
>  
>  static int mt9m111_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
>  {
> -	struct v4l2_rect rect = a->c;
>  	struct i2c_client *client = sd->priv;
>  	struct mt9m111 *mt9m111 = to_mt9m111(client);
> +	struct mt9m111_format format;
> +	struct v4l2_mbus_framefmt *mf = &format.mf;
>  	int ret;
>  
> -	dev_dbg(&client->dev, "%s left=%d, top=%d, width=%d, height=%d\n",
> -		__func__, rect.left, rect.top, rect.width, rect.height);
> +	format.rect	= a->c;
> +	format.mf	= mt9m111->format.mf;
> +
> +	dev_dbg(&client->dev, "%s: rect: left=%d top=%d width=%d height=%d\n",
> +		__func__, a->c.left, a->c.top, a->c.width, a->c.height);
> +	dev_dbg(&client->dev, "%s: mf: width=%d height=%d pixelcode=%d "
> +		"field=%x colorspace=%x\n", __func__, mf->width, mf->height,
> +		mf->code, mf->field, mf->colorspace);
>  
> -	ret = mt9m111_make_rect(client, &rect);
> +	ret = mt9m111_make_rect(client, &format);
>  	if (!ret)
> -		mt9m111->rect = rect;
> +		mt9m111->format = format;
> +
>  	return ret;
>  }
>  
> @@ -483,7 +495,7 @@ static int mt9m111_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
>  	struct i2c_client *client = sd->priv;
>  	struct mt9m111 *mt9m111 = to_mt9m111(client);
>  
> -	a->c	= mt9m111->rect;
> +	a->c	= mt9m111->format.rect;
>  	a->type	= V4L2_BUF_TYPE_VIDEO_CAPTURE;
>  
>  	return 0;
> @@ -514,10 +526,7 @@ static int mt9m111_g_fmt(struct v4l2_subdev *sd,
>  	struct i2c_client *client = sd->priv;
>  	struct mt9m111 *mt9m111 = to_mt9m111(client);
>  
> -	mf->width	= mt9m111->rect.width;
> -	mf->height	= mt9m111->rect.height;
> -	mf->code	= mt9m111->fmt->code;
> -	mf->field	= V4L2_FIELD_NONE;
> +	*mf = mt9m111->format.mf;
>  
>  	return 0;
>  }
> @@ -576,12 +585,8 @@ static int mt9m111_s_fmt(struct v4l2_subdev *sd,
>  	struct i2c_client *client = sd->priv;
>  	const struct mt9m111_datafmt *fmt;
>  	struct mt9m111 *mt9m111 = to_mt9m111(client);
> -	struct v4l2_rect rect = {
> -		.left	= mt9m111->rect.left,
> -		.top	= mt9m111->rect.top,
> -		.width	= mf->width,
> -		.height	= mf->height,
> -	};
> +	struct v4l2_rect *rect;
> +	struct mt9m111_format format;
>  	int ret;
>  
>  	fmt = mt9m111_find_datafmt(mf->code, mt9m111_colour_fmts,
> @@ -589,18 +594,19 @@ static int mt9m111_s_fmt(struct v4l2_subdev *sd,
>  	if (!fmt)
>  		return -EINVAL;
>  
> +	format.rect	= mt9m111->format.rect;
> +	format.mf	= *mf;
> +	rect		= &format.rect;
> +
>  	dev_dbg(&client->dev,
>  		"%s code=%x left=%d, top=%d, width=%d, height=%d\n", __func__,
> -		mf->code, rect.left, rect.top, rect.width, rect.height);
> +		mf->code, rect->left, rect->top, rect->width, rect->height);
>  
> -	ret = mt9m111_make_rect(client, &rect);
> +	ret = mt9m111_make_rect(client, &format);
>  	if (!ret)
> -		ret = mt9m111_set_pixfmt(client, mf->code);
> -	if (!ret) {
> -		mt9m111->rect	= rect;
> -		mt9m111->fmt	= fmt;
> -		mf->colorspace	= fmt->colorspace;
> -	}
> +		ret = mt9m111_set_pixfmt(client, format.mf.code);
> +	if (!ret)
> +		mt9m111->format = format;
>  
>  	return ret;
>  }
> @@ -609,17 +615,14 @@ static int mt9m111_try_fmt(struct v4l2_subdev *sd,
>  			   struct v4l2_mbus_framefmt *mf)
>  {
>  	struct i2c_client *client = sd->priv;
> -	struct mt9m111 *mt9m111 = to_mt9m111(client);
>  	const struct mt9m111_datafmt *fmt;
>  	bool bayer = mf->code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
>  		mf->code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE;
>  
>  	fmt = mt9m111_find_datafmt(mf->code, mt9m111_colour_fmts,
>  				   ARRAY_SIZE(mt9m111_colour_fmts));
> -	if (!fmt) {
> -		fmt = mt9m111->fmt;
> -		mf->code = fmt->code;
> -	}
> +	if (!fmt)
> +		return -EINVAL;
>  
>  	/*
>  	 * With Bayer format enforce even side lengths, but let the user play
> @@ -930,8 +933,8 @@ static int mt9m111_restore_state(struct i2c_client *client)
>  	struct mt9m111 *mt9m111 = to_mt9m111(client);
>  
>  	mt9m111_set_context(client, mt9m111->context);
> -	mt9m111_set_pixfmt(client, mt9m111->fmt->code);
> -	mt9m111_setup_rect(client, &mt9m111->rect);
> +	mt9m111_set_pixfmt(client, mt9m111->format.mf.code);
> +	mt9m111_setup_rect(client, &mt9m111->format);
>  	mt9m111_set_flip(client, mt9m111->hflip, MT9M111_RMB_MIRROR_COLS);
>  	mt9m111_set_flip(client, mt9m111->vflip, MT9M111_RMB_MIRROR_ROWS);
>  	mt9m111_set_global_gain(client, mt9m111->gain);
> @@ -1096,11 +1099,15 @@ static int mt9m111_probe(struct i2c_client *client,
>  	/* Second stage probe - when a capture adapter is there */
>  	icd->ops		= &mt9m111_ops;
>  
> -	mt9m111->rect.left	= MT9M111_MIN_DARK_COLS;
> -	mt9m111->rect.top	= MT9M111_MIN_DARK_ROWS;
> -	mt9m111->rect.width	= MT9M111_MAX_WIDTH;
> -	mt9m111->rect.height	= MT9M111_MAX_HEIGHT;
> -	mt9m111->fmt		= &mt9m111_colour_fmts[0];
> +	mt9m111->format.rect.left       = MT9M111_DEF_DARK_COLS;
> +	mt9m111->format.rect.top        = MT9M111_DEF_DARK_ROWS;
> +	mt9m111->format.rect.width      = MT9M111_DEF_WIDTH;
> +	mt9m111->format.rect.height     = MT9M111_DEF_HEIGHT;
> +	mt9m111->format.mf.width        = MT9M111_DEF_WIDTH;
> +	mt9m111->format.mf.height       = MT9M111_DEF_HEIGHT;
> +	mt9m111->format.mf.code         = mt9m111_colour_fmts[2].code;
> +	mt9m111->format.mf.field        = V4L2_FIELD_NONE;
> +	mt9m111->format.mf.colorspace   = mt9m111_colour_fmts[2].colorspace;
>  
>  	ret = mt9m111_video_probe(icd, client);
>  	if (ret) {
> -- 
> 1.7.1
> 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 12/20] mt9m111: s_crop add calculation of output size

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index cc0f996..2758a97 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -472,11 +472,19 @@ static int mt9m111_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 	struct mt9m111 *mt9m111 = to_mt9m111(client);
 	struct mt9m111_format format;
 	struct v4l2_mbus_framefmt *mf = &format.mf;
+	s32 rectwidth	= mt9m111->format.rect.width;
+	s32 rectheight	= mt9m111->format.rect.height;
+	u32 pixwidth	= mt9m111->format.mf.width;
+	u32 pixheight	= mt9m111->format.mf.height;
 	int ret;
 
 	format.rect	= a->c;
 	format.mf	= mt9m111->format.mf;
 
+	/* calculate output size, maintain current scaling factors */
+	format.mf.width = pixwidth / rectwidth * format.mf.width;
+	format.mf.height = pixheight / rectheight * format.mf.height;
+
 	dev_dbg(&client->dev, "%s: rect: left=%d top=%d width=%d height=%d\n",
 		__func__, a->c.left, a->c.top, a->c.width, a->c.height);
 	dev_dbg(&client->dev, "%s: mf: width=%d height=%d pixelcode=%d "
-- 
1.7.1

Re: [PATCH 12/20] mt9m111: s_crop add calculation of output size

[Guennadi Liakhovetski]

On Fri, 30 Jul 2010, Michael Grzeschik wrote:

> Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/media/video/mt9m111.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
> index cc0f996..2758a97 100644
> --- a/drivers/media/video/mt9m111.c
> +++ b/drivers/media/video/mt9m111.c
> @@ -472,11 +472,19 @@ static int mt9m111_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
>  	struct mt9m111 *mt9m111 = to_mt9m111(client);
>  	struct mt9m111_format format;
>  	struct v4l2_mbus_framefmt *mf = &format.mf;
> +	s32 rectwidth	= mt9m111->format.rect.width;
> +	s32 rectheight	= mt9m111->format.rect.height;
> +	u32 pixwidth	= mt9m111->format.mf.width;
> +	u32 pixheight	= mt9m111->format.mf.height;
>  	int ret;
>  
>  	format.rect	= a->c;
>  	format.mf	= mt9m111->format.mf;
>  
> +	/* calculate output size, maintain current scaling factors */
> +	format.mf.width = pixwidth / rectwidth * format.mf.width;
> +	format.mf.height = pixheight / rectheight * format.mf.height;

Again - don't understand:

	u32 pixwidth    = mt9m111->format.mf.width;
	format.mf       = mt9m111->format.mf;
	format.mf.width = pixwidth / rectwidth * format.mf.width;

this means

	format.mf.width = mt9m111->format.mf.width / rectwidth * mt9m111->format.mf.width;

which makes no sense to me. Can you explain?

> +
>  	dev_dbg(&client->dev, "%s: rect: left=%d top=%d width=%d height=%d\n",
>  		__func__, a->c.left, a->c.top, a->c.width, a->c.height);
>  	dev_dbg(&client->dev, "%s: mf: width=%d height=%d pixelcode=%d "
> -- 
> 1.7.1

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 13/20] mt9m111: s_crop check for VIDEO_CAPTURE type

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 2758a97..4dbaf31 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -478,6 +478,9 @@ static int mt9m111_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 	u32 pixheight	= mt9m111->format.mf.height;
 	int ret;
 
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
 	format.rect	= a->c;
 	format.mf	= mt9m111->format.mf;
 
-- 
1.7.1

[PATCH 14/20] mt9m111: added reg_mask function

[Michael Grzeschik]

reg_mask is basically the same as clearing & setting registers,
but it is more convenient and faster (saves one rw cycle).

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 4dbaf31..161c751 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -128,6 +128,8 @@
 #define reg_write(reg, val) mt9m111_reg_write(client, MT9M111_##reg, (val))
 #define reg_set(reg, val) mt9m111_reg_set(client, MT9M111_##reg, (val))
 #define reg_clear(reg, val) mt9m111_reg_clear(client, MT9M111_##reg, (val))
+#define reg_mask(reg, val, mask) mt9m111_reg_mask(client, MT9M111_##reg, \
+		(val), (mask))
 
 #define MT9M111_MIN_DARK_ROWS	8
 #define MT9M111_MIN_DARK_COLS	26
@@ -265,6 +267,15 @@ static int mt9m111_reg_clear(struct i2c_client *client, const u16 reg,
 	return mt9m111_reg_write(client, reg, ret & ~data);
 }
 
+static int mt9m111_reg_mask(struct i2c_client *client, const u16 reg,
+			    const u16 data, const u16 mask)
+{
+	int ret;
+
+	ret = mt9m111_reg_read(client, reg);
+	return mt9m111_reg_write(client, reg, (ret & ~mask) | data);
+}
+
 static int mt9m111_set_context(struct i2c_client *client,
 			       enum mt9m111_context ctxt)
 {
-- 
1.7.1

[PATCH 15/20] mt9m111: rewrite setup_rect, added soft_crop for smooth panning

[Michael Grzeschik]

-soft_crop: enables the use of the sensors cropping abilities
instead of using real roi. This is needed to make use of the 'pan'
registers for smooth panning.

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |  106 +++++++++++++++++++++++++++++++---------
 1 files changed, 82 insertions(+), 24 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 161c751..11a68b6 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -87,12 +87,16 @@
  */
 #define MT9M111_OPER_MODE_CTRL		0x106
 #define MT9M111_OUTPUT_FORMAT_CTRL	0x108
+#define MT9M111_REDUCER_XPAN_B		0x19f
 #define MT9M111_REDUCER_XZOOM_B		0x1a0
 #define MT9M111_REDUCER_XSIZE_B		0x1a1
+#define MT9M111_REDUCER_YPAN_B		0x1a2
 #define MT9M111_REDUCER_YZOOM_B		0x1a3
 #define MT9M111_REDUCER_YSIZE_B		0x1a4
+#define MT9M111_REDUCER_XPAN_A		0x1a5
 #define MT9M111_REDUCER_XZOOM_A		0x1a6
 #define MT9M111_REDUCER_XSIZE_A		0x1a7
+#define MT9M111_REDUCER_YPAN_A		0x1a8
 #define MT9M111_REDUCER_YZOOM_A		0x1a9
 #define MT9M111_REDUCER_YSIZE_A		0x1aa
 
@@ -101,7 +105,8 @@
 
 #define MT9M111_OPMODE_AUTOEXPO_EN	(1 << 14)
 #define MT9M111_OPMODE_AUTOWHITEBAL_EN	(1 << 1)
-
+#define MT9M111_OUTFMT_CFA_1ST_ROW_BLUE	(1 << 1)
+#define MT9M111_OUTFMT_CFA_1ST_COL_R_B	(1 << 0)
 #define MT9M111_OUTFMT_PROCESSED_BAYER	(1 << 14)
 #define MT9M111_OUTFMT_BYPASS_IFP	(1 << 10)
 #define MT9M111_OUTFMT_INV_PIX_CLOCK	(1 << 9)
@@ -140,6 +145,11 @@
 #define MT9M111_DEF_HEIGHT	1024
 #define MT9M111_DEF_WIDTH	1280
 
+static int soft_crop;
+module_param(soft_crop, int, S_IRUGO);
+MODULE_PARM_DESC(soft_crop, "Enables soft-cropping and thus the use of "
+		"pan register");
+
 /* MT9M111 has only one fixed colorspace per pixelcode */
 struct mt9m111_datafmt {
 	enum v4l2_mbus_pixelcode	code;
@@ -296,42 +306,90 @@ static int mt9m111_setup_rect(struct i2c_client *client,
 			      struct mt9m111_format *format)
 {
 	struct v4l2_rect *rect = &format->rect;
-	int ret, is_raw_format;
-	int width = rect->width;
-	int height = rect->height;
-
-	if (format->mf.code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
-	    format->mf.code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE)
-		is_raw_format = 1;
-	else
-		is_raw_format = 0;
+	struct v4l2_mbus_framefmt *mf = &format->mf;
+	enum v4l2_mbus_pixelcode *code = &format->mf.code;
+	u16 data_outfmt1 = 0, mask_outfmt1;
+	u16 colum_start, row_start, window_width, window_height, xpan, ypan;
+	int ret;
 
-	ret = reg_write(COLUMN_START, rect->left);
-	if (!ret)
-		ret = reg_write(ROW_START, rect->top);
+	dev_dbg(&client->dev, "%s: rect: left=%d top=%d width=%d height=%d "
+		"mf: pixelcode=%d\n", __func__, rect->left, rect->top,
+		rect->width, rect->height, *code);
 
-	if (is_raw_format) {
+	if (*code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE) {
+			ret = reg_write(COLUMN_START, rect->left);
 		if (!ret)
-			ret = reg_write(WINDOW_WIDTH, width);
+			ret = reg_write(ROW_START, rect->top);
 		if (!ret)
-			ret = reg_write(WINDOW_HEIGHT, height);
+			ret = reg_write(WINDOW_WIDTH, rect->width);
+		if (!ret)
+			ret = reg_write(WINDOW_HEIGHT, rect->height);
 	} else {
+		if (soft_crop) {
+			/* use 'soft cropping' through ZOOM and PAN registers */
+			/* enables use of smart zooming and panning functions */
+			colum_start     = MT9M111_MIN_DARK_COLS;
+			row_start       = MT9M111_MIN_DARK_ROWS;
+			window_width    = MT9M111_MAX_WIDTH;
+			window_height   = MT9M111_MAX_HEIGHT;
+			xpan            = rect->left - MT9M111_MIN_DARK_COLS;
+			ypan            = rect->top - MT9M111_MIN_DARK_ROWS;
+		} else {
+			/* use real cropping, smaller roi increases framerate */
+			colum_start     = rect->left;
+			row_start       = rect->top;
+			window_width    = rect->width;
+			window_height   = rect->height;
+			xpan            = 0;
+			ypan            = 0;
+		}
+
+		ret = reg_write(COLUMN_START, colum_start);
+		if (!ret)
+			ret = reg_write(ROW_START, row_start);
 		if (!ret)
-			ret = reg_write(REDUCER_XZOOM_B, MT9M111_MAX_WIDTH);
+			ret = reg_write(WINDOW_WIDTH, window_width);
 		if (!ret)
-			ret = reg_write(REDUCER_YZOOM_B, MT9M111_MAX_HEIGHT);
+			ret = reg_write(WINDOW_HEIGHT, window_height);
 		if (!ret)
-			ret = reg_write(REDUCER_XSIZE_B, width);
+			ret = reg_write(REDUCER_XPAN_A, xpan);
 		if (!ret)
-			ret = reg_write(REDUCER_YSIZE_B, height);
+			ret = reg_write(REDUCER_YPAN_A, ypan);
 		if (!ret)
-			ret = reg_write(REDUCER_XZOOM_A, MT9M111_MAX_WIDTH);
+			ret = reg_write(REDUCER_XZOOM_A, rect->width);
 		if (!ret)
-			ret = reg_write(REDUCER_YZOOM_A, MT9M111_MAX_HEIGHT);
+			ret = reg_write(REDUCER_YZOOM_A, rect->height);
 		if (!ret)
-			ret = reg_write(REDUCER_XSIZE_A, width);
+			ret = reg_write(REDUCER_XSIZE_A, mf->width);
+		if (!ret)
+			ret = reg_write(REDUCER_YSIZE_A, mf->height);
+		if (!ret)
+			ret = reg_write(REDUCER_XPAN_B, xpan);
+		if (!ret)
+			ret = reg_write(REDUCER_YPAN_B, ypan);
+		if (!ret)
+			ret = reg_write(REDUCER_XZOOM_B, rect->width);
+		if (!ret)
+			ret = reg_write(REDUCER_YZOOM_B, rect->height);
+		if (!ret)
+			ret = reg_write(REDUCER_XSIZE_B, mf->width);
+		if (!ret)
+			ret = reg_write(REDUCER_YSIZE_B, mf->height);
+
+		/* not making assumptions about where default and maximum
+		 * rectangles are, we need to do this calculation always
+		 * when IFP is involved */
+		if (row_start % 2)
+			data_outfmt1 |= MT9M111_OUTFMT_CFA_1ST_ROW_BLUE;
+		if (row_start % 2 ^ colum_start % 2)
+			data_outfmt1 |= MT9M111_OUTFMT_CFA_1ST_COL_R_B;
+
+		mask_outfmt1 = MT9M111_OUTFMT_CFA_1ST_ROW_BLUE |
+			MT9M111_OUTFMT_CFA_1ST_COL_R_B;
+
 		if (!ret)
-			ret = reg_write(REDUCER_YSIZE_A, height);
+			ret = reg_mask(OUTPUT_FORMAT_CTRL, data_outfmt1,
+				mask_outfmt1);
 	}
 
 	return ret;
-- 
1.7.1

[PATCH 16/20] mt9m111: added more supported BE colour formats

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 11a68b6..6da9f48 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -175,7 +175,9 @@ static const struct mt9m111_datafmt mt9m111_colour_fmts[] = {
 	{V4L2_MBUS_FMT_YUYV8_2X8_BE, V4L2_COLORSPACE_JPEG},
 	{V4L2_MBUS_FMT_YVYU8_2X8_BE, V4L2_COLORSPACE_JPEG},
 	{V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, V4L2_COLORSPACE_SRGB},
+	{V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, V4L2_COLORSPACE_SRGB},
 	{V4L2_MBUS_FMT_RGB565_2X8_LE, V4L2_COLORSPACE_SRGB},
+	{V4L2_MBUS_FMT_RGB565_2X8_BE, V4L2_COLORSPACE_SRGB},
 	{V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_COLORSPACE_SRGB},
 	{V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, V4L2_COLORSPACE_SRGB},
 };
-- 
1.7.1

[PATCH 17/20] mt9m111: rewrite set_pixfmt

[Michael Grzeschik]

removed pixfmt helper functions and option flags
setting the configuration register directly in set_pixfmt

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |  142 ++++++++++++++++-------------------------
 1 files changed, 56 insertions(+), 86 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 6da9f48..f327177 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -104,7 +104,10 @@
 #define MT9M111_OUTPUT_FORMAT_CTRL2_B	0x19b
 
 #define MT9M111_OPMODE_AUTOEXPO_EN	(1 << 14)
+#define MT9M111_OPMODE_FLICKER_DET_EN	(1 << 7)
 #define MT9M111_OPMODE_AUTOWHITEBAL_EN	(1 << 1)
+#define MT9M111_OUTFMT_FLIP_BAYER_COL	(1 << 9)
+#define MT9M111_OUTFMT_FLIP_BAYER_ROW	(1 << 8)
 #define MT9M111_OUTFMT_CFA_1ST_ROW_BLUE	(1 << 1)
 #define MT9M111_OUTFMT_CFA_1ST_COL_R_B	(1 << 0)
 #define MT9M111_OUTFMT_PROCESSED_BAYER	(1 << 14)
@@ -124,6 +127,7 @@
 #define MT9M111_OUTFMT_SWAP_YCbCr_C_Y	(1 << 1)
 #define MT9M111_OUTFMT_SWAP_RGB_EVEN	(1 << 1)
 #define MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr	(1 << 0)
+#define MT9M111_OUTFMT_SWAP_RGB_R_B	(1 << 0)
 
 /*
  * Camera control register addresses (0x200..0x2ff not implemented)
@@ -204,10 +208,6 @@ struct mt9m111 {
 	unsigned int powered:1;
 	unsigned int hflip:1;
 	unsigned int vflip:1;
-	unsigned int swap_rgb_even_odd:1;
-	unsigned int swap_rgb_red_blue:1;
-	unsigned int swap_yuv_y_chromas:1;
-	unsigned int swap_yuv_cb_cr:1;
 	unsigned int autowhitebalance:1;
 };
 
@@ -397,68 +397,6 @@ static int mt9m111_setup_rect(struct i2c_client *client,
 	return ret;
 }
 
-static int mt9m111_setup_pixfmt(struct i2c_client *client, u16 outfmt)
-{
-	int ret;
-
-	ret = reg_write(OUTPUT_FORMAT_CTRL2_A, outfmt);
-	if (!ret)
-		ret = reg_write(OUTPUT_FORMAT_CTRL2_B, outfmt);
-	return ret;
-}
-
-static int mt9m111_setfmt_bayer8(struct i2c_client *client)
-{
-	return mt9m111_setup_pixfmt(client, MT9M111_OUTFMT_PROCESSED_BAYER |
-				    MT9M111_OUTFMT_RGB);
-}
-
-static int mt9m111_setfmt_bayer10(struct i2c_client *client)
-{
-	return mt9m111_setup_pixfmt(client, MT9M111_OUTFMT_BYPASS_IFP);
-}
-
-static int mt9m111_setfmt_rgb565(struct i2c_client *client)
-{
-	struct mt9m111 *mt9m111 = to_mt9m111(client);
-	int val = 0;
-
-	if (mt9m111->swap_rgb_red_blue)
-		val |= MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr;
-	if (mt9m111->swap_rgb_even_odd)
-		val |= MT9M111_OUTFMT_SWAP_RGB_EVEN;
-	val |= MT9M111_OUTFMT_RGB | MT9M111_OUTFMT_RGB565;
-
-	return mt9m111_setup_pixfmt(client, val);
-}
-
-static int mt9m111_setfmt_rgb555(struct i2c_client *client)
-{
-	struct mt9m111 *mt9m111 = to_mt9m111(client);
-	int val = 0;
-
-	if (mt9m111->swap_rgb_red_blue)
-		val |= MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr;
-	if (mt9m111->swap_rgb_even_odd)
-		val |= MT9M111_OUTFMT_SWAP_RGB_EVEN;
-	val |= MT9M111_OUTFMT_RGB | MT9M111_OUTFMT_RGB555;
-
-	return mt9m111_setup_pixfmt(client, val);
-}
-
-static int mt9m111_setfmt_yuv(struct i2c_client *client)
-{
-	struct mt9m111 *mt9m111 = to_mt9m111(client);
-	int val = 0;
-
-	if (mt9m111->swap_yuv_cb_cr)
-		val |= MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr;
-	if (mt9m111->swap_yuv_y_chromas)
-		val |= MT9M111_OUTFMT_SWAP_YCbCr_C_Y;
-
-	return mt9m111_setup_pixfmt(client, val);
-}
-
 static int mt9m111_enable(struct i2c_client *client)
 {
 	struct mt9m111 *mt9m111 = to_mt9m111(client);
@@ -616,41 +554,49 @@ static int mt9m111_g_fmt(struct v4l2_subdev *sd,
 static int mt9m111_set_pixfmt(struct i2c_client *client,
 			      enum v4l2_mbus_pixelcode code)
 {
-	struct mt9m111 *mt9m111 = to_mt9m111(client);
+	u16 data_outfmt1 = 0, data_outfmt2 = 0, mask_outfmt1, mask_outfmt2;
+	u16 data_opermod;
 	int ret;
 
+	data_opermod = MT9M111_OPMODE_AUTOEXPO_EN |
+		MT9M111_OPMODE_FLICKER_DET_EN | MT9M111_OPMODE_AUTOWHITEBAL_EN;
+
 	switch (code) {
 	case V4L2_MBUS_FMT_SBGGR8_1X8:
-		ret = mt9m111_setfmt_bayer8(client);
+		data_outfmt1 = MT9M111_OUTFMT_FLIP_BAYER_ROW;
+		data_outfmt2 = MT9M111_OUTFMT_PROCESSED_BAYER |
+			MT9M111_OUTFMT_RGB;
 		break;
 	case V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE:
-		ret = mt9m111_setfmt_bayer10(client);
+		data_outfmt2 = MT9M111_OUTFMT_BYPASS_IFP | MT9M111_OUTFMT_RGB;
 		break;
 	case V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE:
-		ret = mt9m111_setfmt_rgb555(client);
+		data_outfmt2 = MT9M111_OUTFMT_SWAP_RGB_EVEN |
+			MT9M111_OUTFMT_RGB |
+			MT9M111_OUTFMT_RGB555;
+		break;
+	case V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE:
+		data_outfmt2 = MT9M111_OUTFMT_RGB | MT9M111_OUTFMT_RGB555;
 		break;
 	case V4L2_MBUS_FMT_RGB565_2X8_LE:
-		ret = mt9m111_setfmt_rgb565(client);
+		data_outfmt2 = MT9M111_OUTFMT_SWAP_RGB_EVEN |
+			MT9M111_OUTFMT_RGB |
+			MT9M111_OUTFMT_RGB565;
+		break;
+	case V4L2_MBUS_FMT_RGB565_2X8_BE:
+		data_outfmt2 = MT9M111_OUTFMT_RGB | MT9M111_OUTFMT_RGB565;
 		break;
 	case V4L2_MBUS_FMT_YUYV8_2X8_BE:
-		mt9m111->swap_yuv_y_chromas = 0;
-		mt9m111->swap_yuv_cb_cr = 0;
-		ret = mt9m111_setfmt_yuv(client);
 		break;
 	case V4L2_MBUS_FMT_YVYU8_2X8_BE:
-		mt9m111->swap_yuv_y_chromas = 0;
-		mt9m111->swap_yuv_cb_cr = 1;
-		ret = mt9m111_setfmt_yuv(client);
+		data_outfmt2 = MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr;
 		break;
 	case V4L2_MBUS_FMT_YUYV8_2X8_LE:
-		mt9m111->swap_yuv_y_chromas = 1;
-		mt9m111->swap_yuv_cb_cr = 0;
-		ret = mt9m111_setfmt_yuv(client);
+		data_outfmt2 = MT9M111_OUTFMT_SWAP_YCbCr_C_Y;
 		break;
 	case V4L2_MBUS_FMT_YVYU8_2X8_LE:
-		mt9m111->swap_yuv_y_chromas = 1;
-		mt9m111->swap_yuv_cb_cr = 1;
-		ret = mt9m111_setfmt_yuv(client);
+		data_outfmt2 = MT9M111_OUTFMT_SWAP_YCbCr_C_Y |
+			MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr;
 		break;
 	default:
 		dev_err(&client->dev, "Pixel format not handled : %x\n",
@@ -658,6 +604,33 @@ static int mt9m111_set_pixfmt(struct i2c_client *client,
 		ret = -EINVAL;
 	}
 
+	mask_outfmt1 = MT9M111_OUTFMT_FLIP_BAYER_COL |
+		MT9M111_OUTFMT_FLIP_BAYER_ROW;
+
+	mask_outfmt2 = MT9M111_OUTFMT_PROCESSED_BAYER |
+		MT9M111_OUTFMT_BYPASS_IFP | MT9M111_OUTFMT_RGB |
+		MT9M111_OUTFMT_RGB565 | MT9M111_OUTFMT_RGB555 |
+		MT9M111_OUTFMT_RGB444x | MT9M111_OUTFMT_RGBx444 |
+		MT9M111_OUTFMT_SWAP_YCbCr_C_Y | MT9M111_OUTFMT_SWAP_RGB_EVEN |
+		MT9M111_OUTFMT_SWAP_YCbCr_Cb_Cr | MT9M111_OUTFMT_SWAP_RGB_R_B;
+
+	ret = reg_mask(OUTPUT_FORMAT_CTRL, data_outfmt1, mask_outfmt1);
+
+	if (!ret)
+		ret = reg_mask(OUTPUT_FORMAT_CTRL2_A, data_outfmt2,
+			mask_outfmt2);
+	if (!ret)
+		ret = reg_mask(OUTPUT_FORMAT_CTRL2_B, data_outfmt2,
+			mask_outfmt2);
+
+	if (code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE) {
+		if (!ret)
+			ret = reg_clear(OPER_MODE_CTRL, data_opermod);
+	} else {
+		if (!ret)
+			ret = reg_set(OPER_MODE_CTRL, data_opermod);
+	}
+
 	return ret;
 }
 
@@ -1081,9 +1054,6 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
 	mt9m111->autoexposure = 1;
 	mt9m111->autowhitebalance = 1;
 
-	mt9m111->swap_rgb_even_odd = 1;
-	mt9m111->swap_rgb_red_blue = 1;
-
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
-- 
1.7.1

[PATCH 18/20] mt9m111: make use of testpattern in debug mode

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |   63 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index f327177..799a735 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -87,6 +87,7 @@
  */
 #define MT9M111_OPER_MODE_CTRL		0x106
 #define MT9M111_OUTPUT_FORMAT_CTRL	0x108
+#define MT9M111_TEST_PATTERN_GEN	0x148
 #define MT9M111_REDUCER_XPAN_B		0x19f
 #define MT9M111_REDUCER_XZOOM_B		0x1a0
 #define MT9M111_REDUCER_XSIZE_B		0x1a1
@@ -110,6 +111,15 @@
 #define MT9M111_OUTFMT_FLIP_BAYER_ROW	(1 << 8)
 #define MT9M111_OUTFMT_CFA_1ST_ROW_BLUE	(1 << 1)
 #define MT9M111_OUTFMT_CFA_1ST_COL_R_B	(1 << 0)
+#define MT9M111_TST_PATT_OFF		(0 << 0)
+#define MT9M111_TST_PATT_1		(1 << 0)
+#define MT9M111_TST_PATT_2		(2 << 0)
+#define MT9M111_TST_PATT_3		(3 << 0)
+#define MT9M111_TST_PATT_4		(4 << 0)
+#define MT9M111_TST_PATT_5		(5 << 0)
+#define MT9M111_TST_PATT_6		(6 << 0)
+#define MT9M111_TST_PATT_COLORBARS	(7 << 0)
+#define MT9M111_TST_PATT_FORCE_WB_GAIN_1 (1 << 7)
 #define MT9M111_OUTFMT_PROCESSED_BAYER	(1 << 14)
 #define MT9M111_OUTFMT_BYPASS_IFP	(1 << 10)
 #define MT9M111_OUTFMT_INV_PIX_CLOCK	(1 << 9)
@@ -149,6 +159,13 @@
 #define MT9M111_DEF_HEIGHT	1024
 #define MT9M111_DEF_WIDTH	1280
 
+#ifdef DEBUG
+static int testpattern;
+module_param(testpattern, int, S_IRUGO);
+MODULE_PARM_DESC(testpattern, "Test pattern: a number from 1 to 10, 0 for "
+		"normal usage");
+#endif
+
 static int soft_crop;
 module_param(soft_crop, int, S_IRUGO);
 MODULE_PARM_DESC(soft_crop, "Enables soft-cropping and thus the use of "
@@ -556,6 +573,9 @@ static int mt9m111_set_pixfmt(struct i2c_client *client,
 {
 	u16 data_outfmt1 = 0, data_outfmt2 = 0, mask_outfmt1, mask_outfmt2;
 	u16 data_opermod;
+#ifdef DEBUG
+	u16 pattern = 0;
+#endif
 	int ret;
 
 	data_opermod = MT9M111_OPMODE_AUTOEXPO_EN |
@@ -616,6 +636,49 @@ static int mt9m111_set_pixfmt(struct i2c_client *client,
 
 	ret = reg_mask(OUTPUT_FORMAT_CTRL, data_outfmt1, mask_outfmt1);
 
+#ifdef DEBUG
+	switch (testpattern) {
+	case 1:
+		pattern = MT9M111_TST_PATT_1 | MT9M111_TST_PATT_FORCE_WB_GAIN_1;
+		break;
+	case 2:
+		pattern = MT9M111_TST_PATT_2 | MT9M111_TST_PATT_FORCE_WB_GAIN_1;
+		break;
+	case 3:
+		pattern = MT9M111_TST_PATT_3 | MT9M111_TST_PATT_FORCE_WB_GAIN_1;
+		break;
+	case 4:
+		pattern = MT9M111_TST_PATT_4 | MT9M111_TST_PATT_FORCE_WB_GAIN_1;
+		break;
+	case 5:
+		pattern = MT9M111_TST_PATT_5 | MT9M111_TST_PATT_FORCE_WB_GAIN_1;
+		break;
+	case 6:
+		pattern = MT9M111_TST_PATT_6 | MT9M111_TST_PATT_FORCE_WB_GAIN_1;
+		break;
+	case 7:
+		pattern = MT9M111_TST_PATT_COLORBARS |
+			MT9M111_TST_PATT_FORCE_WB_GAIN_1;
+		break;
+	case 8:
+		data_outfmt2 |= MT9M111_OUTFMT_TST_RAMP_COL;
+		break;
+	case 9:
+		data_outfmt2 |= MT9M111_OUTFMT_TST_RAMP_ROW;
+		break;
+	case 10:
+		data_outfmt2 |= MT9M111_OUTFMT_TST_RAMP_FRAME;
+		break;
+	}
+
+	dev_dbg(&client->dev, "%s: using testpattern %d\n", __func__,
+			testpattern);
+
+	if (!ret)
+		ret = mt9m111_reg_set(client,
+				MT9M111_TEST_PATTERN_GEN, pattern);
+#endif
+
 	if (!ret)
 		ret = reg_mask(OUTPUT_FORMAT_CTRL2_A, data_outfmt2,
 			mask_outfmt2);
-- 
1.7.1

[PATCH 19/20] mt9m111: try_fmt rewrite to use preset values

[Michael Grzeschik]

make use of the format.rect boundery values

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |   41 +++++++++++++++++++----------------------
 1 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 799a735..f472ca1 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -733,35 +733,32 @@ static int mt9m111_try_fmt(struct v4l2_subdev *sd,
 			   struct v4l2_mbus_framefmt *mf)
 {
 	struct i2c_client *client = sd->priv;
+	struct mt9m111 *mt9m111 = to_mt9m111(client);
+	struct v4l2_rect rect = mt9m111->format.rect;
 	const struct mt9m111_datafmt *fmt;
-	bool bayer = mf->code == V4L2_MBUS_FMT_SBGGR8_1X8 ||
-		mf->code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE;
 
 	fmt = mt9m111_find_datafmt(mf->code, mt9m111_colour_fmts,
 				   ARRAY_SIZE(mt9m111_colour_fmts));
 	if (!fmt)
 		return -EINVAL;
 
-	/*
-	 * With Bayer format enforce even side lengths, but let the user play
-	 * with the starting pixel
-	 */
+	mf->code	= fmt->code;
+	mf->colorspace	= fmt->colorspace;
+	mf->field	= V4L2_FIELD_NONE;
 
-	if (mf->height > MT9M111_MAX_HEIGHT)
-		mf->height = MT9M111_MAX_HEIGHT;
-	else if (mf->height < 2)
-		mf->height = 2;
-	else if (bayer)
-		mf->height = ALIGN(mf->height, 2);
-
-	if (mf->width > MT9M111_MAX_WIDTH)
-		mf->width = MT9M111_MAX_WIDTH;
-	else if (mf->width < 2)
-		mf->width = 2;
-	else if (bayer)
-		mf->width = ALIGN(mf->width, 2);
-
-	mf->colorspace = fmt->colorspace;
+	if (mf->code == V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE) {
+		mf->width	= rect.width;
+		mf->height	= rect.height;
+	} else {
+		if (mf->width > rect.width)
+			mf->width = rect.width;
+		if (mf->height > rect.height)
+			mf->height = rect.height;
+	}
+
+	dev_dbg(&client->dev, "%s: mf: width=%d height=%d pixelcode=%d "
+		"field=%x colorspace=%x\n", __func__, mf->width, mf->height,
+		mf->code, mf->field, mf->colorspace);
 
 	return 0;
 }
-- 
1.7.1

[PATCH 20/20] mt9m111: s_fmt make use of try_fmt

[Michael Grzeschik]

Signed-off-by: Philipp Wiesner <p.wiesner@phytec.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/media/video/mt9m111.c |   60 +++++++++++++++++++----------------------
 1 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index f472ca1..ec758ae 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -697,38 +697,6 @@ static int mt9m111_set_pixfmt(struct i2c_client *client,
 	return ret;
 }
 
-static int mt9m111_s_fmt(struct v4l2_subdev *sd,
-			 struct v4l2_mbus_framefmt *mf)
-{
-	struct i2c_client *client = sd->priv;
-	const struct mt9m111_datafmt *fmt;
-	struct mt9m111 *mt9m111 = to_mt9m111(client);
-	struct v4l2_rect *rect;
-	struct mt9m111_format format;
-	int ret;
-
-	fmt = mt9m111_find_datafmt(mf->code, mt9m111_colour_fmts,
-				   ARRAY_SIZE(mt9m111_colour_fmts));
-	if (!fmt)
-		return -EINVAL;
-
-	format.rect	= mt9m111->format.rect;
-	format.mf	= *mf;
-	rect		= &format.rect;
-
-	dev_dbg(&client->dev,
-		"%s code=%x left=%d, top=%d, width=%d, height=%d\n", __func__,
-		mf->code, rect->left, rect->top, rect->width, rect->height);
-
-	ret = mt9m111_make_rect(client, &format);
-	if (!ret)
-		ret = mt9m111_set_pixfmt(client, format.mf.code);
-	if (!ret)
-		mt9m111->format = format;
-
-	return ret;
-}
-
 static int mt9m111_try_fmt(struct v4l2_subdev *sd,
 			   struct v4l2_mbus_framefmt *mf)
 {
@@ -763,6 +731,34 @@ static int mt9m111_try_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int mt9m111_s_fmt(struct v4l2_subdev *sd,
+			 struct v4l2_mbus_framefmt *mf)
+{
+	struct i2c_client *client = sd->priv;
+	struct mt9m111 *mt9m111 = to_mt9m111(client);
+	struct mt9m111_format format;
+	int ret;
+
+	dev_dbg(&client->dev, "%s: mf: width=%d height=%d pixelcode=%d "
+		"field=%x colorspace=%x\n", __func__, mf->width, mf->height,
+		mf->code, mf->field, mf->colorspace);
+
+	ret = mt9m111_try_fmt(sd, mf);
+
+	if (!ret) {
+		format.rect	= mt9m111->format.rect;
+		format.mf	= *mf;
+
+		ret = mt9m111_make_rect(client, &format);
+	}
+	if (!ret)
+		ret = mt9m111_set_pixfmt(client, format.mf.code);
+	if (!ret)
+		mt9m111->format = format;
+
+	return ret;
+}
+
 static int mt9m111_g_chip_ident(struct v4l2_subdev *sd,
 				struct v4l2_dbg_chip_ident *id)
 {
-- 
1.7.1

Re: [PATCH 00/20] MT9M111/MT9M131

[Guennadi Liakhovetski]

Hi Michael

Thanks for the patches, and for taking care about my holiday - now I will 
definitely not have to be bored, while lying around on tropical beaches of 
Denmark;)

Ok, I will review them, and I hope you realise, this has practically 0 
chance to get into 2.6.36, unless Linus decides to release a couple more 
-rc's. So, I think we shall take our time and prepare these for 2.6.37.

Thanks
Guennadi

On Fri, 30 Jul 2010, Michael Grzeschik wrote:

> Hi everyone,
> 
> the following patchseries was created in a rewrite process of the
> mt9m111 camera driver while it was tested for support of the very
> similar silicon mt9m121. Some patches add functionality like panning or
> test pattern generation or adjust rectengular positioning while others
> do some restructuring. It has been tested as functional. Comments on
> this are very welcome.
> 
> Michael Grzeschik (19):
>   mt9m111: init chip after read CHIP_VERSION
>   mt9m111: register cleanup hex to dec bitoffset
>   mt9m111: added new bit offset defines
>   mt9m111: added default row/col/width/height values
>   mt9m111: changed MAX_{HEIGHT,WIDTH} values to silicon pixelcount
>   mt9m111: changed MIN_DARK_COLS to MT9M131 spec count
>   mt9m111: cropcap use defined default rect properties in defrect
>   mt9m111: cropcap check if type is CAPTURE
>   mt9m111: rewrite make_rect for positioning in debug
>   mt9m111: added mt9m111 format structure
>   mt9m111: s_crop add calculation of output size
>   mt9m111: s_crop check for VIDEO_CAPTURE type
>   mt9m111: added reg_mask function
>   mt9m111: rewrite setup_rect, added soft_crop for smooth panning
>   mt9m111: added more supported BE colour formats
>   mt9m111: rewrite set_pixfmt
>   mt9m111: make use of testpattern in debug mode
>   mt9m111: try_fmt rewrite to use preset values
>   mt9m111: s_fmt make use of try_fmt
> 
> Philipp Wiesner (1):
>   mt9m111: Added indication that MT9M131 is supported by this driver
> 
>  drivers/media/video/Kconfig   |    5 +-
>  drivers/media/video/mt9m111.c |  596 ++++++++++++++++++++++++++---------------
>  2 files changed, 377 insertions(+), 224 deletions(-)
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 00/20] MT9M111/MT9M131

[Robert Jarzmik]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> Hi Michael
>
> Thanks for the patches, and for taking care about my holiday - now I will 
> definitely not have to be bored, while lying around on tropical beaches of 
> Denmark;)
Same thing in here, if my wife lets me play with my computer :)

I'll review them as soon as possible.

Cheers.

--
Robert

Re: [PATCH 00/20] MT9M111/MT9M131

[Michael Grzeschik]

Hey Guennadi, Robert

sorry for that lack of information to those patches. I also just have
been slicing one big patch into several canonical and tried to get rid
of most tofu. You see the result in that patchseries. But since some big
changes i could not figure out correctly, i left them in the stack for
review. I should have adding an RFC to the Subject on those. Since i
still have no response from the original author of these patches and you
also see no sense in some changes i will condense the stack to a
managable amount and will repost this v2 series in short time.

Thanks for your time,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |