Re: [PATCH v2 1/4] ov7670: use the control framework
Hi Hans,
On 28 September 2012 13:05, Hans Verkuil wrote:
> On Fri September 28 2012 12:50:55 Javier Martin wrote:
>> Signed-off-by: Hans Verkuil
>> Signed-off-by: Javier Martin
>> ---
>> Changes since v1:
>> - Use v4l2_ctrl_auto_cluster() for auto_gain and auto_exp.
>>
>> ---
>> drivers/media/i2c/ov7670.c | 310
>>
>> 1 file changed, 112 insertions(+), 198 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
>> index 9d7a92e..babd55c 100644
>> --- a/drivers/media/i2c/ov7670.c
>> +++ b/drivers/media/i2c/ov7670.c
>> @@ -18,6 +18,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -222,9 +223,23 @@ struct ov7670_devtype {
>> struct ov7670_format_struct; /* coming later */
>> struct ov7670_info {
>> struct v4l2_subdev sd;
>> + struct v4l2_ctrl_handler hdl;
>> + struct {
>> + /* gain cluster */
>> + struct v4l2_ctrl *auto_gain;
>> + struct v4l2_ctrl *gain;
>> + };
>> + struct {
>> + /* exposure cluster */
>> + struct v4l2_ctrl *auto_exposure;
>> + struct v4l2_ctrl *exposure;
>> + };
>> + struct {
>> + /* saturation/hue cluster */
>> + struct v4l2_ctrl *saturation;
>> + struct v4l2_ctrl *hue;
>> + };
>> struct ov7670_format_struct *fmt; /* Current format */
>> - unsigned char sat; /* Saturation value */
>> - int hue;/* Hue value */
>> int min_width; /* Filter out smaller sizes */
>> int min_height; /* Filter out smaller sizes */
>> int clock_speed;/* External clock speed (MHz) */
>> @@ -240,6 +255,11 @@ static inline struct ov7670_info *to_state(struct
>> v4l2_subdev *sd)
>> return container_of(sd, struct ov7670_info, sd);
>> }
>>
>> +static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl)
>> +{
>> + return &container_of(ctrl->handler, struct ov7670_info, hdl)->sd;
>> +}
>> +
>>
>>
>> /*
>> @@ -1195,23 +1215,23 @@ static int ov7670_cosine(int theta)
>>
>>
>> static void ov7670_calc_cmatrix(struct ov7670_info *info,
>> - int matrix[CMATRIX_LEN])
>> + int matrix[CMATRIX_LEN], int sat, int hue)
>> {
>> int i;
>> /*
>>* Apply the current saturation setting first.
>>*/
>> for (i = 0; i < CMATRIX_LEN; i++)
>> - matrix[i] = (info->fmt->cmatrix[i]*info->sat) >> 7;
>> + matrix[i] = (info->fmt->cmatrix[i] * sat) >> 7;
>> /*
>>* Then, if need be, rotate the hue value.
>>*/
>> - if (info->hue != 0) {
>> + if (hue != 0) {
>> int sinth, costh, tmpmatrix[CMATRIX_LEN];
>>
>> memcpy(tmpmatrix, matrix, CMATRIX_LEN*sizeof(int));
>> - sinth = ov7670_sine(info->hue);
>> - costh = ov7670_cosine(info->hue);
>> + sinth = ov7670_sine(hue);
>> + costh = ov7670_cosine(hue);
>>
>> matrix[0] = (matrix[3]*sinth + matrix[0]*costh)/1000;
>> matrix[1] = (matrix[4]*sinth + matrix[1]*costh)/1000;
>> @@ -1224,60 +1244,21 @@ static void ov7670_calc_cmatrix(struct ov7670_info
>> *info,
>>
>>
>>
>> -static int ov7670_s_sat(struct v4l2_subdev *sd, int value)
>> +static int ov7670_s_sat_hue(struct v4l2_subdev *sd, int sat, int hue)
>> {
>> struct ov7670_info *info = to_state(sd);
>> int matrix[CMATRIX_LEN];
>> int ret;
>>
>> - info->sat = value;
>> - ov7670_calc_cmatrix(info, matrix);
>> + ov7670_calc_cmatrix(info, matrix, sat, hue);
>> ret = ov7670_store_cmatrix(sd, matrix);
>> return ret;
>> }
>>
>> -static int ov7670_g_sat(struct v4l2_subdev *sd, __s32 *value)
>> -{
>> - struct ov7670_info *info = to_state(sd);
>> -
>> - *value = info->sat;
>> - return 0;
>> -}
>> -
>> -static int ov7670_s_hue(struct v4l2_subdev *sd, int value)
>> -{
>> - struct ov7670_info *info = to_state(sd);
>> - int matrix[CMATRIX_LEN];
>> - int ret;
>> -
>> - if (value < -180 || value > 180)
>> - return -EINVAL;
>> - info->hue = value;
>> - ov7670_calc_cmatrix(info, matrix);
>> - ret = ov7670_store_cmatrix(sd, matrix);
>> - return ret;
>> -}
>> -
>> -
>> -static int ov7670_g_hue(struct v4l2_subdev *sd, __s32 *value)
>> -{
>> - struct ov7670_info *info = to_state(sd);
>> -
>> - *value = info->hue;
>> - return 0;
>> -}
>> -
>>
>> /*
>> * Some weird registers seem to store values in a sign/magnitude format!
>> */
>> -static unsigned char ov7670_sm_to_abs(unsigned char v)
>> -{
>> - if ((v & 0x80) == 0)
>> - return v + 128;
>> - return 128 - (v & 0x7f);
>> -}
>> -
>>
>> static unsigned char ov7670_abs_to_sm(unsigned char v)
>> {
>> @@ -1299,40 +1280,11 @@ static int ov7670_s_brightness(struct v4l2_subdev
>> *sd, int v
Re: [PATCH v2 1/4] ov7670: use the control framework
On Fri September 28 2012 12:50:55 Javier Martin wrote:
> Signed-off-by: Hans Verkuil
> Signed-off-by: Javier Martin
> ---
> Changes since v1:
> - Use v4l2_ctrl_auto_cluster() for auto_gain and auto_exp.
>
> ---
> drivers/media/i2c/ov7670.c | 310
>
> 1 file changed, 112 insertions(+), 198 deletions(-)
>
> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> index 9d7a92e..babd55c 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -222,9 +223,23 @@ struct ov7670_devtype {
> struct ov7670_format_struct; /* coming later */
> struct ov7670_info {
> struct v4l2_subdev sd;
> + struct v4l2_ctrl_handler hdl;
> + struct {
> + /* gain cluster */
> + struct v4l2_ctrl *auto_gain;
> + struct v4l2_ctrl *gain;
> + };
> + struct {
> + /* exposure cluster */
> + struct v4l2_ctrl *auto_exposure;
> + struct v4l2_ctrl *exposure;
> + };
> + struct {
> + /* saturation/hue cluster */
> + struct v4l2_ctrl *saturation;
> + struct v4l2_ctrl *hue;
> + };
> struct ov7670_format_struct *fmt; /* Current format */
> - unsigned char sat; /* Saturation value */
> - int hue;/* Hue value */
> int min_width; /* Filter out smaller sizes */
> int min_height; /* Filter out smaller sizes */
> int clock_speed;/* External clock speed (MHz) */
> @@ -240,6 +255,11 @@ static inline struct ov7670_info *to_state(struct
> v4l2_subdev *sd)
> return container_of(sd, struct ov7670_info, sd);
> }
>
> +static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl)
> +{
> + return &container_of(ctrl->handler, struct ov7670_info, hdl)->sd;
> +}
> +
>
>
> /*
> @@ -1195,23 +1215,23 @@ static int ov7670_cosine(int theta)
>
>
> static void ov7670_calc_cmatrix(struct ov7670_info *info,
> - int matrix[CMATRIX_LEN])
> + int matrix[CMATRIX_LEN], int sat, int hue)
> {
> int i;
> /*
>* Apply the current saturation setting first.
>*/
> for (i = 0; i < CMATRIX_LEN; i++)
> - matrix[i] = (info->fmt->cmatrix[i]*info->sat) >> 7;
> + matrix[i] = (info->fmt->cmatrix[i] * sat) >> 7;
> /*
>* Then, if need be, rotate the hue value.
>*/
> - if (info->hue != 0) {
> + if (hue != 0) {
> int sinth, costh, tmpmatrix[CMATRIX_LEN];
>
> memcpy(tmpmatrix, matrix, CMATRIX_LEN*sizeof(int));
> - sinth = ov7670_sine(info->hue);
> - costh = ov7670_cosine(info->hue);
> + sinth = ov7670_sine(hue);
> + costh = ov7670_cosine(hue);
>
> matrix[0] = (matrix[3]*sinth + matrix[0]*costh)/1000;
> matrix[1] = (matrix[4]*sinth + matrix[1]*costh)/1000;
> @@ -1224,60 +1244,21 @@ static void ov7670_calc_cmatrix(struct ov7670_info
> *info,
>
>
>
> -static int ov7670_s_sat(struct v4l2_subdev *sd, int value)
> +static int ov7670_s_sat_hue(struct v4l2_subdev *sd, int sat, int hue)
> {
> struct ov7670_info *info = to_state(sd);
> int matrix[CMATRIX_LEN];
> int ret;
>
> - info->sat = value;
> - ov7670_calc_cmatrix(info, matrix);
> + ov7670_calc_cmatrix(info, matrix, sat, hue);
> ret = ov7670_store_cmatrix(sd, matrix);
> return ret;
> }
>
> -static int ov7670_g_sat(struct v4l2_subdev *sd, __s32 *value)
> -{
> - struct ov7670_info *info = to_state(sd);
> -
> - *value = info->sat;
> - return 0;
> -}
> -
> -static int ov7670_s_hue(struct v4l2_subdev *sd, int value)
> -{
> - struct ov7670_info *info = to_state(sd);
> - int matrix[CMATRIX_LEN];
> - int ret;
> -
> - if (value < -180 || value > 180)
> - return -EINVAL;
> - info->hue = value;
> - ov7670_calc_cmatrix(info, matrix);
> - ret = ov7670_store_cmatrix(sd, matrix);
> - return ret;
> -}
> -
> -
> -static int ov7670_g_hue(struct v4l2_subdev *sd, __s32 *value)
> -{
> - struct ov7670_info *info = to_state(sd);
> -
> - *value = info->hue;
> - return 0;
> -}
> -
>
> /*
> * Some weird registers seem to store values in a sign/magnitude format!
> */
> -static unsigned char ov7670_sm_to_abs(unsigned char v)
> -{
> - if ((v & 0x80) == 0)
> - return v + 128;
> - return 128 - (v & 0x7f);
> -}
> -
>
> static unsigned char ov7670_abs_to_sm(unsigned char v)
> {
> @@ -1299,40 +1280,11 @@ static int ov7670_s_brightness(struct v4l2_subdev
> *sd, int value)
> return ret;
> }
>
> -static int ov7670_g_brightness(struct v4l2_subdev *sd, __s32 *value)
> -{
> - unsigned char v = 0;
> - int ret = ov7670_read(sd, REG_BRIGHT,
[PATCH v2 1/4] ov7670: use the control framework
Signed-off-by: Hans Verkuil
Signed-off-by: Javier Martin
---
Changes since v1:
- Use v4l2_ctrl_auto_cluster() for auto_gain and auto_exp.
---
drivers/media/i2c/ov7670.c | 310
1 file changed, 112 insertions(+), 198 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 9d7a92e..babd55c 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -222,9 +223,23 @@ struct ov7670_devtype {
struct ov7670_format_struct; /* coming later */
struct ov7670_info {
struct v4l2_subdev sd;
+ struct v4l2_ctrl_handler hdl;
+ struct {
+ /* gain cluster */
+ struct v4l2_ctrl *auto_gain;
+ struct v4l2_ctrl *gain;
+ };
+ struct {
+ /* exposure cluster */
+ struct v4l2_ctrl *auto_exposure;
+ struct v4l2_ctrl *exposure;
+ };
+ struct {
+ /* saturation/hue cluster */
+ struct v4l2_ctrl *saturation;
+ struct v4l2_ctrl *hue;
+ };
struct ov7670_format_struct *fmt; /* Current format */
- unsigned char sat; /* Saturation value */
- int hue;/* Hue value */
int min_width; /* Filter out smaller sizes */
int min_height; /* Filter out smaller sizes */
int clock_speed;/* External clock speed (MHz) */
@@ -240,6 +255,11 @@ static inline struct ov7670_info *to_state(struct
v4l2_subdev *sd)
return container_of(sd, struct ov7670_info, sd);
}
+static inline struct v4l2_subdev *to_sd(struct v4l2_ctrl *ctrl)
+{
+ return &container_of(ctrl->handler, struct ov7670_info, hdl)->sd;
+}
+
/*
@@ -1195,23 +1215,23 @@ static int ov7670_cosine(int theta)
static void ov7670_calc_cmatrix(struct ov7670_info *info,
- int matrix[CMATRIX_LEN])
+ int matrix[CMATRIX_LEN], int sat, int hue)
{
int i;
/*
* Apply the current saturation setting first.
*/
for (i = 0; i < CMATRIX_LEN; i++)
- matrix[i] = (info->fmt->cmatrix[i]*info->sat) >> 7;
+ matrix[i] = (info->fmt->cmatrix[i] * sat) >> 7;
/*
* Then, if need be, rotate the hue value.
*/
- if (info->hue != 0) {
+ if (hue != 0) {
int sinth, costh, tmpmatrix[CMATRIX_LEN];
memcpy(tmpmatrix, matrix, CMATRIX_LEN*sizeof(int));
- sinth = ov7670_sine(info->hue);
- costh = ov7670_cosine(info->hue);
+ sinth = ov7670_sine(hue);
+ costh = ov7670_cosine(hue);
matrix[0] = (matrix[3]*sinth + matrix[0]*costh)/1000;
matrix[1] = (matrix[4]*sinth + matrix[1]*costh)/1000;
@@ -1224,60 +1244,21 @@ static void ov7670_calc_cmatrix(struct ov7670_info
*info,
-static int ov7670_s_sat(struct v4l2_subdev *sd, int value)
+static int ov7670_s_sat_hue(struct v4l2_subdev *sd, int sat, int hue)
{
struct ov7670_info *info = to_state(sd);
int matrix[CMATRIX_LEN];
int ret;
- info->sat = value;
- ov7670_calc_cmatrix(info, matrix);
+ ov7670_calc_cmatrix(info, matrix, sat, hue);
ret = ov7670_store_cmatrix(sd, matrix);
return ret;
}
-static int ov7670_g_sat(struct v4l2_subdev *sd, __s32 *value)
-{
- struct ov7670_info *info = to_state(sd);
-
- *value = info->sat;
- return 0;
-}
-
-static int ov7670_s_hue(struct v4l2_subdev *sd, int value)
-{
- struct ov7670_info *info = to_state(sd);
- int matrix[CMATRIX_LEN];
- int ret;
-
- if (value < -180 || value > 180)
- return -EINVAL;
- info->hue = value;
- ov7670_calc_cmatrix(info, matrix);
- ret = ov7670_store_cmatrix(sd, matrix);
- return ret;
-}
-
-
-static int ov7670_g_hue(struct v4l2_subdev *sd, __s32 *value)
-{
- struct ov7670_info *info = to_state(sd);
-
- *value = info->hue;
- return 0;
-}
-
/*
* Some weird registers seem to store values in a sign/magnitude format!
*/
-static unsigned char ov7670_sm_to_abs(unsigned char v)
-{
- if ((v & 0x80) == 0)
- return v + 128;
- return 128 - (v & 0x7f);
-}
-
static unsigned char ov7670_abs_to_sm(unsigned char v)
{
@@ -1299,40 +1280,11 @@ static int ov7670_s_brightness(struct v4l2_subdev *sd,
int value)
return ret;
}
-static int ov7670_g_brightness(struct v4l2_subdev *sd, __s32 *value)
-{
- unsigned char v = 0;
- int ret = ov7670_read(sd, REG_BRIGHT, &v);
-
- *value = ov7670_sm_to_abs(v);
- return ret;
-}
-
static int ov7670_s_contrast(struct v4l2_subdev *sd, int value)
{
return ov7670_write(sd, REG_CONTRAS, (unsigned char) value);
}
-static int ov7670_
