[PATCH 1/1] smiapp: Implement the test pattern control

[PATCH 1/1] smiapp: Implement the test pattern control

[Sakari Ailus]

Add support for the V4L2_CID_TEST_PATTERN control. When the solid colour
mode is selected, additional controls become available for setting the
solid four solid colour components.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 120 +++++++++++++++++++++++++++++++--
 drivers/media/i2c/smiapp/smiapp.h      |   4 ++
 2 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 446c82c..025342c 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -32,6 +32,7 @@
 #include <linux/gpio.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/smiapp.h>
 #include <linux/regulator/consumer.h>
 #include <linux/v4l2-mediabus.h>
 #include <media/v4l2-device.h>
@@ -404,6 +405,52 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor)
 		pixel_order_str[pixel_order]);
 }
 
+static const char * const smiapp_test_patterns[] = {
+	"Disabled",
+	"Solid colour",
+	"Eight vertical colour bars",
+	"Colour bars with fade to grey",
+	"Pseudorandom sequence (PN9)",
+};
+
+static const struct v4l2_ctrl_ops smiapp_ctrl_ops;
+
+static struct v4l2_ctrl_config
+smiapp_test_pattern_colours[SMIAPP_COLOUR_COMPONENTS] = {
+	{
+		&smiapp_ctrl_ops,
+		V4L2_CID_SMIAPP_TEST_PATTERN_RED,
+		"Solid red pixel value",
+		V4L2_CTRL_TYPE_INTEGER,
+		0, 0, 1, 0,
+		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
+	},
+	{
+		&smiapp_ctrl_ops,
+		V4L2_CID_SMIAPP_TEST_PATTERN_GREENR,
+		"Solid green (red) pixel value",
+		V4L2_CTRL_TYPE_INTEGER,
+		0, 0, 1, 0,
+		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
+	},
+	{
+		&smiapp_ctrl_ops,
+		V4L2_CID_SMIAPP_TEST_PATTERN_BLUE,
+		"Solid blue pixel value",
+		V4L2_CTRL_TYPE_INTEGER,
+		0, 0, 1, 0,
+		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
+	},
+	{
+		&smiapp_ctrl_ops,
+		V4L2_CID_SMIAPP_TEST_PATTERN_GREENB,
+		"Solid green (blue) pixel value",
+		V4L2_CTRL_TYPE_INTEGER,
+		0, 0, 1, 0,
+		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
+	},
+};
+
 static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct smiapp_sensor *sensor =
@@ -477,6 +524,35 @@ static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
 
 		return smiapp_pll_update(sensor);
 
+	case V4L2_CID_TEST_PATTERN: {
+		unsigned int i;
+
+		for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++)
+			v4l2_ctrl_activate(
+				sensor->test_data[i],
+				ctrl->val ==
+				V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR);
+
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_PATTERN_MODE, ctrl->val);
+	}
+
+	case V4L2_CID_SMIAPP_TEST_PATTERN_RED:
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_DATA_RED, ctrl->val);
+
+	case V4L2_CID_SMIAPP_TEST_PATTERN_GREENR:
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_DATA_GREENR, ctrl->val);
+
+	case V4L2_CID_SMIAPP_TEST_PATTERN_BLUE:
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_DATA_BLUE, ctrl->val);
+
+	case V4L2_CID_SMIAPP_TEST_PATTERN_GREENB:
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_DATA_GREENB, ctrl->val);
+
 	default:
 		return -EINVAL;
 	}
@@ -489,10 +565,10 @@ static const struct v4l2_ctrl_ops smiapp_ctrl_ops = {
 static int smiapp_init_controls(struct smiapp_sensor *sensor)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
-	unsigned int max;
+	unsigned int max, i;
 	int rval;
 
-	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 7);
+	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 12);
 	if (rval)
 		return rval;
 	sensor->pixel_array->ctrl_handler.lock = &sensor->mutex;
@@ -535,6 +611,17 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor)
 		&sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops,
 		V4L2_CID_PIXEL_RATE, 0, 0, 1, 0);
 
+	v4l2_ctrl_new_std_menu_items(&sensor->pixel_array->ctrl_handler,
+				     &smiapp_ctrl_ops, V4L2_CID_TEST_PATTERN,
+				     ARRAY_SIZE(smiapp_test_patterns) - 1,
+				     0, 0, smiapp_test_patterns);
+
+	for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++)
+		sensor->test_data[i] =
+			v4l2_ctrl_new_custom(&sensor->pixel_array->ctrl_handler,
+					     &smiapp_test_pattern_colours[i],
+					     NULL);
+
 	if (sensor->pixel_array->ctrl_handler.error) {
 		dev_err(&client->dev,
 			"pixel array controls initialization failed (%d)\n",
@@ -543,6 +630,14 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor)
 		goto error;
 	}
 
+	for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++) {
+		struct v4l2_ctrl *ctrl = sensor->test_data[i];
+
+		ctrl->maximum =
+			ctrl->default_value =
+			ctrl->cur.val = (1 << sensor->csi_format->width) - 1;
+	}
+
 	sensor->pixel_array->sd.ctrl_handler =
 		&sensor->pixel_array->ctrl_handler;
 
