Hi Yunke, Laurent, On 8/24/22 18:20, Laurent Pinchart wrote: > Hi Yunke and Hans, > > Thank you for the patch. > > On Tue, Jun 28, 2022 at 04:57:03PM +0900, Yunke Cao wrote: >> From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >> >> Add the capability of retrieving the min and max values of a >> compound control. >> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >> Signed-off-by: Yunke Cao <yunkec@xxxxxxxxxx> >> --- >> git am from https://lore.kernel.org/all/20191119113457.57833-3-hverkuil-cisco@xxxxxxxxx/ >> -Fixed some merge conflits. >> -Fixed the build error in drivers/media/platform/qcom/venus. >> >> .../media/v4l/vidioc-g-ext-ctrls.rst | 7 +- >> .../media/videodev2.h.rst.exceptions | 2 + >> drivers/media/i2c/imx214.c | 5 +- >> .../media/platform/qcom/venus/venc_ctrls.c | 4 + >> drivers/media/v4l2-core/v4l2-ctrls-api.c | 51 +++++-- >> drivers/media/v4l2-core/v4l2-ctrls-core.c | 135 ++++++++++++++++-- >> drivers/media/v4l2-core/v4l2-ioctl.c | 4 +- >> include/media/v4l2-ctrls.h | 32 ++++- >> include/uapi/linux/videodev2.h | 2 + >> 9 files changed, 214 insertions(+), 28 deletions(-) >> >> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst >> index 7473baa4e977..65a5f878cc28 100644 >> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst >> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst >> @@ -284,14 +284,17 @@ still cause this situation. >> - Which value of the control to get/set/try. >> * - :cspan:`2` ``V4L2_CTRL_WHICH_CUR_VAL`` will return the current value of >> the control, ``V4L2_CTRL_WHICH_DEF_VAL`` will return the default >> + value of the control, ``V4L2_CTRL_WHICH_MIN_VAL`` will return the minimum >> + value of the control, ``V4L2_CTRL_WHICH_MAX_VAL`` will return the maximum > > The concept of minimum and maximum values for compound controls can be > quite ill-defined. Could we document here that the definition of the > minimum and maximum values are provided by the control documentation for > compound controls ? Makes sense! I agree with the updated text in v8. > >> value of the control and ``V4L2_CTRL_WHICH_REQUEST_VAL`` indicates that >> these controls have to be retrieved from a request or tried/set for >> a request. In the latter case the ``request_fd`` field contains the >> file descriptor of the request that should be used. If the device >> does not support requests, then ``EACCES`` will be returned. >> >> - When using ``V4L2_CTRL_WHICH_DEF_VAL`` be aware that you can only >> - get the default value of the control, you cannot set or try it. >> + When using ``V4L2_CTRL_WHICH_DEF_VAL``, ``V4L2_CTRL_WHICH_MIN_VAL`` >> + or ``V4L2_CTRL_WHICH_MAX_VAL`` be aware that you can only get the >> + default/minimum/maximum value of the control, you cannot set or try it. >> >> For backwards compatibility you can also use a control class here >> (see :ref:`ctrl-class`). In that case all controls have to >> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions >> index 7b423475281d..e2dde31d76df 100644 >> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions >> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions >> @@ -553,6 +553,8 @@ ignore define V4L2_CTRL_DRIVER_PRIV >> ignore define V4L2_CTRL_MAX_DIMS >> ignore define V4L2_CTRL_WHICH_CUR_VAL >> ignore define V4L2_CTRL_WHICH_DEF_VAL >> +ignore define V4L2_CTRL_WHICH_MIN_VAL >> +ignore define V4L2_CTRL_WHICH_MAX_VAL >> ignore define V4L2_CTRL_WHICH_REQUEST_VAL >> ignore define V4L2_OUT_CAP_CUSTOM_TIMINGS >> ignore define V4L2_CID_MAX_CTRLS >> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c >> index 83c1737abeec..aa0bfd18f7b7 100644 >> --- a/drivers/media/i2c/imx214.c >> +++ b/drivers/media/i2c/imx214.c >> @@ -1037,7 +1037,10 @@ static int imx214_probe(struct i2c_client *client) >> imx214->unit_size = v4l2_ctrl_new_std_compound(&imx214->ctrls, >> NULL, >> V4L2_CID_UNIT_CELL_SIZE, >> - v4l2_ctrl_ptr_create((void *)&unit_size)); >> + v4l2_ctrl_ptr_create((void *)&unit_size), >> + v4l2_ctrl_ptr_create(NULL), >> + v4l2_ctrl_ptr_create(NULL)); > > Would it make sense to set min = max = default for read-only controls ? No, that doesn't make sense. Read-only controls are only read-only from the PoV of userspace. The kernel *does* set them, so you still want to have a range of values that the control can take. > You should also update the documentation of V4L2_CID_UNIT_CELL_SIZE (and > other controls below) to indicate how their minimum and maximum are > defined. > >> + >> ret = imx214->ctrls.error; >> if (ret) { >> dev_err(&client->dev, "%s control init failed (%d)\n", >> diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c >> index ed44e5800759..4af8f9ca12a6 100644 >> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c >> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c >> @@ -579,10 +579,14 @@ int venc_ctrl_init(struct venus_inst *inst) >> >> v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops, >> V4L2_CID_COLORIMETRY_HDR10_CLL_INFO, >> + v4l2_ctrl_ptr_create(NULL), >> + v4l2_ctrl_ptr_create(NULL), >> v4l2_ctrl_ptr_create(NULL)); >> >> v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops, >> V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY, >> + v4l2_ctrl_ptr_create(NULL), >> + v4l2_ctrl_ptr_create(NULL), >> v4l2_ctrl_ptr_create(NULL)); >> >> v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops, >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c >> index db9baa0bd05f..8a9c816b0dab 100644 >> --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c >> +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c >> @@ -97,6 +97,28 @@ static int def_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl) >> return ptr_to_user(c, ctrl, ctrl->p_new); >> } >> >> +/* Helper function: copy the minimum control value back to the caller */ >> +static int min_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl) >> +{ >> + int idx; >> + >> + for (idx = 0; idx < ctrl->elems; idx++) >> + ctrl->type_ops->minimum(ctrl, idx, ctrl->p_new); >> + >> + return ptr_to_user(c, ctrl, ctrl->p_new); >> +} >> + >> +/* Helper function: copy the maximum control value back to the caller */ >> +static int max_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl) >> +{ >> + int idx; >> + >> + for (idx = 0; idx < ctrl->elems; idx++) >> + ctrl->type_ops->maximum(ctrl, idx, ctrl->p_new); >> + >> + return ptr_to_user(c, ctrl, ctrl->p_new); >> +} >> + >> /* Helper function: copy the caller-provider value to the given control value */ >> static int user_to_ptr(struct v4l2_ext_control *c, >> struct v4l2_ctrl *ctrl, >> @@ -220,8 +242,8 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl, >> cs->error_idx = i; >> >> if (cs->which && >> - cs->which != V4L2_CTRL_WHICH_DEF_VAL && >> - cs->which != V4L2_CTRL_WHICH_REQUEST_VAL && >> + (cs->which < V4L2_CTRL_WHICH_DEF_VAL || >> + cs->which > V4L2_CTRL_WHICH_MAX_VAL) && >> V4L2_CTRL_ID2WHICH(id) != cs->which) { >> dprintk(vdev, >> "invalid which 0x%x or control id 0x%x\n", >> @@ -335,8 +357,8 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl, >> */ >> static int class_check(struct v4l2_ctrl_handler *hdl, u32 which) >> { >> - if (which == 0 || which == V4L2_CTRL_WHICH_DEF_VAL || >> - which == V4L2_CTRL_WHICH_REQUEST_VAL) >> + if (which == 0 || (which >= V4L2_CTRL_WHICH_DEF_VAL && >> + which <= V4L2_CTRL_WHICH_MAX_VAL)) >> return 0; >> return find_ref_lock(hdl, which | 1) ? 0 : -EINVAL; >> } >> @@ -356,10 +378,12 @@ int v4l2_g_ext_ctrls_common(struct v4l2_ctrl_handler *hdl, >> struct v4l2_ctrl_helper *helpers = helper; >> int ret; >> int i, j; >> - bool is_default, is_request; >> + bool is_default, is_request, is_min, is_max; >> >> is_default = (cs->which == V4L2_CTRL_WHICH_DEF_VAL); >> is_request = (cs->which == V4L2_CTRL_WHICH_REQUEST_VAL); >> + is_min = (cs->which == V4L2_CTRL_WHICH_MIN_VAL); >> + is_max = (cs->which == V4L2_CTRL_WHICH_MAX_VAL); >> >> cs->error_idx = cs->count; >> cs->which = V4L2_CTRL_ID2WHICH(cs->which); >> @@ -399,13 +423,14 @@ int v4l2_g_ext_ctrls_common(struct v4l2_ctrl_handler *hdl, >> >> /* >> * g_volatile_ctrl will update the new control values. >> - * This makes no sense for V4L2_CTRL_WHICH_DEF_VAL and >> + * This makes no sense for V4L2_CTRL_WHICH_DEF_VAL, >> + * V4L2_CTRL_WHICH_MIN_VAL, V4L2_CTRL_WHICH_MAX_VAL and >> * V4L2_CTRL_WHICH_REQUEST_VAL. In the case of requests >> * it is v4l2_ctrl_request_complete() that copies the >> * volatile controls at the time of request completion >> * to the request, so you don't want to do that again. >> */ >> - if (!is_default && !is_request && >> + if (!is_default && !is_request && !is_min && !is_max && >> ((master->flags & V4L2_CTRL_FLAG_VOLATILE) || >> (master->has_volatiles && !is_cur_manual(master)))) { >> for (j = 0; j < master->ncontrols; j++) >> @@ -432,6 +457,10 @@ int v4l2_g_ext_ctrls_common(struct v4l2_ctrl_handler *hdl, >> ret = def_to_user(cs->controls + idx, ref->ctrl); >> else if (is_request && ref->valid_p_req) >> ret = req_to_user(cs->controls + idx, ref); >> + else if (is_min) >> + ret = min_to_user(cs->controls + idx, ref->ctrl); >> + else if (is_max) >> + ret = max_to_user(cs->controls + idx, ref->ctrl); >> else if (is_volatile) >> ret = new_to_user(cs->controls + idx, ref->ctrl); >> else >> @@ -523,9 +552,11 @@ int try_set_ext_ctrls_common(struct v4l2_fh *fh, >> >> cs->error_idx = cs->count; >> >> - /* Default value cannot be changed */ >> - if (cs->which == V4L2_CTRL_WHICH_DEF_VAL) { >> - dprintk(vdev, "%s: cannot change default value\n", >> + /* Default/minimum/maximum values cannot be changed */ >> + if (cs->which == V4L2_CTRL_WHICH_DEF_VAL || >> + cs->which == V4L2_CTRL_WHICH_MIN_VAL || >> + cs->which == V4L2_CTRL_WHICH_MAX_VAL) { >> + dprintk(vdev, "%s: cannot change default/min/max value\n", >> video_device_node_name(vdev)); >> return -EINVAL; >> } >> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c >> index 35d43ba650db..8c5828c7c6d3 100644 >> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c >> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c >> @@ -186,6 +186,28 @@ static void std_init_compound(const struct v4l2_ctrl *ctrl, u32 idx, >> } >> } >> >> +static void std_min_compound(const struct v4l2_ctrl *ctrl, u32 idx, >> + union v4l2_ctrl_ptr ptr) >> +{ >> + void *p = ptr.p + idx * ctrl->elem_size; >> + >> + if (ctrl->p_min.p_const) >> + memcpy(p, ctrl->p_min.p_const, ctrl->elem_size); >> + else >> + memset(p, 0, ctrl->elem_size); >> +} >> + >> +static void std_max_compound(const struct v4l2_ctrl *ctrl, u32 idx, >> + union v4l2_ctrl_ptr ptr) >> +{ >> + void *p = ptr.p + idx * ctrl->elem_size; >> + >> + if (ctrl->p_max.p_const) >> + memcpy(p, ctrl->p_max.p_const, ctrl->elem_size); >> + else >> + memset(p, 0, ctrl->elem_size); >> +} >> + >> static void std_init(const struct v4l2_ctrl *ctrl, u32 idx, >> union v4l2_ctrl_ptr ptr) >> { >> @@ -224,6 +246,82 @@ static void std_init(const struct v4l2_ctrl *ctrl, u32 idx, >> } >> } >> >> +static void std_minimum(const struct v4l2_ctrl *ctrl, u32 idx, >> + union v4l2_ctrl_ptr ptr) >> +{ >> + switch (ctrl->type) { >> + case V4L2_CTRL_TYPE_STRING: >> + idx *= ctrl->elem_size; >> + memset(ptr.p_char + idx, ' ', ctrl->minimum); >> + ptr.p_char[idx + ctrl->minimum] = '\0'; >> + break; >> + case V4L2_CTRL_TYPE_INTEGER64: >> + ptr.p_s64[idx] = ctrl->minimum; >> + break; >> + case V4L2_CTRL_TYPE_INTEGER: >> + case V4L2_CTRL_TYPE_INTEGER_MENU: >> + case V4L2_CTRL_TYPE_MENU: >> + case V4L2_CTRL_TYPE_BITMASK: >> + case V4L2_CTRL_TYPE_BOOLEAN: >> + ptr.p_s32[idx] = ctrl->minimum; >> + break; >> + case V4L2_CTRL_TYPE_BUTTON: >> + case V4L2_CTRL_TYPE_CTRL_CLASS: >> + ptr.p_s32[idx] = 0; >> + break; >> + case V4L2_CTRL_TYPE_U8: >> + ptr.p_u8[idx] = ctrl->minimum; >> + break; >> + case V4L2_CTRL_TYPE_U16: >> + ptr.p_u16[idx] = ctrl->minimum; >> + break; >> + case V4L2_CTRL_TYPE_U32: >> + ptr.p_u32[idx] = ctrl->minimum; >> + break; >> + default: >> + std_min_compound(ctrl, idx, ptr); >> + break; >> + } >> +} >> + >> +static void std_maximum(const struct v4l2_ctrl *ctrl, u32 idx, >> + union v4l2_ctrl_ptr ptr) >> +{ >> + switch (ctrl->type) { >> + case V4L2_CTRL_TYPE_STRING: >> + idx *= ctrl->elem_size; >> + memset(ptr.p_char + idx, ' ', ctrl->maximum); >> + ptr.p_char[idx + ctrl->maximum] = '\0'; >> + break; >> + case V4L2_CTRL_TYPE_INTEGER64: >> + ptr.p_s64[idx] = ctrl->maximum; >> + break; >> + case V4L2_CTRL_TYPE_INTEGER: >> + case V4L2_CTRL_TYPE_INTEGER_MENU: >> + case V4L2_CTRL_TYPE_MENU: >> + case V4L2_CTRL_TYPE_BITMASK: >> + case V4L2_CTRL_TYPE_BOOLEAN: >> + ptr.p_s32[idx] = ctrl->maximum; >> + break; >> + case V4L2_CTRL_TYPE_BUTTON: >> + case V4L2_CTRL_TYPE_CTRL_CLASS: >> + ptr.p_s32[idx] = 0; >> + break; >> + case V4L2_CTRL_TYPE_U8: >> + ptr.p_u8[idx] = ctrl->maximum; >> + break; >> + case V4L2_CTRL_TYPE_U16: >> + ptr.p_u16[idx] = ctrl->maximum; >> + break; >> + case V4L2_CTRL_TYPE_U32: >> + ptr.p_u32[idx] = ctrl->maximum; >> + break; >> + default: >> + std_max_compound(ctrl, idx, ptr); >> + break; >> + } >> +} >> + >> static void std_log(const struct v4l2_ctrl *ctrl) >> { >> union v4l2_ctrl_ptr ptr = ctrl->p_cur; >> @@ -986,6 +1084,8 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx, >> static const struct v4l2_ctrl_type_ops std_type_ops = { >> .equal = std_equal, >> .init = std_init, >> + .minimum = std_minimum, >> + .maximum = std_maximum, >> .log = std_log, >> .validate = std_validate, >> }; >> @@ -1368,7 +1468,10 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, >> s64 min, s64 max, u64 step, s64 def, >> const u32 dims[V4L2_CTRL_MAX_DIMS], u32 elem_size, >> u32 flags, const char * const *qmenu, >> - const s64 *qmenu_int, const union v4l2_ctrl_ptr p_def, >> + const s64 *qmenu_int, >> + const union v4l2_ctrl_ptr p_def, >> + const union v4l2_ctrl_ptr p_min, >> + const union v4l2_ctrl_ptr p_max, >> void *priv) >> { >> struct v4l2_ctrl *ctrl; >> @@ -1515,7 +1618,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, >> sz_extra += 2 * tot_ctrl_size; >> >> if (type >= V4L2_CTRL_COMPOUND_TYPES && p_def.p_const) >> - sz_extra += elem_size; >> + sz_extra += elem_size * 3; >> >> ctrl = kvzalloc(sizeof(*ctrl) + sz_extra, GFP_KERNEL); >> if (ctrl == NULL) { >> @@ -1565,6 +1668,13 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, >> ctrl->p_def.p = ctrl->p_cur.p + tot_ctrl_size; >> memcpy(ctrl->p_def.p, p_def.p_const, elem_size); >> } >> + if (type >= V4L2_CTRL_COMPOUND_TYPES && >> + p_min.p_const && p_max.p_const) { >> + ctrl->p_min.p = ctrl->p_cur.p + 2 * tot_ctrl_size; >> + memcpy(ctrl->p_min.p, p_min.p_const, elem_size); >> + ctrl->p_max.p = ctrl->p_cur.p + 3 * tot_ctrl_size; >> + memcpy(ctrl->p_max.p, p_max.p_const, elem_size); >> + } >> >> for (idx = 0; idx < elems; idx++) { >> ctrl->type_ops->init(ctrl, idx, ctrl->p_cur); >> @@ -1617,7 +1727,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl, >> type, min, max, >> is_menu ? cfg->menu_skip_mask : step, def, >> cfg->dims, cfg->elem_size, >> - flags, qmenu, qmenu_int, cfg->p_def, priv); >> + flags, qmenu, qmenu_int, cfg->p_def, cfg->p_min, >> + cfg->p_max, priv); >> if (ctrl) >> ctrl->is_private = cfg->is_private; >> return ctrl; >> @@ -1642,7 +1753,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct v4l2_ctrl_handler *hdl, >> } >> return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, >> min, max, step, def, NULL, 0, >> - flags, NULL, NULL, ptr_null, NULL); >> + flags, NULL, NULL, ptr_null, ptr_null, >> + ptr_null, NULL); >> } >> EXPORT_SYMBOL(v4l2_ctrl_new_std); >> >> @@ -1675,7 +1787,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl, >> } >> return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, >> 0, max, mask, def, NULL, 0, >> - flags, qmenu, qmenu_int, ptr_null, NULL); >> + flags, qmenu, qmenu_int, ptr_null, ptr_null, >> + ptr_null, NULL); >> } >> EXPORT_SYMBOL(v4l2_ctrl_new_std_menu); >> >> @@ -1707,7 +1820,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl, >> } >> return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, >> 0, max, mask, def, NULL, 0, >> - flags, qmenu, NULL, ptr_null, NULL); >> + flags, qmenu, NULL, ptr_null, ptr_null, >> + ptr_null, NULL); >> >> } >> EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items); >> @@ -1715,7 +1829,9 @@ EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items); >> /* Helper function for standard compound controls */ >> struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl, >> const struct v4l2_ctrl_ops *ops, u32 id, >> - const union v4l2_ctrl_ptr p_def) >> + const union v4l2_ctrl_ptr p_def, >> + const union v4l2_ctrl_ptr p_min, >> + const union v4l2_ctrl_ptr p_max) >> { >> const char *name; >> enum v4l2_ctrl_type type; >> @@ -1729,7 +1845,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl, >> } >> return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, >> min, max, step, def, NULL, 0, >> - flags, NULL, NULL, p_def, NULL); >> + flags, NULL, NULL, p_def, p_min, p_max, NULL); >> } >> EXPORT_SYMBOL(v4l2_ctrl_new_std_compound); >> >> @@ -1753,7 +1869,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl, >> } >> return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, >> 0, max, 0, def, NULL, 0, >> - flags, NULL, qmenu_int, ptr_null, NULL); >> + flags, NULL, qmenu_int, ptr_null, ptr_null, >> + ptr_null, NULL); >> } >> EXPORT_SYMBOL(v4l2_ctrl_new_int_menu); >> >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >> index 21470de62d72..0f713b9a95f9 100644 >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >> @@ -893,7 +893,9 @@ static bool check_ext_ctrls(struct v4l2_ext_controls *c, unsigned long ioctl) >> return false; >> break; >> case V4L2_CTRL_WHICH_DEF_VAL: >> - /* Default value cannot be changed */ >> + case V4L2_CTRL_WHICH_MIN_VAL: >> + case V4L2_CTRL_WHICH_MAX_VAL: >> + /* Default, minimum or maximum value cannot be changed */ >> if (ioctl == VIDIOC_S_EXT_CTRLS || >> ioctl == VIDIOC_TRY_EXT_CTRLS) { >> c->error_idx = c->count; >> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h >> index 919e104de50b..c8ab8c44d7c6 100644 >> --- a/include/media/v4l2-ctrls.h >> +++ b/include/media/v4l2-ctrls.h >> @@ -131,6 +131,8 @@ struct v4l2_ctrl_ops { >> * >> * @equal: return true if both values are equal. >> * @init: initialize the value. >> + * @minimum: set the value to the minimum value of the control. >> + * @maximum: set the value to the maximum value of the control. >> * @log: log the value. >> * @validate: validate the value. Return 0 on success and a negative value >> * otherwise. >> @@ -141,6 +143,10 @@ struct v4l2_ctrl_type_ops { >> union v4l2_ctrl_ptr ptr2); >> void (*init)(const struct v4l2_ctrl *ctrl, u32 idx, >> union v4l2_ctrl_ptr ptr); >> + void (*minimum)(const struct v4l2_ctrl *ctrl, u32 idx, >> + union v4l2_ctrl_ptr ptr); >> + void (*maximum)(const struct v4l2_ctrl *ctrl, u32 idx, >> + union v4l2_ctrl_ptr ptr); >> void (*log)(const struct v4l2_ctrl *ctrl); >> int (*validate)(const struct v4l2_ctrl *ctrl, u32 idx, >> union v4l2_ctrl_ptr ptr); >> @@ -237,6 +243,12 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv); >> * @p_def: The control's default value represented via a union which >> * provides a standard way of accessing control types >> * through a pointer (for compound controls only). >> + * @p_min: The control's minimum value represented via a union which >> + * provides a standard way of accessing control types >> + * through a pointer (for compound controls only). >> + * @p_max: The control's maximum value represented via a union which >> + * provides a standard way of accessing control types >> + * through a pointer (for compound controls only). >> * @p_cur: The control's current value represented via a union which >> * provides a standard way of accessing control types >> * through a pointer. >> @@ -292,6 +304,8 @@ struct v4l2_ctrl { >> } cur; >> >> union v4l2_ctrl_ptr p_def; >> + union v4l2_ctrl_ptr p_min; >> + union v4l2_ctrl_ptr p_max; >> union v4l2_ctrl_ptr p_new; >> union v4l2_ctrl_ptr p_cur; >> }; >> @@ -398,6 +412,8 @@ struct v4l2_ctrl_handler { >> * @step: The control's step value for non-menu controls. >> * @def: The control's default value. >> * @p_def: The control's default value for compound controls. >> + * @p_min: The control's minimum value for compound controls. >> + * @p_max: The control's maximum value for compound controls. >> * @dims: The size of each dimension. >> * @elem_size: The size in bytes of the control. >> * @flags: The control's flags. >> @@ -427,6 +443,8 @@ struct v4l2_ctrl_config { >> u64 step; >> s64 def; >> union v4l2_ctrl_ptr p_def; >> + union v4l2_ctrl_ptr p_min; >> + union v4l2_ctrl_ptr p_max; >> u32 dims[V4L2_CTRL_MAX_DIMS]; >> u32 elem_size; >> u32 flags; >> @@ -696,17 +714,21 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl, >> * @ops: The control ops. >> * @id: The control ID. >> * @p_def: The control's default value. >> + * @p_min: The control's minimum value. >> + * @p_max: The control's maximum value. >> * >> - * Sames as v4l2_ctrl_new_std(), but with support to compound controls, thanks >> - * to the @p_def field. Use v4l2_ctrl_ptr_create() to create @p_def from a >> - * pointer. Use v4l2_ctrl_ptr_create(NULL) if the default value of the >> - * compound control should be all zeroes. >> + * Same as v4l2_ctrl_new_std(), but with support to compound controls, thanks >> + * to the @p_def/min/max field. Use v4l2_ctrl_ptr_create() to create > > s/field/fields/ You forgot to fix this in v8. > >> + * @p_def/min/max from a pointer. Use v4l2_ctrl_ptr_create(NULL) if the >> + * default/min/max value of the compound control should be all zeroes. > > Hmmm... I expect that, for some compound control types, the concept of > minimum and maximum won't be defined. Wouldn't it better for a get > control operation on V4L2_CTRL_WHICH_{MIN,MAX}_VAL to return an error in > that case, instead of zeroed memory ? I agree with that. The error code needs to be documented, though. And I don't see that in v8. If I understand it correctly, then you return -EINVAL in v8 if no min or max info is available. I'm not so keen on that since it isn't an invalid argument. Perhaps -ENODATA? Regards, Hans > >> * >> */ >> struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl, >> const struct v4l2_ctrl_ops *ops, >> u32 id, >> - const union v4l2_ctrl_ptr p_def); >> + const union v4l2_ctrl_ptr p_def, >> + const union v4l2_ctrl_ptr p_min, >> + const union v4l2_ctrl_ptr p_max); >> >> /** >> * v4l2_ctrl_new_int_menu() - Create a new standard V4L2 integer menu control. >> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >> index 2e36bb610ea6..134f6a65eacc 100644 >> --- a/include/uapi/linux/videodev2.h >> +++ b/include/uapi/linux/videodev2.h >> @@ -1796,6 +1796,8 @@ struct v4l2_ext_controls { >> #define V4L2_CTRL_WHICH_CUR_VAL 0 >> #define V4L2_CTRL_WHICH_DEF_VAL 0x0f000000 >> #define V4L2_CTRL_WHICH_REQUEST_VAL 0x0f010000 >> +#define V4L2_CTRL_WHICH_MIN_VAL 0x0f020000 >> +#define V4L2_CTRL_WHICH_MAX_VAL 0x0f030000 >> >> enum v4l2_ctrl_type { >> V4L2_CTRL_TYPE_INTEGER = 1, >
