* [PATCH 01/13 v3] soc_camera: add control handler support
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
@ 2011-09-08 8:43 ` Guennadi Liakhovetski
2011-09-08 8:43 ` [PATCH 02/13 v3] sh_mobile_ceu_camera: implement the control handler Guennadi Liakhovetski
` (11 subsequent siblings)
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:43 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
The soc_camera framework is switched over to use the control framework.
After this patch none of the controls in subdevs or host drivers are available,
until those drivers are also converted to the control framework.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: moved code around, fixed problems]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/soc_camera.c | 94 ++++++++------------------------------
include/media/soc_camera.h | 2 +
2 files changed, 21 insertions(+), 75 deletions(-)
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index f0996de..6bd6fd9 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -554,6 +554,7 @@ static int soc_camera_open(struct file *file)
if (ret < 0)
goto einitvb;
}
+ v4l2_ctrl_handler_setup(&icd->ctrl_handler);
}
file->private_data = icd;
@@ -823,78 +824,6 @@ static int soc_camera_streamoff(struct file *file, void *priv,
return 0;
}
-static int soc_camera_queryctrl(struct file *file, void *priv,
- struct v4l2_queryctrl *qc)
-{
- struct soc_camera_device *icd = file->private_data;
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- int i;
-
- WARN_ON(priv != file->private_data);
-
- if (!qc->id)
- return -EINVAL;
-
- /* First check host controls */
- for (i = 0; i < ici->ops->num_controls; i++)
- if (qc->id == ici->ops->controls[i].id) {
- memcpy(qc, &(ici->ops->controls[i]),
- sizeof(*qc));
- return 0;
- }
-
- if (!icd->ops)
- return -EINVAL;
-
- /* Then device controls */
- for (i = 0; i < icd->ops->num_controls; i++)
- if (qc->id == icd->ops->controls[i].id) {
- memcpy(qc, &(icd->ops->controls[i]),
- sizeof(*qc));
- return 0;
- }
-
- return -EINVAL;
-}
-
-static int soc_camera_g_ctrl(struct file *file, void *priv,
- struct v4l2_control *ctrl)
-{
- struct soc_camera_device *icd = file->private_data;
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
- int ret;
-
- WARN_ON(priv != file->private_data);
-
- if (ici->ops->get_ctrl) {
- ret = ici->ops->get_ctrl(icd, ctrl);
- if (ret != -ENOIOCTLCMD)
- return ret;
- }
-
- return v4l2_subdev_call(sd, core, g_ctrl, ctrl);
-}
-
-static int soc_camera_s_ctrl(struct file *file, void *priv,
- struct v4l2_control *ctrl)
-{
- struct soc_camera_device *icd = file->private_data;
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
- int ret;
-
- WARN_ON(priv != file->private_data);
-
- if (ici->ops->set_ctrl) {
- ret = ici->ops->set_ctrl(icd, ctrl);
- if (ret != -ENOIOCTLCMD)
- return ret;
- }
-
- return v4l2_subdev_call(sd, core, s_ctrl, ctrl);
-}
-
static int soc_camera_cropcap(struct file *file, void *fh,
struct v4l2_cropcap *a)
{
@@ -1097,6 +1026,17 @@ static int soc_camera_probe(struct soc_camera_device *icd)
dev_info(icd->pdev, "Probing %s\n", dev_name(icd->pdev));
+ /*
+ * Currently the subdev with the largest number of controls (13) is
+ * ov6550. So let's pick 16 as a hint for the control handler. Note
+ * that this is a hint only: too large and you waste some memory, too
+ * small and there is a (very) small performance hit when looking up
+ * controls in the internal hash.
+ */
+ ret = v4l2_ctrl_handler_init(&icd->ctrl_handler, 16);
+ if (ret < 0)
+ return ret;
+
ret = regulator_bulk_get(icd->pdev, icl->num_regulators,
icl->regulators);
if (ret < 0)
@@ -1157,6 +1097,9 @@ static int soc_camera_probe(struct soc_camera_device *icd)
sd = soc_camera_to_subdev(icd);
sd->grp_id = (long)icd;
+ if (v4l2_ctrl_add_handler(&icd->ctrl_handler, sd->ctrl_handler))
+ goto ectrl;
+
/* At this point client .probe() should have run already */
ret = soc_camera_init_user_formats(icd);
if (ret < 0)
@@ -1201,6 +1144,7 @@ evidstart:
mutex_unlock(&icd->video_lock);
soc_camera_free_user_formats(icd);
eiufmt:
+ectrl:
if (icl->board_info) {
soc_camera_free_i2c(icd);
} else {
@@ -1218,6 +1162,7 @@ eadd:
epower:
regulator_bulk_free(icl->num_regulators, icl->regulators);
ereg:
+ v4l2_ctrl_handler_free(&icd->ctrl_handler);
return ret;
}
@@ -1232,6 +1177,7 @@ static int soc_camera_remove(struct soc_camera_device *icd)
BUG_ON(!icd->parent);
+ v4l2_ctrl_handler_free(&icd->ctrl_handler);
if (vdev) {
video_unregister_device(vdev);
icd->vdev = NULL;
@@ -1439,9 +1385,6 @@ static const struct v4l2_ioctl_ops soc_camera_ioctl_ops = {
.vidioc_prepare_buf = soc_camera_prepare_buf,
.vidioc_streamon = soc_camera_streamon,
.vidioc_streamoff = soc_camera_streamoff,
- .vidioc_queryctrl = soc_camera_queryctrl,
- .vidioc_g_ctrl = soc_camera_g_ctrl,
- .vidioc_s_ctrl = soc_camera_s_ctrl,
.vidioc_cropcap = soc_camera_cropcap,
.vidioc_g_crop = soc_camera_g_crop,
.vidioc_s_crop = soc_camera_s_crop,
@@ -1470,6 +1413,7 @@ static int video_dev_create(struct soc_camera_device *icd)
vdev->ioctl_ops = &soc_camera_ioctl_ops;
vdev->release = video_device_release;
vdev->tvnorms = V4L2_STD_UNKNOWN;
+ vdev->ctrl_handler = &icd->ctrl_handler;
vdev->lock = &icd->video_lock;
icd->vdev = vdev;
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 1864e22..2e15e17 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -19,6 +19,7 @@
#include <linux/videodev2.h>
#include <media/videobuf-core.h>
#include <media/videobuf2-core.h>
+#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
struct file;
@@ -40,6 +41,7 @@ struct soc_camera_device {
struct soc_camera_sense *sense; /* See comment in struct definition */
struct soc_camera_ops *ops;
struct video_device *vdev;
+ struct v4l2_ctrl_handler ctrl_handler;
const struct soc_camera_format_xlate *current_fmt;
struct soc_camera_format_xlate *user_formats;
int num_user_formats;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 02/13 v3] sh_mobile_ceu_camera: implement the control handler.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
2011-09-08 8:43 ` [PATCH 01/13 v3] soc_camera: add control handler support Guennadi Liakhovetski
@ 2011-09-08 8:43 ` Guennadi Liakhovetski
2011-09-08 8:43 ` [PATCH 03/13 v3] ov9640: convert to the control framework Guennadi Liakhovetski
` (10 subsequent siblings)
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:43 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
And since this is the last and only host driver that uses controls, also
remove the now obsolete control fields from soc_camera.h.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: moved code around, fixed problems]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/sh_mobile_ceu_camera.c | 91 ++++++++++++----------------
include/media/soc_camera.h | 4 -
2 files changed, 38 insertions(+), 57 deletions(-)
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 8711aa8..955947a 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -997,6 +997,38 @@ static bool sh_mobile_ceu_packing_supported(const struct soc_mbus_pixelfmt *fmt)
static int client_g_rect(struct v4l2_subdev *sd, struct v4l2_rect *rect);
+static struct soc_camera_device *ctrl_to_icd(struct v4l2_ctrl *ctrl)
+{
+ return container_of(ctrl->handler, struct soc_camera_device,
+ ctrl_handler);
+}
+
+static int sh_mobile_ceu_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct soc_camera_device *icd = ctrl_to_icd(ctrl);
+ struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+ struct sh_mobile_ceu_dev *pcdev = ici->priv;
+
+ switch (ctrl->id) {
+ case V4L2_CID_SHARPNESS:
+ switch (icd->current_fmt->host_fmt->fourcc) {
+ case V4L2_PIX_FMT_NV12:
+ case V4L2_PIX_FMT_NV21:
+ case V4L2_PIX_FMT_NV16:
+ case V4L2_PIX_FMT_NV61:
+ ceu_write(pcdev, CLFCR, !ctrl->val);
+ return 0;
+ }
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct v4l2_ctrl_ops sh_mobile_ceu_ctrl_ops = {
+ .s_ctrl = sh_mobile_ceu_s_ctrl,
+};
+
static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, unsigned int idx,
struct soc_camera_format_xlate *xlate)
{
@@ -1033,6 +1065,12 @@ static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, unsigned int
struct v4l2_rect rect;
int shift = 0;
+ /* Add our control */
+ v4l2_ctrl_new_std(&icd->ctrl_handler, &sh_mobile_ceu_ctrl_ops,
+ V4L2_CID_SHARPNESS, 0, 1, 1, 0);
+ if (icd->ctrl_handler.error)
+ return icd->ctrl_handler.error;
+
/* FIXME: subwindow is lost between close / open */
/* Cache current client geometry */
@@ -1961,55 +1999,6 @@ static int sh_mobile_ceu_init_videobuf(struct vb2_queue *q,
return vb2_queue_init(q);
}
-static int sh_mobile_ceu_get_ctrl(struct soc_camera_device *icd,
- struct v4l2_control *ctrl)
-{
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct sh_mobile_ceu_dev *pcdev = ici->priv;
- u32 val;
-
- switch (ctrl->id) {
- case V4L2_CID_SHARPNESS:
- val = ceu_read(pcdev, CLFCR);
- ctrl->value = val ^ 1;
- return 0;
- }
- return -ENOIOCTLCMD;
-}
-
-static int sh_mobile_ceu_set_ctrl(struct soc_camera_device *icd,
- struct v4l2_control *ctrl)
-{
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct sh_mobile_ceu_dev *pcdev = ici->priv;
-
- switch (ctrl->id) {
- case V4L2_CID_SHARPNESS:
- switch (icd->current_fmt->host_fmt->fourcc) {
- case V4L2_PIX_FMT_NV12:
- case V4L2_PIX_FMT_NV21:
- case V4L2_PIX_FMT_NV16:
- case V4L2_PIX_FMT_NV61:
- ceu_write(pcdev, CLFCR, !ctrl->value);
- return 0;
- }
- return -EINVAL;
- }
- return -ENOIOCTLCMD;
-}
-
-static const struct v4l2_queryctrl sh_mobile_ceu_controls[] = {
- {
- .id = V4L2_CID_SHARPNESS,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Low-pass filter",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 0,
- },
-};
-
static struct soc_camera_host_ops sh_mobile_ceu_host_ops = {
.owner = THIS_MODULE,
.add = sh_mobile_ceu_add_device,
@@ -2021,14 +2010,10 @@ static struct soc_camera_host_ops sh_mobile_ceu_host_ops = {
.set_livecrop = sh_mobile_ceu_set_livecrop,
.set_fmt = sh_mobile_ceu_set_fmt,
.try_fmt = sh_mobile_ceu_try_fmt,
- .set_ctrl = sh_mobile_ceu_set_ctrl,
- .get_ctrl = sh_mobile_ceu_get_ctrl,
.poll = sh_mobile_ceu_poll,
.querycap = sh_mobile_ceu_querycap,
.set_bus_param = sh_mobile_ceu_set_bus_param,
.init_videobuf2 = sh_mobile_ceu_init_videobuf,
- .controls = sh_mobile_ceu_controls,
- .num_controls = ARRAY_SIZE(sh_mobile_ceu_controls),
};
struct bus_wait {
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 2e15e17..d41b8bd 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -96,14 +96,10 @@ struct soc_camera_host_ops {
int (*reqbufs)(struct soc_camera_device *, struct v4l2_requestbuffers *);
int (*querycap)(struct soc_camera_host *, struct v4l2_capability *);
int (*set_bus_param)(struct soc_camera_device *, __u32);
- int (*get_ctrl)(struct soc_camera_device *, struct v4l2_control *);
- int (*set_ctrl)(struct soc_camera_device *, struct v4l2_control *);
int (*get_parm)(struct soc_camera_device *, struct v4l2_streamparm *);
int (*set_parm)(struct soc_camera_device *, struct v4l2_streamparm *);
int (*enum_fsizes)(struct soc_camera_device *, struct v4l2_frmsizeenum *);
unsigned int (*poll)(struct file *, poll_table *);
- const struct v4l2_queryctrl *controls;
- int num_controls;
};
#define SOCAM_SENSOR_INVERT_PCLK (1 << 0)
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 03/13 v3] ov9640: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
2011-09-08 8:43 ` [PATCH 01/13 v3] soc_camera: add control handler support Guennadi Liakhovetski
2011-09-08 8:43 ` [PATCH 02/13 v3] sh_mobile_ceu_camera: implement the control handler Guennadi Liakhovetski
@ 2011-09-08 8:43 ` Guennadi Liakhovetski
2011-09-08 8:43 ` [PATCH 04/13 v3] ov772x: " Guennadi Liakhovetski
` (9 subsequent siblings)
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:43 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/ov9640.c | 119 +++++++++++++-----------------------------
drivers/media/video/ov9640.h | 4 +-
2 files changed, 38 insertions(+), 85 deletions(-)
diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
index 352a8fb..12d33a9 100644
--- a/drivers/media/video/ov9640.c
+++ b/drivers/media/video/ov9640.c
@@ -30,6 +30,7 @@
#include <media/soc_mediabus.h>
#include <media/v4l2-chip-ident.h>
#include <media/v4l2-common.h>
+#include <media/v4l2-ctrls.h>
#include "ov9640.h"
@@ -164,27 +165,6 @@ static enum v4l2_mbus_pixelcode ov9640_codes[] = {
V4L2_MBUS_FMT_RGB565_2X8_LE,
};
-static const struct v4l2_queryctrl ov9640_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,
- },
-};
-
/* read a register */
static int ov9640_reg_read(struct i2c_client *client, u8 reg, u8 *val)
{
@@ -286,52 +266,25 @@ static int ov9640_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
}
-/* Get status of additional camera capabilities */
-static int ov9640_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
- struct ov9640_priv *priv = to_ov9640_sensor(sd);
-
- switch (ctrl->id) {
- case V4L2_CID_VFLIP:
- ctrl->value = priv->flag_vflip;
- break;
- case V4L2_CID_HFLIP:
- ctrl->value = priv->flag_hflip;
- break;
- }
- return 0;
-}
-
/* Set status of additional camera capabilities */
-static int ov9640_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov9640_s_ctrl(struct v4l2_ctrl *ctrl)
{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct ov9640_priv *priv = to_ov9640_sensor(sd);
-
- int ret = 0;
+ struct ov9640_priv *priv = container_of(ctrl->handler, struct ov9640_priv, hdl);
+ struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev);
switch (ctrl->id) {
case V4L2_CID_VFLIP:
- priv->flag_vflip = ctrl->value;
- if (ctrl->value)
- ret = ov9640_reg_rmw(client, OV9640_MVFP,
+ if (ctrl->val)
+ return ov9640_reg_rmw(client, OV9640_MVFP,
OV9640_MVFP_V, 0);
- else
- ret = ov9640_reg_rmw(client, OV9640_MVFP,
- 0, OV9640_MVFP_V);
- break;
+ return ov9640_reg_rmw(client, OV9640_MVFP, 0, OV9640_MVFP_V);
case V4L2_CID_HFLIP:
- priv->flag_hflip = ctrl->value;
- if (ctrl->value)
- ret = ov9640_reg_rmw(client, OV9640_MVFP,
+ if (ctrl->val)
+ return ov9640_reg_rmw(client, OV9640_MVFP,
OV9640_MVFP_H, 0);
- else
- ret = ov9640_reg_rmw(client, OV9640_MVFP,
- 0, OV9640_MVFP_H);
- break;
+ return ov9640_reg_rmw(client, OV9640_MVFP, 0, OV9640_MVFP_H);
}
-
- return ret;
+ return -EINVAL;
}
/* Get chip identification */
@@ -643,20 +596,14 @@ static int ov9640_video_probe(struct soc_camera_device *icd,
*/
ret = ov9640_reg_read(client, OV9640_PID, &pid);
+ if (!ret)
+ ret = ov9640_reg_read(client, OV9640_VER, &ver);
+ if (!ret)
+ ret = ov9640_reg_read(client, OV9640_MIDH, &midh);
+ if (!ret)
+ ret = ov9640_reg_read(client, OV9640_MIDL, &midl);
if (ret)
- goto err;
-
- ret = ov9640_reg_read(client, OV9640_VER, &ver);
- if (ret)
- goto err;
-
- ret = ov9640_reg_read(client, OV9640_MIDH, &midh);
- if (ret)
- goto err;
-
- ret = ov9640_reg_read(client, OV9640_MIDL, &midl);
- if (ret)
- goto err;
+ return ret;
switch (VERSION(pid, ver)) {
case OV9640_V2:
@@ -670,25 +617,20 @@ static int ov9640_video_probe(struct soc_camera_device *icd,
break;
default:
dev_err(&client->dev, "Product ID error %x:%x\n", pid, ver);
- ret = -ENODEV;
- goto err;
+ return -ENODEV;
}
dev_info(&client->dev, "%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
devname, pid, ver, midh, midl);
-err:
- return ret;
+ return v4l2_ctrl_handler_setup(&priv->hdl);
}
-static struct soc_camera_ops ov9640_ops = {
- .controls = ov9640_controls,
- .num_controls = ARRAY_SIZE(ov9640_controls),
+static const struct v4l2_ctrl_ops ov9640_ctrl_ops = {
+ .s_ctrl = ov9640_s_ctrl,
};
static struct v4l2_subdev_core_ops ov9640_core_ops = {
- .g_ctrl = ov9640_g_ctrl,
- .s_ctrl = ov9640_s_ctrl,
.g_chip_ident = ov9640_g_chip_ident,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = ov9640_get_register,
@@ -760,12 +702,23 @@ static int ov9640_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(&priv->subdev, client, &ov9640_subdev_ops);
- icd->ops = &ov9640_ops;
+ v4l2_ctrl_handler_init(&priv->hdl, 2);
+ v4l2_ctrl_new_std(&priv->hdl, &ov9640_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&priv->hdl, &ov9640_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ priv->subdev.ctrl_handler = &priv->hdl;
+ if (priv->hdl.error) {
+ int err = priv->hdl.error;
+
+ kfree(priv);
+ return err;
+ }
ret = ov9640_video_probe(icd, client);
if (ret) {
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
}
@@ -777,6 +730,8 @@ static int ov9640_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct ov9640_priv *priv = to_ov9640_sensor(sd);
+ v4l2_device_unregister_subdev(&priv->subdev);
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
return 0;
}
diff --git a/drivers/media/video/ov9640.h b/drivers/media/video/ov9640.h
index f8a51b7..6b33a97 100644
--- a/drivers/media/video/ov9640.h
+++ b/drivers/media/video/ov9640.h
@@ -198,12 +198,10 @@ struct ov9640_reg {
struct ov9640_priv {
struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
int model;
int revision;
-
- bool flag_vflip;
- bool flag_hflip;
};
#endif /* __DRIVERS_MEDIA_VIDEO_OV9640_H__ */
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 04/13 v3] ov772x: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (2 preceding siblings ...)
2011-09-08 8:43 ` [PATCH 03/13 v3] ov9640: convert to the control framework Guennadi Liakhovetski
@ 2011-09-08 8:43 ` Guennadi Liakhovetski
2011-09-08 8:43 ` [PATCH 05/13 v3] rj54n1cb0c: " Guennadi Liakhovetski
` (8 subsequent siblings)
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:43 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/ov772x.c | 114 +++++++++++++----------------------------
1 files changed, 36 insertions(+), 78 deletions(-)
diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index f77e899..9b54042 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -25,6 +25,7 @@
#include <media/ov772x.h>
#include <media/soc_camera.h>
#include <media/soc_mediabus.h>
+#include <media/v4l2-ctrls.h>
#include <media/v4l2-chip-ident.h>
#include <media/v4l2-subdev.h>
@@ -401,6 +402,7 @@ struct ov772x_win_size {
struct ov772x_priv {
struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
struct ov772x_camera_info *info;
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
@@ -518,36 +520,6 @@ 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,
- },
- {
- .id = V4L2_CID_BAND_STOP_FILTER,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Band-stop filter",
- .minimum = 0,
- .maximum = 256,
- .step = 1,
- .default_value = 0,
- },
-};
-
/*
* general function
*/
@@ -621,52 +593,30 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
}
-static int ov772x_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
- struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
-
- switch (ctrl->id) {
- case V4L2_CID_VFLIP:
- ctrl->value = priv->flag_vflip;
- break;
- case V4L2_CID_HFLIP:
- ctrl->value = priv->flag_hflip;
- break;
- case V4L2_CID_BAND_STOP_FILTER:
- ctrl->value = priv->band_filter;
- break;
- }
- return 0;
-}
-
-static int ov772x_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
{
+ struct ov772x_priv *priv = container_of(ctrl->handler,
+ struct ov772x_priv, hdl);
+ struct v4l2_subdev *sd = &priv->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct ov772x_priv *priv = container_of(sd, struct ov772x_priv, subdev);
int ret = 0;
u8 val;
switch (ctrl->id) {
case V4L2_CID_VFLIP:
- val = ctrl->value ? VFLIP_IMG : 0x00;
- priv->flag_vflip = ctrl->value;
+ val = ctrl->val ? VFLIP_IMG : 0x00;
+ priv->flag_vflip = ctrl->val;
if (priv->info->flags & OV772X_FLAG_VFLIP)
val ^= VFLIP_IMG;
- ret = ov772x_mask_set(client, COM3, VFLIP_IMG, val);
- break;
+ return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
case V4L2_CID_HFLIP:
- val = ctrl->value ? HFLIP_IMG : 0x00;
- priv->flag_hflip = ctrl->value;
+ val = ctrl->val ? HFLIP_IMG : 0x00;
+ priv->flag_hflip = ctrl->val;
if (priv->info->flags & OV772X_FLAG_HFLIP)
val ^= HFLIP_IMG;
- ret = ov772x_mask_set(client, COM3, HFLIP_IMG, val);
- break;
+ return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
case V4L2_CID_BAND_STOP_FILTER:
- if ((unsigned)ctrl->value > 256)
- ctrl->value = 256;
- if (ctrl->value == priv->band_filter)
- break;
- if (!ctrl->value) {
+ if (!ctrl->val) {
/* Switch the filter off, it is on now */
ret = ov772x_mask_set(client, BDBASE, 0xff, 0xff);
if (!ret)
@@ -674,7 +624,7 @@ static int ov772x_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
BNDF_ON_OFF, 0);
} else {
/* Switch the filter on, set AEC low limit */
- val = 256 - ctrl->value;
+ val = 256 - ctrl->val;
ret = ov772x_mask_set(client, COM8,
BNDF_ON_OFF, BNDF_ON_OFF);
if (!ret)
@@ -682,11 +632,11 @@ static int ov772x_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
0xff, val);
}
if (!ret)
- priv->band_filter = ctrl->value;
- break;
+ priv->band_filter = ctrl->val;
+ return ret;
}
- return ret;
+ return -EINVAL;
}
static int ov772x_g_chip_ident(struct v4l2_subdev *sd,
@@ -1042,18 +992,14 @@ static int ov772x_video_probe(struct soc_camera_device *icd,
ver,
i2c_smbus_read_byte_data(client, MIDH),
i2c_smbus_read_byte_data(client, MIDL));
-
- return 0;
+ return v4l2_ctrl_handler_setup(&priv->hdl);
}
-static struct soc_camera_ops ov772x_ops = {
- .controls = ov772x_controls,
- .num_controls = ARRAY_SIZE(ov772x_controls),
+static const struct v4l2_ctrl_ops ov772x_ctrl_ops = {
+ .s_ctrl = ov772x_s_ctrl,
};
static struct v4l2_subdev_core_ops ov772x_subdev_core_ops = {
- .g_ctrl = ov772x_g_ctrl,
- .s_ctrl = ov772x_s_ctrl,
.g_chip_ident = ov772x_g_chip_ident,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = ov772x_g_register,
@@ -1139,12 +1085,24 @@ static int ov772x_probe(struct i2c_client *client,
priv->info = icl->priv;
v4l2_i2c_subdev_init(&priv->subdev, client, &ov772x_subdev_ops);
+ v4l2_ctrl_handler_init(&priv->hdl, 3);
+ v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
+ V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
+ priv->subdev.ctrl_handler = &priv->hdl;
+ if (priv->hdl.error) {
+ int err = priv->hdl.error;
- icd->ops = &ov772x_ops;
+ kfree(priv);
+ return err;
+ }
ret = ov772x_video_probe(icd, client);
if (ret) {
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
}
@@ -1154,9 +1112,9 @@ static int ov772x_probe(struct i2c_client *client,
static int ov772x_remove(struct i2c_client *client)
{
struct ov772x_priv *priv = to_ov772x(client);
- struct soc_camera_device *icd = client->dev.platform_data;
- icd->ops = NULL;
+ v4l2_device_unregister_subdev(&priv->subdev);
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
return 0;
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 05/13 v3] rj54n1cb0c: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (3 preceding siblings ...)
2011-09-08 8:43 ` [PATCH 04/13 v3] ov772x: " Guennadi Liakhovetski
@ 2011-09-08 8:43 ` Guennadi Liakhovetski
2011-09-08 8:43 ` [PATCH 06/13 v3] mt9v022: " Guennadi Liakhovetski
` (7 subsequent siblings)
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:43 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/rj54n1cb0c.c | 141 ++++++++++---------------------------
1 files changed, 38 insertions(+), 103 deletions(-)
diff --git a/drivers/media/video/rj54n1cb0c.c b/drivers/media/video/rj54n1cb0c.c
index c302211..9a87153 100644
--- a/drivers/media/video/rj54n1cb0c.c
+++ b/drivers/media/video/rj54n1cb0c.c
@@ -18,6 +18,7 @@
#include <media/soc_mediabus.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
#define RJ54N1_DEV_CODE 0x0400
#define RJ54N1_DEV_CODE2 0x0401
@@ -148,6 +149,7 @@ struct rj54n1_clock_div {
struct rj54n1 {
struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
struct rj54n1_clock_div clk_div;
const struct rj54n1_datafmt *fmt;
struct v4l2_rect rect; /* Sensor window */
@@ -1177,132 +1179,51 @@ static int rj54n1_s_register(struct v4l2_subdev *sd,
}
#endif
-static const struct v4l2_queryctrl rj54n1_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,
- }, {
- .id = V4L2_CID_GAIN,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Gain",
- .minimum = 0,
- .maximum = 127,
- .step = 1,
- .default_value = 66,
- .flags = V4L2_CTRL_FLAG_SLIDER,
- }, {
- .id = V4L2_CID_AUTO_WHITE_BALANCE,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Auto white balance",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 1,
- },
-};
-
-static struct soc_camera_ops rj54n1_ops = {
- .controls = rj54n1_controls,
- .num_controls = ARRAY_SIZE(rj54n1_controls),
-};
-
-static int rj54n1_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int rj54n1_s_ctrl(struct v4l2_ctrl *ctrl)
{
+ struct rj54n1 *rj54n1 = container_of(ctrl->handler, struct rj54n1, hdl);
+ struct v4l2_subdev *sd = &rj54n1->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct rj54n1 *rj54n1 = to_rj54n1(client);
int data;
switch (ctrl->id) {
case V4L2_CID_VFLIP:
- data = reg_read(client, RJ54N1_MIRROR_STILL_MODE);
- if (data < 0)
- return -EIO;
- ctrl->value = !(data & 1);
- break;
- case V4L2_CID_HFLIP:
- data = reg_read(client, RJ54N1_MIRROR_STILL_MODE);
- if (data < 0)
- return -EIO;
- ctrl->value = !(data & 2);
- break;
- case V4L2_CID_GAIN:
- data = reg_read(client, RJ54N1_Y_GAIN);
- if (data < 0)
- return -EIO;
-
- ctrl->value = data / 2;
- break;
- case V4L2_CID_AUTO_WHITE_BALANCE:
- ctrl->value = rj54n1->auto_wb;
- break;
- }
-
- return 0;
-}
-
-static int rj54n1_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
- int data;
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct rj54n1 *rj54n1 = to_rj54n1(client);
- const struct v4l2_queryctrl *qctrl;
-
- qctrl = soc_camera_find_qctrl(&rj54n1_ops, ctrl->id);
- if (!qctrl)
- return -EINVAL;
-
- switch (ctrl->id) {
- case V4L2_CID_VFLIP:
- if (ctrl->value)
+ if (ctrl->val)
data = reg_set(client, RJ54N1_MIRROR_STILL_MODE, 0, 1);
else
data = reg_set(client, RJ54N1_MIRROR_STILL_MODE, 1, 1);
if (data < 0)
return -EIO;
- break;
+ return 0;
case V4L2_CID_HFLIP:
- if (ctrl->value)
+ if (ctrl->val)
data = reg_set(client, RJ54N1_MIRROR_STILL_MODE, 0, 2);
else
data = reg_set(client, RJ54N1_MIRROR_STILL_MODE, 2, 2);
if (data < 0)
return -EIO;
- break;
+ return 0;
case V4L2_CID_GAIN:
- if (ctrl->value > qctrl->maximum ||
- ctrl->value < qctrl->minimum)
- return -EINVAL;
- else if (reg_write(client, RJ54N1_Y_GAIN, ctrl->value * 2) < 0)
+ if (reg_write(client, RJ54N1_Y_GAIN, ctrl->val * 2) < 0)
return -EIO;
- break;
+ return 0;
case V4L2_CID_AUTO_WHITE_BALANCE:
/* Auto WB area - whole image */
- if (reg_set(client, RJ54N1_WB_SEL_WEIGHT_I, ctrl->value << 7,
+ if (reg_set(client, RJ54N1_WB_SEL_WEIGHT_I, ctrl->val << 7,
0x80) < 0)
return -EIO;
- rj54n1->auto_wb = ctrl->value;
- break;
+ rj54n1->auto_wb = ctrl->val;
+ return 0;
}
- return 0;
+ return -EINVAL;
}
+static const struct v4l2_ctrl_ops rj54n1_ctrl_ops = {
+ .s_ctrl = rj54n1_s_ctrl,
+};
+
static struct v4l2_subdev_core_ops rj54n1_subdev_core_ops = {
- .g_ctrl = rj54n1_g_ctrl,
- .s_ctrl = rj54n1_s_ctrl,
.g_chip_ident = rj54n1_g_chip_ident,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = rj54n1_g_register,
@@ -1432,8 +1353,22 @@ static int rj54n1_probe(struct i2c_client *client,
return -ENOMEM;
v4l2_i2c_subdev_init(&rj54n1->subdev, client, &rj54n1_subdev_ops);
+ v4l2_ctrl_handler_init(&rj54n1->hdl, 4);
+ v4l2_ctrl_new_std(&rj54n1->hdl, &rj54n1_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&rj54n1->hdl, &rj54n1_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&rj54n1->hdl, &rj54n1_ctrl_ops,
+ V4L2_CID_GAIN, 0, 127, 1, 66);
+ v4l2_ctrl_new_std(&rj54n1->hdl, &rj54n1_ctrl_ops,
+ V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
+ rj54n1->subdev.ctrl_handler = &rj54n1->hdl;
+ if (rj54n1->hdl.error) {
+ int err = rj54n1->hdl.error;
- icd->ops = &rj54n1_ops;
+ kfree(rj54n1);
+ return err;
+ }
rj54n1->clk_div = clk_div;
rj54n1->rect.left = RJ54N1_COLUMN_SKIP;
@@ -1449,12 +1384,11 @@ static int rj54n1_probe(struct i2c_client *client,
ret = rj54n1_video_probe(icd, client, rj54n1_priv);
if (ret < 0) {
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&rj54n1->hdl);
kfree(rj54n1);
return ret;
}
-
- return ret;
+ return v4l2_ctrl_handler_setup(&rj54n1->hdl);
}
static int rj54n1_remove(struct i2c_client *client)
@@ -1463,9 +1397,10 @@ static int rj54n1_remove(struct i2c_client *client)
struct soc_camera_device *icd = client->dev.platform_data;
struct soc_camera_link *icl = to_soc_camera_link(icd);
- icd->ops = NULL;
+ v4l2_device_unregister_subdev(&rj54n1->subdev);
if (icl->free_bus)
icl->free_bus(icl);
+ v4l2_ctrl_handler_free(&rj54n1->hdl);
kfree(rj54n1);
return 0;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 06/13 v3] mt9v022: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (4 preceding siblings ...)
2011-09-08 8:43 ` [PATCH 05/13 v3] rj54n1cb0c: " Guennadi Liakhovetski
@ 2011-09-08 8:43 ` Guennadi Liakhovetski
2011-09-08 8:44 ` [PATCH 07/13 v3] ov2640: " Guennadi Liakhovetski
` (6 subsequent siblings)
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:43 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/mt9v022.c | 265 +++++++++++++++++------------------------
1 files changed, 107 insertions(+), 158 deletions(-)
diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
index 53149a7..7e2aeda 100644
--- a/drivers/media/video/mt9v022.c
+++ b/drivers/media/video/mt9v022.c
@@ -18,6 +18,7 @@
#include <media/soc_mediabus.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
/*
* mt9v022 i2c address 0x48, 0x4c, 0x58, 0x5c
@@ -101,6 +102,17 @@ static const struct mt9v022_datafmt mt9v022_monochrome_fmts[] = {
struct mt9v022 {
struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
+ struct {
+ /* exposure/auto-exposure cluster */
+ struct v4l2_ctrl *autoexposure;
+ struct v4l2_ctrl *exposure;
+ };
+ struct {
+ /* gain/auto-gain cluster */
+ struct v4l2_ctrl *autogain;
+ struct v4l2_ctrl *gain;
+ };
struct v4l2_rect rect; /* Sensor window */
const struct mt9v022_datafmt *fmt;
const struct mt9v022_datafmt *fmts;
@@ -179,6 +191,8 @@ static int mt9v022_init(struct i2c_client *client)
ret = reg_clear(client, MT9V022_BLACK_LEVEL_CALIB_CTRL, 1);
if (!ret)
ret = reg_write(client, MT9V022_DIGITAL_TEST_PATTERN, 0);
+ if (!ret)
+ return v4l2_ctrl_handler_setup(&mt9v022->hdl);
return ret;
}
@@ -431,215 +445,117 @@ static int mt9v022_s_register(struct v4l2_subdev *sd,
}
#endif
-static const struct v4l2_queryctrl mt9v022_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,
- }, {
- .id = V4L2_CID_GAIN,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Analog Gain",
- .minimum = 64,
- .maximum = 127,
- .step = 1,
- .default_value = 64,
- .flags = V4L2_CTRL_FLAG_SLIDER,
- }, {
- .id = V4L2_CID_EXPOSURE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Exposure",
- .minimum = 1,
- .maximum = 255,
- .step = 1,
- .default_value = 255,
- .flags = V4L2_CTRL_FLAG_SLIDER,
- }, {
- .id = V4L2_CID_AUTOGAIN,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Automatic Gain",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 1,
- }, {
- .id = V4L2_CID_EXPOSURE_AUTO,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Automatic Exposure",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 1,
- }
-};
-
-static struct soc_camera_ops mt9v022_ops = {
- .controls = mt9v022_controls,
- .num_controls = ARRAY_SIZE(mt9v022_controls),
-};
-
-static int mt9v022_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9v022_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
+ struct mt9v022 *mt9v022 = container_of(ctrl->handler,
+ struct mt9v022, hdl);
+ struct v4l2_subdev *sd = &mt9v022->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- const struct v4l2_queryctrl *qctrl;
+ struct v4l2_ctrl *gain = mt9v022->gain;
+ struct v4l2_ctrl *exp = mt9v022->exposure;
unsigned long range;
int data;
- qctrl = soc_camera_find_qctrl(&mt9v022_ops, ctrl->id);
-
switch (ctrl->id) {
- case V4L2_CID_VFLIP:
- data = reg_read(client, MT9V022_READ_MODE);
- if (data < 0)
- return -EIO;
- ctrl->value = !!(data & 0x10);
- break;
- case V4L2_CID_HFLIP:
- data = reg_read(client, MT9V022_READ_MODE);
- if (data < 0)
- return -EIO;
- ctrl->value = !!(data & 0x20);
- break;
- case V4L2_CID_EXPOSURE_AUTO:
- data = reg_read(client, MT9V022_AEC_AGC_ENABLE);
- if (data < 0)
- return -EIO;
- ctrl->value = !!(data & 0x1);
- break;
case V4L2_CID_AUTOGAIN:
- data = reg_read(client, MT9V022_AEC_AGC_ENABLE);
- if (data < 0)
- return -EIO;
- ctrl->value = !!(data & 0x2);
- break;
- case V4L2_CID_GAIN:
data = reg_read(client, MT9V022_ANALOG_GAIN);
if (data < 0)
return -EIO;
- range = qctrl->maximum - qctrl->minimum;
- ctrl->value = ((data - 16) * range + 24) / 48 + qctrl->minimum;
-
- break;
- case V4L2_CID_EXPOSURE:
+ range = gain->maximum - gain->minimum;
+ gain->val = ((data - 16) * range + 24) / 48 + gain->minimum;
+ return 0;
+ case V4L2_CID_EXPOSURE_AUTO:
data = reg_read(client, MT9V022_TOTAL_SHUTTER_WIDTH);
if (data < 0)
return -EIO;
- range = qctrl->maximum - qctrl->minimum;
- ctrl->value = ((data - 1) * range + 239) / 479 + qctrl->minimum;
-
- break;
+ range = exp->maximum - exp->minimum;
+ exp->val = ((data - 1) * range + 239) / 479 + exp->minimum;
+ return 0;
}
- return 0;
+ return -EINVAL;
}
-static int mt9v022_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9v022_s_ctrl(struct v4l2_ctrl *ctrl)
{
- int data;
+ struct mt9v022 *mt9v022 = container_of(ctrl->handler,
+ struct mt9v022, hdl);
+ struct v4l2_subdev *sd = &mt9v022->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- const struct v4l2_queryctrl *qctrl;
-
- qctrl = soc_camera_find_qctrl(&mt9v022_ops, ctrl->id);
- if (!qctrl)
- return -EINVAL;
+ int data;
switch (ctrl->id) {
case V4L2_CID_VFLIP:
- if (ctrl->value)
+ if (ctrl->val)
data = reg_set(client, MT9V022_READ_MODE, 0x10);
else
data = reg_clear(client, MT9V022_READ_MODE, 0x10);
if (data < 0)
return -EIO;
- break;
+ return 0;
case V4L2_CID_HFLIP:
- if (ctrl->value)
+ if (ctrl->val)
data = reg_set(client, MT9V022_READ_MODE, 0x20);
else
data = reg_clear(client, MT9V022_READ_MODE, 0x20);
if (data < 0)
return -EIO;
- break;
- case V4L2_CID_GAIN:
- /* mt9v022 has minimum == default */
- if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
- return -EINVAL;
- else {
- unsigned long range = qctrl->maximum - qctrl->minimum;
+ return 0;
+ case V4L2_CID_AUTOGAIN:
+ if (ctrl->val) {
+ if (reg_set(client, MT9V022_AEC_AGC_ENABLE, 0x2) < 0)
+ return -EIO;
+ } else {
+ struct v4l2_ctrl *gain = mt9v022->gain;
+ /* mt9v022 has minimum == default */
+ unsigned long range = gain->maximum - gain->minimum;
/* Valid values 16 to 64, 32 to 64 must be even. */
- unsigned long gain = ((ctrl->value - qctrl->minimum) *
+ unsigned long gain_val = ((gain->val - gain->minimum) *
48 + range / 2) / range + 16;
- if (gain >= 32)
- gain &= ~1;
+
+ if (gain_val >= 32)
+ gain_val &= ~1;
+
/*
* The user wants to set gain manually, hope, she
* knows, what she's doing... Switch AGC off.
*/
-
if (reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x2) < 0)
return -EIO;
dev_dbg(&client->dev, "Setting gain from %d to %lu\n",
- reg_read(client, MT9V022_ANALOG_GAIN), gain);
- if (reg_write(client, MT9V022_ANALOG_GAIN, gain) < 0)
+ reg_read(client, MT9V022_ANALOG_GAIN), gain_val);
+ if (reg_write(client, MT9V022_ANALOG_GAIN, gain_val) < 0)
return -EIO;
}
- break;
- case V4L2_CID_EXPOSURE:
- /* mt9v022 has maximum == default */
- if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
- return -EINVAL;
- else {
- unsigned long range = qctrl->maximum - qctrl->minimum;
- unsigned long shutter = ((ctrl->value - qctrl->minimum) *
- 479 + range / 2) / range + 1;
+ return 0;
+ case V4L2_CID_EXPOSURE_AUTO:
+ if (ctrl->val == V4L2_EXPOSURE_AUTO) {
+ data = reg_set(client, MT9V022_AEC_AGC_ENABLE, 0x1);
+ } else {
+ struct v4l2_ctrl *exp = mt9v022->exposure;
+ unsigned long range = exp->maximum - exp->minimum;
+ unsigned long shutter = ((exp->val - exp->minimum) *
+ 479 + range / 2) / range + 1;
+
/*
* The user wants to set shutter width manually, hope,
* she knows, what she's doing... Switch AEC off.
*/
-
- if (reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x1) < 0)
+ data = reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x1);
+ if (data < 0)
return -EIO;
-
dev_dbg(&client->dev, "Shutter width from %d to %lu\n",
- reg_read(client, MT9V022_TOTAL_SHUTTER_WIDTH),
- shutter);
+ reg_read(client, MT9V022_TOTAL_SHUTTER_WIDTH),
+ shutter);
if (reg_write(client, MT9V022_TOTAL_SHUTTER_WIDTH,
- shutter) < 0)
+ shutter) < 0)
return -EIO;
}
- break;
- case V4L2_CID_AUTOGAIN:
- if (ctrl->value)
- data = reg_set(client, MT9V022_AEC_AGC_ENABLE, 0x2);
- else
- data = reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x2);
- if (data < 0)
- return -EIO;
- break;
- case V4L2_CID_EXPOSURE_AUTO:
- if (ctrl->value)
- data = reg_set(client, MT9V022_AEC_AGC_ENABLE, 0x1);
- else
- data = reg_clear(client, MT9V022_AEC_AGC_ENABLE, 0x1);
- if (data < 0)
- return -EIO;
- break;
+ return 0;
}
- return 0;
+ return -EINVAL;
}
/*
@@ -752,9 +668,12 @@ static int mt9v022_g_skip_top_lines(struct v4l2_subdev *sd, u32 *lines)
return 0;
}
+static const struct v4l2_ctrl_ops mt9v022_ctrl_ops = {
+ .g_volatile_ctrl = mt9v022_g_volatile_ctrl,
+ .s_ctrl = mt9v022_s_ctrl,
+};
+
static struct v4l2_subdev_core_ops mt9v022_subdev_core_ops = {
- .g_ctrl = mt9v022_g_ctrl,
- .s_ctrl = mt9v022_s_ctrl,
.g_chip_ident = mt9v022_g_chip_ident,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = mt9v022_g_register,
@@ -906,10 +825,39 @@ static int mt9v022_probe(struct i2c_client *client,
return -ENOMEM;
v4l2_i2c_subdev_init(&mt9v022->subdev, client, &mt9v022_subdev_ops);
+ v4l2_ctrl_handler_init(&mt9v022->hdl, 6);
+ v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ mt9v022->autogain = v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+ V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
+ mt9v022->gain = v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+ V4L2_CID_GAIN, 0, 127, 1, 64);
+
+ /*
+ * Simulated autoexposure. If enabled, we calculate shutter width
+ * ourselves in the driver based on vertical blanking and frame width
+ */
+ mt9v022->autoexposure = v4l2_ctrl_new_std_menu(&mt9v022->hdl,
+ &mt9v022_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+ V4L2_EXPOSURE_AUTO);
+ mt9v022->exposure = v4l2_ctrl_new_std(&mt9v022->hdl, &mt9v022_ctrl_ops,
+ V4L2_CID_EXPOSURE, 1, 255, 1, 255);
+
+ mt9v022->subdev.ctrl_handler = &mt9v022->hdl;
+ if (mt9v022->hdl.error) {
+ int err = mt9v022->hdl.error;
+
+ kfree(mt9v022);
+ return err;
+ }
+ v4l2_ctrl_auto_cluster(2, &mt9v022->autoexposure,
+ V4L2_EXPOSURE_MANUAL, true);
+ v4l2_ctrl_auto_cluster(2, &mt9v022->autogain, 0, true);
mt9v022->chip_control = MT9V022_CHIP_CONTROL_DEFAULT;
- icd->ops = &mt9v022_ops;
/*
* MT9V022 _really_ corrupts the first read out line.
* TODO: verify on i.MX31
@@ -922,7 +870,7 @@ static int mt9v022_probe(struct i2c_client *client,
ret = mt9v022_video_probe(icd, client);
if (ret) {
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&mt9v022->hdl);
kfree(mt9v022);
}
@@ -934,8 +882,9 @@ static int mt9v022_remove(struct i2c_client *client)
struct mt9v022 *mt9v022 = to_mt9v022(client);
struct soc_camera_device *icd = client->dev.platform_data;
- icd->ops = NULL;
+ v4l2_device_unregister_subdev(&mt9v022->subdev);
mt9v022_video_remove(icd);
+ v4l2_ctrl_handler_free(&mt9v022->hdl);
kfree(mt9v022);
return 0;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 07/13 v3] ov2640: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (5 preceding siblings ...)
2011-09-08 8:43 ` [PATCH 06/13 v3] mt9v022: " Guennadi Liakhovetski
@ 2011-09-08 8:44 ` Guennadi Liakhovetski
2011-09-08 8:44 ` [PATCH 08/13 v3] ov6650: " Guennadi Liakhovetski
` (5 subsequent siblings)
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:44 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/ov2640.c | 90 ++++++++++++-----------------------------
1 files changed, 27 insertions(+), 63 deletions(-)
diff --git a/drivers/media/video/ov2640.c b/drivers/media/video/ov2640.c
index 2826aff..981767f 100644
--- a/drivers/media/video/ov2640.c
+++ b/drivers/media/video/ov2640.c
@@ -24,6 +24,7 @@
#include <media/soc_mediabus.h>
#include <media/v4l2-chip-ident.h>
#include <media/v4l2-subdev.h>
+#include <media/v4l2-ctrls.h>
#define VAL_SET(x, mask, rshift, lshift) \
((((x) >> rshift) & mask) << lshift)
@@ -300,11 +301,10 @@ struct ov2640_win_size {
struct ov2640_priv {
struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
enum v4l2_mbus_pixelcode cfmt_code;
const struct ov2640_win_size *win;
int model;
- u16 flag_vflip:1;
- u16 flag_hflip:1;
};
/*
@@ -610,29 +610,6 @@ static enum v4l2_mbus_pixelcode ov2640_codes[] = {
};
/*
- * Supported controls
- */
-static const struct v4l2_queryctrl ov2640_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 functions
*/
static struct ov2640_priv *to_ov2640(const struct i2c_client *client)
@@ -701,43 +678,23 @@ static int ov2640_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
}
-static int ov2640_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct ov2640_priv *priv = to_ov2640(client);
-
- switch (ctrl->id) {
- case V4L2_CID_VFLIP:
- ctrl->value = priv->flag_vflip;
- break;
- case V4L2_CID_HFLIP:
- ctrl->value = priv->flag_hflip;
- break;
- }
- return 0;
-}
-
-static int ov2640_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
{
+ struct v4l2_subdev *sd =
+ &container_of(ctrl->handler, struct ov2640_priv, hdl)->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct ov2640_priv *priv = to_ov2640(client);
- int ret = 0;
u8 val;
switch (ctrl->id) {
case V4L2_CID_VFLIP:
- val = ctrl->value ? REG04_VFLIP_IMG : 0x00;
- priv->flag_vflip = ctrl->value ? 1 : 0;
- ret = ov2640_mask_set(client, REG04, REG04_VFLIP_IMG, val);
- break;
+ val = ctrl->val ? REG04_VFLIP_IMG : 0x00;
+ return ov2640_mask_set(client, REG04, REG04_VFLIP_IMG, val);
case V4L2_CID_HFLIP:
- val = ctrl->value ? REG04_HFLIP_IMG : 0x00;
- priv->flag_hflip = ctrl->value ? 1 : 0;
- ret = ov2640_mask_set(client, REG04, REG04_HFLIP_IMG, val);
- break;
+ val = ctrl->val ? REG04_HFLIP_IMG : 0x00;
+ return ov2640_mask_set(client, REG04, REG04_HFLIP_IMG, val);
}
- return ret;
+ return -EINVAL;
}
static int ov2640_g_chip_ident(struct v4l2_subdev *sd,
@@ -1022,20 +979,17 @@ static int ov2640_video_probe(struct soc_camera_device *icd,
"%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
devname, pid, ver, midh, midl);
- return 0;
+ return v4l2_ctrl_handler_setup(&priv->hdl);
err:
return ret;
}
-static struct soc_camera_ops ov2640_ops = {
- .controls = ov2640_controls,
- .num_controls = ARRAY_SIZE(ov2640_controls),
+static const struct v4l2_ctrl_ops ov2640_ctrl_ops = {
+ .s_ctrl = ov2640_s_ctrl,
};
static struct v4l2_subdev_core_ops ov2640_subdev_core_ops = {
- .g_ctrl = ov2640_g_ctrl,
- .s_ctrl = ov2640_s_ctrl,
.g_chip_ident = ov2640_g_chip_ident,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = ov2640_g_register,
@@ -1113,12 +1067,22 @@ static int ov2640_probe(struct i2c_client *client,
}
v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
+ v4l2_ctrl_handler_init(&priv->hdl, 2);
+ v4l2_ctrl_new_std(&priv->hdl, &ov2640_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&priv->hdl, &ov2640_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ priv->subdev.ctrl_handler = &priv->hdl;
+ if (priv->hdl.error) {
+ int err = priv->hdl.error;
- icd->ops = &ov2640_ops;
+ kfree(priv);
+ return err;
+ }
ret = ov2640_video_probe(icd, client);
if (ret) {
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
} else {
dev_info(&adapter->dev, "OV2640 Probed\n");
@@ -1130,9 +1094,9 @@ static int ov2640_probe(struct i2c_client *client,
static int ov2640_remove(struct i2c_client *client)
{
struct ov2640_priv *priv = to_ov2640(client);
- struct soc_camera_device *icd = client->dev.platform_data;
- icd->ops = NULL;
+ v4l2_device_unregister_subdev(&priv->subdev);
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
return 0;
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 08/13 v3] ov6650: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (6 preceding siblings ...)
2011-09-08 8:44 ` [PATCH 07/13 v3] ov2640: " Guennadi Liakhovetski
@ 2011-09-08 8:44 ` Guennadi Liakhovetski
2011-09-09 17:07 ` Janusz Krzysztofik
2011-09-08 8:44 ` [PATCH 09/13 v3] ov9740: " Guennadi Liakhovetski
` (4 subsequent siblings)
12 siblings, 1 reply; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:44 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/ov6650.c | 381 +++++++++++++-----------------------------
1 files changed, 115 insertions(+), 266 deletions(-)
diff --git a/drivers/media/video/ov6650.c b/drivers/media/video/ov6650.c
index 654b2f5..089a4aa 100644
--- a/drivers/media/video/ov6650.c
+++ b/drivers/media/video/ov6650.c
@@ -32,6 +32,7 @@
#include <media/soc_camera.h>
#include <media/soc_mediabus.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
/* Register definitions */
#define REG_GAIN 0x00 /* range 00 - 3F */
@@ -177,20 +178,23 @@ struct ov6650_reg {
struct ov6650 {
struct v4l2_subdev subdev;
-
- int gain;
- int blue;
- int red;
- int saturation;
- int hue;
- int brightness;
- int exposure;
- int gamma;
- int aec;
- bool vflip;
- bool hflip;
- bool awb;
- bool agc;
+ struct v4l2_ctrl_handler hdl;
+ struct {
+ /* exposure/autoexposure cluster */
+ struct v4l2_ctrl *autoexposure;
+ struct v4l2_ctrl *exposure;
+ };
+ struct {
+ /* gain/autogain cluster */
+ struct v4l2_ctrl *autogain;
+ struct v4l2_ctrl *gain;
+ };
+ struct {
+ /* blue/red/autowhitebalance cluster */
+ struct v4l2_ctrl *autowb;
+ struct v4l2_ctrl *blue;
+ struct v4l2_ctrl *red;
+ };
bool half_scale; /* scale down output by 2 */
struct v4l2_rect rect; /* sensor cropping window */
unsigned long pclk_limit; /* from host */
@@ -210,126 +214,6 @@ static enum v4l2_mbus_pixelcode ov6650_codes[] = {
V4L2_MBUS_FMT_Y8_1X8,
};
-static const struct v4l2_queryctrl ov6650_controls[] = {
- {
- .id = V4L2_CID_AUTOGAIN,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "AGC",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 1,
- },
- {
- .id = V4L2_CID_GAIN,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Gain",
- .minimum = 0,
- .maximum = 0x3f,
- .step = 1,
- .default_value = DEF_GAIN,
- },
- {
- .id = V4L2_CID_AUTO_WHITE_BALANCE,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "AWB",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 1,
- },
- {
- .id = V4L2_CID_BLUE_BALANCE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Blue",
- .minimum = 0,
- .maximum = 0xff,
- .step = 1,
- .default_value = DEF_BLUE,
- },
- {
- .id = V4L2_CID_RED_BALANCE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Red",
- .minimum = 0,
- .maximum = 0xff,
- .step = 1,
- .default_value = DEF_RED,
- },
- {
- .id = V4L2_CID_SATURATION,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Saturation",
- .minimum = 0,
- .maximum = 0xf,
- .step = 1,
- .default_value = 0x8,
- },
- {
- .id = V4L2_CID_HUE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Hue",
- .minimum = 0,
- .maximum = HUE_MASK,
- .step = 1,
- .default_value = DEF_HUE,
- },
- {
- .id = V4L2_CID_BRIGHTNESS,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Brightness",
- .minimum = 0,
- .maximum = 0xff,
- .step = 1,
- .default_value = 0x80,
- },
- {
- .id = V4L2_CID_EXPOSURE_AUTO,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "AEC",
- .minimum = 0,
- .maximum = 3,
- .step = 1,
- .default_value = 0,
- },
- {
- .id = V4L2_CID_EXPOSURE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Exposure",
- .minimum = 0,
- .maximum = 0xff,
- .step = 1,
- .default_value = DEF_AECH,
- },
- {
- .id = V4L2_CID_GAMMA,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Gamma",
- .minimum = 0,
- .maximum = 0xff,
- .step = 1,
- .default_value = 0x12,
- },
- {
- .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,
- },
-};
-
/* read a register */
static int ov6650_reg_read(struct i2c_client *client, u8 reg, u8 *val)
{
@@ -420,166 +304,91 @@ static int ov6650_s_stream(struct v4l2_subdev *sd, int enable)
}
/* Get status of additional camera capabilities */
-static int ov6650_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov6550_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
+ struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
+ struct v4l2_subdev *sd = &priv->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct ov6650 *priv = to_ov6650(client);
- uint8_t reg;
+ uint8_t reg, reg2;
int ret = 0;
switch (ctrl->id) {
case V4L2_CID_AUTOGAIN:
- ctrl->value = priv->agc;
- break;
- case V4L2_CID_GAIN:
- if (priv->agc) {
- ret = ov6650_reg_read(client, REG_GAIN, ®);
- ctrl->value = reg;
- } else {
- ctrl->value = priv->gain;
- }
- break;
+ ret = ov6650_reg_read(client, REG_GAIN, ®);
+ if (!ret)
+ priv->gain->val = reg;
+ return ret;
case V4L2_CID_AUTO_WHITE_BALANCE:
- ctrl->value = priv->awb;
- break;
- case V4L2_CID_BLUE_BALANCE:
- if (priv->awb) {
- ret = ov6650_reg_read(client, REG_BLUE, ®);
- ctrl->value = reg;
- } else {
- ctrl->value = priv->blue;
- }
- break;
- case V4L2_CID_RED_BALANCE:
- if (priv->awb) {
- ret = ov6650_reg_read(client, REG_RED, ®);
- ctrl->value = reg;
- } else {
- ctrl->value = priv->red;
+ ret = ov6650_reg_read(client, REG_BLUE, ®);
+ if (!ret)
+ ret = ov6650_reg_read(client, REG_RED, ®2);
+ if (!ret) {
+ priv->blue->val = reg;
+ priv->red->val = reg2;
}
- break;
- case V4L2_CID_SATURATION:
- ctrl->value = priv->saturation;
- break;
- case V4L2_CID_HUE:
- ctrl->value = priv->hue;
- break;
- case V4L2_CID_BRIGHTNESS:
- ctrl->value = priv->brightness;
- break;
+ return ret;
case V4L2_CID_EXPOSURE_AUTO:
- ctrl->value = priv->aec;
- break;
- case V4L2_CID_EXPOSURE:
- if (priv->aec) {
- ret = ov6650_reg_read(client, REG_AECH, ®);
- ctrl->value = reg;
- } else {
- ctrl->value = priv->exposure;
- }
- break;
- case V4L2_CID_GAMMA:
- ctrl->value = priv->gamma;
- break;
- case V4L2_CID_VFLIP:
- ctrl->value = priv->vflip;
- break;
- case V4L2_CID_HFLIP:
- ctrl->value = priv->hflip;
- break;
+ ret = ov6650_reg_read(client, REG_AECH, ®);
+ if (!ret)
+ priv->exposure->val = reg;
+ return ret;
}
- return ret;
+ return -EINVAL;
}
/* Set status of additional camera capabilities */
-static int ov6650_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
{
+ struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
+ struct v4l2_subdev *sd = &priv->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct ov6650 *priv = to_ov6650(client);
int ret = 0;
switch (ctrl->id) {
case V4L2_CID_AUTOGAIN:
ret = ov6650_reg_rmw(client, REG_COMB,
- ctrl->value ? COMB_AGC : 0, COMB_AGC);
- if (!ret)
- priv->agc = ctrl->value;
- break;
- case V4L2_CID_GAIN:
- ret = ov6650_reg_write(client, REG_GAIN, ctrl->value);
- if (!ret)
- priv->gain = ctrl->value;
- break;
+ ctrl->val ? COMB_AGC : 0, COMB_AGC);
+ if (!ret && !ctrl->val)
+ ret = ov6650_reg_write(client, REG_GAIN, priv->gain->val);
+ return ret;
case V4L2_CID_AUTO_WHITE_BALANCE:
ret = ov6650_reg_rmw(client, REG_COMB,
- ctrl->value ? COMB_AWB : 0, COMB_AWB);
- if (!ret)
- priv->awb = ctrl->value;
- break;
- case V4L2_CID_BLUE_BALANCE:
- ret = ov6650_reg_write(client, REG_BLUE, ctrl->value);
- if (!ret)
- priv->blue = ctrl->value;
- break;
- case V4L2_CID_RED_BALANCE:
- ret = ov6650_reg_write(client, REG_RED, ctrl->value);
- if (!ret)
- priv->red = ctrl->value;
- break;
+ ctrl->val ? COMB_AWB : 0, COMB_AWB);
+ if (!ret && !ctrl->val) {
+ ret = ov6650_reg_write(client, REG_BLUE, priv->blue->val);
+ if (!ret)
+ ret = ov6650_reg_write(client, REG_BLUE,
+ priv->red->val);
+ }
+ return ret;
case V4L2_CID_SATURATION:
- ret = ov6650_reg_rmw(client, REG_SAT, SET_SAT(ctrl->value),
+ return ov6650_reg_rmw(client, REG_SAT, SET_SAT(ctrl->val),
SAT_MASK);
- if (!ret)
- priv->saturation = ctrl->value;
- break;
case V4L2_CID_HUE:
- ret = ov6650_reg_rmw(client, REG_HUE, SET_HUE(ctrl->value),
+ return ov6650_reg_rmw(client, REG_HUE, SET_HUE(ctrl->val),
HUE_MASK);
- if (!ret)
- priv->hue = ctrl->value;
- break;
case V4L2_CID_BRIGHTNESS:
- ret = ov6650_reg_write(client, REG_BRT, ctrl->value);
- if (!ret)
- priv->brightness = ctrl->value;
- break;
+ return ov6650_reg_write(client, REG_BRT, ctrl->val);
case V4L2_CID_EXPOSURE_AUTO:
- switch (ctrl->value) {
- case V4L2_EXPOSURE_AUTO:
+ if (ctrl->val == V4L2_EXPOSURE_AUTO)
ret = ov6650_reg_rmw(client, REG_COMB, COMB_AEC, 0);
- break;
- default:
+ else
ret = ov6650_reg_rmw(client, REG_COMB, 0, COMB_AEC);
- break;
- }
- if (!ret)
- priv->aec = ctrl->value;
- break;
- case V4L2_CID_EXPOSURE:
- ret = ov6650_reg_write(client, REG_AECH, ctrl->value);
- if (!ret)
- priv->exposure = ctrl->value;
- break;
+ if (!ret && ctrl->val == V4L2_EXPOSURE_MANUAL)
+ ret = ov6650_reg_write(client, REG_AECH,
+ priv->exposure->val);
+ return ret;
case V4L2_CID_GAMMA:
- ret = ov6650_reg_write(client, REG_GAM1, ctrl->value);
- if (!ret)
- priv->gamma = ctrl->value;
- break;
+ return ov6650_reg_write(client, REG_GAM1, ctrl->val);
case V4L2_CID_VFLIP:
- ret = ov6650_reg_rmw(client, REG_COMB,
- ctrl->value ? COMB_FLIP_V : 0, COMB_FLIP_V);
- if (!ret)
- priv->vflip = ctrl->value;
- break;
+ return ov6650_reg_rmw(client, REG_COMB,
+ ctrl->val ? COMB_FLIP_V : 0, COMB_FLIP_V);
case V4L2_CID_HFLIP:
- ret = ov6650_reg_rmw(client, REG_COMB,
- ctrl->value ? COMB_FLIP_H : 0, COMB_FLIP_H);
- if (!ret)
- priv->hflip = ctrl->value;
- break;
+ return ov6650_reg_rmw(client, REG_COMB,
+ ctrl->val ? COMB_FLIP_H : 0, COMB_FLIP_H);
}
- return ret;
+ return -EINVAL;
}
/* Get chip identification */
@@ -1048,14 +857,12 @@ static int ov6650_video_probe(struct soc_camera_device *icd,
return ret;
}
-static struct soc_camera_ops ov6650_ops = {
- .controls = ov6650_controls,
- .num_controls = ARRAY_SIZE(ov6650_controls),
+static const struct v4l2_ctrl_ops ov6550_ctrl_ops = {
+ .g_volatile_ctrl = ov6550_g_volatile_ctrl,
+ .s_ctrl = ov6550_s_ctrl,
};
static struct v4l2_subdev_core_ops ov6650_core_ops = {
- .g_ctrl = ov6650_g_ctrl,
- .s_ctrl = ov6650_s_ctrl,
.g_chip_ident = ov6650_g_chip_ident,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = ov6650_get_register,
@@ -1164,8 +971,46 @@ static int ov6650_probe(struct i2c_client *client,
}
v4l2_i2c_subdev_init(&priv->subdev, client, &ov6650_subdev_ops);
+ v4l2_ctrl_handler_init(&priv->hdl, 13);
+ v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ priv->autogain = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
+ priv->gain = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_GAIN, 0, 0x3f, 1, DEF_GAIN);
+ priv->autowb = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
+ priv->blue = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_BLUE_BALANCE, 0, 0xff, 1, DEF_BLUE);
+ priv->red = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_RED_BALANCE, 0, 0xff, 1, DEF_RED);
+ v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_SATURATION, 0, 0xf, 1, 0x8);
+ v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_HUE, 0, HUE_MASK, 1, DEF_HUE);
+ v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_BRIGHTNESS, 0, 0xff, 1, 0x80);
+ priv->autoexposure = v4l2_ctrl_new_std_menu(&priv->hdl,
+ &ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+ V4L2_EXPOSURE_AUTO);
+ priv->exposure = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_EXPOSURE, 0, 0xff, 1, DEF_AECH);
+ v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
+ V4L2_CID_GAMMA, 0, 0xff, 1, 0x12);
+
+ priv->subdev.ctrl_handler = &priv->hdl;
+ if (priv->hdl.error) {
+ int err = priv->hdl.error;
- icd->ops = &ov6650_ops;
+ kfree(priv);
+ return err;
+ }
+ v4l2_ctrl_auto_cluster(2, &priv->autogain, 0, true);
+ v4l2_ctrl_auto_cluster(3, &priv->autowb, 0, true);
+ v4l2_ctrl_auto_cluster(2, &priv->autoexposure,
+ V4L2_EXPOSURE_MANUAL, true);
priv->rect.left = DEF_HSTRT << 1;
priv->rect.top = DEF_VSTRT << 1;
@@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
priv->colorspace = V4L2_COLORSPACE_JPEG;
ret = ov6650_video_probe(icd, client);
+ if (!ret)
+ ret = v4l2_ctrl_handler_setup(&priv->hdl);
if (ret) {
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
}
@@ -1189,6 +1036,8 @@ static int ov6650_remove(struct i2c_client *client)
{
struct ov6650 *priv = to_ov6650(client);
+ v4l2_device_unregister_subdev(&priv->subdev);
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
return 0;
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 08/13 v3] ov6650: convert to the control framework.
2011-09-08 8:44 ` [PATCH 08/13 v3] ov6650: " Guennadi Liakhovetski
@ 2011-09-09 17:07 ` Janusz Krzysztofik
2011-09-09 18:01 ` Guennadi Liakhovetski
0 siblings, 1 reply; 22+ messages in thread
From: Janusz Krzysztofik @ 2011-09-09 17:07 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-media, Hans Verkuil, Mauro Carvalho Chehab
On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> [g.liakhovetski@gmx.de: simplified pointer arithmetic]
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Hi,
I've successfully tested this one for you, to the extent possible using
mplayer, i.e., only saturation, hue and brightness controls, and an
overall functionality.
Modifications to other (not runtime tested) controls look good to me,
except for one copy-paste mistake, see below. With that erratic REG_BLUE
corrected:
Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
There are also a few minor comments for you to consider.
Thanks,
Janusz
> ---
> drivers/media/video/ov6650.c | 381 +++++++++++++-----------------------------
> 1 files changed, 115 insertions(+), 266 deletions(-)
>
> diff --git a/drivers/media/video/ov6650.c b/drivers/media/video/ov6650.c
> index 654b2f5..089a4aa 100644
> --- a/drivers/media/video/ov6650.c
> +++ b/drivers/media/video/ov6650.c
> @@ -32,6 +32,7 @@
> #include <media/soc_camera.h>
> #include <media/soc_mediabus.h>
> #include <media/v4l2-chip-ident.h>
> +#include <media/v4l2-ctrls.h>
>
> /* Register definitions */
> #define REG_GAIN 0x00 /* range 00 - 3F */
> @@ -177,20 +178,23 @@ struct ov6650_reg {
>
> struct ov6650 {
> struct v4l2_subdev subdev;
> -
> - int gain;
> - int blue;
> - int red;
> - int saturation;
> - int hue;
> - int brightness;
> - int exposure;
> - int gamma;
> - int aec;
> - bool vflip;
> - bool hflip;
> - bool awb;
> - bool agc;
> + struct v4l2_ctrl_handler hdl;
> + struct {
> + /* exposure/autoexposure cluster */
> + struct v4l2_ctrl *autoexposure;
> + struct v4l2_ctrl *exposure;
> + };
> + struct {
> + /* gain/autogain cluster */
> + struct v4l2_ctrl *autogain;
> + struct v4l2_ctrl *gain;
> + };
> + struct {
> + /* blue/red/autowhitebalance cluster */
> + struct v4l2_ctrl *autowb;
> + struct v4l2_ctrl *blue;
> + struct v4l2_ctrl *red;
> + };
> bool half_scale; /* scale down output by 2 */
> struct v4l2_rect rect; /* sensor cropping window */
> unsigned long pclk_limit; /* from host */
> @@ -210,126 +214,6 @@ static enum v4l2_mbus_pixelcode ov6650_codes[] = {
> V4L2_MBUS_FMT_Y8_1X8,
> };
>
> -static const struct v4l2_queryctrl ov6650_controls[] = {
> - {
> - .id = V4L2_CID_AUTOGAIN,
> - .type = V4L2_CTRL_TYPE_BOOLEAN,
> - .name = "AGC",
> - .minimum = 0,
> - .maximum = 1,
> - .step = 1,
> - .default_value = 1,
> - },
> - {
> - .id = V4L2_CID_GAIN,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Gain",
> - .minimum = 0,
> - .maximum = 0x3f,
> - .step = 1,
> - .default_value = DEF_GAIN,
> - },
> - {
> - .id = V4L2_CID_AUTO_WHITE_BALANCE,
> - .type = V4L2_CTRL_TYPE_BOOLEAN,
> - .name = "AWB",
> - .minimum = 0,
> - .maximum = 1,
> - .step = 1,
> - .default_value = 1,
> - },
> - {
> - .id = V4L2_CID_BLUE_BALANCE,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Blue",
> - .minimum = 0,
> - .maximum = 0xff,
> - .step = 1,
> - .default_value = DEF_BLUE,
> - },
> - {
> - .id = V4L2_CID_RED_BALANCE,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Red",
> - .minimum = 0,
> - .maximum = 0xff,
> - .step = 1,
> - .default_value = DEF_RED,
> - },
> - {
> - .id = V4L2_CID_SATURATION,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Saturation",
> - .minimum = 0,
> - .maximum = 0xf,
> - .step = 1,
> - .default_value = 0x8,
> - },
> - {
> - .id = V4L2_CID_HUE,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Hue",
> - .minimum = 0,
> - .maximum = HUE_MASK,
> - .step = 1,
> - .default_value = DEF_HUE,
> - },
> - {
> - .id = V4L2_CID_BRIGHTNESS,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Brightness",
> - .minimum = 0,
> - .maximum = 0xff,
> - .step = 1,
> - .default_value = 0x80,
> - },
> - {
> - .id = V4L2_CID_EXPOSURE_AUTO,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "AEC",
> - .minimum = 0,
> - .maximum = 3,
> - .step = 1,
> - .default_value = 0,
> - },
> - {
> - .id = V4L2_CID_EXPOSURE,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Exposure",
> - .minimum = 0,
> - .maximum = 0xff,
> - .step = 1,
> - .default_value = DEF_AECH,
> - },
> - {
> - .id = V4L2_CID_GAMMA,
> - .type = V4L2_CTRL_TYPE_INTEGER,
> - .name = "Gamma",
> - .minimum = 0,
> - .maximum = 0xff,
> - .step = 1,
> - .default_value = 0x12,
> - },
> - {
> - .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,
> - },
> -};
> -
> /* read a register */
> static int ov6650_reg_read(struct i2c_client *client, u8 reg, u8 *val)
> {
> @@ -420,166 +304,91 @@ static int ov6650_s_stream(struct v4l2_subdev *sd, int enable)
> }
>
> /* Get status of additional camera capabilities */
> -static int ov6650_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +static int ov6550_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> {
> + struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
> + struct v4l2_subdev *sd = &priv->subdev;
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct ov6650 *priv = to_ov6650(client);
> - uint8_t reg;
> + uint8_t reg, reg2;
> int ret = 0;
With no "retrun ret;" at the end, there is no need to initialize ret any
longer.
> switch (ctrl->id) {
> case V4L2_CID_AUTOGAIN:
> - ctrl->value = priv->agc;
> - break;
> - case V4L2_CID_GAIN:
> - if (priv->agc) {
> - ret = ov6650_reg_read(client, REG_GAIN, ®);
> - ctrl->value = reg;
> - } else {
> - ctrl->value = priv->gain;
> - }
> - break;
> + ret = ov6650_reg_read(client, REG_GAIN, ®);
> + if (!ret)
> + priv->gain->val = reg;
> + return ret;
> case V4L2_CID_AUTO_WHITE_BALANCE:
> - ctrl->value = priv->awb;
> - break;
> - case V4L2_CID_BLUE_BALANCE:
> - if (priv->awb) {
> - ret = ov6650_reg_read(client, REG_BLUE, ®);
> - ctrl->value = reg;
> - } else {
> - ctrl->value = priv->blue;
> - }
> - break;
> - case V4L2_CID_RED_BALANCE:
> - if (priv->awb) {
> - ret = ov6650_reg_read(client, REG_RED, ®);
> - ctrl->value = reg;
> - } else {
> - ctrl->value = priv->red;
> + ret = ov6650_reg_read(client, REG_BLUE, ®);
> + if (!ret)
> + ret = ov6650_reg_read(client, REG_RED, ®2);
> + if (!ret) {
> + priv->blue->val = reg;
> + priv->red->val = reg2;
> }
> - break;
> - case V4L2_CID_SATURATION:
> - ctrl->value = priv->saturation;
> - break;
> - case V4L2_CID_HUE:
> - ctrl->value = priv->hue;
> - break;
> - case V4L2_CID_BRIGHTNESS:
> - ctrl->value = priv->brightness;
> - break;
> + return ret;
> case V4L2_CID_EXPOSURE_AUTO:
> - ctrl->value = priv->aec;
> - break;
> - case V4L2_CID_EXPOSURE:
> - if (priv->aec) {
> - ret = ov6650_reg_read(client, REG_AECH, ®);
> - ctrl->value = reg;
> - } else {
> - ctrl->value = priv->exposure;
> - }
> - break;
> - case V4L2_CID_GAMMA:
> - ctrl->value = priv->gamma;
> - break;
> - case V4L2_CID_VFLIP:
> - ctrl->value = priv->vflip;
> - break;
> - case V4L2_CID_HFLIP:
> - ctrl->value = priv->hflip;
> - break;
> + ret = ov6650_reg_read(client, REG_AECH, ®);
> + if (!ret)
> + priv->exposure->val = reg;
> + return ret;
> }
> - return ret;
> + return -EINVAL;
> }
>
> /* Set status of additional camera capabilities */
> -static int ov6650_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
> +static int ov6550_s_ctrl(struct v4l2_ctrl *ctrl)
> {
> + struct ov6650 *priv = container_of(ctrl->handler, struct ov6650, hdl);
> + struct v4l2_subdev *sd = &priv->subdev;
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> - struct ov6650 *priv = to_ov6650(client);
> int ret = 0;
ditto
> switch (ctrl->id) {
> case V4L2_CID_AUTOGAIN:
> ret = ov6650_reg_rmw(client, REG_COMB,
> - ctrl->value ? COMB_AGC : 0, COMB_AGC);
> - if (!ret)
> - priv->agc = ctrl->value;
> - break;
> - case V4L2_CID_GAIN:
> - ret = ov6650_reg_write(client, REG_GAIN, ctrl->value);
> - if (!ret)
> - priv->gain = ctrl->value;
> - break;
> + ctrl->val ? COMB_AGC : 0, COMB_AGC);
> + if (!ret && !ctrl->val)
> + ret = ov6650_reg_write(client, REG_GAIN, priv->gain->val);
> + return ret;
> case V4L2_CID_AUTO_WHITE_BALANCE:
> ret = ov6650_reg_rmw(client, REG_COMB,
> - ctrl->value ? COMB_AWB : 0, COMB_AWB);
> - if (!ret)
> - priv->awb = ctrl->value;
> - break;
> - case V4L2_CID_BLUE_BALANCE:
> - ret = ov6650_reg_write(client, REG_BLUE, ctrl->value);
> - if (!ret)
> - priv->blue = ctrl->value;
> - break;
> - case V4L2_CID_RED_BALANCE:
> - ret = ov6650_reg_write(client, REG_RED, ctrl->value);
> - if (!ret)
> - priv->red = ctrl->value;
> - break;
> + ctrl->val ? COMB_AWB : 0, COMB_AWB);
> + if (!ret && !ctrl->val) {
> + ret = ov6650_reg_write(client, REG_BLUE, priv->blue->val);
> + if (!ret)
> + ret = ov6650_reg_write(client, REG_BLUE,
REG_RED
> + priv->red->val);
> + }
> + return ret;
> case V4L2_CID_SATURATION:
> - ret = ov6650_reg_rmw(client, REG_SAT, SET_SAT(ctrl->value),
> + return ov6650_reg_rmw(client, REG_SAT, SET_SAT(ctrl->val),
> SAT_MASK);
> - if (!ret)
> - priv->saturation = ctrl->value;
> - break;
> case V4L2_CID_HUE:
> - ret = ov6650_reg_rmw(client, REG_HUE, SET_HUE(ctrl->value),
> + return ov6650_reg_rmw(client, REG_HUE, SET_HUE(ctrl->val),
> HUE_MASK);
> - if (!ret)
> - priv->hue = ctrl->value;
> - break;
> case V4L2_CID_BRIGHTNESS:
> - ret = ov6650_reg_write(client, REG_BRT, ctrl->value);
> - if (!ret)
> - priv->brightness = ctrl->value;
> - break;
> + return ov6650_reg_write(client, REG_BRT, ctrl->val);
> case V4L2_CID_EXPOSURE_AUTO:
> - switch (ctrl->value) {
> - case V4L2_EXPOSURE_AUTO:
For consitency with other cases (V4L2_CID_AUTOGAIN,
V4L2_CID_AUTO_WHITE_BALANCE, V4L2_CID_VFLIP, V4L2_CID_HFLIP), the
following snippet:
> + if (ctrl->val == V4L2_EXPOSURE_AUTO)
> ret = ov6650_reg_rmw(client, REG_COMB, COMB_AEC, 0);
> - break;
> - default:
> + else
> ret = ov6650_reg_rmw(client, REG_COMB, 0, COMB_AEC);
could perhaps be replaced with something like:
- if (ctrl->val == V4L2_EXPOSURE_AUTO)
- ret = ov6650_reg_rmw(client, REG_COMB, COMB_AEC, 0);
- break;
- default:
- ret = ov6650_reg_rmw(client, REG_COMB, 0, COMB_AEC);
+ ret = ov6650_reg_rmw(client, REG_COMB, ctrl->val ==
+ V4L2_EXPOSURE_AUTO ? COMB_AEC : 0, COMB_AEC);
or, the other way around, conditional expressions could be replaced with
if...else constructs in those other cases consequently.
> - break;
> - }
> - if (!ret)
> - priv->aec = ctrl->value;
> - break;
> - case V4L2_CID_EXPOSURE:
> - ret = ov6650_reg_write(client, REG_AECH, ctrl->value);
> - if (!ret)
> - priv->exposure = ctrl->value;
> - break;
> + if (!ret && ctrl->val == V4L2_EXPOSURE_MANUAL)
> + ret = ov6650_reg_write(client, REG_AECH,
> + priv->exposure->val);
> + return ret;
> case V4L2_CID_GAMMA:
> - ret = ov6650_reg_write(client, REG_GAM1, ctrl->value);
> - if (!ret)
> - priv->gamma = ctrl->value;
> - break;
> + return ov6650_reg_write(client, REG_GAM1, ctrl->val);
> case V4L2_CID_VFLIP:
> - ret = ov6650_reg_rmw(client, REG_COMB,
> - ctrl->value ? COMB_FLIP_V : 0, COMB_FLIP_V);
> - if (!ret)
> - priv->vflip = ctrl->value;
> - break;
> + return ov6650_reg_rmw(client, REG_COMB,
> + ctrl->val ? COMB_FLIP_V : 0, COMB_FLIP_V);
> case V4L2_CID_HFLIP:
> - ret = ov6650_reg_rmw(client, REG_COMB,
> - ctrl->value ? COMB_FLIP_H : 0, COMB_FLIP_H);
> - if (!ret)
> - priv->hflip = ctrl->value;
> - break;
> + return ov6650_reg_rmw(client, REG_COMB,
> + ctrl->val ? COMB_FLIP_H : 0, COMB_FLIP_H);
> }
>
> - return ret;
> + return -EINVAL;
> }
>
> /* Get chip identification */
> @@ -1048,14 +857,12 @@ static int ov6650_video_probe(struct soc_camera_device *icd,
> return ret;
> }
>
> -static struct soc_camera_ops ov6650_ops = {
> - .controls = ov6650_controls,
> - .num_controls = ARRAY_SIZE(ov6650_controls),
> +static const struct v4l2_ctrl_ops ov6550_ctrl_ops = {
> + .g_volatile_ctrl = ov6550_g_volatile_ctrl,
> + .s_ctrl = ov6550_s_ctrl,
> };
>
> static struct v4l2_subdev_core_ops ov6650_core_ops = {
> - .g_ctrl = ov6650_g_ctrl,
> - .s_ctrl = ov6650_s_ctrl,
> .g_chip_ident = ov6650_g_chip_ident,
> #ifdef CONFIG_VIDEO_ADV_DEBUG
> .g_register = ov6650_get_register,
> @@ -1164,8 +971,46 @@ static int ov6650_probe(struct i2c_client *client,
> }
>
> v4l2_i2c_subdev_init(&priv->subdev, client, &ov6650_subdev_ops);
> + v4l2_ctrl_handler_init(&priv->hdl, 13);
> + v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_VFLIP, 0, 1, 1, 0);
> + v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_HFLIP, 0, 1, 1, 0);
> + priv->autogain = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_AUTOGAIN, 0, 1, 1, 1);
> + priv->gain = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_GAIN, 0, 0x3f, 1, DEF_GAIN);
> + priv->autowb = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
> + priv->blue = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_BLUE_BALANCE, 0, 0xff, 1, DEF_BLUE);
> + priv->red = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_RED_BALANCE, 0, 0xff, 1, DEF_RED);
> + v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_SATURATION, 0, 0xf, 1, 0x8);
> + v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_HUE, 0, HUE_MASK, 1, DEF_HUE);
> + v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_BRIGHTNESS, 0, 0xff, 1, 0x80);
> + priv->autoexposure = v4l2_ctrl_new_std_menu(&priv->hdl,
> + &ov6550_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
max value of V4L2_EXPOSURE_MANUAL instead of equivalent 1 could be more
clear.
> + V4L2_EXPOSURE_AUTO);
> + priv->exposure = v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_EXPOSURE, 0, 0xff, 1, DEF_AECH);
> + v4l2_ctrl_new_std(&priv->hdl, &ov6550_ctrl_ops,
> + V4L2_CID_GAMMA, 0, 0xff, 1, 0x12);
> +
> + priv->subdev.ctrl_handler = &priv->hdl;
> + if (priv->hdl.error) {
> + int err = priv->hdl.error;
>
> - icd->ops = &ov6650_ops;
> + kfree(priv);
> + return err;
> + }
> + v4l2_ctrl_auto_cluster(2, &priv->autogain, 0, true);
> + v4l2_ctrl_auto_cluster(3, &priv->autowb, 0, true);
> + v4l2_ctrl_auto_cluster(2, &priv->autoexposure,
> + V4L2_EXPOSURE_MANUAL, true);
>
> priv->rect.left = DEF_HSTRT << 1;
> priv->rect.top = DEF_VSTRT << 1;
> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> priv->colorspace = V4L2_COLORSPACE_JPEG;
>
> ret = ov6650_video_probe(icd, client);
> + if (!ret)
> + ret = v4l2_ctrl_handler_setup(&priv->hdl);
Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
fails? Its usage is documented as optional.
>
> if (ret) {
> - icd->ops = NULL;
> + v4l2_ctrl_handler_free(&priv->hdl);
> kfree(priv);
> }
>
> @@ -1189,6 +1036,8 @@ static int ov6650_remove(struct i2c_client *client)
> {
> struct ov6650 *priv = to_ov6650(client);
>
> + v4l2_device_unregister_subdev(&priv->subdev);
> + v4l2_ctrl_handler_free(&priv->hdl);
> kfree(priv);
> return 0;
> }
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 08/13 v3] ov6650: convert to the control framework.
2011-09-09 17:07 ` Janusz Krzysztofik
@ 2011-09-09 18:01 ` Guennadi Liakhovetski
2011-09-09 18:23 ` Sylwester Nawrocki
2011-09-09 20:39 ` Janusz Krzysztofik
0 siblings, 2 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-09 18:01 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: linux-media, Hans Verkuil, Mauro Carvalho Chehab
Hi Janusz
On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:
> On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> >
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > [g.liakhovetski@gmx.de: simplified pointer arithmetic]
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>
> Hi,
> I've successfully tested this one for you, to the extent possible using
> mplayer, i.e., only saturation, hue and brightness controls, and an
> overall functionality.
Thanks for testing and reviewing!
> Modifications to other (not runtime tested) controls look good to me,
> except for one copy-paste mistake, see below. With that erratic REG_BLUE
> corrected:
>
> Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
>
> There are also a few minor comments for you to consider.
Well, some of them are not so minor, I would say;-) But I personally would
be happy to have this just as an incremental patch. Would you like to
prepare one or should I do it?
I basically agree with all your comments apart from maybe
[snip]
> > @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> > priv->colorspace = V4L2_COLORSPACE_JPEG;
> >
> > ret = ov6650_video_probe(icd, client);
> > + if (!ret)
> > + ret = v4l2_ctrl_handler_setup(&priv->hdl);
>
> Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
> fails? Its usage is documented as optional.
Not sure what the standard really meant, but it looks like this is done in
all patches in this series. So, we'd have to change this everywhere. Most
other drivers indeed do not care.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 08/13 v3] ov6650: convert to the control framework.
2011-09-09 18:01 ` Guennadi Liakhovetski
@ 2011-09-09 18:23 ` Sylwester Nawrocki
2011-09-09 20:58 ` Janusz Krzysztofik
2011-09-09 20:39 ` Janusz Krzysztofik
1 sibling, 1 reply; 22+ messages in thread
From: Sylwester Nawrocki @ 2011-09-09 18:23 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Janusz Krzysztofik, linux-media, Hans Verkuil,
Mauro Carvalho Chehab
Hi,
On 09/09/2011 08:01 PM, Guennadi Liakhovetski wrote:
> Hi Janusz
>
> On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:
>
>> On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
>>> From: Hans Verkuil<hans.verkuil@cisco.com>
>>>
>>> Signed-off-by: Hans Verkuil<hans.verkuil@cisco.com>
>>> [g.liakhovetski@gmx.de: simplified pointer arithmetic]
>>> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
>>
>> Hi,
>> I've successfully tested this one for you, to the extent possible using
>> mplayer, i.e., only saturation, hue and brightness controls, and an
>> overall functionality.
>
> Thanks for testing and reviewing!
>
>> Modifications to other (not runtime tested) controls look good to me,
>> except for one copy-paste mistake, see below. With that erratic REG_BLUE
>> corrected:
>>
>> Acked-by: Janusz Krzysztofik<jkrzyszt@tis.icnet.pl>
>>
>> There are also a few minor comments for you to consider.
>
> Well, some of them are not so minor, I would say;-) But I personally would
> be happy to have this just as an incremental patch. Would you like to
> prepare one or should I do it?
>
> I basically agree with all your comments apart from maybe
>
> [snip]
>
>>> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
>>> priv->colorspace = V4L2_COLORSPACE_JPEG;
>>>
>>> ret = ov6650_video_probe(icd, client);
>>> + if (!ret)
>>> + ret = v4l2_ctrl_handler_setup(&priv->hdl);
>>
>> Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
>> fails? Its usage is documented as optional.
>
> Not sure what the standard really meant, but it looks like this is done in
> all patches in this series. So, we'd have to change this everywhere. Most
> other drivers indeed do not care.
The usage of v4l2_ctrl_handler_setup() is optional, but if this function
is not used, then AFAIU the driver writer needs to ensure the control's
values after the device is initialized are exactly as those specified during
the control creation. Of course v4l2_ctrl_handler_setup() failure might
mean s_ctrl op failed, which might be caused by some H/W access errors.
So IMHO it is always a good idea to check the return value if we know
the batch controls setup shouldn't fail.
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 08/13 v3] ov6650: convert to the control framework.
2011-09-09 18:23 ` Sylwester Nawrocki
@ 2011-09-09 20:58 ` Janusz Krzysztofik
2011-09-10 10:30 ` Guennadi Liakhovetski
2011-09-10 11:14 ` Sylwester Nawrocki
0 siblings, 2 replies; 22+ messages in thread
From: Janusz Krzysztofik @ 2011-09-09 20:58 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Guennadi Liakhovetski, linux-media, Hans Verkuil,
Mauro Carvalho Chehab
On Fri, 9 Sep 2011 at 20:23:38 Sylwester Nawrocki wrote:
> Hi,
>
> On 09/09/2011 08:01 PM, Guennadi Liakhovetski wrote:
[snip]
> > I basically agree with all your comments apart from maybe
> >
> > [snip]
> >
> >>> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> >>> priv->colorspace = V4L2_COLORSPACE_JPEG;
> >>>
> >>> ret = ov6650_video_probe(icd, client);
> >>> + if (!ret)
> >>> + ret = v4l2_ctrl_handler_setup(&priv->hdl);
> >>
> >> Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
> >> fails? Its usage is documented as optional.
> >
> > Not sure what the standard really meant, but it looks like this is done in
> > all patches in this series. So, we'd have to change this everywhere. Most
> > other drivers indeed do not care.
>
> The usage of v4l2_ctrl_handler_setup() is optional, but if this function
> is not used, then AFAIU the driver writer needs to ensure the control's
> values after the device is initialized are exactly as those specified during
> the control creation. Of course v4l2_ctrl_handler_setup() failure might
> mean s_ctrl op failed, which might be caused by some H/W access errors.
> So IMHO it is always a good idea to check the return value if we know
> the batch controls setup shouldn't fail.
I'm not for ignoring that return value, only wondering if the i2c_driver
.probe handler should really fail in such cases, effectivelly preventing
the device from being accessible at all.
Perhaps a warning message would be sufficient?
Thanks,
Janusz
>
> --
> Regards,
> Sylwester
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 08/13 v3] ov6650: convert to the control framework.
2011-09-09 20:58 ` Janusz Krzysztofik
@ 2011-09-10 10:30 ` Guennadi Liakhovetski
2011-09-10 11:14 ` Sylwester Nawrocki
1 sibling, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-10 10:30 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Sylwester Nawrocki, linux-media, Hans Verkuil,
Mauro Carvalho Chehab
On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:
> On Fri, 9 Sep 2011 at 20:23:38 Sylwester Nawrocki wrote:
> > Hi,
> >
> > On 09/09/2011 08:01 PM, Guennadi Liakhovetski wrote:
>
> [snip]
>
> > > I basically agree with all your comments apart from maybe
> > >
> > > [snip]
> > >
> > >>> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> > >>> priv->colorspace = V4L2_COLORSPACE_JPEG;
> > >>>
> > >>> ret = ov6650_video_probe(icd, client);
> > >>> + if (!ret)
> > >>> + ret = v4l2_ctrl_handler_setup(&priv->hdl);
> > >>
> > >> Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
> > >> fails? Its usage is documented as optional.
> > >
> > > Not sure what the standard really meant, but it looks like this is done in
> > > all patches in this series. So, we'd have to change this everywhere. Most
> > > other drivers indeed do not care.
> >
> > The usage of v4l2_ctrl_handler_setup() is optional, but if this function
> > is not used, then AFAIU the driver writer needs to ensure the control's
> > values after the device is initialized are exactly as those specified during
> > the control creation. Of course v4l2_ctrl_handler_setup() failure might
> > mean s_ctrl op failed, which might be caused by some H/W access errors.
> > So IMHO it is always a good idea to check the return value if we know
> > the batch controls setup shouldn't fail.
>
> I'm not for ignoring that return value, only wondering if the i2c_driver
> .probe handler should really fail in such cases, effectivelly preventing
> the device from being accessible at all.
>
> Perhaps a warning message would be sufficient?
Well, restoring the state is normally something pretty basic like writing
a couple of registers via i2c. If that didn't work out, there's little
chance you'll be able to use the device anyway. So, I'd feel pretty safe
with erroring out. If we ever encounter a case, where this can fail in a
non-fatal way, we can always relax it.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 08/13 v3] ov6650: convert to the control framework.
2011-09-09 20:58 ` Janusz Krzysztofik
2011-09-10 10:30 ` Guennadi Liakhovetski
@ 2011-09-10 11:14 ` Sylwester Nawrocki
1 sibling, 0 replies; 22+ messages in thread
From: Sylwester Nawrocki @ 2011-09-10 11:14 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Guennadi Liakhovetski, linux-media, Hans Verkuil,
Mauro Carvalho Chehab
On 09/09/2011 10:58 PM, Janusz Krzysztofik wrote:
> On Fri, 9 Sep 2011 at 20:23:38 Sylwester Nawrocki wrote:
>> On 09/09/2011 08:01 PM, Guennadi Liakhovetski wrote:
>
> [snip]
>
>>> I basically agree with all your comments apart from maybe
>>>
>>> [snip]
>>>
>>>>> @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
>>>>> priv->colorspace = V4L2_COLORSPACE_JPEG;
>>>>>
>>>>> ret = ov6650_video_probe(icd, client);
>>>>> + if (!ret)
>>>>> + ret = v4l2_ctrl_handler_setup(&priv->hdl);
>>>>
>>>> Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
>>>> fails? Its usage is documented as optional.
>>>
>>> Not sure what the standard really meant, but it looks like this is done in
>>> all patches in this series. So, we'd have to change this everywhere. Most
>>> other drivers indeed do not care.
>>
>> The usage of v4l2_ctrl_handler_setup() is optional, but if this function
>> is not used, then AFAIU the driver writer needs to ensure the control's
>> values after the device is initialized are exactly as those specified during
>> the control creation. Of course v4l2_ctrl_handler_setup() failure might
>> mean s_ctrl op failed, which might be caused by some H/W access errors.
>> So IMHO it is always a good idea to check the return value if we know
>> the batch controls setup shouldn't fail.
>
> I'm not for ignoring that return value, only wondering if the i2c_driver
> .probe handler should really fail in such cases, effectivelly preventing
> the device from being accessible at all.
>
> Perhaps a warning message would be sufficient?
I guess, on individual cases - yes, but I wouldn't take it as a general rule.
If the device fails from the beginning it will probably not be really usable
thereafter.
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 08/13 v3] ov6650: convert to the control framework.
2011-09-09 18:01 ` Guennadi Liakhovetski
2011-09-09 18:23 ` Sylwester Nawrocki
@ 2011-09-09 20:39 ` Janusz Krzysztofik
2011-09-09 21:00 ` Guennadi Liakhovetski
1 sibling, 1 reply; 22+ messages in thread
From: Janusz Krzysztofik @ 2011-09-09 20:39 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linux-media, Hans Verkuil, Mauro Carvalho Chehab
On Fri, 9 Sep 2011 at 20:01:14 Guennadi Liakhovetski wrote:
> Hi Janusz
>
> On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:
>
> > On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
> > > From: Hans Verkuil <hans.verkuil@cisco.com>
> > >
> > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > > [g.liakhovetski@gmx.de: simplified pointer arithmetic]
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> >
> > Hi,
> > I've successfully tested this one for you, to the extent possible using
> > mplayer, i.e., only saturation, hue and brightness controls, and an
> > overall functionality.
>
> Thanks for testing and reviewing!
>
> > Modifications to other (not runtime tested) controls look good to me,
> > except for one copy-paste mistake, see below. With that erratic REG_BLUE
> > corrected:
> >
> > Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> >
> > There are also a few minor comments for you to consider.
>
> Well, some of them are not so minor, I would say;-) But I personally would
> be happy to have this just as an incremental patch. Would you like to
> prepare one or should I do it?
OK, I can do it.
Do you want them all (except the one below) go into the incremental
patch, or would you rather fix that REG_RED bug still before applying?
Thanks,
Janusz
>
> I basically agree with all your comments apart from maybe
>
> [snip]
>
> > > @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> > > priv->colorspace = V4L2_COLORSPACE_JPEG;
> > >
> > > ret = ov6650_video_probe(icd, client);
> > > + if (!ret)
> > > + ret = v4l2_ctrl_handler_setup(&priv->hdl);
> >
> > Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
> > fails? Its usage is documented as optional.
>
> Not sure what the standard really meant, but it looks like this is done in
> all patches in this series. So, we'd have to change this everywhere. Most
> other drivers indeed do not care.
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> --
> 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
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 08/13 v3] ov6650: convert to the control framework.
2011-09-09 20:39 ` Janusz Krzysztofik
@ 2011-09-09 21:00 ` Guennadi Liakhovetski
0 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-09 21:00 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: linux-media, Hans Verkuil, Mauro Carvalho Chehab
On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:
> On Fri, 9 Sep 2011 at 20:01:14 Guennadi Liakhovetski wrote:
> > Hi Janusz
> >
> > On Fri, 9 Sep 2011, Janusz Krzysztofik wrote:
> >
> > > On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote:
> > > > From: Hans Verkuil <hans.verkuil@cisco.com>
> > > >
> > > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > > > [g.liakhovetski@gmx.de: simplified pointer arithmetic]
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > >
> > > Hi,
> > > I've successfully tested this one for you, to the extent possible using
> > > mplayer, i.e., only saturation, hue and brightness controls, and an
> > > overall functionality.
> >
> > Thanks for testing and reviewing!
> >
> > > Modifications to other (not runtime tested) controls look good to me,
> > > except for one copy-paste mistake, see below. With that erratic REG_BLUE
> > > corrected:
> > >
> > > Acked-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> > >
> > > There are also a few minor comments for you to consider.
> >
> > Well, some of them are not so minor, I would say;-) But I personally would
> > be happy to have this just as an incremental patch. Would you like to
> > prepare one or should I do it?
>
> OK, I can do it.
>
> Do you want them all (except the one below) go into the incremental
> patch, or would you rather fix that REG_RED bug still before applying?
Just put them all in one.
Thanks
Guennadi
>
> Thanks,
> Janusz
>
> >
> > I basically agree with all your comments apart from maybe
> >
> > [snip]
> >
> > > > @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client,
> > > > priv->colorspace = V4L2_COLORSPACE_JPEG;
> > > >
> > > > ret = ov6650_video_probe(icd, client);
> > > > + if (!ret)
> > > > + ret = v4l2_ctrl_handler_setup(&priv->hdl);
> > >
> > > Are you sure the probe function should fail if v4l2_ctrl_handler_setup()
> > > fails? Its usage is documented as optional.
> >
> > Not sure what the standard really meant, but it looks like this is done in
> > all patches in this series. So, we'd have to change this everywhere. Most
> > other drivers indeed do not care.
> >
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> > --
> > 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
> >
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 09/13 v3] ov9740: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (7 preceding siblings ...)
2011-09-08 8:44 ` [PATCH 08/13 v3] ov6650: " Guennadi Liakhovetski
@ 2011-09-08 8:44 ` Guennadi Liakhovetski
2011-09-08 8:44 ` [PATCH 10/13 v3] mt9m001: " Guennadi Liakhovetski
` (3 subsequent siblings)
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:44 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/ov9740.c | 83 ++++++++++++++----------------------------
1 files changed, 28 insertions(+), 55 deletions(-)
diff --git a/drivers/media/video/ov9740.c b/drivers/media/video/ov9740.c
index 5920f60..3dd910d 100644
--- a/drivers/media/video/ov9740.c
+++ b/drivers/media/video/ov9740.c
@@ -18,6 +18,7 @@
#include <media/soc_camera.h>
#include <media/soc_mediabus.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
#define to_ov9740(sd) container_of(sd, struct ov9740_priv, subdev)
@@ -194,6 +195,7 @@ struct ov9740_reg {
struct ov9740_priv {
struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
int ident;
u16 model;
@@ -394,27 +396,6 @@ static enum v4l2_mbus_pixelcode ov9740_codes[] = {
V4L2_MBUS_FMT_YUYV8_2X8,
};
-static const struct v4l2_queryctrl ov9740_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,
- },
-};
-
/* read a register */
static int ov9740_reg_read(struct i2c_client *client, u16 reg, u8 *val)
{
@@ -771,36 +752,18 @@ static int ov9740_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
return 0;
}
-/* Get status of additional camera capabilities */
-static int ov9740_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
- struct ov9740_priv *priv = to_ov9740(sd);
-
- switch (ctrl->id) {
- case V4L2_CID_VFLIP:
- ctrl->value = priv->flag_vflip;
- break;
- case V4L2_CID_HFLIP:
- ctrl->value = priv->flag_hflip;
- break;
- default:
- return -EINVAL;
- }
-
- return 0;
-}
-
/* Set status of additional camera capabilities */
-static int ov9740_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int ov9740_s_ctrl(struct v4l2_ctrl *ctrl)
{
- struct ov9740_priv *priv = to_ov9740(sd);
+ struct ov9740_priv *priv =
+ container_of(ctrl->handler, struct ov9740_priv, hdl);
switch (ctrl->id) {
case V4L2_CID_VFLIP:
- priv->flag_vflip = ctrl->value;
+ priv->flag_vflip = ctrl->val;
break;
case V4L2_CID_HFLIP:
- priv->flag_hflip = ctrl->value;
+ priv->flag_hflip = ctrl->val;
break;
default:
return -EINVAL;
@@ -925,11 +888,6 @@ err:
return ret;
}
-static struct soc_camera_ops ov9740_ops = {
- .controls = ov9740_controls,
- .num_controls = ARRAY_SIZE(ov9740_controls),
-};
-
/* Request bus settings on camera side */
static int ov9740_g_mbus_config(struct v4l2_subdev *sd,
struct v4l2_mbus_config *cfg)
@@ -958,8 +916,6 @@ static struct v4l2_subdev_video_ops ov9740_video_ops = {
};
static struct v4l2_subdev_core_ops ov9740_core_ops = {
- .g_ctrl = ov9740_g_ctrl,
- .s_ctrl = ov9740_s_ctrl,
.g_chip_ident = ov9740_g_chip_ident,
.s_power = ov9740_s_power,
#ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -973,6 +929,10 @@ static struct v4l2_subdev_ops ov9740_subdev_ops = {
.video = &ov9740_video_ops,
};
+static const struct v4l2_ctrl_ops ov9740_ctrl_ops = {
+ .s_ctrl = ov9740_s_ctrl,
+};
+
/*
* i2c_driver function
*/
@@ -980,7 +940,7 @@ static int ov9740_probe(struct i2c_client *client,
const struct i2c_device_id *did)
{
struct ov9740_priv *priv;
- struct soc_camera_device *icd = client->dev.platform_data;
+ struct soc_camera_device *icd = client->dev.platform_data;
struct soc_camera_link *icl;
int ret;
@@ -1002,12 +962,24 @@ static int ov9740_probe(struct i2c_client *client,
}
v4l2_i2c_subdev_init(&priv->subdev, client, &ov9740_subdev_ops);
+ v4l2_ctrl_handler_init(&priv->hdl, 13);
+ v4l2_ctrl_new_std(&priv->hdl, &ov9740_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&priv->hdl, &ov9740_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ priv->subdev.ctrl_handler = &priv->hdl;
+ if (priv->hdl.error) {
+ int err = priv->hdl.error;
- icd->ops = &ov9740_ops;
+ kfree(priv);
+ return err;
+ }
ret = ov9740_video_probe(icd, client);
+ if (!ret)
+ ret = v4l2_ctrl_handler_setup(&priv->hdl);
if (ret < 0) {
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
}
@@ -1018,8 +990,9 @@ static int ov9740_remove(struct i2c_client *client)
{
struct ov9740_priv *priv = i2c_get_clientdata(client);
+ v4l2_device_unregister_subdev(&priv->subdev);
+ v4l2_ctrl_handler_free(&priv->hdl);
kfree(priv);
-
return 0;
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 10/13 v3] mt9m001: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (8 preceding siblings ...)
2011-09-08 8:44 ` [PATCH 09/13 v3] ov9740: " Guennadi Liakhovetski
@ 2011-09-08 8:44 ` Guennadi Liakhovetski
2011-09-08 8:44 ` [PATCH 11/13 v3] mt9m111: " Guennadi Liakhovetski
` (2 subsequent siblings)
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:44 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/mt9m001.c | 218 +++++++++++++++--------------------------
1 files changed, 77 insertions(+), 141 deletions(-)
diff --git a/drivers/media/video/mt9m001.c b/drivers/media/video/mt9m001.c
index 3555e85..42bb3c8 100644
--- a/drivers/media/video/mt9m001.c
+++ b/drivers/media/video/mt9m001.c
@@ -17,6 +17,7 @@
#include <media/soc_mediabus.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-ctrls.h>
/*
* mt9m001 i2c address 0x5d
@@ -85,15 +86,19 @@ static const struct mt9m001_datafmt mt9m001_monochrome_fmts[] = {
struct mt9m001 {
struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
+ struct {
+ /* exposure/auto-exposure cluster */
+ struct v4l2_ctrl *autoexposure;
+ struct v4l2_ctrl *exposure;
+ };
struct v4l2_rect rect; /* Sensor window */
const struct mt9m001_datafmt *fmt;
const struct mt9m001_datafmt *fmts;
int num_fmts;
int model; /* V4L2_IDENT_MT9M001* codes from v4l2-chip-ident.h */
- unsigned int gain;
- unsigned int exposure;
+ unsigned int total_h;
unsigned short y_skip_top; /* Lines to skip at the top */
- unsigned char autoexposure;
};
static struct mt9m001 *to_mt9m001(const struct i2c_client *client)
@@ -171,10 +176,8 @@ static int mt9m001_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct mt9m001 *mt9m001 = to_mt9m001(client);
struct v4l2_rect rect = a->c;
- struct soc_camera_device *icd = client->dev.platform_data;
int ret;
const u16 hblank = 9, vblank = 25;
- unsigned int total_h;
if (mt9m001->fmts == mt9m001_colour_fmts)
/*
@@ -193,7 +196,7 @@ static int mt9m001_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
soc_camera_limit_side(&rect.top, &rect.height,
MT9M001_ROW_SKIP, MT9M001_MIN_HEIGHT, MT9M001_MAX_HEIGHT);
- total_h = rect.height + mt9m001->y_skip_top + vblank;
+ mt9m001->total_h = rect.height + mt9m001->y_skip_top + vblank;
/* Blanking and start values - default... */
ret = reg_write(client, MT9M001_HORIZONTAL_BLANKING, hblank);
@@ -213,17 +216,8 @@ static int mt9m001_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
if (!ret)
ret = reg_write(client, MT9M001_WINDOW_HEIGHT,
rect.height + mt9m001->y_skip_top - 1);
- if (!ret && mt9m001->autoexposure) {
- ret = reg_write(client, MT9M001_SHUTTER_WIDTH, total_h);
- if (!ret) {
- const struct v4l2_queryctrl *qctrl =
- soc_camera_find_qctrl(icd->ops,
- V4L2_CID_EXPOSURE);
- mt9m001->exposure = (524 + (total_h - 1) *
- (qctrl->maximum - qctrl->minimum)) /
- 1048 + qctrl->minimum;
- }
- }
+ if (!ret && v4l2_ctrl_g_ctrl(mt9m001->autoexposure) == V4L2_EXPOSURE_AUTO)
+ ret = reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h);
if (!ret)
mt9m001->rect = rect;
@@ -383,105 +377,48 @@ static int mt9m001_s_register(struct v4l2_subdev *sd,
}
#endif
-static const struct v4l2_queryctrl mt9m001_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_GAIN,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Gain",
- .minimum = 0,
- .maximum = 127,
- .step = 1,
- .default_value = 64,
- .flags = V4L2_CTRL_FLAG_SLIDER,
- }, {
- .id = V4L2_CID_EXPOSURE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Exposure",
- .minimum = 1,
- .maximum = 255,
- .step = 1,
- .default_value = 255,
- .flags = V4L2_CTRL_FLAG_SLIDER,
- }, {
- .id = V4L2_CID_EXPOSURE_AUTO,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Automatic Exposure",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 1,
- }
-};
-
-static struct soc_camera_ops mt9m001_ops = {
- .controls = mt9m001_controls,
- .num_controls = ARRAY_SIZE(mt9m001_controls),
-};
-
-static int mt9m001_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9m001_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct mt9m001 *mt9m001 = to_mt9m001(client);
- int data;
+ struct mt9m001 *mt9m001 = container_of(ctrl->handler,
+ struct mt9m001, hdl);
+ s32 min, max;
switch (ctrl->id) {
- case V4L2_CID_VFLIP:
- data = reg_read(client, MT9M001_READ_OPTIONS2);
- if (data < 0)
- return -EIO;
- ctrl->value = !!(data & 0x8000);
- break;
case V4L2_CID_EXPOSURE_AUTO:
- ctrl->value = mt9m001->autoexposure;
- break;
- case V4L2_CID_GAIN:
- ctrl->value = mt9m001->gain;
- break;
- case V4L2_CID_EXPOSURE:
- ctrl->value = mt9m001->exposure;
+ min = mt9m001->exposure->minimum;
+ max = mt9m001->exposure->maximum;
+ mt9m001->exposure->val =
+ (524 + (mt9m001->total_h - 1) * (max - min)) / 1048 + min;
break;
}
return 0;
}
-static int mt9m001_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9m001_s_ctrl(struct v4l2_ctrl *ctrl)
{
+ struct mt9m001 *mt9m001 = container_of(ctrl->handler,
+ struct mt9m001, hdl);
+ struct v4l2_subdev *sd = &mt9m001->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct mt9m001 *mt9m001 = to_mt9m001(client);
- struct soc_camera_device *icd = client->dev.platform_data;
- const struct v4l2_queryctrl *qctrl;
+ struct v4l2_ctrl *exp = mt9m001->exposure;
int data;
- qctrl = soc_camera_find_qctrl(&mt9m001_ops, ctrl->id);
-
- if (!qctrl)
- return -EINVAL;
-
switch (ctrl->id) {
case V4L2_CID_VFLIP:
- if (ctrl->value)
+ if (ctrl->val)
data = reg_set(client, MT9M001_READ_OPTIONS2, 0x8000);
else
data = reg_clear(client, MT9M001_READ_OPTIONS2, 0x8000);
if (data < 0)
return -EIO;
- break;
+ return 0;
+
case V4L2_CID_GAIN:
- if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
- return -EINVAL;
/* See Datasheet Table 7, Gain settings. */
- if (ctrl->value <= qctrl->default_value) {
+ if (ctrl->val <= ctrl->default_value) {
/* Pack it into 0..1 step 0.125, register values 0..8 */
- unsigned long range = qctrl->default_value - qctrl->minimum;
- data = ((ctrl->value - qctrl->minimum) * 8 + range / 2) / range;
+ unsigned long range = ctrl->default_value - ctrl->minimum;
+ data = ((ctrl->val - ctrl->minimum) * 8 + range / 2) / range;
dev_dbg(&client->dev, "Setting gain %d\n", data);
data = reg_write(client, MT9M001_GLOBAL_GAIN, data);
@@ -490,8 +427,8 @@ static int mt9m001_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
} else {
/* Pack it into 1.125..15 variable step, register values 9..67 */
/* We assume qctrl->maximum - qctrl->default_value - 1 > 0 */
- unsigned long range = qctrl->maximum - qctrl->default_value - 1;
- unsigned long gain = ((ctrl->value - qctrl->default_value - 1) *
+ unsigned long range = ctrl->maximum - ctrl->default_value - 1;
+ unsigned long gain = ((ctrl->val - ctrl->default_value - 1) *
111 + range / 2) / range + 9;
if (gain <= 32)
@@ -507,47 +444,30 @@ static int mt9m001_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
if (data < 0)
return -EIO;
}
+ return 0;
- /* Success */
- mt9m001->gain = ctrl->value;
- break;
- case V4L2_CID_EXPOSURE:
- /* mt9m001 has maximum == default */
- if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
- return -EINVAL;
- else {
- unsigned long range = qctrl->maximum - qctrl->minimum;
- unsigned long shutter = ((ctrl->value - qctrl->minimum) * 1048 +
+ case V4L2_CID_EXPOSURE_AUTO:
+ if (ctrl->val == V4L2_EXPOSURE_MANUAL) {
+ unsigned long range = exp->maximum - exp->minimum;
+ unsigned long shutter = ((exp->val - exp->minimum) * 1048 +
range / 2) / range + 1;
dev_dbg(&client->dev,
"Setting shutter width from %d to %lu\n",
- reg_read(client, MT9M001_SHUTTER_WIDTH),
- shutter);
+ reg_read(client, MT9M001_SHUTTER_WIDTH), shutter);
if (reg_write(client, MT9M001_SHUTTER_WIDTH, shutter) < 0)
return -EIO;
- mt9m001->exposure = ctrl->value;
- mt9m001->autoexposure = 0;
- }
- break;
- case V4L2_CID_EXPOSURE_AUTO:
- if (ctrl->value) {
+ } else {
const u16 vblank = 25;
- unsigned int total_h = mt9m001->rect.height +
+
+ mt9m001->total_h = mt9m001->rect.height +
mt9m001->y_skip_top + vblank;
- if (reg_write(client, MT9M001_SHUTTER_WIDTH,
- total_h) < 0)
+ if (reg_write(client, MT9M001_SHUTTER_WIDTH, mt9m001->total_h) < 0)
return -EIO;
- qctrl = soc_camera_find_qctrl(icd->ops, V4L2_CID_EXPOSURE);
- mt9m001->exposure = (524 + (total_h - 1) *
- (qctrl->maximum - qctrl->minimum)) /
- 1048 + qctrl->minimum;
- mt9m001->autoexposure = 1;
- } else
- mt9m001->autoexposure = 0;
- break;
+ }
+ return 0;
}
- return 0;
+ return -EINVAL;
}
/*
@@ -621,10 +541,7 @@ static int mt9m001_video_probe(struct soc_camera_device *icd,
dev_err(&client->dev, "Failed to initialise the camera\n");
/* mt9m001_init() has reset the chip, returning registers to defaults */
- mt9m001->gain = 64;
- mt9m001->exposure = 255;
-
- return ret;
+ return v4l2_ctrl_handler_setup(&mt9m001->hdl);
}
static void mt9m001_video_remove(struct soc_camera_device *icd)
@@ -647,9 +564,12 @@ static int mt9m001_g_skip_top_lines(struct v4l2_subdev *sd, u32 *lines)
return 0;
}
+static const struct v4l2_ctrl_ops mt9m001_ctrl_ops = {
+ .g_volatile_ctrl = mt9m001_g_volatile_ctrl,
+ .s_ctrl = mt9m001_s_ctrl,
+};
+
static struct v4l2_subdev_core_ops mt9m001_subdev_core_ops = {
- .g_ctrl = mt9m001_g_ctrl,
- .s_ctrl = mt9m001_s_ctrl,
.g_chip_ident = mt9m001_g_chip_ident,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = mt9m001_g_register,
@@ -765,25 +685,40 @@ static int mt9m001_probe(struct i2c_client *client,
return -ENOMEM;
v4l2_i2c_subdev_init(&mt9m001->subdev, client, &mt9m001_subdev_ops);
+ v4l2_ctrl_handler_init(&mt9m001->hdl, 4);
+ v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
+ V4L2_CID_GAIN, 0, 127, 1, 64);
+ mt9m001->exposure = v4l2_ctrl_new_std(&mt9m001->hdl, &mt9m001_ctrl_ops,
+ V4L2_CID_EXPOSURE, 1, 255, 1, 255);
+ /*
+ * Simulated autoexposure. If enabled, we calculate shutter width
+ * ourselves in the driver based on vertical blanking and frame width
+ */
+ mt9m001->autoexposure = v4l2_ctrl_new_std_menu(&mt9m001->hdl,
+ &mt9m001_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+ V4L2_EXPOSURE_AUTO);
+ mt9m001->subdev.ctrl_handler = &mt9m001->hdl;
+ if (mt9m001->hdl.error) {
+ int err = mt9m001->hdl.error;
- /* Second stage probe - when a capture adapter is there */
- icd->ops = &mt9m001_ops;
+ kfree(mt9m001);
+ return err;
+ }
+ v4l2_ctrl_auto_cluster(2, &mt9m001->autoexposure,
+ V4L2_EXPOSURE_MANUAL, true);
+ /* Second stage probe - when a capture adapter is there */
mt9m001->y_skip_top = 0;
mt9m001->rect.left = MT9M001_COLUMN_SKIP;
mt9m001->rect.top = MT9M001_ROW_SKIP;
mt9m001->rect.width = MT9M001_MAX_WIDTH;
mt9m001->rect.height = MT9M001_MAX_HEIGHT;
- /*
- * Simulated autoexposure. If enabled, we calculate shutter width
- * ourselves in the driver based on vertical blanking and frame width
- */
- mt9m001->autoexposure = 1;
-
ret = mt9m001_video_probe(icd, client);
if (ret) {
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&mt9m001->hdl);
kfree(mt9m001);
}
@@ -795,7 +730,8 @@ static int mt9m001_remove(struct i2c_client *client)
struct mt9m001 *mt9m001 = to_mt9m001(client);
struct soc_camera_device *icd = client->dev.platform_data;
- icd->ops = NULL;
+ v4l2_device_unregister_subdev(&mt9m001->subdev);
+ v4l2_ctrl_handler_free(&mt9m001->hdl);
mt9m001_video_remove(icd);
kfree(mt9m001);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 11/13 v3] mt9m111: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (9 preceding siblings ...)
2011-09-08 8:44 ` [PATCH 10/13 v3] mt9m001: " Guennadi Liakhovetski
@ 2011-09-08 8:44 ` Guennadi Liakhovetski
2011-09-08 8:44 ` [PATCH 12/13 v3] mt9t031: " Guennadi Liakhovetski
2011-09-08 8:44 ` [PATCH 13/13 v3] soc_camera: remove the now obsolete struct soc_camera_ops Guennadi Liakhovetski
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:44 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/mt9m111.c | 203 ++++++++++-------------------------------
1 files changed, 48 insertions(+), 155 deletions(-)
diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index d7a0773..41df6c3 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -17,6 +17,7 @@
#include <media/soc_camera.h>
#include <media/soc_mediabus.h>
#include <media/v4l2-common.h>
+#include <media/v4l2-ctrls.h>
#include <media/v4l2-chip-ident.h>
/*
@@ -178,6 +179,8 @@ enum mt9m111_context {
struct mt9m111 {
struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
+ struct v4l2_ctrl *gain;
int model; /* V4L2_IDENT_MT9M111 or V4L2_IDENT_MT9M112 code
* from v4l2-chip-ident.h */
enum mt9m111_context context;
@@ -185,13 +188,8 @@ struct mt9m111 {
struct mutex power_lock; /* lock to protect power_count */
int power_count;
const struct mt9m111_datafmt *fmt;
- unsigned int gain;
- unsigned char autoexposure;
unsigned char datawidth;
unsigned int powered:1;
- unsigned int hflip:1;
- unsigned int vflip:1;
- unsigned int autowhitebalance:1;
};
static struct mt9m111 *to_mt9m111(const struct i2c_client *client)
@@ -645,48 +643,6 @@ static int mt9m111_s_register(struct v4l2_subdev *sd,
}
#endif
-static const struct v4l2_queryctrl mt9m111_controls[] = {
- {
- .id = V4L2_CID_VFLIP,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Flip Verticaly",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 0,
- }, {
- .id = V4L2_CID_HFLIP,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Flip Horizontaly",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 0,
- }, { /* gain = 1/32*val (=>gain=1 if val==32) */
- .id = V4L2_CID_GAIN,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Gain",
- .minimum = 0,
- .maximum = 63 * 2 * 2,
- .step = 1,
- .default_value = 32,
- .flags = V4L2_CTRL_FLAG_SLIDER,
- }, {
- .id = V4L2_CID_EXPOSURE_AUTO,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Auto Exposure",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 1,
- }
-};
-
-static struct soc_camera_ops mt9m111_ops = {
- .controls = mt9m111_controls,
- .num_controls = ARRAY_SIZE(mt9m111_controls),
-};
-
static int mt9m111_set_flip(struct mt9m111 *mt9m111, int flip, int mask)
{
struct i2c_client *client = v4l2_get_subdevdata(&mt9m111->subdev);
@@ -727,7 +683,6 @@ static int mt9m111_set_global_gain(struct mt9m111 *mt9m111, int gain)
if (gain > 63 * 2 * 2)
return -EINVAL;
- mt9m111->gain = gain;
if ((gain >= 64 * 2) && (gain < 63 * 2 * 2))
val = (1 << 10) | (1 << 9) | (gain / 4);
else if ((gain >= 64) && (gain < 64 * 2))
@@ -741,118 +696,47 @@ static int mt9m111_set_global_gain(struct mt9m111 *mt9m111, int gain)
static int mt9m111_set_autoexposure(struct mt9m111 *mt9m111, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(&mt9m111->subdev);
- int ret;
if (on)
- ret = reg_set(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOEXPO_EN);
- else
- ret = reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOEXPO_EN);
-
- if (!ret)
- mt9m111->autoexposure = on;
-
- return ret;
+ return reg_set(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOEXPO_EN);
+ return reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOEXPO_EN);
}
static int mt9m111_set_autowhitebalance(struct mt9m111 *mt9m111, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(&mt9m111->subdev);
- int ret;
if (on)
- ret = reg_set(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
- else
- ret = reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
-
- if (!ret)
- mt9m111->autowhitebalance = on;
-
- return ret;
+ return reg_set(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
+ return reg_clear(OPER_MODE_CTRL, MT9M111_OPMODE_AUTOWHITEBAL_EN);
}
-static int mt9m111_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9m111_s_ctrl(struct v4l2_ctrl *ctrl)
{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
- int data;
+ struct mt9m111 *mt9m111 = container_of(ctrl->handler,
+ struct mt9m111, hdl);
switch (ctrl->id) {
case V4L2_CID_VFLIP:
- if (mt9m111->context == HIGHPOWER)
- data = reg_read(READ_MODE_B);
- else
- data = reg_read(READ_MODE_A);
-
- if (data < 0)
- return -EIO;
- ctrl->value = !!(data & MT9M111_RMB_MIRROR_ROWS);
- break;
- case V4L2_CID_HFLIP:
- if (mt9m111->context == HIGHPOWER)
- data = reg_read(READ_MODE_B);
- else
- data = reg_read(READ_MODE_A);
-
- if (data < 0)
- return -EIO;
- ctrl->value = !!(data & MT9M111_RMB_MIRROR_COLS);
- break;
- case V4L2_CID_GAIN:
- data = mt9m111_get_global_gain(mt9m111);
- if (data < 0)
- return data;
- ctrl->value = data;
- break;
- case V4L2_CID_EXPOSURE_AUTO:
- ctrl->value = mt9m111->autoexposure;
- break;
- case V4L2_CID_AUTO_WHITE_BALANCE:
- ctrl->value = mt9m111->autowhitebalance;
- break;
- }
- return 0;
-}
-
-static int mt9m111_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
- struct mt9m111 *mt9m111 = container_of(sd, struct mt9m111, subdev);
- const struct v4l2_queryctrl *qctrl;
- int ret;
-
- qctrl = soc_camera_find_qctrl(&mt9m111_ops, ctrl->id);
- if (!qctrl)
- return -EINVAL;
-
- switch (ctrl->id) {
- case V4L2_CID_VFLIP:
- mt9m111->vflip = ctrl->value;
- ret = mt9m111_set_flip(mt9m111, ctrl->value,
+ return mt9m111_set_flip(mt9m111, ctrl->val,
MT9M111_RMB_MIRROR_ROWS);
- break;
case V4L2_CID_HFLIP:
- mt9m111->hflip = ctrl->value;
- ret = mt9m111_set_flip(mt9m111, ctrl->value,
+ return mt9m111_set_flip(mt9m111, ctrl->val,
MT9M111_RMB_MIRROR_COLS);
- break;
case V4L2_CID_GAIN:
- ret = mt9m111_set_global_gain(mt9m111, ctrl->value);
- break;
+ return mt9m111_set_global_gain(mt9m111, ctrl->val);
case V4L2_CID_EXPOSURE_AUTO:
- ret = mt9m111_set_autoexposure(mt9m111, ctrl->value);
- break;
+ return mt9m111_set_autoexposure(mt9m111, ctrl->val);
case V4L2_CID_AUTO_WHITE_BALANCE:
- ret = mt9m111_set_autowhitebalance(mt9m111, ctrl->value);
- break;
- default:
- ret = -EINVAL;
+ return mt9m111_set_autowhitebalance(mt9m111, ctrl->val);
}
- return ret;
+ return -EINVAL;
}
static int mt9m111_suspend(struct mt9m111 *mt9m111)
{
- mt9m111->gain = mt9m111_get_global_gain(mt9m111);
+ v4l2_ctrl_s_ctrl(mt9m111->gain, mt9m111_get_global_gain(mt9m111));
return 0;
}
@@ -862,11 +746,7 @@ static void mt9m111_restore_state(struct mt9m111 *mt9m111)
mt9m111_set_context(mt9m111, mt9m111->context);
mt9m111_set_pixfmt(mt9m111, mt9m111->fmt->code);
mt9m111_setup_rect(mt9m111, &mt9m111->rect);
- mt9m111_set_flip(mt9m111, mt9m111->hflip, MT9M111_RMB_MIRROR_COLS);
- mt9m111_set_flip(mt9m111, mt9m111->vflip, MT9M111_RMB_MIRROR_ROWS);
- mt9m111_set_global_gain(mt9m111, mt9m111->gain);
- mt9m111_set_autoexposure(mt9m111, mt9m111->autoexposure);
- mt9m111_set_autowhitebalance(mt9m111, mt9m111->autowhitebalance);
+ v4l2_ctrl_handler_setup(&mt9m111->hdl);
}
static int mt9m111_resume(struct mt9m111 *mt9m111)
@@ -894,8 +774,6 @@ static int mt9m111_init(struct mt9m111 *mt9m111)
ret = mt9m111_reset(mt9m111);
if (!ret)
ret = mt9m111_set_context(mt9m111, mt9m111->context);
- if (!ret)
- ret = mt9m111_set_autoexposure(mt9m111, mt9m111->autoexposure);
if (ret)
dev_err(&client->dev, "mt9m111 init failed: %d\n", ret);
return ret;
@@ -916,9 +794,6 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
BUG_ON(!icd->parent ||
to_soc_camera_host(icd->parent)->nr != icd->iface);
- mt9m111->autoexposure = 1;
- mt9m111->autowhitebalance = 1;
-
data = reg_read(CHIP_VERSION);
switch (data) {
@@ -932,17 +807,16 @@ static int mt9m111_video_probe(struct soc_camera_device *icd,
dev_info(&client->dev, "Detected a MT9M112 chip ID %x\n", data);
break;
default:
- ret = -ENODEV;
dev_err(&client->dev,
"No MT9M111/MT9M112/MT9M131 chip detected register read %x\n",
data);
- goto ei2c;
+ return -ENODEV;
}
ret = mt9m111_init(mt9m111);
-
-ei2c:
- return ret;
+ if (ret)
+ return ret;
+ return v4l2_ctrl_handler_setup(&mt9m111->hdl);
}
static int mt9m111_s_power(struct v4l2_subdev *sd, int on)
@@ -979,9 +853,11 @@ out:
return ret;
}
+static const struct v4l2_ctrl_ops mt9m111_ctrl_ops = {
+ .s_ctrl = mt9m111_s_ctrl,
+};
+
static struct v4l2_subdev_core_ops mt9m111_subdev_core_ops = {
- .g_ctrl = mt9m111_g_ctrl,
- .s_ctrl = mt9m111_s_ctrl,
.g_chip_ident = mt9m111_g_chip_ident,
.s_power = mt9m111_s_power,
#ifdef CONFIG_VIDEO_ADV_DEBUG
@@ -1063,10 +939,27 @@ static int mt9m111_probe(struct i2c_client *client,
return -ENOMEM;
v4l2_i2c_subdev_init(&mt9m111->subdev, client, &mt9m111_subdev_ops);
+ v4l2_ctrl_handler_init(&mt9m111->hdl, 5);
+ v4l2_ctrl_new_std(&mt9m111->hdl, &mt9m111_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&mt9m111->hdl, &mt9m111_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&mt9m111->hdl, &mt9m111_ctrl_ops,
+ V4L2_CID_AUTO_WHITE_BALANCE, 0, 1, 1, 1);
+ mt9m111->gain = v4l2_ctrl_new_std(&mt9m111->hdl, &mt9m111_ctrl_ops,
+ V4L2_CID_GAIN, 0, 63 * 2 * 2, 1, 32);
+ v4l2_ctrl_new_std_menu(&mt9m111->hdl,
+ &mt9m111_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+ V4L2_EXPOSURE_AUTO);
+ mt9m111->subdev.ctrl_handler = &mt9m111->hdl;
+ if (mt9m111->hdl.error) {
+ int err = mt9m111->hdl.error;
- /* Second stage probe - when a capture adapter is there */
- icd->ops = &mt9m111_ops;
+ kfree(mt9m111);
+ return err;
+ }
+ /* Second stage probe - when a capture adapter is there */
mt9m111->rect.left = MT9M111_MIN_DARK_COLS;
mt9m111->rect.top = MT9M111_MIN_DARK_ROWS;
mt9m111->rect.width = MT9M111_MAX_WIDTH;
@@ -1075,7 +968,7 @@ static int mt9m111_probe(struct i2c_client *client,
ret = mt9m111_video_probe(icd, client);
if (ret) {
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&mt9m111->hdl);
kfree(mt9m111);
}
@@ -1085,9 +978,9 @@ static int mt9m111_probe(struct i2c_client *client,
static int mt9m111_remove(struct i2c_client *client)
{
struct mt9m111 *mt9m111 = to_mt9m111(client);
- struct soc_camera_device *icd = client->dev.platform_data;
- icd->ops = NULL;
+ v4l2_device_unregister_subdev(&mt9m111->subdev);
+ v4l2_ctrl_handler_free(&mt9m111->hdl);
kfree(mt9m111);
return 0;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 12/13 v3] mt9t031: convert to the control framework.
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (10 preceding siblings ...)
2011-09-08 8:44 ` [PATCH 11/13 v3] mt9m111: " Guennadi Liakhovetski
@ 2011-09-08 8:44 ` Guennadi Liakhovetski
2011-09-08 8:44 ` [PATCH 13/13 v3] soc_camera: remove the now obsolete struct soc_camera_ops Guennadi Liakhovetski
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:44 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: simplified pointer arithmetic]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/mt9t031.c | 252 ++++++++++++++---------------------------
1 files changed, 86 insertions(+), 166 deletions(-)
diff --git a/drivers/media/video/mt9t031.c b/drivers/media/video/mt9t031.c
index 25fb833..7ce3799 100644
--- a/drivers/media/video/mt9t031.c
+++ b/drivers/media/video/mt9t031.c
@@ -19,6 +19,7 @@
#include <media/soc_mediabus.h>
#include <media/v4l2-chip-ident.h>
#include <media/v4l2-subdev.h>
+#include <media/v4l2-ctrls.h>
/*
* mt9t031 i2c address 0x5d
@@ -60,14 +61,18 @@
struct mt9t031 {
struct v4l2_subdev subdev;
+ struct v4l2_ctrl_handler hdl;
+ struct {
+ /* exposure/auto-exposure cluster */
+ struct v4l2_ctrl *autoexposure;
+ struct v4l2_ctrl *exposure;
+ };
struct v4l2_rect rect; /* Sensor window */
int model; /* V4L2_IDENT_MT9T031* codes from v4l2-chip-ident.h */
u16 xskip;
u16 yskip;
- unsigned int gain;
+ unsigned int total_h;
unsigned short y_skip_top; /* Lines to skip at the top */
- unsigned int exposure;
- unsigned char autoexposure;
};
static struct mt9t031 *to_mt9t031(const struct i2c_client *client)
@@ -175,69 +180,6 @@ static int mt9t031_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
}
-enum {
- MT9T031_CTRL_VFLIP,
- MT9T031_CTRL_HFLIP,
- MT9T031_CTRL_GAIN,
- MT9T031_CTRL_EXPOSURE,
- MT9T031_CTRL_EXPOSURE_AUTO,
-};
-
-static const struct v4l2_queryctrl mt9t031_controls[] = {
- [MT9T031_CTRL_VFLIP] = {
- .id = V4L2_CID_VFLIP,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Flip Vertically",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 0,
- },
- [MT9T031_CTRL_HFLIP] = {
- .id = V4L2_CID_HFLIP,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Flip Horizontally",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 0,
- },
- [MT9T031_CTRL_GAIN] = {
- .id = V4L2_CID_GAIN,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Gain",
- .minimum = 0,
- .maximum = 127,
- .step = 1,
- .default_value = 64,
- .flags = V4L2_CTRL_FLAG_SLIDER,
- },
- [MT9T031_CTRL_EXPOSURE] = {
- .id = V4L2_CID_EXPOSURE,
- .type = V4L2_CTRL_TYPE_INTEGER,
- .name = "Exposure",
- .minimum = 1,
- .maximum = 255,
- .step = 1,
- .default_value = 255,
- .flags = V4L2_CTRL_FLAG_SLIDER,
- },
- [MT9T031_CTRL_EXPOSURE_AUTO] = {
- .id = V4L2_CID_EXPOSURE_AUTO,
- .type = V4L2_CTRL_TYPE_BOOLEAN,
- .name = "Automatic Exposure",
- .minimum = 0,
- .maximum = 1,
- .step = 1,
- .default_value = 1,
- }
-};
-
-static struct soc_camera_ops mt9t031_ops = {
- .controls = mt9t031_controls,
- .num_controls = ARRAY_SIZE(mt9t031_controls),
-};
-
/* target must be _even_ */
static u16 mt9t031_skip(s32 *source, s32 target, s32 max)
{
@@ -334,17 +276,10 @@ static int mt9t031_set_params(struct i2c_client *client,
if (ret >= 0)
ret = reg_write(client, MT9T031_WINDOW_HEIGHT,
rect->height + mt9t031->y_skip_top - 1);
- if (ret >= 0 && mt9t031->autoexposure) {
- unsigned int total_h = rect->height + mt9t031->y_skip_top + vblank;
- ret = set_shutter(client, total_h);
- if (ret >= 0) {
- const u32 shutter_max = MT9T031_MAX_HEIGHT + vblank;
- const struct v4l2_queryctrl *qctrl =
- &mt9t031_controls[MT9T031_CTRL_EXPOSURE];
- mt9t031->exposure = (shutter_max / 2 + (total_h - 1) *
- (qctrl->maximum - qctrl->minimum)) /
- shutter_max + qctrl->minimum;
- }
+ if (ret >= 0 && v4l2_ctrl_g_ctrl(mt9t031->autoexposure) == V4L2_EXPOSURE_AUTO) {
+ mt9t031->total_h = rect->height + mt9t031->y_skip_top + vblank;
+
+ ret = set_shutter(client, mt9t031->total_h);
}
/* Re-enable register update, commit all changes */
@@ -513,71 +448,57 @@ static int mt9t031_s_register(struct v4l2_subdev *sd,
}
#endif
-static int mt9t031_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9t031_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct mt9t031 *mt9t031 = to_mt9t031(client);
- int data;
+ struct mt9t031 *mt9t031 = container_of(ctrl->handler,
+ struct mt9t031, hdl);
+ const u32 shutter_max = MT9T031_MAX_HEIGHT + MT9T031_VERTICAL_BLANK;
+ s32 min, max;
switch (ctrl->id) {
- case V4L2_CID_VFLIP:
- data = reg_read(client, MT9T031_READ_MODE_2);
- if (data < 0)
- return -EIO;
- ctrl->value = !!(data & 0x8000);
- break;
- case V4L2_CID_HFLIP:
- data = reg_read(client, MT9T031_READ_MODE_2);
- if (data < 0)
- return -EIO;
- ctrl->value = !!(data & 0x4000);
- break;
case V4L2_CID_EXPOSURE_AUTO:
- ctrl->value = mt9t031->autoexposure;
- break;
- case V4L2_CID_GAIN:
- ctrl->value = mt9t031->gain;
- break;
- case V4L2_CID_EXPOSURE:
- ctrl->value = mt9t031->exposure;
+ min = mt9t031->exposure->minimum;
+ max = mt9t031->exposure->maximum;
+ mt9t031->exposure->val =
+ (shutter_max / 2 + (mt9t031->total_h - 1) * (max - min))
+ / shutter_max + min;
break;
}
return 0;
}
-static int mt9t031_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
+static int mt9t031_s_ctrl(struct v4l2_ctrl *ctrl)
{
+ struct mt9t031 *mt9t031 = container_of(ctrl->handler,
+ struct mt9t031, hdl);
+ struct v4l2_subdev *sd = &mt9t031->subdev;
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct mt9t031 *mt9t031 = to_mt9t031(client);
- const struct v4l2_queryctrl *qctrl;
+ struct v4l2_ctrl *exp = mt9t031->exposure;
int data;
switch (ctrl->id) {
case V4L2_CID_VFLIP:
- if (ctrl->value)
+ if (ctrl->val)
data = reg_set(client, MT9T031_READ_MODE_2, 0x8000);
else
data = reg_clear(client, MT9T031_READ_MODE_2, 0x8000);
if (data < 0)
return -EIO;
- break;
+ return 0;
case V4L2_CID_HFLIP:
- if (ctrl->value)
+ if (ctrl->val)
data = reg_set(client, MT9T031_READ_MODE_2, 0x4000);
else
data = reg_clear(client, MT9T031_READ_MODE_2, 0x4000);
if (data < 0)
return -EIO;
- break;
+ return 0;
case V4L2_CID_GAIN:
- qctrl = &mt9t031_controls[MT9T031_CTRL_GAIN];
- if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
- return -EINVAL;
/* See Datasheet Table 7, Gain settings. */
- if (ctrl->value <= qctrl->default_value) {
+ if (ctrl->val <= ctrl->default_value) {
/* Pack it into 0..1 step 0.125, register values 0..8 */
- unsigned long range = qctrl->default_value - qctrl->minimum;
- data = ((ctrl->value - qctrl->minimum) * 8 + range / 2) / range;
+ unsigned long range = ctrl->default_value - ctrl->minimum;
+ data = ((ctrl->val - ctrl->minimum) * 8 + range / 2) / range;
dev_dbg(&client->dev, "Setting gain %d\n", data);
data = reg_write(client, MT9T031_GLOBAL_GAIN, data);
@@ -586,9 +507,9 @@ static int mt9t031_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
} else {
/* Pack it into 1.125..128 variable step, register values 9..0x7860 */
/* We assume qctrl->maximum - qctrl->default_value - 1 > 0 */
- unsigned long range = qctrl->maximum - qctrl->default_value - 1;
+ unsigned long range = ctrl->maximum - ctrl->default_value - 1;
/* calculated gain: map 65..127 to 9..1024 step 0.125 */
- unsigned long gain = ((ctrl->value - qctrl->default_value - 1) *
+ unsigned long gain = ((ctrl->val - ctrl->default_value - 1) *
1015 + range / 2) / range + 9;
if (gain <= 32) /* calculated gain 9..32 -> 9..32 */
@@ -605,19 +526,13 @@ static int mt9t031_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
if (data < 0)
return -EIO;
}
+ return 0;
- /* Success */
- mt9t031->gain = ctrl->value;
- break;
- case V4L2_CID_EXPOSURE:
- qctrl = &mt9t031_controls[MT9T031_CTRL_EXPOSURE];
- /* mt9t031 has maximum == default */
- if (ctrl->value > qctrl->maximum || ctrl->value < qctrl->minimum)
- return -EINVAL;
- else {
- const unsigned long range = qctrl->maximum - qctrl->minimum;
- const u32 shutter = ((ctrl->value - qctrl->minimum) * 1048 +
- range / 2) / range + 1;
+ case V4L2_CID_EXPOSURE_AUTO:
+ if (ctrl->val == V4L2_EXPOSURE_MANUAL) {
+ unsigned int range = exp->maximum - exp->minimum;
+ unsigned int shutter = ((exp->val - exp->minimum) * 1048 +
+ range / 2) / range + 1;
u32 old;
get_shutter(client, &old);
@@ -625,27 +540,15 @@ static int mt9t031_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
old, shutter);
if (set_shutter(client, shutter) < 0)
return -EIO;
- mt9t031->exposure = ctrl->value;
- mt9t031->autoexposure = 0;
- }
- break;
- case V4L2_CID_EXPOSURE_AUTO:
- if (ctrl->value) {
+ } else {
const u16 vblank = MT9T031_VERTICAL_BLANK;
- const u32 shutter_max = MT9T031_MAX_HEIGHT + vblank;
- unsigned int total_h = mt9t031->rect.height +
+ mt9t031->total_h = mt9t031->rect.height +
mt9t031->y_skip_top + vblank;
- if (set_shutter(client, total_h) < 0)
+ if (set_shutter(client, mt9t031->total_h) < 0)
return -EIO;
- qctrl = &mt9t031_controls[MT9T031_CTRL_EXPOSURE];
- mt9t031->exposure = (shutter_max / 2 + (total_h - 1) *
- (qctrl->maximum - qctrl->minimum)) /
- shutter_max + qctrl->minimum;
- mt9t031->autoexposure = 1;
- } else
- mt9t031->autoexposure = 0;
- break;
+ }
+ return 0;
default:
return -EINVAL;
}
@@ -735,15 +638,12 @@ static int mt9t031_video_probe(struct i2c_client *client)
dev_info(&client->dev, "Detected a MT9T031 chip ID %x\n", data);
ret = mt9t031_idle(client);
- if (ret < 0)
+ if (ret < 0) {
dev_err(&client->dev, "Failed to initialise the camera\n");
- else
+ } else {
vdev->dev.type = &mt9t031_dev_type;
-
- /* mt9t031_idle() has reset the chip to default. */
- mt9t031->exposure = 255;
- mt9t031->gain = 64;
-
+ v4l2_ctrl_handler_setup(&mt9t031->hdl);
+ }
return ret;
}
@@ -757,9 +657,12 @@ static int mt9t031_g_skip_top_lines(struct v4l2_subdev *sd, u32 *lines)
return 0;
}
+static const struct v4l2_ctrl_ops mt9t031_ctrl_ops = {
+ .g_volatile_ctrl = mt9t031_g_volatile_ctrl,
+ .s_ctrl = mt9t031_s_ctrl,
+};
+
static struct v4l2_subdev_core_ops mt9t031_subdev_core_ops = {
- .g_ctrl = mt9t031_g_ctrl,
- .s_ctrl = mt9t031_s_ctrl,
.g_chip_ident = mt9t031_g_chip_ident,
#ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = mt9t031_g_register,
@@ -844,8 +747,6 @@ static int mt9t031_probe(struct i2c_client *client,
dev_err(&client->dev, "MT9T031 driver needs platform data\n");
return -EINVAL;
}
-
- icd->ops = &mt9t031_ops;
}
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) {
@@ -859,6 +760,33 @@ static int mt9t031_probe(struct i2c_client *client,
return -ENOMEM;
v4l2_i2c_subdev_init(&mt9t031->subdev, client, &mt9t031_subdev_ops);
+ v4l2_ctrl_handler_init(&mt9t031->hdl, 5);
+ v4l2_ctrl_new_std(&mt9t031->hdl, &mt9t031_ctrl_ops,
+ V4L2_CID_VFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&mt9t031->hdl, &mt9t031_ctrl_ops,
+ V4L2_CID_HFLIP, 0, 1, 1, 0);
+ v4l2_ctrl_new_std(&mt9t031->hdl, &mt9t031_ctrl_ops,
+ V4L2_CID_GAIN, 0, 127, 1, 64);
+
+ /*
+ * Simulated autoexposure. If enabled, we calculate shutter width
+ * ourselves in the driver based on vertical blanking and frame width
+ */
+ mt9t031->autoexposure = v4l2_ctrl_new_std_menu(&mt9t031->hdl,
+ &mt9t031_ctrl_ops, V4L2_CID_EXPOSURE_AUTO, 1, 0,
+ V4L2_EXPOSURE_AUTO);
+ mt9t031->exposure = v4l2_ctrl_new_std(&mt9t031->hdl, &mt9t031_ctrl_ops,
+ V4L2_CID_EXPOSURE, 1, 255, 1, 255);
+
+ mt9t031->subdev.ctrl_handler = &mt9t031->hdl;
+ if (mt9t031->hdl.error) {
+ int err = mt9t031->hdl.error;
+
+ kfree(mt9t031);
+ return err;
+ }
+ v4l2_ctrl_auto_cluster(2, &mt9t031->autoexposure,
+ V4L2_EXPOSURE_MANUAL, true);
mt9t031->y_skip_top = 0;
mt9t031->rect.left = MT9T031_COLUMN_SKIP;
@@ -866,12 +794,6 @@ static int mt9t031_probe(struct i2c_client *client,
mt9t031->rect.width = MT9T031_MAX_WIDTH;
mt9t031->rect.height = MT9T031_MAX_HEIGHT;
- /*
- * Simulated autoexposure. If enabled, we calculate shutter width
- * ourselves in the driver based on vertical blanking and frame width
- */
- mt9t031->autoexposure = 1;
-
mt9t031->xskip = 1;
mt9t031->yskip = 1;
@@ -882,8 +804,7 @@ static int mt9t031_probe(struct i2c_client *client,
mt9t031_disable(client);
if (ret) {
- if (icd)
- icd->ops = NULL;
+ v4l2_ctrl_handler_free(&mt9t031->hdl);
kfree(mt9t031);
}
@@ -893,10 +814,9 @@ static int mt9t031_probe(struct i2c_client *client,
static int mt9t031_remove(struct i2c_client *client)
{
struct mt9t031 *mt9t031 = to_mt9t031(client);
- struct soc_camera_device *icd = client->dev.platform_data;
- if (icd)
- icd->ops = NULL;
+ v4l2_device_unregister_subdev(&mt9t031->subdev);
+ v4l2_ctrl_handler_free(&mt9t031->hdl);
kfree(mt9t031);
return 0;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 13/13 v3] soc_camera: remove the now obsolete struct soc_camera_ops
2011-09-08 8:43 [PATCH/RFC 00/13 v3] Converting soc_camera to the control framework Guennadi Liakhovetski
` (11 preceding siblings ...)
2011-09-08 8:44 ` [PATCH 12/13 v3] mt9t031: " Guennadi Liakhovetski
@ 2011-09-08 8:44 ` Guennadi Liakhovetski
12 siblings, 0 replies; 22+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-08 8:44 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Mauro Carvalho Chehab
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[g.liakhovetski@gmx.de: mt9m001 hunk moved to an earlier patch]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/video/imx074.c | 1 -
drivers/media/video/mt9t112.c | 2 --
drivers/media/video/soc_camera_platform.c | 2 --
drivers/media/video/tw9910.c | 1 -
include/media/soc_camera.h | 18 ------------------
5 files changed, 0 insertions(+), 24 deletions(-)
diff --git a/drivers/media/video/imx074.c b/drivers/media/video/imx074.c
index 20756e0..3f5d4de 100644
--- a/drivers/media/video/imx074.c
+++ b/drivers/media/video/imx074.c
@@ -437,7 +437,6 @@ static int imx074_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(&priv->subdev, client, &imx074_subdev_ops);
- icd->ops = NULL;
priv->fmt = &imx074_colour_fmts[0];
ret = imx074_video_probe(icd, client);
diff --git a/drivers/media/video/mt9t112.c b/drivers/media/video/mt9t112.c
index 25cdcb9..b8da7fe 100644
--- a/drivers/media/video/mt9t112.c
+++ b/drivers/media/video/mt9t112.c
@@ -1095,8 +1095,6 @@ static int mt9t112_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops);
- icd->ops = NULL;
-
ret = mt9t112_camera_probe(icd, client);
if (ret)
kfree(priv);
diff --git a/drivers/media/video/soc_camera_platform.c b/drivers/media/video/soc_camera_platform.c
index c8f6b18..4402a8a 100644
--- a/drivers/media/video/soc_camera_platform.c
+++ b/drivers/media/video/soc_camera_platform.c
@@ -150,8 +150,6 @@ static int soc_camera_platform_probe(struct platform_device *pdev)
/* Set the control device reference */
icd->control = &pdev->dev;
- icd->ops = NULL;
-
ici = to_soc_camera_host(icd->parent);
v4l2_subdev_init(&priv->subdev, &platform_subdev_ops);
diff --git a/drivers/media/video/tw9910.c b/drivers/media/video/tw9910.c
index 40cc149..2fddd1f 100644
--- a/drivers/media/video/tw9910.c
+++ b/drivers/media/video/tw9910.c
@@ -921,7 +921,6 @@ static int tw9910_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(&priv->subdev, client, &tw9910_subdev_ops);
- icd->ops = NULL;
icd->iface = icl->bus_id;
ret = tw9910_video_probe(icd, client);
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index d41b8bd..6398ff0 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -39,7 +39,6 @@ struct soc_camera_device {
unsigned char iface; /* Host number */
unsigned char devnum; /* Device number per host */
struct soc_camera_sense *sense; /* See comment in struct definition */
- struct soc_camera_ops *ops;
struct video_device *vdev;
struct v4l2_ctrl_handler ctrl_handler;
const struct soc_camera_format_xlate *current_fmt;
@@ -192,11 +191,6 @@ struct soc_camera_format_xlate {
const struct soc_mbus_pixelfmt *host_fmt;
};
-struct soc_camera_ops {
- const struct v4l2_queryctrl *controls;
- int num_controls;
-};
-
#define SOCAM_SENSE_PCLK_CHANGED (1 << 0)
/**
@@ -223,18 +217,6 @@ struct soc_camera_sense {
unsigned long pixel_clock;
};
-static inline struct v4l2_queryctrl const *soc_camera_find_qctrl(
- struct soc_camera_ops *ops, int id)
-{
- int i;
-
- for (i = 0; i < ops->num_controls; i++)
- if (ops->controls[i].id == id)
- return &ops->controls[i];
-
- return NULL;
-}
-
#define SOCAM_DATAWIDTH(x) BIT((x) - 1)
#define SOCAM_DATAWIDTH_4 SOCAM_DATAWIDTH(4)
#define SOCAM_DATAWIDTH_8 SOCAM_DATAWIDTH(8)
--
1.7.2.5
^ permalink raw reply related [flat|nested] 22+ messages in thread