@@ -1670,17 +1765,34 @@ static int smiapp_set_format(struct v4l2_subdev *subdev,
 	if (fmt->pad == ssd->source_pad) {
 		u32 code = fmt->format.code;
 		int rval = __smiapp_get_format(subdev, fh, fmt);
+		bool range_changed = false;
+		unsigned int i;
 
 		if (!rval && subdev == &sensor->src->sd) {
 			const struct smiapp_csi_data_format *csi_format =
 				smiapp_validate_csi_data_format(sensor, code);
-			if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+
+			if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+				if (csi_format->width !=
+				    sensor->csi_format->width)
+					range_changed = true;
+
 				sensor->csi_format = csi_format;
+			}
+
 			fmt->format.code = csi_format->code;
 		}
 
 		mutex_unlock(&sensor->mutex);
-		return rval;
+		if (rval || !range_changed)
+			return rval;
+
+		for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++)
+			v4l2_ctrl_modify_range(
+				sensor->test_data[i],
+				0, (1 << sensor->csi_format->width) - 1, 1, 0);
+
+		return 0;
 	}
 
 	/* Sink pad. Width and height are changeable here. */
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index 7cc5aae..874b49f 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -54,6 +54,8 @@
 	(1000 +	(SMIAPP_RESET_DELAY_CLOCKS * 1000	\
 		 + (clk) / 1000 - 1) / ((clk) / 1000))
 
+#define SMIAPP_COLOUR_COMPONENTS	4
+
 #include "smiapp-limits.h"
 
 struct smiapp_quirk;
@@ -241,6 +243,8 @@ struct smiapp_sensor {
 	/* src controls */
 	struct v4l2_ctrl *link_freq;
 	struct v4l2_ctrl *pixel_rate_csi;
+	/* test pattern colour components */
+	struct v4l2_ctrl *test_data[SMIAPP_COLOUR_COMPONENTS];
 };
 
 #define to_smiapp_subdev(_sd)				\
-- 
1.8.3.2

[PATCH 1/1] smiapp: Add driver-specific control class, test pattern controls

[Sakari Ailus]

Add smiapp driver specific control sub-class for test pattern controls. More
controls are expected since a fair amount of the standard functionality is
still unsupported. There are sensor model specific functionality as well and
expectedly thus also sensor specific controls. So reserve 128 controls for
this driver.

This patch also adds test pattern controls for the four colour components.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
This patch comes before the previous patch I sent to the thread. I missed
this when sending it.

 include/uapi/linux/smiapp.h        | 34 ++++++++++++++++++++++++++++++++++
 include/uapi/linux/v4l2-controls.h |  4 ++++
 2 files changed, 38 insertions(+)
 create mode 100644 include/uapi/linux/smiapp.h

diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h
new file mode 100644
index 0000000..116fc69
--- /dev/null
+++ b/include/uapi/linux/smiapp.h
@@ -0,0 +1,34 @@
+/*
+ * include/media/smiapp.h
+ *
+ * Generic driver for SMIA/SMIA++ compliant camera modules
+ *
+ * Copyright (C) 2014 Intel Corporation
+ * Contact: Sakari Ailus <sakari.ailus@iki.fi>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ */
+
+#ifndef __UAPI_LINUX_SMIAPP_H_
+#define __UAPI_LINUX_SMIAPP_H_
+
+#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED			0
+#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR		1
+#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS		2
+#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY		3
+#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9			4
+
+#define V4L2_CID_SMIAPP_TEST_PATTERN_RED	(V4L2_CID_USER_SMIAPP_BASE | 0x01)
+#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENR	(V4L2_CID_USER_SMIAPP_BASE | 0x02)
+#define V4L2_CID_SMIAPP_TEST_PATTERN_BLUE	(V4L2_CID_USER_SMIAPP_BASE | 0x03)
+#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENB	(V4L2_CID_USER_SMIAPP_BASE | 0x04)
+
+#endif /* __UAPI_LINUX_SMIAPP_H_ */
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 2ac5597..8b5502f 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -169,6 +169,10 @@ enum v4l2_colorfx {
  * We reserve 16 controls for this driver. */
 #define V4L2_CID_USER_SAA7134_BASE		(V4L2_CID_USER_BASE + 0x1060)
 
+/* The base for the smiapp driver controls. See include/media/smiapp.h
+ * for the list of controls. 128 controls are reserved for this driver. */
+#define V4L2_CID_USER_SMIAPP_BASE		(V4L2_CID_USER_BASE + 0x1070)
+
 /* MPEG-class control IDs */
 /* The MPEG controls are applicable to all codec controls
  * and the 'MPEG' part of the define is historical */
-- 
1.8.3.2

Re: [PATCH 1/1] smiapp: Implement the test pattern control

[Hans Verkuil]

On 05/27/14 14:43, Sakari Ailus wrote:
> Add support for the V4L2_CID_TEST_PATTERN control. When the solid colour
> mode is selected, additional controls become available for setting the
> solid four solid colour components.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/smiapp/smiapp-core.c | 120 +++++++++++++++++++++++++++++++--
>  drivers/media/i2c/smiapp/smiapp.h      |   4 ++
>  2 files changed, 120 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> index 446c82c..025342c 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -32,6 +32,7 @@
>  #include <linux/gpio.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/smiapp.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/v4l2-mediabus.h>
>  #include <media/v4l2-device.h>
> @@ -404,6 +405,52 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor)
>  		pixel_order_str[pixel_order]);
>  }
>  
> +static const char * const smiapp_test_patterns[] = {
> +	"Disabled",
> +	"Solid colour",
> +	"Eight vertical colour bars",
> +	"Colour bars with fade to grey",
> +	"Pseudorandom sequence (PN9)",
> +};

Capitalize the strings (same rules as are used for book titles in english).

