| Message ID | u63kbpxm9.wl%morimoto.kuninori@renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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
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 */ @@ -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, diff --git a/include/media/ov772x.h b/include/media/ov772x.h index e391d55..57db48d 100644 --- a/include/media/ov772x.h +++ b/include/media/ov772x.h @@ -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; };
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(-)