[PATCH 1/3] ov7670: use the control framework

2012-09-28 Thread Javier Martin
Signed-off-by: Hans Verkuil hverk...@xs4all.nl
Signed-off-by: Javier Martin javier.mar...@vista-silicon.com
---
 drivers/media/i2c/ov7670.c |  295 +---
 1 file changed, 115 insertions(+), 180 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 9d7a92e..eead1b4 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -18,6 +18,7 @@
 #include linux/videodev2.h
 #include media/v4l2-device.h
 #include media/v4l2-chip-ident.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-mediabus.h
 #include media/ov7670.h
 
@@ -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)
-{
-   struct ov7670_info *info = to_state(sd);
-   int matrix[CMATRIX_LEN];
-   int ret;
-
-   info-sat = value;
-   ov7670_calc_cmatrix(info, matrix);
-   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)
+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;
 
-   if (value  -180 || value  180)
-   return -EINVAL;
-   info-hue = 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_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, 

Re: [PATCH 1/3] ov7670: use the control framework

2012-09-28 Thread Hans Verkuil
On Fri September 28 2012 09:48:01 Javier Martin wrote:
 Signed-off-by: Hans Verkuil hverk...@xs4all.nl
 Signed-off-by: Javier Martin javier.mar...@vista-silicon.com
 ---
  drivers/media/i2c/ov7670.c |  295 
 +---
  1 file changed, 115 insertions(+), 180 deletions(-)
 
 diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
 index 9d7a92e..eead1b4 100644
 --- a/drivers/media/i2c/ov7670.c
 +++ b/drivers/media/i2c/ov7670.c
 @@ -18,6 +18,7 @@
  #include linux/videodev2.h
  #include media/v4l2-device.h
  #include media/v4l2-chip-ident.h
 +#include media/v4l2-ctrls.h
  #include media/v4l2-mediabus.h
  #include media/ov7670.h
  
 @@ -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)
 -{
 - struct ov7670_info *info = to_state(sd);
 - int matrix[CMATRIX_LEN];
 - int ret;
 -
 - info-sat = value;
 - ov7670_calc_cmatrix(info, matrix);
 - 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)
 +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;
  
 - if (value  -180 || value  180)
 - return -EINVAL;
 - info-hue = 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_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 

Re: [PATCH 1/3] ov7670: use the control framework

2012-09-28 Thread javier Martin
Hi Hans,

On 28 September 2012 10:23, Hans Verkuil hverk...@xs4all.nl wrote:
 On Fri September 28 2012 09:48:01 Javier Martin wrote:
  static const struct v4l2_subdev_core_ops ov7670_core_ops = {
   .g_chip_ident = ov7670_g_chip_ident,
 - .g_ctrl = ov7670_g_ctrl,
 - .s_ctrl = ov7670_s_ctrl,
 - .queryctrl = ov7670_queryctrl,
 + .g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
 + .try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
 + .s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
 + .g_ctrl = v4l2_subdev_g_ctrl,
 + .s_ctrl = v4l2_subdev_s_ctrl,
 + .queryctrl = v4l2_subdev_queryctrl,
 + .querymenu = v4l2_subdev_querymenu,

 Can you make a fourth patch removing these lines again after mcam-core and
 via-camera are converted to the control framework? These control ops are
 only needed if there are bridge drivers using this sensor driver that are
 not yet converted to the control framework. Since that limitation is now
 lifted, these ops can be removed as well.

Fine, I will do that.

   .reset = ov7670_reset,
   .init = ov7670_init,
  #ifdef CONFIG_VIDEO_ADV_DEBUG
 @@ -1732,7 +1630,6 @@ static int ov7670_probe(struct i2c_client *client,

   info-devtype = ov7670_devdata[id-driver_data];
   info-fmt = ov7670_formats[0];
 - info-sat = 128;/* Review this */
   info-clkrc = 0;

   /* Set default frame rate to 30 fps */
 @@ -1743,6 +1640,42 @@ static int ov7670_probe(struct i2c_client *client,
   if (info-pclk_hb_disable)
   ov7670_write(sd, REG_COM10, COM10_PCLK_HB);

 + v4l2_ctrl_handler_init(info-hdl, 10);
 + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops,
 + V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
 + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops,
 + V4L2_CID_CONTRAST, 0, 127, 1, 64);
 + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops,
 + V4L2_CID_VFLIP, 0, 1, 1, 0);
 + v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops,
 + V4L2_CID_HFLIP, 0, 1, 1, 0);
 + info-saturation = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops,
 + V4L2_CID_SATURATION, 0, 256, 1, 128);
 + info-hue = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops,
 + V4L2_CID_HUE, -180, 180, 5, 0);
 + info-gain = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops,
 + V4L2_CID_GAIN, 0, 255, 1, 128);
 + info-auto_gain = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops,
 + V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
 + info-exposure = v4l2_ctrl_new_std(info-hdl, ov7670_ctrl_ops,
 + V4L2_CID_EXPOSURE, 0, 65535, 1, 500);
 + info-auto_exposure = v4l2_ctrl_new_std_menu(info-hdl, 
 ov7670_ctrl_ops,
 + V4L2_CID_EXPOSURE_AUTO, 1, 0, 0);
 + sd-ctrl_handler = info-hdl;
 + if (info-hdl.error) {
 + int err = info-hdl.error;
 +
 + v4l2_ctrl_handler_free(info-hdl);
 + kfree(info);
 + return err;
 + }
 + info-gain-flags |= V4L2_CTRL_FLAG_VOLATILE;
 + info-exposure-flags |= V4L2_CTRL_FLAG_VOLATILE;
 + v4l2_ctrl_cluster(2, info-auto_gain);
 + v4l2_ctrl_cluster(2, info-auto_exposure);

 You need to use v4l2_ctrl_auto_cluster for these two. Not having that function
 at the time was the reason my work on this driver stalled. The auto cluster
 functionality takes care of most of the nitty gritty details of handling
 these situations.

OK, let me take a look.

Regards.
-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html