> +
> +static const struct v4l2_ctrl_ops smiapp_ctrl_ops;
> +
> +static struct v4l2_ctrl_config
> +smiapp_test_pattern_colours[SMIAPP_COLOUR_COMPONENTS] = {
> +	{
> +		&smiapp_ctrl_ops,
> +		V4L2_CID_SMIAPP_TEST_PATTERN_RED,
> +		"Solid red pixel value",
> +		V4L2_CTRL_TYPE_INTEGER,
> +		0, 0, 1, 0,
> +		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
> +	},
> +	{
> +		&smiapp_ctrl_ops,
> +		V4L2_CID_SMIAPP_TEST_PATTERN_GREENR,
> +		"Solid green (red) pixel value",
> +		V4L2_CTRL_TYPE_INTEGER,
> +		0, 0, 1, 0,
> +		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
> +	},
> +	{
> +		&smiapp_ctrl_ops,
> +		V4L2_CID_SMIAPP_TEST_PATTERN_BLUE,
> +		"Solid blue pixel value",
> +		V4L2_CTRL_TYPE_INTEGER,
> +		0, 0, 1, 0,
> +		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
> +	},
> +	{
> +		&smiapp_ctrl_ops,
> +		V4L2_CID_SMIAPP_TEST_PATTERN_GREENB,
> +		"Solid green (blue) pixel value",
> +		V4L2_CTRL_TYPE_INTEGER,
> +		0, 0, 1, 0,
> +		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
> +	},
> +};

Ditto for the control names.

After that you can add my:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> +
>  static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct smiapp_sensor *sensor =
> @@ -477,6 +524,35 @@ static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
>  
>  		return smiapp_pll_update(sensor);
>  
> +	case V4L2_CID_TEST_PATTERN: {
> +		unsigned int i;
> +
> +		for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++)
> +			v4l2_ctrl_activate(
> +				sensor->test_data[i],
> +				ctrl->val ==
> +				V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR);
> +
> +		return smiapp_write(
> +			sensor, SMIAPP_REG_U16_TEST_PATTERN_MODE, ctrl->val);
> +	}
> +
> +	case V4L2_CID_SMIAPP_TEST_PATTERN_RED:
> +		return smiapp_write(
> +			sensor, SMIAPP_REG_U16_TEST_DATA_RED, ctrl->val);
> +
> +	case V4L2_CID_SMIAPP_TEST_PATTERN_GREENR:
> +		return smiapp_write(
> +			sensor, SMIAPP_REG_U16_TEST_DATA_GREENR, ctrl->val);
> +
> +	case V4L2_CID_SMIAPP_TEST_PATTERN_BLUE:
> +		return smiapp_write(
> +			sensor, SMIAPP_REG_U16_TEST_DATA_BLUE, ctrl->val);
> +
> +	case V4L2_CID_SMIAPP_TEST_PATTERN_GREENB:
> +		return smiapp_write(
> +			sensor, SMIAPP_REG_U16_TEST_DATA_GREENB, ctrl->val);
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -489,10 +565,10 @@ static const struct v4l2_ctrl_ops smiapp_ctrl_ops = {
>  static int smiapp_init_controls(struct smiapp_sensor *sensor)
>  {
>  	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
> -	unsigned int max;
> +	unsigned int max, i;
>  	int rval;
>  
> -	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 7);
> +	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 12);
>  	if (rval)
>  		return rval;
>  	sensor->pixel_array->ctrl_handler.lock = &sensor->mutex;
> @@ -535,6 +611,17 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor)
>  		&sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops,
>  		V4L2_CID_PIXEL_RATE, 0, 0, 1, 0);
>  
> +	v4l2_ctrl_new_std_menu_items(&sensor->pixel_array->ctrl_handler,
> +				     &smiapp_ctrl_ops, V4L2_CID_TEST_PATTERN,
> +				     ARRAY_SIZE(smiapp_test_patterns) - 1,
> +				     0, 0, smiapp_test_patterns);
> +
> +	for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++)
> +		sensor->test_data[i] =
> +			v4l2_ctrl_new_custom(&sensor->pixel_array->ctrl_handler,
> +					     &smiapp_test_pattern_colours[i],
> +					     NULL);
> +
>  	if (sensor->pixel_array->ctrl_handler.error) {
>  		dev_err(&client->dev,
>  			"pixel array controls initialization failed (%d)\n",
> @@ -543,6 +630,14 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor)
>  		goto error;
>  	}
>  
> +	for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++) {
> +		struct v4l2_ctrl *ctrl = sensor->test_data[i];
> +
> +		ctrl->maximum =
> +			ctrl->default_value =
> +			ctrl->cur.val = (1 << sensor->csi_format->width) - 1;
> +	}
> +
>  	sensor->pixel_array->sd.ctrl_handler =
>  		&sensor->pixel_array->ctrl_handler;
>  
> @@ -1670,17 +1765,34 @@ static int smiapp_set_format(struct v4l2_subdev *subdev,
>  	if (fmt->pad == ssd->source_pad) {
>  		u32 code = fmt->format.code;
>  		int rval = __smiapp_get_format(subdev, fh, fmt);
> +		bool range_changed = false;
> +		unsigned int i;
>  
>  		if (!rval && subdev == &sensor->src->sd) {
>  			const struct smiapp_csi_data_format *csi_format =
>  				smiapp_validate_csi_data_format(sensor, code);
> -			if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> +
> +			if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> +				if (csi_format->width !=
> +				    sensor->csi_format->width)
> +					range_changed = true;
> +
>  				sensor->csi_format = csi_format;
> +			}
> +
>  			fmt->format.code = csi_format->code;
>  		}
>  
>  		mutex_unlock(&sensor->mutex);
> -		return rval;
> +		if (rval || !range_changed)
> +			return rval;
> +
> +		for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++)
> +			v4l2_ctrl_modify_range(
> +				sensor->test_data[i],
> +				0, (1 << sensor->csi_format->width) - 1, 1, 0);
> +
> +		return 0;
>  	}
>  
>  	/* Sink pad. Width and height are changeable here. */
> diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
> index 7cc5aae..874b49f 100644
> --- a/drivers/media/i2c/smiapp/smiapp.h
> +++ b/drivers/media/i2c/smiapp/smiapp.h
> @@ -54,6 +54,8 @@
>  	(1000 +	(SMIAPP_RESET_DELAY_CLOCKS * 1000	\
>  		 + (clk) / 1000 - 1) / ((clk) / 1000))
>  
> +#define SMIAPP_COLOUR_COMPONENTS	4
> +
>  #include "smiapp-limits.h"
>  
>  struct smiapp_quirk;
> @@ -241,6 +243,8 @@ struct smiapp_sensor {
>  	/* src controls */
>  	struct v4l2_ctrl *link_freq;
>  	struct v4l2_ctrl *pixel_rate_csi;
> +	/* test pattern colour components */
> +	struct v4l2_ctrl *test_data[SMIAPP_COLOUR_COMPONENTS];
>  };
>  
>  #define to_smiapp_subdev(_sd)				\
> 

