Hi,
On 14-Nov-24 8:10 PM, Ricardo Ribalda wrote:
> From: Yunke Cao <yunkec@xxxxxxxxxx>
>
> Add support for V4L2_CTRL_WHICH_MIN/MAX_VAL in uvc driver.
> It is needed for the V4L2_CID_UVC_REGION_OF_INTEREST_RECT control.
>
> Signed-off-by: Yunke Cao <yunkec@xxxxxxxxxx>
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Regards,
Hans
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 96 ++++++++++++++++++++++++++++++++--------
> drivers/media/usb/uvc/uvc_v4l2.c | 2 +
> 2 files changed, 79 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index b591d7fddc37..0dae5e8c3ca0 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1270,6 +1270,37 @@ static int uvc_query_v4l2_class(struct uvc_video_chain *chain, u32 req_id,
> return 0;
> }
>
> +static bool uvc_ctrl_is_readable(u32 which, struct uvc_control *ctrl,
> + struct uvc_control_mapping *mapping)
> +{
> + if (which == V4L2_CTRL_WHICH_CUR_VAL)
> + return !!(ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR);
> +
> + if (which == V4L2_CTRL_WHICH_DEF_VAL)
> + return !!(ctrl->info.flags & UVC_CTRL_FLAG_GET_DEF);
> +
> + /* Types with implicit boundaries. */
> + switch (mapping->v4l2_type) {
> + case V4L2_CTRL_TYPE_MENU:
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + case V4L2_CTRL_TYPE_BUTTON:
> + return true;
> + case V4L2_CTRL_TYPE_BITMASK:
> + return (ctrl->info.flags & UVC_CTRL_FLAG_GET_RES) ||
> + (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX);
> + default:
> + break;
> + }
> +
> + if (which == V4L2_CTRL_WHICH_MIN_VAL)
> + return !!(ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN);
> +
> + if (which == V4L2_CTRL_WHICH_MAX_VAL)
> + return !!(ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX);
> +
> + return false;
> +}
> +
> /*
> * Check if control @v4l2_id can be accessed by the given control @ioctl
> * (VIDIOC_G_EXT_CTRLS, VIDIOC_TRY_EXT_CTRLS or VIDIOC_S_EXT_CTRLS).
> @@ -1288,7 +1319,6 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
> struct uvc_control *master_ctrl = NULL;
> struct uvc_control_mapping *mapping;
> struct uvc_control *ctrl;
> - bool read = ioctl == VIDIOC_G_EXT_CTRLS;
> s32 val;
> int ret;
> int i;
> @@ -1300,10 +1330,10 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
> if (!ctrl)
> return -EINVAL;
>
> - if (!(ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) && read)
> - return -EACCES;
> + if (ioctl == VIDIOC_G_EXT_CTRLS)
> + return uvc_ctrl_is_readable(ctrls->which, ctrl, mapping);
>
> - if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR) && !read)
> + if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
> return -EACCES;
>
> if (ioctl != VIDIOC_S_EXT_CTRLS || !mapping->master_id)
> @@ -1451,6 +1481,9 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> v4l2_ctrl->flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
> if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
> v4l2_ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> + if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX) &&
> + (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN))
> + v4l2_ctrl->flags |= V4L2_CTRL_FLAG_HAS_WHICH_MIN_MAX;
>
> if (mapping->master_id)
> __uvc_find_control(ctrl->entity, mapping->master_id,
> @@ -2037,16 +2070,18 @@ static int uvc_mapping_get_xctrl_compound(struct uvc_video_chain *chain,
>
> switch (which) {
> case V4L2_CTRL_WHICH_CUR_VAL:
> - ret = __uvc_ctrl_load_cur(chain, ctrl);
> - if (ret < 0)
> - return ret;
> id = UVC_CTRL_DATA_CURRENT;
> query = UVC_GET_CUR;
> break;
> + case V4L2_CTRL_WHICH_MIN_VAL:
> + id = UVC_CTRL_DATA_MIN;
> + query = UVC_GET_MIN;
> + break;
> + case V4L2_CTRL_WHICH_MAX_VAL:
> + id = UVC_CTRL_DATA_MAX;
> + query = UVC_GET_MAX;
> + break;
> case V4L2_CTRL_WHICH_DEF_VAL:
> - ret = uvc_ctrl_populate_cache(chain, ctrl);
> - if (ret < 0)
> - return ret;
> id = UVC_CTRL_DATA_DEF;
> query = UVC_GET_DEF;
> break;
> @@ -2064,6 +2099,14 @@ static int uvc_mapping_get_xctrl_compound(struct uvc_video_chain *chain,
> if (!data)
> return -ENOMEM;
>
> + if (which == V4L2_CTRL_WHICH_CUR_VAL)
> + ret = __uvc_ctrl_load_cur(chain, ctrl);
> + else
> + ret = uvc_ctrl_populate_cache(chain, ctrl);
> +
> + if (ret < 0)
> + return ret;
> +
> ret = mapping->get(mapping, query, uvc_ctrl_data(ctrl, id), size, data);
> if (ret < 0)
> return ret;
> @@ -2076,22 +2119,37 @@ static int uvc_mapping_get_xctrl_std(struct uvc_video_chain *chain,
> struct uvc_control_mapping *mapping,
> u32 which, struct v4l2_ext_control *xctrl)
> {
> + struct v4l2_queryctrl qc;
> + int ret;
> +
> switch (which) {
> case V4L2_CTRL_WHICH_CUR_VAL:
> return __uvc_ctrl_get(chain, ctrl, mapping, &xctrl->value);
> case V4L2_CTRL_WHICH_DEF_VAL:
> - if (!ctrl->cached) {
> - int ret = uvc_ctrl_populate_cache(chain, ctrl);
> + case V4L2_CTRL_WHICH_MIN_VAL:
> + case V4L2_CTRL_WHICH_MAX_VAL:
> + break;
> + default:
> + return -EINVAL;
> + }
>
> - if (ret < 0)
> - return ret;
> - }
> - xctrl->value = uvc_mapping_get_s32(mapping, UVC_GET_DEF,
> - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF));
> - return 0;
> + ret = __uvc_queryctrl_boundaries(chain, ctrl, mapping, &qc);
> + if (ret < 0)
> + return ret;
> +
> + switch (which) {
> + case V4L2_CTRL_WHICH_DEF_VAL:
> + xctrl->value = qc.default_value;
> + break;
> + case V4L2_CTRL_WHICH_MIN_VAL:
> + xctrl->value = qc.minimum;
> + break;
> + case V4L2_CTRL_WHICH_MAX_VAL:
> + xctrl->value = qc.maximum;
> + break;
> }
>
> - return -EINVAL;
> + return 0;
> }
>
> static int uvc_mapping_get_xctrl(struct uvc_video_chain *chain,
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 65dbb53b1e75..7e284770149d 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -1087,6 +1087,8 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
> switch (ctrls->which) {
> case V4L2_CTRL_WHICH_DEF_VAL:
> case V4L2_CTRL_WHICH_CUR_VAL:
> + case V4L2_CTRL_WHICH_MAX_VAL:
> + case V4L2_CTRL_WHICH_MIN_VAL:
> which = ctrls->which;
> break;
> default:
>
