[PATCH 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[PATCH 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[Antonio Ospite]

Hi,

here are a couple more of controls for the gspca ov534 subdriver.

In order to control the HUE value the sensor expects sin(HUE) and
cos(HUE) to be set so I decided to reuse the fixed point implementation
of sine and cosine from drivers/input/fixp-arith.h, see patches 2 and 3.

Dmitry, can the movement of fixp-arith.h in patch 2 go via the media
tree?  That should ease up the integration of patch 3 in this series
I think.

Jonathan, maybe fixp_sin() and fixp_cos() can be used in
drivers/media/video/ov7670.c too where currently ov7670_sine() and
ov7670_cosine() are defined, but I didn't want to send a patch I could
not test.

BTW What is the usual way to communicate these cross-subsystem stuff?
I CC-ed everybody only on the cover letter and on patches 2 and 3 which
are the ones concerning somehow both "input" and "media".

Thanks,
   Antonio


Antonio Ospite (3):
  [media] gspca - ov534: Add Saturation control
  Input: move drivers/input/fixp-arith.h to include/linux
  [media] gspca - ov534: Add Hue control

 drivers/input/ff-memless.c        |    3 +-
 drivers/input/fixp-arith.h        |   87 ------------------------------------
 drivers/media/video/gspca/ov534.c |   89 ++++++++++++++++++++++++++++++++++++-
 include/linux/fixp-arith.h        |   87 ++++++++++++++++++++++++++++++++++++
 4 files changed, 176 insertions(+), 90 deletions(-)
 delete mode 100644 drivers/input/fixp-arith.h
 create mode 100644 include/linux/fixp-arith.h

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[PATCH 1/3] [media] gspca - ov534: Add Saturation control

[Antonio Ospite]

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 drivers/media/video/gspca/ov534.c |   31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/media/video/gspca/ov534.c b/drivers/media/video/gspca/ov534.c
index 0475339..45139b5 100644
--- a/drivers/media/video/gspca/ov534.c
+++ b/drivers/media/video/gspca/ov534.c
@@ -53,6 +53,7 @@ MODULE_LICENSE("GPL");
 
 /* controls */
 enum e_ctrl {
+	SATURATION,
 	BRIGHTNESS,
 	CONTRAST,
 	GAIN,
@@ -87,6 +88,7 @@ enum sensors {
 };
 
 /* V4L2 controls supported by the driver */
+static void setsaturation(struct gspca_dev *gspca_dev);
 static void setbrightness(struct gspca_dev *gspca_dev);
 static void setcontrast(struct gspca_dev *gspca_dev);
 static void setgain(struct gspca_dev *gspca_dev);
@@ -103,6 +105,18 @@ static int sd_start(struct gspca_dev *gspca_dev);
 static void sd_stopN(struct gspca_dev *gspca_dev);
 
 static const struct ctrl sd_ctrls[] = {
+[SATURATION] = {
+		{
+			.id      = V4L2_CID_SATURATION,
+			.type    = V4L2_CTRL_TYPE_INTEGER,
+			.name    = "Saturation",
+			.minimum = 0,
+			.maximum = 255,
+			.step    = 1,
+			.default_value = 64,
+		},
+		.set_control = setsaturation
+	},
 [BRIGHTNESS] = {
 		{
 			.id      = V4L2_CID_BRIGHTNESS,
@@ -684,7 +698,7 @@ static const u8 sensor_init_772x[][2] = {
 	{ 0x9c, 0x20 },
 	{ 0x9e, 0x81 },
 
-	{ 0xa6, 0x04 },
+	{ 0xa6, 0x06 },
 	{ 0x7e, 0x0c },
 	{ 0x7f, 0x16 },
 	{ 0x80, 0x2a },
@@ -955,6 +969,20 @@ static void set_frame_rate(struct gspca_dev *gspca_dev)
 	PDEBUG(D_PROBE, "frame_rate: %d", r->fps);
 }
 
+static void setsaturation(struct gspca_dev *gspca_dev)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+	int val;
+
+	val = sd->ctrls[SATURATION].val;
+	if (sd->sensor == SENSOR_OV767x) {
+		/* TBD */
+	} else {
+		sccb_reg_write(gspca_dev, 0xa7, val); /* U saturation */
+		sccb_reg_write(gspca_dev, 0xa8, val); /* V saturation */
+	}
+}
+
 static void setbrightness(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -1309,6 +1337,7 @@ static int sd_start(struct gspca_dev *gspca_dev)
 	if (!(gspca_dev->ctrl_dis & (1 << GAIN)))
 		setgain(gspca_dev);
 	setexposure(gspca_dev);
+	setsaturation(gspca_dev);
 	setbrightness(gspca_dev);
 	setcontrast(gspca_dev);
 	if (!(gspca_dev->ctrl_dis & (1 << SHARPNESS)))
-- 
1.7.10

[PATCH 2/3] Input: move drivers/input/fixp-arith.h to include/linux

[Antonio Ospite]

Move drivers/input/fixp-arith.h to include/linux so that the functions
defined there can be used by other subsystems, for instance some video
devices ISPs can control the output HUE value by setting registers for
sin(HUE) and cos(HUE).

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 drivers/input/ff-memless.c                    |    3 +--
 {drivers/input => include/linux}/fixp-arith.h |    0
 2 files changed, 1 insertion(+), 2 deletions(-)
 rename {drivers/input => include/linux}/fixp-arith.h (100%)

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index 117a59a..5f55885 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -31,8 +31,7 @@
 #include <linux/mutex.h>
 #include <linux/spinlock.h>
 #include <linux/jiffies.h>
-
-#include "fixp-arith.h"
+#include <linux/fixp-arith.h>
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Anssi Hannula <anssi.hannula@gmail.com>");
diff --git a/drivers/input/fixp-arith.h b/include/linux/fixp-arith.h
similarity index 100%
rename from drivers/input/fixp-arith.h
rename to include/linux/fixp-arith.h
-- 
1.7.10

[PATCH 3/3] [media] gspca - ov534: Add Hue control

[Antonio Ospite]

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 drivers/media/video/gspca/ov534.c |   61 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/media/video/gspca/ov534.c b/drivers/media/video/gspca/ov534.c
index 45139b5..1a437b5 100644
--- a/drivers/media/video/gspca/ov534.c
+++ b/drivers/media/video/gspca/ov534.c
@@ -34,6 +34,8 @@
 
 #include "gspca.h"
 
+#include <linux/fixp-arith.h>
+
 #define OV534_REG_ADDRESS	0xf1	/* sensor address */
 #define OV534_REG_SUBADDR	0xf2
 #define OV534_REG_WRITE		0xf3
@@ -53,6 +55,7 @@ MODULE_LICENSE("GPL");
 
 /* controls */
 enum e_ctrl {
+	HUE,
 	SATURATION,
 	BRIGHTNESS,
 	CONTRAST,
@@ -88,6 +91,7 @@ enum sensors {
 };
 
 /* V4L2 controls supported by the driver */
+static void sethue(struct gspca_dev *gspca_dev);
 static void setsaturation(struct gspca_dev *gspca_dev);
 static void setbrightness(struct gspca_dev *gspca_dev);
 static void setcontrast(struct gspca_dev *gspca_dev);
@@ -105,6 +109,18 @@ static int sd_start(struct gspca_dev *gspca_dev);
 static void sd_stopN(struct gspca_dev *gspca_dev);
 
 static const struct ctrl sd_ctrls[] = {
+[HUE] = {
+		{
+			.id      = V4L2_CID_HUE,
+			.type    = V4L2_CTRL_TYPE_INTEGER,
+			.name    = "Hue",
+			.minimum = -90,
+			.maximum = 90,
+			.step    = 1,
+			.default_value = 0,
+		},
+		.set_control = sethue
+	},
 [SATURATION] = {
 		{
 			.id      = V4L2_CID_SATURATION,
@@ -698,7 +714,7 @@ static const u8 sensor_init_772x[][2] = {
 	{ 0x9c, 0x20 },
 	{ 0x9e, 0x81 },
 
-	{ 0xa6, 0x06 },
+	{ 0xa6, 0x07 },
 	{ 0x7e, 0x0c },
 	{ 0x7f, 0x16 },
 	{ 0x80, 0x2a },
@@ -969,6 +985,48 @@ static void set_frame_rate(struct gspca_dev *gspca_dev)
 	PDEBUG(D_PROBE, "frame_rate: %d", r->fps);
 }
 
+static void sethue(struct gspca_dev *gspca_dev)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+	int val;
+
+	val = sd->ctrls[HUE].val;
+	if (sd->sensor == SENSOR_OV767x) {
+		/* TBD */
+	} else {
+		s16 huesin;
+		s16 huecos;
+
+		/* fixp_sin and fixp_cos accept only positive values, while
+		 * our val is between -90 and 90
+		 */
+		val += 360;
+
+		/* According to the datasheet the registers expect HUESIN and
+		 * HUECOS to be the result of the trigonometric functions,
+		 * scaled by 0x80.
+		 *
+		 * The 0x100 here represents the maximun absolute value
+		 * returned byt fixp_sin and fixp_cos, so the scaling will
+		 * consider the result like in the interval [-1.0, 1.0].
+		 */
+		huesin = fixp_sin(val) * 0x80 / 0x100;
+		huecos = fixp_cos(val) * 0x80 / 0x100;
+
+		if (huesin < 0) {
+			sccb_reg_write(gspca_dev, 0xab,
+				sccb_reg_read(gspca_dev, 0xab) | 0x2);
+			huesin = -huesin;
+		} else {
+			sccb_reg_write(gspca_dev, 0xab,
+				sccb_reg_read(gspca_dev, 0xab) & ~0x2);
+
+		}
+		sccb_reg_write(gspca_dev, 0xa9, (u8)huecos);
+		sccb_reg_write(gspca_dev, 0xaa, (u8)huesin);
+	}
+}
+
 static void setsaturation(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -1337,6 +1395,7 @@ static int sd_start(struct gspca_dev *gspca_dev)
 	if (!(gspca_dev->ctrl_dis & (1 << GAIN)))
 		setgain(gspca_dev);
 	setexposure(gspca_dev);
+	sethue(gspca_dev);
 	setsaturation(gspca_dev);
 	setbrightness(gspca_dev);
 	setcontrast(gspca_dev);
-- 
1.7.10

Re: [PATCH 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 309 bytes --]

If you "reply to all" please fix Johann Deneux's address, I mis-wrote
it, sorry.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/3] Input: move drivers/input/fixp-arith.h to include/linux

[Dmitry Torokhov]

On Mon, Apr 23, 2012 at 03:21:06PM +0200, Antonio Ospite wrote:
> Move drivers/input/fixp-arith.h to include/linux so that the functions
> defined there can be used by other subsystems, for instance some video
> devices ISPs can control the output HUE value by setting registers for
> sin(HUE) and cos(HUE).
> 
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
>  drivers/input/ff-memless.c                    |    3 +--
>  {drivers/input => include/linux}/fixp-arith.h |    0
>  2 files changed, 1 insertion(+), 2 deletions(-)
>  rename {drivers/input => include/linux}/fixp-arith.h (100%)
> 
> diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
> index 117a59a..5f55885 100644
> --- a/drivers/input/ff-memless.c
> +++ b/drivers/input/ff-memless.c
> @@ -31,8 +31,7 @@
>  #include <linux/mutex.h>
>  #include <linux/spinlock.h>
>  #include <linux/jiffies.h>
> -
> -#include "fixp-arith.h"
> +#include <linux/fixp-arith.h>
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Anssi Hannula <anssi.hannula@gmail.com>");
> diff --git a/drivers/input/fixp-arith.h b/include/linux/fixp-arith.h
> similarity index 100%
> rename from drivers/input/fixp-arith.h
> rename to include/linux/fixp-arith.h
> -- 
> 1.7.10
> 

-- 
Dmitry

Re: [PATCH 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[Dmitry Torokhov]

On Mon, Apr 23, 2012 at 03:21:04PM +0200, Antonio Ospite wrote:
> Hi,
> 
> here are a couple more of controls for the gspca ov534 subdriver.
> 
> In order to control the HUE value the sensor expects sin(HUE) and
> cos(HUE) to be set so I decided to reuse the fixed point implementation
> of sine and cosine from drivers/input/fixp-arith.h, see patches 2 and 3.
> 
> Dmitry, can the movement of fixp-arith.h in patch 2 go via the media
> tree?  That should ease up the integration of patch 3 in this series
> I think.

Yep, this is fine with me.

> 
> Jonathan, maybe fixp_sin() and fixp_cos() can be used in
> drivers/media/video/ov7670.c too where currently ov7670_sine() and
> ov7670_cosine() are defined, but I didn't want to send a patch I could
> not test.
> 
> BTW What is the usual way to communicate these cross-subsystem stuff?
> I CC-ed everybody only on the cover letter and on patches 2 and 3 which
> are the ones concerning somehow both "input" and "media".
> 
> Thanks,
>    Antonio
> 
> 
> Antonio Ospite (3):
>   [media] gspca - ov534: Add Saturation control
>   Input: move drivers/input/fixp-arith.h to include/linux
>   [media] gspca - ov534: Add Hue control
> 
>  drivers/input/ff-memless.c        |    3 +-
>  drivers/input/fixp-arith.h        |   87 ------------------------------------
>  drivers/media/video/gspca/ov534.c |   89 ++++++++++++++++++++++++++++++++++++-
>  include/linux/fixp-arith.h        |   87 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 176 insertions(+), 90 deletions(-)
>  delete mode 100644 drivers/input/fixp-arith.h
>  create mode 100644 include/linux/fixp-arith.h
> 
> -- 
> Antonio Ospite
> http://ao2.it
> 
> A: Because it messes up the order in which people normally read text.
>    See http://en.wikipedia.org/wiki/Posting_style
> Q: Why is top-posting such a bad thing?

-- 
Dmitry

Re: [PATCH 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[Jonathan Corbet]

On Mon, 23 Apr 2012 15:21:04 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Jonathan, maybe fixp_sin() and fixp_cos() can be used in
> drivers/media/video/ov7670.c too where currently ov7670_sine() and
> ov7670_cosine() are defined, but I didn't want to send a patch I could
> not test.

Seems like a good idea.  No reason to have multiple such hacks in the
kernel; I'll look at dumping the ov7670 version when I get a chance.  That
may not be all that soon, though; life is a bit challenging at the moment.

One concern is that if we're going to add users to fixp-arith.h, some of
it should maybe go to a C file.  Otherwise we'll create duplicated copies
of the cos_table array for each user.  I'm not sure the functions need to
be inline either; nobody expects cos() to be blindingly fast.

Thanks,

jon

Re: [PATCH 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

On Mon, 23 Apr 2012 14:16:25 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> On Mon, 23 Apr 2012 15:21:04 +0200
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > Jonathan, maybe fixp_sin() and fixp_cos() can be used in
> > drivers/media/video/ov7670.c too where currently ov7670_sine() and
> > ov7670_cosine() are defined, but I didn't want to send a patch I could
> > not test.
> 
> Seems like a good idea.  No reason to have multiple such hacks in the
> kernel; I'll look at dumping the ov7670 version when I get a chance.  That
> may not be all that soon, though; life is a bit challenging at the moment.
> 
> One concern is that if we're going to add users to fixp-arith.h, some of
> it should maybe go to a C file.  Otherwise we'll create duplicated copies
> of the cos_table array for each user.  I'm not sure the functions need to
> be inline either; nobody expects cos() to be blindingly fast.
> 

Jonathan, does lib/fixp-arith.c sound OK to you? I'll put that on my
TODO, unless Johann wans to do some more rework; in any case I think we
can still merge this patchset like it is right now.

Jean-Francois will you take care of that?

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]

On Mon, 30 Apr 2012 15:51:01 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> On Mon, 23 Apr 2012 14:16:25 -0600
> Jonathan Corbet <corbet@lwn.net> wrote:
> 
> > On Mon, 23 Apr 2012 15:21:04 +0200
> > Antonio Ospite <ospite@studenti.unina.it> wrote:
> > 
> > > Jonathan, maybe fixp_sin() and fixp_cos() can be used in
> > > drivers/media/video/ov7670.c too where currently ov7670_sine() and
> > > ov7670_cosine() are defined, but I didn't want to send a patch I could
> > > not test.
> > 
> > Seems like a good idea.  No reason to have multiple such hacks in the
> > kernel; I'll look at dumping the ov7670 version when I get a chance.  That
> > may not be all that soon, though; life is a bit challenging at the moment.
> > 
> > One concern is that if we're going to add users to fixp-arith.h, some of
> > it should maybe go to a C file.  Otherwise we'll create duplicated copies
> > of the cos_table array for each user.  I'm not sure the functions need to
> > be inline either; nobody expects cos() to be blindingly fast.
> > 
> 
> Jonathan, does lib/fixp-arith.c sound OK to you? I'll put that on my
> TODO, unless Johann wans to do some more rework; in any case I think we
> can still merge this patchset like it is right now.
> 
> Jean-Francois will you take care of that?
> 

Or wait :)

I'll send a v2, I think I should merge the current COLORS control used
for SENSOR_OV767x and the SATURATION one I added for SENSOR_OV772x as
they both are V4L2_CID_SATURATION.

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[PATCH v2 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[Antonio Ospite]

Hi,

I am sending a second version of this patchset, changes since version 1 are
annotated per-patch.

Just FYI I intend to work on porting ov534 to v4l2 framework too once these
ones go in and the gspca core changes about that settle a bit.

Thanks,
   Antonio

Antonio Ospite (3):
  gspca - ov534: Add Saturation control
  Input: move drivers/input/fixp-arith.h to include/linux
  gspca - ov534: Add Hue control

 drivers/input/ff-memless.c        |    3 +-
 drivers/input/fixp-arith.h        |   87 ----------------------
 drivers/media/video/gspca/ov534.c |  146 +++++++++++++++++++++++++++----------
 include/linux/fixp-arith.h        |   87 ++++++++++++++++++++++
 4 files changed, 195 insertions(+), 128 deletions(-)
 delete mode 100644 drivers/input/fixp-arith.h
 create mode 100644 include/linux/fixp-arith.h

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[PATCH v2 1/3] gspca - ov534: Add Saturation control

[Antonio Ospite]

Also merge the "COLORS" control into it as it was V4L2_CID_SATURATION
anyway.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---

Changes since version 1:

  - Merged SATURATION and COLORS controls

 drivers/media/video/gspca/ov534.c |   83 ++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/drivers/media/video/gspca/ov534.c b/drivers/media/video/gspca/ov534.c
index 0475339..c15cf23 100644
--- a/drivers/media/video/gspca/ov534.c
+++ b/drivers/media/video/gspca/ov534.c
@@ -53,6 +53,7 @@ MODULE_LICENSE("GPL");
 
 /* controls */
 enum e_ctrl {
+	SATURATION,
 	BRIGHTNESS,
 	CONTRAST,
 	GAIN,
@@ -63,7 +64,6 @@ enum e_ctrl {
 	SHARPNESS,
 	HFLIP,
 	VFLIP,
-	COLORS,
 	LIGHTFREQ,
 	NCTRLS		/* number of controls */
 };
@@ -87,6 +87,7 @@ enum sensors {
 };
 
 /* V4L2 controls supported by the driver */
+static void setsaturation(struct gspca_dev *gspca_dev);
 static void setbrightness(struct gspca_dev *gspca_dev);
 static void setcontrast(struct gspca_dev *gspca_dev);
 static void setgain(struct gspca_dev *gspca_dev);
@@ -96,13 +97,24 @@ static void setawb(struct gspca_dev *gspca_dev);
 static void setaec(struct gspca_dev *gspca_dev);
 static void setsharpness(struct gspca_dev *gspca_dev);
 static void sethvflip(struct gspca_dev *gspca_dev);
-static void setcolors(struct gspca_dev *gspca_dev);
 static void setlightfreq(struct gspca_dev *gspca_dev);
 
 static int sd_start(struct gspca_dev *gspca_dev);
 static void sd_stopN(struct gspca_dev *gspca_dev);
 
 static const struct ctrl sd_ctrls[] = {
+[SATURATION] = {
+		{
+			.id      = V4L2_CID_SATURATION,
+			.type    = V4L2_CTRL_TYPE_INTEGER,
+			.name    = "Saturation",
+			.minimum = 0,
+			.maximum = 255,
+			.step    = 1,
+			.default_value = 64,
+		},
+		.set_control = setsaturation
+	},
 [BRIGHTNESS] = {
 		{
 			.id      = V4L2_CID_BRIGHTNESS,
@@ -223,18 +235,6 @@ static const struct ctrl sd_ctrls[] = {
 		},
 		.set_control = sethvflip
 	},
-[COLORS] = {
-		{
-			.id      = V4L2_CID_SATURATION,
-			.type    = V4L2_CTRL_TYPE_INTEGER,
-			.name    = "Saturation",
-			.minimum = 0,
-			.maximum = 6,
-			.step    = 1,
-			.default_value = 3,
-		},
-		.set_control = setcolors
-	},
 [LIGHTFREQ] = {
 		{
 			.id      = V4L2_CID_POWER_LINE_FREQUENCY,
@@ -684,7 +684,7 @@ static const u8 sensor_init_772x[][2] = {
 	{ 0x9c, 0x20 },
 	{ 0x9e, 0x81 },
 
-	{ 0xa6, 0x04 },
+	{ 0xa6, 0x06 },
 	{ 0x7e, 0x0c },
 	{ 0x7f, 0x16 },
 	{ 0x80, 0x2a },
@@ -955,6 +955,32 @@ static void set_frame_rate(struct gspca_dev *gspca_dev)
 	PDEBUG(D_PROBE, "frame_rate: %d", r->fps);
 }
 
+static void setsaturation(struct gspca_dev *gspca_dev)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+	int val;
+
+	val = sd->ctrls[SATURATION].val;
+	if (sd->sensor == SENSOR_OV767x) {
+		int i;
+		static u8 color_tb[][6] = {
+			{0x42, 0x42, 0x00, 0x11, 0x30, 0x41},
+			{0x52, 0x52, 0x00, 0x16, 0x3c, 0x52},
+			{0x66, 0x66, 0x00, 0x1b, 0x4b, 0x66},
+			{0x80, 0x80, 0x00, 0x22, 0x5e, 0x80},
+			{0x9a, 0x9a, 0x00, 0x29, 0x71, 0x9a},
+			{0xb8, 0xb8, 0x00, 0x31, 0x87, 0xb8},
+			{0xdd, 0xdd, 0x00, 0x3b, 0xa2, 0xdd},
+		};
+
+		for (i = 0; i < ARRAY_SIZE(color_tb[0]); i++)
+			sccb_reg_write(gspca_dev, 0x4f + i, color_tb[val][i]);
+	} else {
+		sccb_reg_write(gspca_dev, 0xa7, val); /* U saturation */
+		sccb_reg_write(gspca_dev, 0xa8, val); /* V saturation */
+	}
+}
+
 static void setbrightness(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -1132,26 +1158,6 @@ static void sethvflip(struct gspca_dev *gspca_dev)
 	}
 }
 
-static void setcolors(struct gspca_dev *gspca_dev)
-{
-	struct sd *sd = (struct sd *) gspca_dev;
-	u8 val;
-	int i;
-	static u8 color_tb[][6] = {
-		{0x42, 0x42, 0x00, 0x11, 0x30, 0x41},
-		{0x52, 0x52, 0x00, 0x16, 0x3c, 0x52},
-		{0x66, 0x66, 0x00, 0x1b, 0x4b, 0x66},
-		{0x80, 0x80, 0x00, 0x22, 0x5e, 0x80},
-		{0x9a, 0x9a, 0x00, 0x29, 0x71, 0x9a},
-		{0xb8, 0xb8, 0x00, 0x31, 0x87, 0xb8},
-		{0xdd, 0xdd, 0x00, 0x3b, 0xa2, 0xdd},
-	};
-
-	val = sd->ctrls[COLORS].val;
-	for (i = 0; i < ARRAY_SIZE(color_tb[0]); i++)
-		sccb_reg_write(gspca_dev, 0x4f + i, color_tb[val][i]);
-}
-
 static void setlightfreq(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -1228,6 +1234,9 @@ static int sd_init(struct gspca_dev *gspca_dev)
 		gspca_dev->ctrl_dis = (1 << GAIN) |
 					(1 << AGC) |
 					(1 << SHARPNESS);	/* auto */
+		sd->ctrls[SATURATION].min = 0,
+		sd->ctrls[SATURATION].max = 6,
+		sd->ctrls[SATURATION].def = 3,
 		sd->ctrls[BRIGHTNESS].min = -127;
 		sd->ctrls[BRIGHTNESS].max = 127;
 		sd->ctrls[BRIGHTNESS].def = 0;
@@ -1243,7 +1252,6 @@ static int sd_init(struct gspca_dev *gspca_dev)
 		gspca_dev->cam.nmodes = ARRAY_SIZE(ov767x_mode);
 	} else {
 		sd->sensor = SENSOR_OV772x;
-		gspca_dev->ctrl_dis = (1 << COLORS);
 		gspca_dev->cam.bulk = 1;
 		gspca_dev->cam.bulk_size = 16384;
 		gspca_dev->cam.bulk_nurbs = 2;
@@ -1302,6 +1310,7 @@ static int sd_start(struct gspca_dev *gspca_dev)
 
 	set_frame_rate(gspca_dev);
 
+	setsaturation(gspca_dev);
 	if (!(gspca_dev->ctrl_dis & (1 << AGC)))
 		setagc(gspca_dev);
 	setawb(gspca_dev);
@@ -1314,8 +1323,6 @@ static int sd_start(struct gspca_dev *gspca_dev)
 	if (!(gspca_dev->ctrl_dis & (1 << SHARPNESS)))
 		setsharpness(gspca_dev);
 	sethvflip(gspca_dev);
-	if (!(gspca_dev->ctrl_dis & (1 << COLORS)))
-		setcolors(gspca_dev);
 	setlightfreq(gspca_dev);
 
 	ov534_set_led(gspca_dev, 1);
-- 
1.7.10

[PATCH v2 2/3] Input: move drivers/input/fixp-arith.h to include/linux

[Antonio Ospite]

Move drivers/input/fixp-arith.h to include/linux so that the functions
defined there can be used by other subsystems, for instance some video
devices ISPs can control the output HUE value by setting registers for
sin(HUE) and cos(HUE).

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Changes since version 1:
  
  - Just added the Acked-by: Dmitry to the commit message

 drivers/input/ff-memless.c |    3 +-
 drivers/input/fixp-arith.h |   87 --------------------------------------------
 include/linux/fixp-arith.h |   87 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+), 89 deletions(-)
 delete mode 100644 drivers/input/fixp-arith.h
 create mode 100644 include/linux/fixp-arith.h

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index 117a59a..5f55885 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -31,8 +31,7 @@
 #include <linux/mutex.h>
 #include <linux/spinlock.h>
 #include <linux/jiffies.h>
-
-#include "fixp-arith.h"
+#include <linux/fixp-arith.h>
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Anssi Hannula <anssi.hannula@gmail.com>");
diff --git a/drivers/input/fixp-arith.h b/drivers/input/fixp-arith.h
deleted file mode 100644
index 3089d73..0000000
--- a/drivers/input/fixp-arith.h
+++ /dev/null
@@ -1,87 +0,0 @@
-#ifndef _FIXP_ARITH_H
-#define _FIXP_ARITH_H
-
-/*
- * Simplistic fixed-point arithmetics.
- * Hmm, I'm probably duplicating some code :(
- *
- * Copyright (c) 2002 Johann Deneux
- */
-
-/*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * 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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- * Should you need to contact me, the author, you can do so by
- * e-mail - mail your message to <johann.deneux@gmail.com>
- */
-
-#include <linux/types.h>
-
-/* The type representing fixed-point values */
-typedef s16 fixp_t;
-
-#define FRAC_N 8
-#define FRAC_MASK ((1<<FRAC_N)-1)
-
-/* Not to be used directly. Use fixp_{cos,sin} */
-static const fixp_t cos_table[46] = {
-	0x0100,	0x00FF,	0x00FF,	0x00FE,	0x00FD,	0x00FC,	0x00FA,	0x00F8,
-	0x00F6,	0x00F3,	0x00F0,	0x00ED,	0x00E9,	0x00E6,	0x00E2,	0x00DD,
-	0x00D9,	0x00D4,	0x00CF,	0x00C9,	0x00C4,	0x00BE,	0x00B8,	0x00B1,
-	0x00AB,	0x00A4,	0x009D,	0x0096,	0x008F,	0x0087,	0x0080,	0x0078,
-	0x0070,	0x0068,	0x005F,	0x0057,	0x004F,	0x0046,	0x003D,	0x0035,
-	0x002C,	0x0023,	0x001A,	0x0011,	0x0008, 0x0000
-};
-
-
-/* a: 123 -> 123.0 */
-static inline fixp_t fixp_new(s16 a)
-{
-	return a<<FRAC_N;
-}
-
-/* a: 0xFFFF -> -1.0
-      0x8000 -> 1.0
-      0x0000 -> 0.0
-*/
-static inline fixp_t fixp_new16(s16 a)
-{
-	return ((s32)a)>>(16-FRAC_N);
-}
-
-static inline fixp_t fixp_cos(unsigned int degrees)
-{
-	int quadrant = (degrees / 90) & 3;
-	unsigned int i = degrees % 90;
-
-	if (quadrant == 1 || quadrant == 3)
-		i = 90 - i;
-
-	i >>= 1;
-
-	return (quadrant == 1 || quadrant == 2)? -cos_table[i] : cos_table[i];
-}
-
-static inline fixp_t fixp_sin(unsigned int degrees)
-{
-	return -fixp_cos(degrees + 90);
-}
-
-static inline fixp_t fixp_mult(fixp_t a, fixp_t b)
-{
-	return ((s32)(a*b))>>FRAC_N;
-}
-
-#endif
diff --git a/include/linux/fixp-arith.h b/include/linux/fixp-arith.h
new file mode 100644
index 0000000..3089d73
--- /dev/null
+++ b/include/linux/fixp-arith.h
@@ -0,0 +1,87 @@
+#ifndef _FIXP_ARITH_H
+#define _FIXP_ARITH_H
+
+/*
+ * Simplistic fixed-point arithmetics.
+ * Hmm, I'm probably duplicating some code :(
+ *
+ * Copyright (c) 2002 Johann Deneux
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Should you need to contact me, the author, you can do so by
+ * e-mail - mail your message to <johann.deneux@gmail.com>
+ */
+
+#include <linux/types.h>
+
+/* The type representing fixed-point values */
+typedef s16 fixp_t;
+
+#define FRAC_N 8
+#define FRAC_MASK ((1<<FRAC_N)-1)
+
+/* Not to be used directly. Use fixp_{cos,sin} */
+static const fixp_t cos_table[46] = {
+	0x0100,	0x00FF,	0x00FF,	0x00FE,	0x00FD,	0x00FC,	0x00FA,	0x00F8,
+	0x00F6,	0x00F3,	0x00F0,	0x00ED,	0x00E9,	0x00E6,	0x00E2,	0x00DD,
+	0x00D9,	0x00D4,	0x00CF,	0x00C9,	0x00C4,	0x00BE,	0x00B8,	0x00B1,
+	0x00AB,	0x00A4,	0x009D,	0x0096,	0x008F,	0x0087,	0x0080,	0x0078,
+	0x0070,	0x0068,	0x005F,	0x0057,	0x004F,	0x0046,	0x003D,	0x0035,
+	0x002C,	0x0023,	0x001A,	0x0011,	0x0008, 0x0000
+};
+
+
+/* a: 123 -> 123.0 */
+static inline fixp_t fixp_new(s16 a)
+{
+	return a<<FRAC_N;
+}
+
+/* a: 0xFFFF -> -1.0
+      0x8000 -> 1.0
+      0x0000 -> 0.0
+*/
+static inline fixp_t fixp_new16(s16 a)
+{
+	return ((s32)a)>>(16-FRAC_N);
+}
+
+static inline fixp_t fixp_cos(unsigned int degrees)
+{
+	int quadrant = (degrees / 90) & 3;
+	unsigned int i = degrees % 90;
+
+	if (quadrant == 1 || quadrant == 3)
+		i = 90 - i;
+
+	i >>= 1;
+
+	return (quadrant == 1 || quadrant == 2)? -cos_table[i] : cos_table[i];
+}
+
+static inline fixp_t fixp_sin(unsigned int degrees)
+{
+	return -fixp_cos(degrees + 90);
+}
+
+static inline fixp_t fixp_mult(fixp_t a, fixp_t b)
+{
+	return ((s32)(a*b))>>FRAC_N;
+}
+
+#endif
-- 
1.7.10

[PATCH v2 3/3] gspca - ov534: Add Hue control

[Antonio Ospite]

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---

Changes since version 1:

  - Disabled the HUE control for SENSOR_OV767x

 drivers/media/video/gspca/ov534.c |   65 +++++++++++++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/gspca/ov534.c b/drivers/media/video/gspca/ov534.c
index c15cf23..b5acb1e 100644
--- a/drivers/media/video/gspca/ov534.c
+++ b/drivers/media/video/gspca/ov534.c
@@ -34,6 +34,8 @@
 
 #include "gspca.h"
 
+#include <linux/fixp-arith.h>
+
 #define OV534_REG_ADDRESS	0xf1	/* sensor address */
 #define OV534_REG_SUBADDR	0xf2
 #define OV534_REG_WRITE		0xf3
@@ -53,6 +55,7 @@ MODULE_LICENSE("GPL");
 
 /* controls */
 enum e_ctrl {
+	HUE,
 	SATURATION,
 	BRIGHTNESS,
 	CONTRAST,
@@ -87,6 +90,7 @@ enum sensors {
 };
 
 /* V4L2 controls supported by the driver */
+static void sethue(struct gspca_dev *gspca_dev);
 static void setsaturation(struct gspca_dev *gspca_dev);
 static void setbrightness(struct gspca_dev *gspca_dev);
 static void setcontrast(struct gspca_dev *gspca_dev);
@@ -103,6 +107,18 @@ static int sd_start(struct gspca_dev *gspca_dev);
 static void sd_stopN(struct gspca_devSENSOR_OV767x *gspca_dev);
 
 static const struct ctrl sd_ctrls[] = {
+[HUE] = {
+		{
+			.id      = V4L2_CID_HUE,
+			.type    = V4L2_CTRL_TYPE_INTEGER,
+			.name    = "Hue",
+			.minimum = -90,
+			.maximum = 90,
+			.step    = 1,
+			.default_value = 0,
+		},
+		.set_control = sethue
+	},
 [SATURATION] = {
 		{
 			.id      = V4L2_CID_SATURATION,
@@ -684,7 +700,7 @@ static const u8 sensor_init_772x[][2] = {
 	{ 0x9c, 0x20 },
 	{ 0x9e, 0x81 },
 
-	{ 0xa6, 0x06 },
+	{ 0xa6, 0x07 },
 	{ 0x7e, 0x0c },
 	{ 0x7f, 0x16 },
 	{ 0x80, 0x2a },
@@ -955,6 +971,48 @@ static void set_frame_rate(struct gspca_dev *gspca_dev)
 	PDEBUG(D_PROBE, "frame_rate: %d", r->fps);
 }
 
+static void sethue(struct gspca_dev *gspca_dev)
+{
+	struct sd *sd = (struct sd *) gspca_dev;
+	int val;
+
+	val = sd->ctrls[HUE].val;
+	if (sd->sensor == SENSOR_OV767x) {
+		/* TBD */
+	} else {
+		s16 huesin;
+		s16 huecos;
+
+		/* fixp_sin and fixp_cos accept only positive values, while
+		 * our val is between -90 and 90
+		 */
+		val += 360;
+
+		/* According to the datasheet the registers expect HUESIN and
+		 * HUECOS to be the result of the trigonometric functions,
+		 * scaled by 0x80.
+		 *
+		 * The 0x100 here represents the maximun absolute value
+		 * returned byt fixp_sin and fixp_cos, so the scaling will
+		 * consider the result like in the interval [-1.0, 1.0].
+		 */
+		huesin = fixp_sin(val) * 0x80 / 0x100;
+		huecos = fixp_cos(val) * 0x80 / 0x100;
+
+		if (huesin < 0) {
+			sccb_reg_write(gspca_dev, 0xab,
+				sccb_reg_read(gspca_dev, 0xab) | 0x2);
+			huesin = -huesin;
+		} else {
+			sccb_reg_write(gspca_dev, 0xab,
+				sccb_reg_read(gspca_dev, 0xab) & ~0x2);
+
+		}
+		sccb_reg_write(gspca_dev, 0xa9, (u8)huecos);
+		sccb_reg_write(gspca_dev, 0xaa, (u8)huesin);
+	}
+}
+
 static void setsaturation(struct gspca_dev *gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;
@@ -1231,7 +1289,8 @@ static int sd_init(struct gspca_dev *gspca_dev)
 
 	if ((sensor_id & 0xfff0) == 0x7670) {
 		sd->sensor = SENSOR_OV767x;
-		gspca_dev->ctrl_dis = (1 << GAIN) |
+		gspca_dev->ctrl_dis = (1 << HUE) |
+					(1 << GAIN) |
 					(1 << AGC) |
 					(1 << SHARPNESS);	/* auto */
 		sd->ctrls[SATURATION].min = 0,
@@ -1310,6 +1369,8 @@ static int sd_start(struct gspca_dev *gspca_dev)
 
 	set_frame_rate(gspca_dev);
 
+	if (!(gspca_dev->ctrl_dis & (1 << HUE)))
+		sethue(gspca_dev);
 	setsaturation(gspca_dev);
 	if (!(gspca_dev->ctrl_dis & (1 << AGC)))
 		setagc(gspca_dev);
-- 
1.7.10

Re: [PATCH v2 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

On Sun,  6 May 2012 12:14:55 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Hi,
> 
> I am sending a second version of this patchset, changes since version 1 are
> annotated per-patch.
> 
> Just FYI I intend to work on porting ov534 to v4l2 framework too once these
> ones go in and the gspca core changes about that settle a bit.
> 
> Thanks,
>    Antonio
>

Ping.

Added HdG to CC.

Dmitry agreed to carry the small drivers/input changes through the media
tree.

> Antonio Ospite (3):
>   gspca - ov534: Add Saturation control
>   Input: move drivers/input/fixp-arith.h to include/linux
>   gspca - ov534: Add Hue control
> 
>  drivers/input/ff-memless.c        |    3 +-
>  drivers/input/fixp-arith.h        |   87 ----------------------
>  drivers/media/video/gspca/ov534.c |  146 +++++++++++++++++++++++++++----------
>  include/linux/fixp-arith.h        |   87 ++++++++++++++++++++++
>  4 files changed, 195 insertions(+), 128 deletions(-)
>  delete mode 100644 drivers/input/fixp-arith.h
>  create mode 100644 include/linux/fixp-arith.h
> 

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v2 0/3] gspca - ov534: saturation and hue (using fixp-arith.h)

[Antonio Ospite]

[-- Attachment #1: Type: text/plain, Size: 605 bytes --]

On Sun,  6 May 2012 12:14:55 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Hi,

[...]
> 
> Antonio Ospite (3):
>   gspca - ov534: Add Saturation control
>   Input: move drivers/input/fixp-arith.h to include/linux
>   gspca - ov534: Add Hue control
> 

These changes are now in
http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.5

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]