Re: [PATCH 1/1] smiapp: Add driver-specific control class, test pattern controls

[Hans Verkuil]

On 05/28/14 11:00, Sakari Ailus wrote:
> Add smiapp driver specific control sub-class for test pattern controls. More
> controls are expected since a fair amount of the standard functionality is
> still unsupported. There are sensor model specific functionality as well and
> expectedly thus also sensor specific controls. So reserve 128 controls for
> this driver.
> 
> This patch also adds test pattern controls for the four colour components.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
> This patch comes before the previous patch I sent to the thread. I missed
> this when sending it.
> 
>  include/uapi/linux/smiapp.h        | 34 ++++++++++++++++++++++++++++++++++
>  include/uapi/linux/v4l2-controls.h |  4 ++++
>  2 files changed, 38 insertions(+)
>  create mode 100644 include/uapi/linux/smiapp.h
> 
> diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h
> new file mode 100644
> index 0000000..116fc69
> --- /dev/null
> +++ b/include/uapi/linux/smiapp.h
> @@ -0,0 +1,34 @@
> +/*
> + * include/media/smiapp.h
> + *
> + * Generic driver for SMIA/SMIA++ compliant camera modules
> + *
> + * Copyright (C) 2014 Intel Corporation
> + * Contact: Sakari Ailus <sakari.ailus@iki.fi>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + */
> +
> +#ifndef __UAPI_LINUX_SMIAPP_H_
> +#define __UAPI_LINUX_SMIAPP_H_
> +
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED			0
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR		1
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS		2
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY		3
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9			4
> +
> +#define V4L2_CID_SMIAPP_TEST_PATTERN_RED	(V4L2_CID_USER_SMIAPP_BASE | 0x01)
> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENR	(V4L2_CID_USER_SMIAPP_BASE | 0x02)
> +#define V4L2_CID_SMIAPP_TEST_PATTERN_BLUE	(V4L2_CID_USER_SMIAPP_BASE | 0x03)
> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENB	(V4L2_CID_USER_SMIAPP_BASE | 0x04)
> +
> +#endif /* __UAPI_LINUX_SMIAPP_H_ */
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 2ac5597..8b5502f 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -169,6 +169,10 @@ enum v4l2_colorfx {
>   * We reserve 16 controls for this driver. */
>  #define V4L2_CID_USER_SAA7134_BASE		(V4L2_CID_USER_BASE + 0x1060)
>  
> +/* The base for the smiapp driver controls. See include/media/smiapp.h
> + * for the list of controls. 128 controls are reserved for this driver. */
> +#define V4L2_CID_USER_SMIAPP_BASE		(V4L2_CID_USER_BASE + 0x1070)
> +
>  /* MPEG-class control IDs */
>  /* The MPEG controls are applicable to all codec controls
>   * and the 'MPEG' part of the define is historical */
> 

[PATCH v2 2/2] smiapp: Implement the test pattern control

[Sakari Ailus]

Add support for the V4L2_CID_TEST_PATTERN control. When the solid colour
mode is selected, additional controls become available for setting the
solid four solid colour components.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
since v1:
- Capitalised the first letters in menu item and custom control words.

 drivers/media/i2c/smiapp/smiapp-core.c | 120 +++++++++++++++++++++++++++++++--
 drivers/media/i2c/smiapp/smiapp.h      |   4 ++
 2 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 446c82c..eeaa91e 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -32,6 +32,7 @@
 #include <linux/gpio.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/smiapp.h>
 #include <linux/regulator/consumer.h>
 #include <linux/v4l2-mediabus.h>
 #include <media/v4l2-device.h>
@@ -404,6 +405,52 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor)
 		pixel_order_str[pixel_order]);
 }
 
