[v2,2/2] ov772x: Add image flip support
Commit Message
o ov772x_camera_info :: flags supports default image flip.
o V4L2_CID_VFLIP/HFLIP supports image flip on user side.
Thank Magnus for advice.
Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
drivers/media/video/ov772x.c | 97 ++++++++++++++++++++++++++++++++++++++++--
include/media/ov772x.h | 5 ++
2 files changed, 98 insertions(+), 4 deletions(-)
Comments
On Mon, 19 Jan 2009, Kuninori Morimoto wrote:
> o ov772x_camera_info :: flags supports default image flip.
> o V4L2_CID_VFLIP/HFLIP supports image flip on user side.
> Thank Magnus for advice.
>
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
> ---
> drivers/media/video/ov772x.c | 97 ++++++++++++++++++++++++++++++++++++++++--
> include/media/ov772x.h | 5 ++
> 2 files changed, 98 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
> index 3341857..ca0cf3c 100644
> --- a/drivers/media/video/ov772x.c
> +++ b/drivers/media/video/ov772x.c
> @@ -218,9 +218,10 @@
>
> /* COM3 */
> #define SWAP_MASK 0x38
> +#define IMG_MASK 0xC0
>
> -#define VFIMG_ON_OFF 0x80 /* Vertical flip image ON/OFF selection */
> -#define HMIMG_ON_OFF 0x40 /* Horizontal mirror image ON/OFF selection */
> +#define VFLIP_IMG 0x80 /* Vertical flip image ON/OFF selection */
> +#define HFLIP_IMG 0x40 /* Horizontal mirror image ON/OFF selection */
> #define SWAP_RGB 0x20 /* Swap B/R output sequence in RGB mode */
> #define SWAP_YUV 0x10 /* Swap Y/UV output sequence in YUV mode */
> #define SWAP_ML 0x08 /* Swap output MSB/LSB */
Please, put SWAP_MASK and IMG_MASK below single bit defines and define
them as
#define SWAP_MASK (SWAP_RGB | SWAP_YUV | SWAP_ML)
#define IMG_MASK (VFLIP_IMG | HFLIP_IMG)
> @@ -540,6 +541,27 @@ static const struct ov772x_win_size ov772x_win_qvga = {
> .regs = ov772x_qvga_regs,
> };
>
> +static const struct v4l2_queryctrl ov772x_controls[] = {
> + {
> + .id = V4L2_CID_VFLIP,
> + .type = V4L2_CTRL_TYPE_BOOLEAN,
> + .name = "Flip Vertically",
> + .minimum = 0,
> + .maximum = 1,
> + .step = 1,
> + .default_value = 0,
> + },
> + {
> + .id = V4L2_CID_HFLIP,
> + .type = V4L2_CTRL_TYPE_BOOLEAN,
> + .name = "Flip Horizontally",
> + .minimum = 0,
> + .maximum = 1,
> + .step = 1,
> + .default_value = 0,
> + },
> +};
> +
>
> /*
> * general function
> @@ -650,6 +672,60 @@ static unsigned long ov772x_query_bus_param(struct soc_camera_device *icd)
> return soc_camera_apply_sensor_flags(icl, flags);
> }
>
> +static int ov772x_get_control(struct soc_camera_device *icd,
> + struct v4l2_control *ctrl)
> +{
> + struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
> + s32 val;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_VFLIP:
> + val = i2c_smbus_read_byte_data(priv->client, COM3);
> + if (val < 0)
> + return val;
> + if (priv->info->flags & OV772X_FLAG_VFLIP)
> + val ^= VFLIP_IMG;
> + val &= VFLIP_IMG;
> + ctrl->value = !!val;
> + break;
> + case V4L2_CID_HFLIP:
> + val = i2c_smbus_read_byte_data(priv->client, COM3);
> + if (val < 0)
> + return val;
> + if (priv->info->flags & OV772X_FLAG_HFLIP)
> + val ^= HFLIP_IMG;
> + val &= HFLIP_IMG;
> + ctrl->value = !!val;
> + break;
> + }
> + return 0;
> +}
> +
> +static int ov772x_set_control(struct soc_camera_device *icd,
> + struct v4l2_control *ctrl)
> +{
> + struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
> + int ret = 0;
> + u8 val;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_VFLIP:
> + val = (ctrl->value) ? VFLIP_IMG : 0x00;
Superfluous parenthesis
> + if (priv->info->flags & OV772X_FLAG_VFLIP)
> + val ^= VFLIP_IMG;
> + ret = ov772x_mask_set(priv->client, COM3, VFLIP_IMG, val);
> + break;
> + case V4L2_CID_HFLIP:
> + val = (ctrl->value) ? HFLIP_IMG : 0x00;
ditto
> + if (priv->info->flags & OV772X_FLAG_HFLIP)
> + val ^= HFLIP_IMG;
> + ret = ov772x_mask_set(priv->client, COM3, HFLIP_IMG, val);
> + break;
> + }
> +
> + return ret;
> +}
> +
> static int ov772x_get_chip_id(struct soc_camera_device *icd,
> struct v4l2_dbg_chip_ident *id)
> {
> @@ -720,7 +796,7 @@ static int ov772x_set_fmt(struct soc_camera_device *icd,
> {
> struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
> int ret = -EINVAL;
> - u8 val;
> + u8 val, mask;
> int i;
>
> /*
> @@ -768,8 +844,17 @@ static int ov772x_set_fmt(struct soc_camera_device *icd,
> * set COM3
> */
> val = priv->fmt->com3;
> + if (priv->info->flags & OV772X_FLAG_VFLIP)
> + val |= VFLIP_IMG;
> + if (priv->info->flags & OV772X_FLAG_HFLIP)
> + val |= HFLIP_IMG;
> +
> + mask = SWAP_MASK;
> + if (IMG_MASK & val)
> + mask |= IMG_MASK;
> +
> ret = ov772x_mask_set(priv->client,
> - COM3, SWAP_MASK, val);
> + COM3, mask, val);
Do I understand it right, that this throws away any flip control settings
performed before S_FMT? You probably want to set priv->fmt->com3 in your
set_control and XOR instead of an OR here as well. Or was this
intentional?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Guennadi
Thank you for checking patch
> > @@ -768,8 +844,17 @@ static int ov772x_set_fmt(struct soc_camera_device *icd,
> > * set COM3
> > */
> > val = priv->fmt->com3;
> > + if (priv->info->flags & OV772X_FLAG_VFLIP)
> > + val |= VFLIP_IMG;
> > + if (priv->info->flags & OV772X_FLAG_HFLIP)
> > + val |= HFLIP_IMG;
> > +
> > + mask = SWAP_MASK;
> > + if (IMG_MASK & val)
> > + mask |= IMG_MASK;
> > +
> > ret = ov772x_mask_set(priv->client,
> > - COM3, SWAP_MASK, val);
> > + COM3, mask, val);
>
> Do I understand it right, that this throws away any flip control settings
> performed before S_FMT? You probably want to set priv->fmt->com3 in your
> set_control and XOR instead of an OR here as well. Or was this
> intentional?
Sorry, I can not understand what you want to say.
I think set_fmt function set default flip control.
And set_control function change flip on/off.
Therefore OR operation on set_fmt is correct I think.
And set_control use only XOR. priv->fmt->com3 is not needed here.
Do you say should I remember flip value ?
Or am I wrong understanding ?
Best regards
--
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 19 Jan 2009, morimoto.kuninori@renesas.com wrote:
> > > @@ -768,8 +844,17 @@ static int ov772x_set_fmt(struct soc_camera_device *icd,
> > > * set COM3
> > > */
> > > val = priv->fmt->com3;
> > > + if (priv->info->flags & OV772X_FLAG_VFLIP)
> > > + val |= VFLIP_IMG;
> > > + if (priv->info->flags & OV772X_FLAG_HFLIP)
> > > + val |= HFLIP_IMG;
> > > +
> > > + mask = SWAP_MASK;
> > > + if (IMG_MASK & val)
> > > + mask |= IMG_MASK;
> > > +
> > > ret = ov772x_mask_set(priv->client,
> > > - COM3, SWAP_MASK, val);
> > > + COM3, mask, val);
> >
> > Do I understand it right, that this throws away any flip control settings
> > performed before S_FMT? You probably want to set priv->fmt->com3 in your
> > set_control and XOR instead of an OR here as well. Or was this
> > intentional?
>
> Sorry, I can not understand what you want to say.
> I think set_fmt function set default flip control.
> And set_control function change flip on/off.
> Therefore OR operation on set_fmt is correct I think.
> And set_control use only XOR. priv->fmt->com3 is not needed here.
> Do you say should I remember flip value ?
I think, yes. If someone sets vertical or horizontal flip using the
respective control, and then calls S_FMT, I think, flip should be
preserved. S_FMT is in no way a reset, it only sets fields that are
explicitly passed with it - pixel format, image size, etc.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -218,9 +218,10 @@
/* COM3 */
#define SWAP_MASK 0x38
+#define IMG_MASK 0xC0
-#define VFIMG_ON_OFF 0x80 /* Vertical flip image ON/OFF selection */
-#define HMIMG_ON_OFF 0x40 /* Horizontal mirror image ON/OFF selection */
+#define VFLIP_IMG 0x80 /* Vertical flip image ON/OFF selection */
+#define HFLIP_IMG 0x40 /* Horizontal mirror image ON/OFF selection */
#define SWAP_RGB 0x20 /* Swap B/R output sequence in RGB mode */
#define SWAP_YUV 0x10 /* Swap Y/UV output sequence in YUV mode */
#define SWAP_ML 0x08 /* Swap output MSB/LSB */
@@ -540,6 +541,27 @@ static const struct ov772x_win_size ov772x_win_qvga = {
.regs = ov772x_qvga_regs,
};
+static const struct v4l2_queryctrl ov772x_controls[] = {
+ {
+ .id = V4L2_CID_VFLIP,
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ .name = "Flip Vertically",
+ .minimum = 0,
+ .maximum = 1,
+ .step = 1,
+ .default_value = 0,
+ },
+ {
+ .id = V4L2_CID_HFLIP,
+ .type = V4L2_CTRL_TYPE_BOOLEAN,
+ .name = "Flip Horizontally",
+ .minimum = 0,
+ .maximum = 1,
+ .step = 1,
+ .default_value = 0,
+ },
+};
+
/*
* general function
@@ -650,6 +672,60 @@ static unsigned long ov772x_query_bus_param(struct soc_camera_device *icd)
return soc_camera_apply_sensor_flags(icl, flags);
}
+static int ov772x_get_control(struct soc_camera_device *icd,
+ struct v4l2_control *ctrl)
+{
+ struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+ s32 val;
+
+ switch (ctrl->id) {
+ case V4L2_CID_VFLIP:
+ val = i2c_smbus_read_byte_data(priv->client, COM3);
+ if (val < 0)
+ return val;
+ if (priv->info->flags & OV772X_FLAG_VFLIP)
+ val ^= VFLIP_IMG;
+ val &= VFLIP_IMG;
+ ctrl->value = !!val;
+ break;
+ case V4L2_CID_HFLIP:
+ val = i2c_smbus_read_byte_data(priv->client, COM3);
+ if (val < 0)
+ return val;
+ if (priv->info->flags & OV772X_FLAG_HFLIP)
+ val ^= HFLIP_IMG;
+ val &= HFLIP_IMG;
+ ctrl->value = !!val;
+ break;
+ }
+ return 0;
+}
+
+static int ov772x_set_control(struct soc_camera_device *icd,
+ struct v4l2_control *ctrl)
+{
+ struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
+ int ret = 0;
+ u8 val;
+
+ switch (ctrl->id) {
+ case V4L2_CID_VFLIP:
+ val = (ctrl->value) ? VFLIP_IMG : 0x00;
+ if (priv->info->flags & OV772X_FLAG_VFLIP)
+ val ^= VFLIP_IMG;
+ ret = ov772x_mask_set(priv->client, COM3, VFLIP_IMG, val);
+ break;
+ case V4L2_CID_HFLIP:
+ val = (ctrl->value) ? HFLIP_IMG : 0x00;
+ if (priv->info->flags & OV772X_FLAG_HFLIP)
+ val ^= HFLIP_IMG;
+ ret = ov772x_mask_set(priv->client, COM3, HFLIP_IMG, val);
+ break;
+ }
+
+ return ret;
+}
+
static int ov772x_get_chip_id(struct soc_camera_device *icd,
struct v4l2_dbg_chip_ident *id)
{
@@ -720,7 +796,7 @@ static int ov772x_set_fmt(struct soc_camera_device *icd,
{
struct ov772x_priv *priv = container_of(icd, struct ov772x_priv, icd);
int ret = -EINVAL;
- u8 val;
+ u8 val, mask;
int i;
/*
@@ -768,8 +844,17 @@ static int ov772x_set_fmt(struct soc_camera_device *icd,
* set COM3
*/
val = priv->fmt->com3;
+ if (priv->info->flags & OV772X_FLAG_VFLIP)
+ val |= VFLIP_IMG;
+ if (priv->info->flags & OV772X_FLAG_HFLIP)
+ val |= HFLIP_IMG;
+
+ mask = SWAP_MASK;
+ if (IMG_MASK & val)
+ mask |= IMG_MASK;
+
ret = ov772x_mask_set(priv->client,
- COM3, SWAP_MASK, val);
+ COM3, mask, val);
if (ret < 0)
goto ov772x_set_fmt_error;
@@ -887,6 +972,10 @@ static struct soc_camera_ops ov772x_ops = {
.try_fmt = ov772x_try_fmt,
.set_bus_param = ov772x_set_bus_param,
.query_bus_param = ov772x_query_bus_param,
+ .controls = ov772x_controls,
+ .num_controls = ARRAY_SIZE(ov772x_controls),
+ .get_control = ov772x_get_control,
+ .set_control = ov772x_set_control,
.get_chip_id = ov772x_get_chip_id,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.get_register = ov772x_get_register,
@@ -13,8 +13,13 @@
#include <media/soc_camera.h>
+/* for flags */
+#define OV772X_FLAG_VFLIP 0x00000001 /* Vertical flip image */
+#define OV772X_FLAG_HFLIP 0x00000002 /* Horizontal flip image */
+
struct ov772x_camera_info {
unsigned long buswidth;
+ unsigned long flags;
struct soc_camera_link link;
};