Re: [PATCH] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control

2012-09-25 Thread Prabhakar Lad
Hi Laurent,

Thanks for the review.

On Tue, Sep 25, 2012 at 9:30 PM, Laurent Pinchart
 wrote:
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Tuesday 25 September 2012 18:29:25 Prabhakar wrote:
>> From: Lad, Prabhakar 
>>
>> Signed-off-by: Lad, Prabhakar 
>> Signed-off-by: Manjunath Hadli 
>> ---
>>  drivers/media/i2c/mt9p031.c |   27 
>>  drivers/media/i2c/mt9t001.c |   33 +++---
>>  drivers/media/i2c/mt9v032.c |   46 
>>  3 files changed, 66 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
>> index 2c0f407..a45c2ea 100644
>> --- a/drivers/media/i2c/mt9p031.c
>> +++ b/drivers/media/i2c/mt9p031.c
>> @@ -574,11 +574,10 @@ static int mt9p031_set_crop(struct v4l2_subdev
>> *subdev, * V4L2 subdev control operations
>>   */
>>
>> -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
>> -#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1002)
>> -#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1003)
>> -#define V4L2_CID_BLC_ANALOG_OFFSET   (V4L2_CID_USER_BASE | 0x1004)
>> -#define V4L2_CID_BLC_DIGITAL_OFFSET  (V4L2_CID_USER_BASE | 0x1005)
>> +#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1001)
>> +#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1002)
>> +#define V4L2_CID_BLC_ANALOG_OFFSET   (V4L2_CID_USER_BASE | 0x1003)
>> +#define V4L2_CID_BLC_DIGITAL_OFFSET  (V4L2_CID_USER_BASE | 0x1004)
>
> Let's not change the value of the other device-specific controls.
>
Ok

>>  static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
>>  {
>> @@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] =
>> { static const struct v4l2_ctrl_config mt9p031_ctrls[] = {
>>   {
>>   .ops= &mt9p031_ctrl_ops,
>> - .id = V4L2_CID_TEST_PATTERN,
>> - .type   = V4L2_CTRL_TYPE_MENU,
>> - .name   = "Test Pattern",
>> - .min= 0,
>> - .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1,
>> - .step   = 0,
>> - .def= 0,
>> - .flags  = 0,
>> - .menu_skip_mask = 0,
>> - .qmenu  = mt9p031_test_pattern_menu,
>> - }, {
>> - .ops= &mt9p031_ctrl_ops,
>>   .id = V4L2_CID_BLC_AUTO,
>>   .type   = V4L2_CTRL_TYPE_BOOLEAN,
>>   .name   = "BLC, Auto",
>> @@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client,
>>   mt9p031->model = did->driver_data;
>>   mt9p031->reset = -1;
>>
>> - v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5);
>> + v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6);
>>
>>   v4l2_ctrl_new_std(&mt9p031->ctrls, &mt9p031_ctrl_ops,
>> V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN,
>> @@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client,
>>   v4l2_ctrl_new_std(&mt9p031->ctrls, &mt9p031_ctrl_ops,
>> V4L2_CID_PIXEL_RATE, pdata->target_freq,
>> pdata->target_freq, 1, pdata->target_freq);
>> + v4l2_ctrl_new_std_menu_items(&mt9p031->ctrls, &mt9p031_ctrl_ops,
>> +   V4L2_CID_TEST_PATTERN,
>> +   ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0,
>> +   0, mt9p031_test_pattern_menu);
>>
>>   for (i = 0; i < ARRAY_SIZE(mt9p031_ctrls); ++i)
>>   v4l2_ctrl_new_custom(&mt9p031->ctrls, &mt9p031_ctrls[i], NULL);
>> diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
>> index 6d343ad..16eac3f 100644
>> --- a/drivers/media/i2c/mt9t001.c
>> +++ b/drivers/media/i2c/mt9t001.c
>> @@ -124,6 +124,7 @@ struct mt9t001 {
>>
>>   u16 output_control;
>>   u16 black_level;
>> + bool test_pattern;
>>  };
>>
>>  static inline struct mt9t001 *to_mt9t001(struct v4l2_subdev *sd)
>> @@ -371,10 +372,10 @@ static int mt9t001_set_crop(struct v4l2_subdev
>> *subdev, * V4L2 subdev control operations
>>   */
>>
>> -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
>> -#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002)
>> -#define V4L2_CID_BLACK_LEVEL_OFFSET  (V4L2_CID_USER_BASE | 0x1003)
>> -#define V4L2_CID_BLACK_LEVEL_CALIBRATE   (V4L2_CID_USER_BASE | 0x1004)
>> +#define V4L2_CID_PARAMETRIC_TEST_PATTERN (V4L2_CID_USER_BASE | 0x1001)
>
> That's a bit of a long name. What about V4L2_CID_TEST_PATTERN_COLOR ?
>
Ok.

>> +#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002)
>> +#define V4L2_CID_BLACK_LEVEL_OFFSET  (V4L2_CID_USER_BASE | 0x1003)
>> +#define V4L2_CID_BLACK_LEVEL_CALIBRATE   (V4L2_CID_USER_BASE | 
>> 0x1004)
>>
>>  #define V4L2_CID_GAIN_RED(V4L2_C

Re: [PATCH] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control

2012-09-25 Thread Laurent Pinchart
Hi Prabhakar,

Thank you for the patch.

On Tuesday 25 September 2012 18:29:25 Prabhakar wrote:
> From: Lad, Prabhakar 
> 
> Signed-off-by: Lad, Prabhakar 
> Signed-off-by: Manjunath Hadli 
> ---
>  drivers/media/i2c/mt9p031.c |   27 
>  drivers/media/i2c/mt9t001.c |   33 +++---
>  drivers/media/i2c/mt9v032.c |   46 
>  3 files changed, 66 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index 2c0f407..a45c2ea 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -574,11 +574,10 @@ static int mt9p031_set_crop(struct v4l2_subdev
> *subdev, * V4L2 subdev control operations
>   */
> 
> -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
> -#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1002)
> -#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1003)
> -#define V4L2_CID_BLC_ANALOG_OFFSET   (V4L2_CID_USER_BASE | 0x1004)
> -#define V4L2_CID_BLC_DIGITAL_OFFSET  (V4L2_CID_USER_BASE | 0x1005)
> +#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1001)
> +#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1002)
> +#define V4L2_CID_BLC_ANALOG_OFFSET   (V4L2_CID_USER_BASE | 0x1003)
> +#define V4L2_CID_BLC_DIGITAL_OFFSET  (V4L2_CID_USER_BASE | 0x1004)

Let's not change the value of the other device-specific controls.

>  static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
> @@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] =
> { static const struct v4l2_ctrl_config mt9p031_ctrls[] = {
>   {
>   .ops= &mt9p031_ctrl_ops,
> - .id = V4L2_CID_TEST_PATTERN,
> - .type   = V4L2_CTRL_TYPE_MENU,
> - .name   = "Test Pattern",
> - .min= 0,
> - .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1,
> - .step   = 0,
> - .def= 0,
> - .flags  = 0,
> - .menu_skip_mask = 0,
> - .qmenu  = mt9p031_test_pattern_menu,
> - }, {
> - .ops= &mt9p031_ctrl_ops,
>   .id = V4L2_CID_BLC_AUTO,
>   .type   = V4L2_CTRL_TYPE_BOOLEAN,
>   .name   = "BLC, Auto",
> @@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client,
>   mt9p031->model = did->driver_data;
>   mt9p031->reset = -1;
> 
> - v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5);
> + v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6);
> 
>   v4l2_ctrl_new_std(&mt9p031->ctrls, &mt9p031_ctrl_ops,
> V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN,
> @@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client,
>   v4l2_ctrl_new_std(&mt9p031->ctrls, &mt9p031_ctrl_ops,
> V4L2_CID_PIXEL_RATE, pdata->target_freq,
> pdata->target_freq, 1, pdata->target_freq);
> + v4l2_ctrl_new_std_menu_items(&mt9p031->ctrls, &mt9p031_ctrl_ops,
> +   V4L2_CID_TEST_PATTERN,
> +   ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0,
> +   0, mt9p031_test_pattern_menu);
> 
>   for (i = 0; i < ARRAY_SIZE(mt9p031_ctrls); ++i)
>   v4l2_ctrl_new_custom(&mt9p031->ctrls, &mt9p031_ctrls[i], NULL);
> diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
> index 6d343ad..16eac3f 100644
> --- a/drivers/media/i2c/mt9t001.c
> +++ b/drivers/media/i2c/mt9t001.c
> @@ -124,6 +124,7 @@ struct mt9t001 {
> 
>   u16 output_control;
>   u16 black_level;
> + bool test_pattern;
>  };
> 
>  static inline struct mt9t001 *to_mt9t001(struct v4l2_subdev *sd)
> @@ -371,10 +372,10 @@ static int mt9t001_set_crop(struct v4l2_subdev
> *subdev, * V4L2 subdev control operations
>   */
> 
> -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
> -#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002)
> -#define V4L2_CID_BLACK_LEVEL_OFFSET  (V4L2_CID_USER_BASE | 0x1003)
> -#define V4L2_CID_BLACK_LEVEL_CALIBRATE   (V4L2_CID_USER_BASE | 0x1004)
> +#define V4L2_CID_PARAMETRIC_TEST_PATTERN (V4L2_CID_USER_BASE | 0x1001)

That's a bit of a long name. What about V4L2_CID_TEST_PATTERN_COLOR ?

> +#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002)
> +#define V4L2_CID_BLACK_LEVEL_OFFSET  (V4L2_CID_USER_BASE | 0x1003)
> +#define V4L2_CID_BLACK_LEVEL_CALIBRATE   (V4L2_CID_USER_BASE | 
> 0x1004)
> 
>  #define V4L2_CID_GAIN_RED(V4L2_CTRL_CLASS_CAMERA | 0x1001)
>  #define V4L2_CID_GAIN_GREEN_RED  (V4L2_CTRL_CLASS_CAMERA | 
> 0x1002)
> @@ -485,8 +486,15 @@ static int mt9t001_s_ctrl(struct v4l2_ctrl *ctrl)
> 
>   r

[PATCH] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control

2012-09-25 Thread Prabhakar
From: Lad, Prabhakar 

Signed-off-by: Lad, Prabhakar 
Signed-off-by: Manjunath Hadli 
---
 drivers/media/i2c/mt9p031.c |   27 
 drivers/media/i2c/mt9t001.c |   33 +++---
 drivers/media/i2c/mt9v032.c |   46 +-
 3 files changed, 66 insertions(+), 40 deletions(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 2c0f407..a45c2ea 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -574,11 +574,10 @@ static int mt9p031_set_crop(struct v4l2_subdev *subdev,
  * V4L2 subdev control operations
  */
 
-#define V4L2_CID_TEST_PATTERN  (V4L2_CID_USER_BASE | 0x1001)
-#define V4L2_CID_BLC_AUTO  (V4L2_CID_USER_BASE | 0x1002)
-#define V4L2_CID_BLC_TARGET_LEVEL  (V4L2_CID_USER_BASE | 0x1003)
-#define V4L2_CID_BLC_ANALOG_OFFSET (V4L2_CID_USER_BASE | 0x1004)
-#define V4L2_CID_BLC_DIGITAL_OFFSET(V4L2_CID_USER_BASE | 0x1005)
+#define V4L2_CID_BLC_AUTO  (V4L2_CID_USER_BASE | 0x1001)
+#define V4L2_CID_BLC_TARGET_LEVEL  (V4L2_CID_USER_BASE | 0x1002)
+#define V4L2_CID_BLC_ANALOG_OFFSET (V4L2_CID_USER_BASE | 0x1003)
+#define V4L2_CID_BLC_DIGITAL_OFFSET(V4L2_CID_USER_BASE | 0x1004)
 
 static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
 {
@@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] = {
 static const struct v4l2_ctrl_config mt9p031_ctrls[] = {
{
.ops= &mt9p031_ctrl_ops,
-   .id = V4L2_CID_TEST_PATTERN,
-   .type   = V4L2_CTRL_TYPE_MENU,
-   .name   = "Test Pattern",
-   .min= 0,
-   .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1,
-   .step   = 0,
-   .def= 0,
-   .flags  = 0,
-   .menu_skip_mask = 0,
-   .qmenu  = mt9p031_test_pattern_menu,
-   }, {
-   .ops= &mt9p031_ctrl_ops,
.id = V4L2_CID_BLC_AUTO,
.type   = V4L2_CTRL_TYPE_BOOLEAN,
.name   = "BLC, Auto",
@@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client,
mt9p031->model = did->driver_data;
mt9p031->reset = -1;
 
-   v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5);
+   v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6);
 
v4l2_ctrl_new_std(&mt9p031->ctrls, &mt9p031_ctrl_ops,
  V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN,
@@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client,
v4l2_ctrl_new_std(&mt9p031->ctrls, &mt9p031_ctrl_ops,
  V4L2_CID_PIXEL_RATE, pdata->target_freq,
  pdata->target_freq, 1, pdata->target_freq);
+   v4l2_ctrl_new_std_menu_items(&mt9p031->ctrls, &mt9p031_ctrl_ops,
+ V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0,
+ 0, mt9p031_test_pattern_menu);
 
for (i = 0; i < ARRAY_SIZE(mt9p031_ctrls); ++i)
v4l2_ctrl_new_custom(&mt9p031->ctrls, &mt9p031_ctrls[i], NULL);
diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
index 6d343ad..16eac3f 100644
--- a/drivers/media/i2c/mt9t001.c
+++ b/drivers/media/i2c/mt9t001.c
@@ -124,6 +124,7 @@ struct mt9t001 {
 
u16 output_control;
u16 black_level;
+   bool test_pattern;
 };
 
 static inline struct mt9t001 *to_mt9t001(struct v4l2_subdev *sd)
@@ -371,10 +372,10 @@ static int mt9t001_set_crop(struct v4l2_subdev *subdev,
  * V4L2 subdev control operations
  */
 
-#define V4L2_CID_TEST_PATTERN  (V4L2_CID_USER_BASE | 0x1001)
-#define V4L2_CID_BLACK_LEVEL_AUTO  (V4L2_CID_USER_BASE | 0x1002)
-#define V4L2_CID_BLACK_LEVEL_OFFSET(V4L2_CID_USER_BASE | 0x1003)
-#define V4L2_CID_BLACK_LEVEL_CALIBRATE (V4L2_CID_USER_BASE | 0x1004)
+#define V4L2_CID_PARAMETRIC_TEST_PATTERN   (V4L2_CID_USER_BASE | 0x1001)
+#define V4L2_CID_BLACK_LEVEL_AUTO  (V4L2_CID_USER_BASE | 0x1002)
+#define V4L2_CID_BLACK_LEVEL_OFFSET(V4L2_CID_USER_BASE | 0x1003)
+#define V4L2_CID_BLACK_LEVEL_CALIBRATE (V4L2_CID_USER_BASE | 0x1004)
 
 #define V4L2_CID_GAIN_RED  (V4L2_CTRL_CLASS_CAMERA | 0x1001)
 #define V4L2_CID_GAIN_GREEN_RED(V4L2_CTRL_CLASS_CAMERA | 
0x1002)
@@ -485,8 +486,15 @@ static int mt9t001_s_ctrl(struct v4l2_ctrl *ctrl)
 
return mt9t001_write(client, MT9T001_SHUTTER_WIDTH_HIGH,
 ctrl->val >> 16);
-
case V4L2_CID_TEST_PATTERN:
+   mt9t001->test_pattern = ctrl->val;
+   break;
+
+   case V4L2_CID_PARAMETRIC_TEST_PATTERN:
+   if (!mt9t001->test_pattern) {
+