+static const char * const smiapp_test_patterns[] = {
+	"Disabled",
+	"Solid Colour",
+	"Eight Vertical Colour Bars",
+	"Colour Bars With Fade to Grey",
+	"Pseudorandom Sequence (PN9)",
+};
+
+static const struct v4l2_ctrl_ops smiapp_ctrl_ops;
+
+static struct v4l2_ctrl_config
+smiapp_test_pattern_colours[SMIAPP_COLOUR_COMPONENTS] = {
+	{
+		&smiapp_ctrl_ops,
+		V4L2_CID_SMIAPP_TEST_PATTERN_RED,
+		"Solid Red Pixel Value",
+		V4L2_CTRL_TYPE_INTEGER,
+		0, 0, 1, 0,
+		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
+	},
+	{
+		&smiapp_ctrl_ops,
+		V4L2_CID_SMIAPP_TEST_PATTERN_GREENR,
+		"Solid Green (Red) Pixel Value",
+		V4L2_CTRL_TYPE_INTEGER,
+		0, 0, 1, 0,
+		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
+	},
+	{
+		&smiapp_ctrl_ops,
+		V4L2_CID_SMIAPP_TEST_PATTERN_BLUE,
+		"Solid Blue Pixel Value",
+		V4L2_CTRL_TYPE_INTEGER,
+		0, 0, 1, 0,
+		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
+	},
+	{
+		&smiapp_ctrl_ops,
+		V4L2_CID_SMIAPP_TEST_PATTERN_GREENB,
+		"Solid Green (Blue) Pixel Value",
+		V4L2_CTRL_TYPE_INTEGER,
+		0, 0, 1, 0,
+		V4L2_CTRL_FLAG_INACTIVE, 0, NULL, NULL, 0
+	},
+};
+
 static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct smiapp_sensor *sensor =
@@ -477,6 +524,35 @@ static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl)
 
 		return smiapp_pll_update(sensor);
 
+	case V4L2_CID_TEST_PATTERN: {
+		unsigned int i;
+
+		for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++)
+			v4l2_ctrl_activate(
+				sensor->test_data[i],
+				ctrl->val ==
+				V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR);
+
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_PATTERN_MODE, ctrl->val);
+	}
+
+	case V4L2_CID_SMIAPP_TEST_PATTERN_RED:
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_DATA_RED, ctrl->val);
+
+	case V4L2_CID_SMIAPP_TEST_PATTERN_GREENR:
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_DATA_GREENR, ctrl->val);
+
+	case V4L2_CID_SMIAPP_TEST_PATTERN_BLUE:
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_DATA_BLUE, ctrl->val);
+
+	case V4L2_CID_SMIAPP_TEST_PATTERN_GREENB:
+		return smiapp_write(
+			sensor, SMIAPP_REG_U16_TEST_DATA_GREENB, ctrl->val);
+
 	default:
 		return -EINVAL;
 	}
@@ -489,10 +565,10 @@ static const struct v4l2_ctrl_ops smiapp_ctrl_ops = {
 static int smiapp_init_controls(struct smiapp_sensor *sensor)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
-	unsigned int max;
+	unsigned int max, i;
 	int rval;
 
-	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 7);
+	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 12);
 	if (rval)
 		return rval;
 	sensor->pixel_array->ctrl_handler.lock = &sensor->mutex;
@@ -535,6 +611,17 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor)
 		&sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops,
 		V4L2_CID_PIXEL_RATE, 0, 0, 1, 0);
 
+	v4l2_ctrl_new_std_menu_items(&sensor->pixel_array->ctrl_handler,
+				     &smiapp_ctrl_ops, V4L2_CID_TEST_PATTERN,
+				     ARRAY_SIZE(smiapp_test_patterns) - 1,
+				     0, 0, smiapp_test_patterns);
+
+	for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++)
+		sensor->test_data[i] =
+			v4l2_ctrl_new_custom(&sensor->pixel_array->ctrl_handler,
+					     &smiapp_test_pattern_colours[i],
+					     NULL);
+
 	if (sensor->pixel_array->ctrl_handler.error) {
 		dev_err(&client->dev,
 			"pixel array controls initialization failed (%d)\n",
@@ -543,6 +630,14 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor)
 		goto error;
 	}
 
+	for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++) {
+		struct v4l2_ctrl *ctrl = sensor->test_data[i];
+
+		ctrl->maximum =
+			ctrl->default_value =
+			ctrl->cur.val = (1 << sensor->csi_format->width) - 1;
+	}
+
 	sensor->pixel_array->sd.ctrl_handler =
 		&sensor->pixel_array->ctrl_handler;
 
@@ -1670,17 +1765,34 @@ static int smiapp_set_format(struct v4l2_subdev *subdev,
 	if (fmt->pad == ssd->source_pad) {
 		u32 code = fmt->format.code;
 		int rval = __smiapp_get_format(subdev, fh, fmt);
+		bool range_changed = false;
+		unsigned int i;
 
 		if (!rval && subdev == &sensor->src->sd) {
 			const struct smiapp_csi_data_format *csi_format =
 				smiapp_validate_csi_data_format(sensor, code);
-			if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+
+			if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
+				if (csi_format->width !=
+				    sensor->csi_format->width)
+					range_changed = true;
+
 				sensor->csi_format = csi_format;
+			}
+
 			fmt->format.code = csi_format->code;
 		}
 
 		mutex_unlock(&sensor->mutex);
-		return rval;
+		if (rval || !range_changed)
+			return rval;
+
+		for (i = 0; i < ARRAY_SIZE(smiapp_test_pattern_colours); i++)
+			v4l2_ctrl_modify_range(
+				sensor->test_data[i],
+				0, (1 << sensor->csi_format->width) - 1, 1, 0);
+
+		return 0;
 	}
 
 	/* Sink pad. Width and height are changeable here. */
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index 7cc5aae..874b49f 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -54,6 +54,8 @@
 	(1000 +	(SMIAPP_RESET_DELAY_CLOCKS * 1000	\
 		 + (clk) / 1000 - 1) / ((clk) / 1000))
 
