Em 07-06-2011 12:05, Hans Verkuil escreveu: > From: Hans Verkuil <hans.verkuil@xxxxxxxxx> > > It is a bit tricky to handle autogain/gain type scenerios correctly. Such > controls need to be clustered and the V4L2_CTRL_FLAG_UPDATE should be set on > the autofoo controls. In addition, the manual controls should be marked > inactive when the automatic mode is on, and active when the manual mode is on. > This also requires specialized volatile handling. > > The chances of drivers doing all these things correctly are pretty remote. > So a new v4l2_ctrl_auto_cluster function was added that takes care of these > issues. > > Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> > --- > drivers/media/video/v4l2-ctrls.c | 69 +++++++++++++++++++++++++++++++------ > include/media/v4l2-ctrls.h | 45 ++++++++++++++++++++++++ > 2 files changed, 102 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c > index a46d5c1..c39ab0c 100644 > --- a/drivers/media/video/v4l2-ctrls.c > +++ b/drivers/media/video/v4l2-ctrls.c > @@ -39,6 +39,20 @@ struct ctrl_helper { > bool handled; > }; > > +/* Small helper function to determine if the autocluster is set to manual > + mode. In that case the is_volatile flag should be ignored. */ > +static bool is_cur_manual(const struct v4l2_ctrl *master) > +{ > + return master->is_auto && master->cur.val == master->manual_mode_value; > +} > + > +/* Same as above, but this checks the against the new value instead of the > + current value. */ > +static bool is_new_manual(const struct v4l2_ctrl *master) > +{ > + return master->is_auto && master->val == master->manual_mode_value; > +} > + > /* Returns NULL or a character pointer array containing the menu for > the given control ID. The pointer array ends with a NULL pointer. > An empty string signifies a menu entry that is invalid. This allows > @@ -643,7 +657,7 @@ static int ctrl_is_volatile(struct v4l2_ext_control *c, > } > > /* Copy the new value to the current value. */ > -static void new_to_cur(struct v4l2_ctrl *ctrl) > +static void new_to_cur(struct v4l2_ctrl *ctrl, bool update_inactive) > { > if (ctrl == NULL) > return; > @@ -659,6 +673,11 @@ static void new_to_cur(struct v4l2_ctrl *ctrl) > ctrl->cur.val = ctrl->val; > break; > } > + if (update_inactive) { > + ctrl->flags &= ~V4L2_CTRL_FLAG_INACTIVE; > + if (!is_cur_manual(ctrl->cluster[0])) > + ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE; > + } > } > > /* Copy the current value to the new value */ > @@ -1166,7 +1185,7 @@ void v4l2_ctrl_cluster(unsigned ncontrols, struct v4l2_ctrl **controls) > int i; > > /* The first control is the master control and it must not be NULL */ > - BUG_ON(controls[0] == NULL); > + BUG_ON(ncontrols == 0 || controls[0] == NULL); > > for (i = 0; i < ncontrols; i++) { > if (controls[i]) { > @@ -1177,6 +1196,28 @@ void v4l2_ctrl_cluster(unsigned ncontrols, struct v4l2_ctrl **controls) > } > EXPORT_SYMBOL(v4l2_ctrl_cluster); > > +void v4l2_ctrl_auto_cluster(unsigned ncontrols, struct v4l2_ctrl **controls, > + u8 manual_val, bool set_volatile) > +{ > + struct v4l2_ctrl *master = controls[0]; > + u32 flag; > + int i; > + > + v4l2_ctrl_cluster(ncontrols, controls); > + WARN_ON(ncontrols <= 1); > + master->is_auto = true; > + master->manual_mode_value = manual_val; You'll have an overflow if manual_val is higher than 31 here. > + master->flags |= V4L2_CTRL_FLAG_UPDATE; > + flag = is_cur_manual(master) ? 0 : V4L2_CTRL_FLAG_INACTIVE; > + > + for (i = 1; i < ncontrols; i++) > + if (controls[i]) { > + controls[i]->is_volatile = set_volatile; > + controls[i]->flags |= flag; > + } > +} > +EXPORT_SYMBOL(v4l2_ctrl_auto_cluster); > + > /* Activate/deactivate a control. */ > void v4l2_ctrl_activate(struct v4l2_ctrl *ctrl, bool active) > { > @@ -1595,7 +1636,7 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs > ctrl_is_volatile); > > /* g_volatile_ctrl will update the new control values */ > - if (has_volatiles) { > + if (has_volatiles && !is_cur_manual(master)) { > for (j = 0; j < master->ncontrols; j++) > cur_to_new(master->cluster[j]); > ret = call_op(master, g_volatile_ctrl); > @@ -1633,7 +1674,7 @@ static int get_ctrl(struct v4l2_ctrl *ctrl, s32 *val) > > v4l2_ctrl_lock(master); > /* g_volatile_ctrl will update the current control values */ > - if (ctrl->is_volatile) { > + if (ctrl->is_volatile && !is_cur_manual(master)) { > for (i = 0; i < master->ncontrols; i++) > cur_to_new(master->cluster[i]); > ret = call_op(master, g_volatile_ctrl); > @@ -1678,6 +1719,7 @@ EXPORT_SYMBOL(v4l2_ctrl_g_ctrl); > Must be called with ctrl->handler->lock held. */ > static int try_or_set_control_cluster(struct v4l2_ctrl *master, bool set) > { > + bool update_flag; > bool try = !set; > int ret = 0; > int i; > @@ -1717,14 +1759,17 @@ static int try_or_set_control_cluster(struct v4l2_ctrl *master, bool set) > ret = call_op(master, try_ctrl); > > /* Don't set if there is no change */ > - if (!ret && set && cluster_changed(master)) { > - ret = call_op(master, s_ctrl); > - /* If OK, then make the new values permanent. */ > - if (!ret) > - for (i = 0; i < master->ncontrols; i++) > - new_to_cur(master->cluster[i]); > - } > - return ret; > + if (ret || !set || !cluster_changed(master)) > + return ret; > + ret = call_op(master, s_ctrl); > + /* If OK, then make the new values permanent. */ > + if (ret) > + return ret; > + > + update_flag = is_cur_manual(master) != is_new_manual(master); > + for (i = 0; i < master->ncontrols; i++) > + new_to_cur(master->cluster[i], update_flag && i > 0); > + return 0; > } > > /* Try or set controls. */ > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h > index 97d0638..56323e3 100644 > --- a/include/media/v4l2-ctrls.h > +++ b/include/media/v4l2-ctrls.h > @@ -65,6 +65,15 @@ struct v4l2_ctrl_ops { > * control's current value cannot be cached and needs to be > * retrieved through the g_volatile_ctrl op. Drivers can set > * this flag. > + * @is_auto: If set, then this control selects whether the other cluster > + * members are in 'automatic' mode or 'manual' mode. This is > + * used for autogain/gain type clusters. Drivers should never > + * set this flag directly. > + * @manual_mode_value: If the is_auto flag is set, then this is the value > + * of the auto control that determines if that control is in > + * manual mode. So if the value of the auto control equals this > + * value, then the whole cluster is in manual mode. Drivers should > + * never set this flag directly. > * @ops: The control ops. > * @id: The control ID. > * @name: The control name. > @@ -105,6 +114,8 @@ struct v4l2_ctrl { > unsigned int is_new:1; > unsigned int is_private:1; > unsigned int is_volatile:1; > + unsigned int is_auto:1; > + unsigned int manual_mode_value:5; Why are you using 5 bits for it? It seems too arbitrary to limit it to 32, especially since I failed to see any rationale for that in the documentation. > > const struct v4l2_ctrl_ops *ops; > u32 id; > @@ -363,6 +374,40 @@ int v4l2_ctrl_add_handler(struct v4l2_ctrl_handler *hdl, > void v4l2_ctrl_cluster(unsigned ncontrols, struct v4l2_ctrl **controls); > > > +/** v4l2_ctrl_auto_cluster() - Mark all controls in the cluster as belonging to > + * that cluster and set it up for autofoo/foo-type handling. > + * @ncontrols: The number of controls in this cluster. > + * @controls: The cluster control array of size @ncontrols. The first control > + * must be the 'auto' control (e.g. autogain, autoexposure, etc.) > + * @manual_val: The value for the first control in the cluster that equals the > + * manual setting. > + * @set_volatile: If true, then all controls except the first auto control will > + * have is_volatile set to true. If false, then is_volatile will not > + * be touched. > + * > + * Use for control groups where one control selects some automatic feature and > + * the other controls are only active whenever the automatic feature is turned > + * off (manual mode). Typical examples: autogain vs gain, auto-whitebalance vs > + * red and blue balance, etc. > + * > + * The behavior of such controls is as follows: > + * > + * When the autofoo control is set to automatic, then any manual controls > + * are set to inactive and any reads will call g_volatile_ctrl (if the control > + * was marked volatile). > + * > + * When the autofoo control is set to manual, then any manual controls will > + * be marked active, and any reads will just return the current value without > + * going through g_volatile_ctrl. > + * > + * In addition, this function will set the V4L2_CTRL_FLAG_UPDATE flag > + * on the autofoo control and V4L2_CTRL_FLAG_INACTIVE on the foo control(s) > + * if autofoo is in auto mode. > + */ > +void v4l2_ctrl_auto_cluster(unsigned ncontrols, struct v4l2_ctrl **controls, > + u8 manual_val, bool set_volatile); If you're limiting it to 5 bits, why are you allowing 8 bits here? > + > + > /** v4l2_ctrl_find() - Find a control with the given ID. > * @hdl: The control handler. > * @id: The control ID to find. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html