+#define SMIAPP_COLOUR_COMPONENTS	4
+
 #include "smiapp-limits.h"
 
 struct smiapp_quirk;
@@ -241,6 +243,8 @@ struct smiapp_sensor {
 	/* src controls */
 	struct v4l2_ctrl *link_freq;
 	struct v4l2_ctrl *pixel_rate_csi;
+	/* test pattern colour components */
+	struct v4l2_ctrl *test_data[SMIAPP_COLOUR_COMPONENTS];
 };
 
 #define to_smiapp_subdev(_sd)				\
-- 
1.8.3.2

Re: [PATCH 1/1] smiapp: Add driver-specific control class, test pattern controls

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Wednesday 28 May 2014 12:00:38 Sakari Ailus wrote:
> Add smiapp driver specific control sub-class for test pattern controls. More
> controls are expected since a fair amount of the standard functionality is
> still unsupported. There are sensor model specific functionality as well
> and expectedly thus also sensor specific controls. So reserve 128 controls
> for this driver.
> 
> This patch also adds test pattern controls for the four colour components.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> This patch comes before the previous patch I sent to the thread. I missed
> this when sending it.
> 
>  include/uapi/linux/smiapp.h        | 34 ++++++++++++++++++++++++++++++++++
>  include/uapi/linux/v4l2-controls.h |  4 ++++
>  2 files changed, 38 insertions(+)
>  create mode 100644 include/uapi/linux/smiapp.h
> 
> diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h
> new file mode 100644
> index 0000000..116fc69
> --- /dev/null
> +++ b/include/uapi/linux/smiapp.h
> @@ -0,0 +1,34 @@
> +/*
> + * include/media/smiapp.h
> + *
> + * Generic driver for SMIA/SMIA++ compliant camera modules
> + *
> + * Copyright (C) 2014 Intel Corporation
> + * Contact: Sakari Ailus <sakari.ailus@iki.fi>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + */
> +
> +#ifndef __UAPI_LINUX_SMIAPP_H_
> +#define __UAPI_LINUX_SMIAPP_H_
> +
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED			0
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR		1
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS		2
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY		3
> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9			4
> +
> +#define V4L2_CID_SMIAPP_TEST_PATTERN_RED	(V4L2_CID_USER_SMIAPP_BASE |
> 0x01)
> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENR	(V4L2_CID_USER_SMIAPP_BASE |
> 0x02)
> +#define V4L2_CID_SMIAPP_TEST_PATTERN_BLUE	(V4L2_CID_USER_SMIAPP_BASE |
> 0x03)
> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENB	(V4L2_CID_USER_SMIAPP_BASE |
> 0x04)

Wouldn't it make sense to create a standard test pattern color control instead 
? Several sensors can control the test pattern color in a way or another. Some 
of them might need more than one color though, so I'm not sure how much 
standardization would be possible.

> +
> +#endif /* __UAPI_LINUX_SMIAPP_H_ */
> diff --git a/include/uapi/linux/v4l2-controls.h
> b/include/uapi/linux/v4l2-controls.h index 2ac5597..8b5502f 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -169,6 +169,10 @@ enum v4l2_colorfx {
>   * We reserve 16 controls for this driver. */
>  #define V4L2_CID_USER_SAA7134_BASE		(V4L2_CID_USER_BASE + 0x1060)
> 
> +/* The base for the smiapp driver controls. See include/media/smiapp.h
> + * for the list of controls. 128 controls are reserved for this driver. */
> +#define V4L2_CID_USER_SMIAPP_BASE		(V4L2_CID_USER_BASE + 0x1070)
> +
>  /* MPEG-class control IDs */
>  /* The MPEG controls are applicable to all codec controls
>   * and the 'MPEG' part of the define is historical */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/1] smiapp: Add driver-specific control class, test pattern controls

[Sakari Ailus]

Hi Laurent,

Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
>
> On Wednesday 28 May 2014 12:00:38 Sakari Ailus wrote:
>> Add smiapp driver specific control sub-class for test pattern controls. More
>> controls are expected since a fair amount of the standard functionality is
>> still unsupported. There are sensor model specific functionality as well
>> and expectedly thus also sensor specific controls. So reserve 128 controls
>> for this driver.
>>
>> This patch also adds test pattern controls for the four colour components.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>> This patch comes before the previous patch I sent to the thread. I missed
>> this when sending it.
>>
>>   include/uapi/linux/smiapp.h        | 34 ++++++++++++++++++++++++++++++++++
>>   include/uapi/linux/v4l2-controls.h |  4 ++++
>>   2 files changed, 38 insertions(+)
>>   create mode 100644 include/uapi/linux/smiapp.h
>>
>> diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h
>> new file mode 100644
>> index 0000000..116fc69
>> --- /dev/null
>> +++ b/include/uapi/linux/smiapp.h
>> @@ -0,0 +1,34 @@
>> +/*
>> + * include/media/smiapp.h
>> + *
>> + * Generic driver for SMIA/SMIA++ compliant camera modules
>> + *
>> + * Copyright (C) 2014 Intel Corporation
>> + * Contact: Sakari Ailus <sakari.ailus@iki.fi>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + *
>> + */
>> +
>> +#ifndef __UAPI_LINUX_SMIAPP_H_
>> +#define __UAPI_LINUX_SMIAPP_H_
>> +
>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED			0
>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR		1
>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS		2
>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY		3
>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9			4
>> +
>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_RED	(V4L2_CID_USER_SMIAPP_BASE |
>> 0x01)
>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENR	(V4L2_CID_USER_SMIAPP_BASE |
>> 0x02)
>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_BLUE	(V4L2_CID_USER_SMIAPP_BASE |
>> 0x03)
>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENB	(V4L2_CID_USER_SMIAPP_BASE |
>> 0x04)
>
> Wouldn't it make sense to create a standard test pattern color control instead
> ? Several sensors can control the test pattern color in a way or another. Some
> of them might need more than one color though, so I'm not sure how much
> standardization would be possible.

Now that you mention it, I'd guess many raw bayer sensors can set 
colours for the test pattern (or image). The menu control has no 
standardised values so I didn't think of standardising controls that 
depend on it.

I'll update the patches (and add a new one for the standard controls).

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH 1/1] smiapp: Add driver-specific control class, test pattern controls

[Laurent Pinchart]

Hi Sakari,

On Wednesday 28 May 2014 15:16:58 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > On Wednesday 28 May 2014 12:00:38 Sakari Ailus wrote:
> >> Add smiapp driver specific control sub-class for test pattern controls.
> >> More controls are expected since a fair amount of the standard
> >> functionality is still unsupported. There are sensor model specific
> >> functionality as well and expectedly thus also sensor specific controls.
> >> So reserve 128 controls for this driver.
> >> 
> >> This patch also adds test pattern controls for the four colour
> >> components.
> >> 
> >> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >> ---
> >> This patch comes before the previous patch I sent to the thread. I missed
> >> this when sending it.
> >> 
> >>   include/uapi/linux/smiapp.h        | 34 +++++++++++++++++++++++++++++++
> >>   include/uapi/linux/v4l2-controls.h |  4 ++++
> >>   2 files changed, 38 insertions(+)
> >>   create mode 100644 include/uapi/linux/smiapp.h
> >> 
> >> diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h
> >> new file mode 100644
> >> index 0000000..116fc69
> >> --- /dev/null
> >> +++ b/include/uapi/linux/smiapp.h
> >> @@ -0,0 +1,34 @@
> >> +/*
> >> + * include/media/smiapp.h
> >> + *
> >> + * Generic driver for SMIA/SMIA++ compliant camera modules
> >> + *
> >> + * Copyright (C) 2014 Intel Corporation
> >> + * Contact: Sakari Ailus <sakari.ailus@iki.fi>
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License
> >> + * version 2 as published by the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful, but
> >> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> + * General Public License for more details.
> >> + *
> >> + */
> >> +
> >> +#ifndef __UAPI_LINUX_SMIAPP_H_
> >> +#define __UAPI_LINUX_SMIAPP_H_
> >> +
> >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED			0
> >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR		1
> >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS		2
> >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY		3
> >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9			4
> >> +
> >> +#define V4L2_CID_SMIAPP_TEST_PATTERN_RED	(V4L2_CID_USER_SMIAPP_BASE |
> >> 0x01)
> >> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENR	(V4L2_CID_USER_SMIAPP_BASE |
> >> 0x02)
> >> +#define V4L2_CID_SMIAPP_TEST_PATTERN_BLUE	(V4L2_CID_USER_SMIAPP_BASE |
> >> 0x03)
> >> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENB	(V4L2_CID_USER_SMIAPP_BASE |
> >> 0x04)
> > 
> > Wouldn't it make sense to create a standard test pattern color control
> > instead ? Several sensors can control the test pattern color in a way or
> > another. Some of them might need more than one color though, so I'm not
> > sure how much standardization would be possible.
> 
> Now that you mention it, I'd guess many raw bayer sensors can set
> colours for the test pattern (or image). The menu control has no
> standardised values so I didn't think of standardising controls that
> depend on it.
> 
> I'll update the patches (and add a new one for the standard controls).

The color format might differ between devices though, some might not be able 
to differentiate between Gr and Gb for the test pattern. A standard test 
pattern color control should thus be flexible in the color format I suppose.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/1] smiapp: Add driver-specific control class, test pattern controls

[Sakari Ailus]

Laurent Pinchart wrote:
> Hi Sakari,
>
> On Wednesday 28 May 2014 15:16:58 Sakari Ailus wrote:
>> Laurent Pinchart wrote:
>>> On Wednesday 28 May 2014 12:00:38 Sakari Ailus wrote:
>>>> Add smiapp driver specific control sub-class for test pattern controls.
>>>> More controls are expected since a fair amount of the standard
>>>> functionality is still unsupported. There are sensor model specific
>>>> functionality as well and expectedly thus also sensor specific controls.
>>>> So reserve 128 controls for this driver.
>>>>
>>>> This patch also adds test pattern controls for the four colour
>>>> components.
>>>>
>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>> ---
>>>> This patch comes before the previous patch I sent to the thread. I missed
>>>> this when sending it.
>>>>
>>>>    include/uapi/linux/smiapp.h        | 34 +++++++++++++++++++++++++++++++
>>>>    include/uapi/linux/v4l2-controls.h |  4 ++++
>>>>    2 files changed, 38 insertions(+)
>>>>    create mode 100644 include/uapi/linux/smiapp.h
>>>>
>>>> diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h
>>>> new file mode 100644
>>>> index 0000000..116fc69
>>>> --- /dev/null
>>>> +++ b/include/uapi/linux/smiapp.h
>>>> @@ -0,0 +1,34 @@
>>>> +/*
>>>> + * include/media/smiapp.h
>>>> + *
>>>> + * Generic driver for SMIA/SMIA++ compliant camera modules
>>>> + *
>>>> + * Copyright (C) 2014 Intel Corporation
>>>> + * Contact: Sakari Ailus <sakari.ailus@iki.fi>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU General Public License
>>>> + * version 2 as published by the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful, but
>>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> + * General Public License for more details.
>>>> + *
>>>> + */
>>>> +
>>>> +#ifndef __UAPI_LINUX_SMIAPP_H_
>>>> +#define __UAPI_LINUX_SMIAPP_H_
>>>> +
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED			0
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR		1
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS		2
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY		3
>>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9			4
>>>> +
>>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_RED	(V4L2_CID_USER_SMIAPP_BASE |
>>>> 0x01)
>>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENR	(V4L2_CID_USER_SMIAPP_BASE |
>>>> 0x02)
>>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_BLUE	(V4L2_CID_USER_SMIAPP_BASE |
>>>> 0x03)
>>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENB	(V4L2_CID_USER_SMIAPP_BASE |
>>>> 0x04)
>>>
>>> Wouldn't it make sense to create a standard test pattern color control
>>> instead ? Several sensors can control the test pattern color in a way or
>>> another. Some of them might need more than one color though, so I'm not
>>> sure how much standardization would be possible.
>>
>> Now that you mention it, I'd guess many raw bayer sensors can set
>> colours for the test pattern (or image). The menu control has no
>> standardised values so I didn't think of standardising controls that
>> depend on it.
>>
>> I'll update the patches (and add a new one for the standard controls).
>
> The color format might differ between devices though, some might not be able
> to differentiate between Gr and Gb for the test pattern. A standard test
> pattern color control should thus be flexible in the color format I suppose.

I think we should simply create a separate control for each component. 
There aren't that many components after all.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH 1/1] smiapp: Add driver-specific control class, test pattern controls

[Laurent Pinchart]

On Wednesday 28 May 2014 15:25:23 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > On Wednesday 28 May 2014 15:16:58 Sakari Ailus wrote:
> >> Laurent Pinchart wrote:
> >>> On Wednesday 28 May 2014 12:00:38 Sakari Ailus wrote:
> >>>> Add smiapp driver specific control sub-class for test pattern controls.
> >>>> More controls are expected since a fair amount of the standard
> >>>> functionality is still unsupported. There are sensor model specific
> >>>> functionality as well and expectedly thus also sensor specific
> >>>> controls. So reserve 128 controls for this driver.
> >>>> 
> >>>> This patch also adds test pattern controls for the four colour
> >>>> components.
> >>>> 
> >>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>>> ---
> >>>> This patch comes before the previous patch I sent to the thread. I
> >>>> missed this when sending it.
> >>>> 
> >>>>   include/uapi/linux/smiapp.h        | 34 +++++++++++++++++++++++++++++
> >>>>   include/uapi/linux/v4l2-controls.h |  4 ++++
> >>>>   2 files changed, 38 insertions(+)
> >>>>   create mode 100644 include/uapi/linux/smiapp.h
> >>>> 
> >>>> diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h
> >>>> new file mode 100644
> >>>> index 0000000..116fc69
> >>>> --- /dev/null
> >>>> +++ b/include/uapi/linux/smiapp.h
> >>>> @@ -0,0 +1,34 @@
> >>>> +/*
> >>>> + * include/media/smiapp.h
> >>>> + *
> >>>> + * Generic driver for SMIA/SMIA++ compliant camera modules
> >>>> + *
> >>>> + * Copyright (C) 2014 Intel Corporation
> >>>> + * Contact: Sakari Ailus <sakari.ailus@iki.fi>
> >>>> + *
> >>>> + * This program is free software; you can redistribute it and/or
> >>>> + * modify it under the terms of the GNU General Public License
> >>>> + * version 2 as published by the Free Software Foundation.
> >>>> + *
> >>>> + * This program is distributed in the hope that it will be useful, but
> >>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >>>> + * General Public License for more details.
> >>>> + *
> >>>> + */
> >>>> +
> >>>> +#ifndef __UAPI_LINUX_SMIAPP_H_
> >>>> +#define __UAPI_LINUX_SMIAPP_H_
> >>>> +
> >>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED			0
> >>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR		1
> >>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS		2
> >>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY		3
> >>>> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9			4
> >>>> +
> >>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_RED
> >>>> (V4L2_CID_USER_SMIAPP_BASE | 0x01)
> >>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENR	
> >>>> (V4L2_CID_USER_SMIAPP_BASE | 0x02)
> >>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_BLUE
> >>>> (V4L2_CID_USER_SMIAPP_BASE | 0x03)
> >>>> +#define V4L2_CID_SMIAPP_TEST_PATTERN_GREENB
> >>>> (V4L2_CID_USER_SMIAPP_BASE | 0x04)
> >>> 
> >>> Wouldn't it make sense to create a standard test pattern color control
> >>> instead ? Several sensors can control the test pattern color in a way or
> >>> another. Some of them might need more than one color though, so I'm not
> >>> sure how much standardization would be possible.
> >> 
> >> Now that you mention it, I'd guess many raw bayer sensors can set
> >> colours for the test pattern (or image). The menu control has no
> >> standardised values so I didn't think of standardising controls that
> >> depend on it.
> >> 
> >> I'll update the patches (and add a new one for the standard controls).
> > 
> > The color format might differ between devices though, some might not be
> > able to differentiate between Gr and Gb for the test pattern. A standard
> > test pattern color control should thus be flexible in the color format I
> > suppose.
>
> I think we should simply create a separate control for each component.
> There aren't that many components after all.

Given that components can be more than 8 bits wide that's probably a good 
idea.

-- 
Regards,

Laurent Pinchart