All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/5] media: Implement UVC v1.5 ROI
@ 2022-05-26  5:07 Yunke Cao
  2022-05-26  5:07 ` [PATCH v5 1/5] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Yunke Cao @ 2022-05-26  5:07 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne
  Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao

This patch set implements UVC v1.5 region of interest using V4L2
control API.

ROI control is consisted two uvc specific controls.
1. A rectangle control with a newly added type V4L2_CTRL_TYPE_RECT.
2. An auto control with type bitmask.

V4L2_CTRL_WHICH_MIN/MAX_VAL is added to support the rectangle control.

Tested on two different usb cameras using v4l2-ctl and calling ioctls.

Changelog since v4:
-Cherry-pick the original patch
 "v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL".
-Split patch "media: uvcvideo: implement UVC v1.5 ROI" into two patches.
 The codes for supporting min/max in uvc are in patch 4/5 now.
-Minor fixes. Detailed changelog in patches
Changelog since v3:
- Reordered/sliced the patches.
  1. Add rect type.
  2. Add min/max.
  3. Add the roi controls (including init to default).
  4. Document the roi controls.
- Define the roi controls as uvc-specific in uvcvideo.h.
- Modified documentation.
- Removed the vivid change. Given the controls are now uvc-specific.
  I'm not sure how valuable it is to add it in vivid. Let me know
  otherwise.

*** BLURB HERE ***

Hans Verkuil (1):
  v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL

Yunke Cao (4):
  media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT
  media: uvcvideo: implement UVC v1.5 ROI
  media: uvcvideo: support V4L2_CTRL_WHICH_MIN/MAX_VAL
  media: uvcvideo: document UVC v1.5 ROI

 .../userspace-api/media/drivers/uvcvideo.rst  |  61 +++++
 .../media/v4l/vidioc-g-ext-ctrls.rst          |  11 +-
 .../media/videodev2.h.rst.exceptions          |   3 +
 drivers/media/i2c/imx214.c                    |   5 +-
 .../media/platform/qcom/venus/venc_ctrls.c    |   4 +
 drivers/media/usb/uvc/uvc_ctrl.c              | 217 ++++++++++++++++--
 drivers/media/usb/uvc/uvc_v4l2.c              |  12 +-
 drivers/media/usb/uvc/uvcvideo.h              |  10 +-
 drivers/media/v4l2-core/v4l2-ctrls-api.c      |  51 +++-
 drivers/media/v4l2-core/v4l2-ctrls-core.c     | 155 ++++++++++++-
 drivers/media/v4l2-core/v4l2-ioctl.c          |   4 +-
 include/media/v4l2-ctrls.h                    |  34 ++-
 include/uapi/linux/usb/video.h                |   1 +
 include/uapi/linux/uvcvideo.h                 |  13 ++
 include/uapi/linux/v4l2-controls.h            |   8 +
 include/uapi/linux/videodev2.h                |   4 +
 16 files changed, 540 insertions(+), 53 deletions(-)

-- 
2.36.1.124.g0e6072fb45-goog


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v5 1/5] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT
  2022-05-26  5:07 [PATCH v5 0/5] media: Implement UVC v1.5 ROI Yunke Cao
@ 2022-05-26  5:07 ` Yunke Cao
  2022-05-26 14:15   ` Ricardo Ribalda
  2022-05-30  1:35   ` Sergey Senozhatsky
  2022-05-26  5:07 ` [PATCH v5 2/5] v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Yunke Cao @ 2022-05-26  5:07 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne
  Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao

Add p_rect to struct v4l2_ext_control with basic support in
v4l2-ctrls.

Signed-off-by: Yunke Cao <yunkec@google.com>
---
Changelog since v4:
- Fix typo.
 .../media/v4l/vidioc-g-ext-ctrls.rst          |  4 ++++
 .../media/videodev2.h.rst.exceptions          |  1 +
 drivers/media/v4l2-core/v4l2-ctrls-core.c     | 20 +++++++++++++++++++
 include/media/v4l2-ctrls.h                    |  2 ++
 include/uapi/linux/videodev2.h                |  2 ++
 5 files changed, 29 insertions(+)

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 29971a45a2d4..7473baa4e977 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
@@ -189,6 +189,10 @@ still cause this situation.
       - ``p_area``
       - A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
         of type ``V4L2_CTRL_TYPE_AREA``.
+    * - struct :c:type:`v4l2_rect` *
+      - ``p_rect``
+      - A pointer to a struct :c:type:`v4l2_rect`. Valid if this control is
+        of type ``V4L2_CTRL_TYPE_RECT``.
     * - struct :c:type:`v4l2_ctrl_h264_sps` *
       - ``p_h264_sps``
       - A pointer to a struct :c:type:`v4l2_ctrl_h264_sps`. Valid if this control is
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index 9cbb7a0c354a..7b423475281d 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -147,6 +147,7 @@ replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
+replace symbol V4L2_CTRL_TYPE_RECT :c:type:`v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_FWHT_PARAMS :c:type:`v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_VP8_FRAME :c:type:`v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR :c:type:`v4l2_ctrl_type`
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index 8968cec8454e..384d12a9638b 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -84,6 +84,11 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
 		return ptr1.p_u16[idx] == ptr2.p_u16[idx];
 	case V4L2_CTRL_TYPE_U32:
 		return ptr1.p_u32[idx] == ptr2.p_u32[idx];
+	case V4L2_CTRL_TYPE_RECT:
+		return ptr1.p_rect->top == ptr2.p_rect->top &&
+		       ptr1.p_rect->left == ptr2.p_rect->left &&
+		       ptr1.p_rect->height == ptr2.p_rect->height &&
+		       ptr1.p_rect->width == ptr2.p_rect->width;
 	default:
 		if (ctrl->is_int)
 			return ptr1.p_s32[idx] == ptr2.p_s32[idx];
@@ -307,6 +312,11 @@ static void std_log(const struct v4l2_ctrl *ctrl)
 	case V4L2_CTRL_TYPE_VP9_FRAME:
 		pr_cont("VP9_FRAME");
 		break;
+	case V4L2_CTRL_TYPE_RECT:
+		pr_cont("%ux%u@%dx%d",
+			ptr.p_rect->width, ptr.p_rect->height,
+			ptr.p_rect->left, ptr.p_rect->top);
+		break;
 	default:
 		pr_cont("unknown type %d", ctrl->type);
 		break;
@@ -525,6 +535,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 	struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
 	struct v4l2_ctrl_hevc_decode_params *p_hevc_decode_params;
 	struct v4l2_area *area;
+	struct v4l2_rect *rect;
 	void *p = ptr.p + idx * ctrl->elem_size;
 	unsigned int i;
 
@@ -888,6 +899,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 			return -EINVAL;
 		break;
 
+	case V4L2_CTRL_TYPE_RECT:
+		rect = p;
+		if (!rect->width || !rect->height)
+			return -EINVAL;
+		break;
+
 	default:
 		return -EINVAL;
 	}
@@ -1456,6 +1473,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_AREA:
 		elem_size = sizeof(struct v4l2_area);
 		break;
+	case V4L2_CTRL_TYPE_RECT:
+		elem_size = sizeof(struct v4l2_rect);
+		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
 			elem_size = sizeof(s32);
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index b3ce438f1329..919e104de50b 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -58,6 +58,7 @@ struct video_device;
  * @p_hdr10_cll:		Pointer to an HDR10 Content Light Level structure.
  * @p_hdr10_mastering:		Pointer to an HDR10 Mastering Display structure.
  * @p_area:			Pointer to an area.
+ * @p_rect:			Pointer to a rectangle.
  * @p:				Pointer to a compound value.
  * @p_const:			Pointer to a constant compound value.
  */
@@ -87,6 +88,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_hdr10_cll_info *p_hdr10_cll;
 	struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
 	struct v4l2_area *p_area;
+	struct v4l2_rect *p_rect;
 	void *p;
 	const void *p_const;
 };
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 3768a0a80830..b712412cf763 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1751,6 +1751,7 @@ struct v4l2_ext_control {
 		__u16 __user *p_u16;
 		__u32 __user *p_u32;
 		struct v4l2_area __user *p_area;
+		struct v4l2_rect __user *p_rect;
 		struct v4l2_ctrl_h264_sps __user *p_h264_sps;
 		struct v4l2_ctrl_h264_pps *p_h264_pps;
 		struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scaling_matrix;
@@ -1810,6 +1811,7 @@ enum v4l2_ctrl_type {
 	V4L2_CTRL_TYPE_U16	     = 0x0101,
 	V4L2_CTRL_TYPE_U32	     = 0x0102,
 	V4L2_CTRL_TYPE_AREA          = 0x0106,
+	V4L2_CTRL_TYPE_RECT	     = 0x0107,
 
 	V4L2_CTRL_TYPE_HDR10_CLL_INFO		= 0x0110,
 	V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY	= 0x0111,
-- 
2.36.1.124.g0e6072fb45-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 2/5] v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL
  2022-05-26  5:07 [PATCH v5 0/5] media: Implement UVC v1.5 ROI Yunke Cao
  2022-05-26  5:07 ` [PATCH v5 1/5] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
@ 2022-05-26  5:07 ` Yunke Cao
  2022-05-26  5:07 ` [PATCH v5 3/5] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Yunke Cao @ 2022-05-26  5:07 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne
  Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Add the capability of retrieving the min and max values of a
compound control.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Yunke Cao <yunkec@google.com>
---
git am from https://lore.kernel.org/all/20191119113457.57833-3-hverkuil-cisco@xs4all.nl/
-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
 	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));
+
 	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 ea5805e71c14..bdeb2311c1f3 100644
--- a/drivers/media/platform/qcom/venus/venc_ctrls.c
+++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
@@ -576,10 +576,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(&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 384d12a9638b..7b6bf85814fe 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,
 };
@@ -1369,7 +1469,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;
@@ -1516,7 +1619,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) {
@@ -1566,6 +1669,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);
@@ -1618,7 +1728,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;
@@ -1643,7 +1754,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);
 
@@ -1676,7 +1788,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);
 
@@ -1708,7 +1821,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);
@@ -1716,7 +1830,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;
@@ -1730,7 +1846,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);
 
@@ -1754,7 +1870,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 96e307fe3aab..6ed6ef87c7ff 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -892,7 +892,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
+ * @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.
  *
  */
 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 b712412cf763..d22ebb0102d4 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1793,6 +1793,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,
-- 
2.36.1.124.g0e6072fb45-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 3/5] media: uvcvideo: implement UVC v1.5 ROI
  2022-05-26  5:07 [PATCH v5 0/5] media: Implement UVC v1.5 ROI Yunke Cao
  2022-05-26  5:07 ` [PATCH v5 1/5] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
  2022-05-26  5:07 ` [PATCH v5 2/5] v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
@ 2022-05-26  5:07 ` Yunke Cao
  2022-05-26 14:35   ` Ricardo Ribalda
  2022-05-30  1:59   ` Sergey Senozhatsky
  2022-05-26  5:07 ` [PATCH v5 4/5] media: uvcvideo: support V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
  2022-05-26  5:07 ` [PATCH v5 5/5] media: uvcvideo: document UVC v1.5 ROI Yunke Cao
  4 siblings, 2 replies; 11+ messages in thread
From: Yunke Cao @ 2022-05-26  5:07 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne
  Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao

Implement support for ROI as described in UVC 1.5:
4.2.2.1.20 Digital Region of Interest (ROI) Control

ROI control is implemented using V4L2 control API as
two uvc-specific controls:
V4L2_CID_UVC_REGION_OF_INTEREST_RECT and
V4L2_CID_UVC_REGION_OF_INTEREST_AUTO.

Signed-off-by: Yunke Cao <yunkec@google.com>
---
References a rejected attempt that uses v4l2 selection API:
https://lore.kernel.org/lkml/20210501082001.100533-2-senozhatsky@chromium.org
Changelog since v4:
-Check boundary condition: width or height == 0.
-Populate xctrl->id and xctrl->size.
-Split code for V4L2_CTRL_WHICH_MIN/MAX_VAL to patch 4/5.

 drivers/media/usb/uvc/uvc_ctrl.c   | 207 ++++++++++++++++++++++++++---
 drivers/media/usb/uvc/uvc_v4l2.c   |   8 +-
 drivers/media/usb/uvc/uvcvideo.h   |  10 +-
 include/uapi/linux/usb/video.h     |   1 +
 include/uapi/linux/uvcvideo.h      |  13 ++
 include/uapi/linux/v4l2-controls.h |   8 ++
 6 files changed, 223 insertions(+), 24 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index b4f6edf968bc..c470861e408a 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -355,6 +355,16 @@ static const struct uvc_control_info uvc_ctrls[] = {
 		.flags		= UVC_CTRL_FLAG_GET_CUR
 				| UVC_CTRL_FLAG_AUTO_UPDATE,
 	},
+	{
+		.entity		= UVC_GUID_UVC_CAMERA,
+		.selector	= UVC_CT_REGION_OF_INTEREST_CONTROL,
+		.index		= 21,
+		.size		= 10,
+		.flags		= UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_CUR
+				| UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
+				| UVC_CTRL_FLAG_GET_DEF
+				| UVC_CTRL_FLAG_AUTO_UPDATE,
+	},
 };
 
 static const u32 uvc_control_classes[] = {
@@ -728,6 +738,24 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.v4l2_type	= V4L2_CTRL_TYPE_BOOLEAN,
 		.data_type	= UVC_CTRL_DATA_TYPE_BOOLEAN,
 	},
+	{
+		.id		= V4L2_CID_UVC_REGION_OF_INTEREST_RECT,
+		.entity		= UVC_GUID_UVC_CAMERA,
+		.selector	= UVC_CT_REGION_OF_INTEREST_CONTROL,
+		.size		= 64,
+		.offset		= 0,
+		.v4l2_type	= V4L2_CTRL_TYPE_RECT,
+		.data_type	= UVC_CTRL_DATA_TYPE_RECT,
+	},
+	{
+		.id		= V4L2_CID_UVC_REGION_OF_INTEREST_AUTO,
+		.entity		= UVC_GUID_UVC_CAMERA,
+		.selector	= UVC_CT_REGION_OF_INTEREST_CONTROL,
+		.size		= 16,
+		.offset		= 64,
+		.v4l2_type	= V4L2_CTRL_TYPE_BITMASK,
+		.data_type	= UVC_CTRL_DATA_TYPE_BITMASK,
+	},
 };
 
 /* ------------------------------------------------------------------------
@@ -749,6 +777,34 @@ static inline void uvc_clear_bit(u8 *data, int bit)
 	data[bit >> 3] &= ~(1 << (bit & 7));
 }
 
+static void uvc_to_v4l2_rect(struct v4l2_rect *v4l2_rect,
+	const struct uvc_rect *uvc_rect)
+{
+	v4l2_rect->top = uvc_rect->top;
+	v4l2_rect->left = uvc_rect->left;
+	v4l2_rect->height = uvc_rect->bottom - uvc_rect->top + 1;
+	v4l2_rect->width = uvc_rect->right - uvc_rect->left + 1;
+}
+
+static int v4l2_to_uvc_rect(struct uvc_rect *uvc_rect,
+	const struct v4l2_rect *v4l2_rect)
+{
+	// Safely converts s32 and u32 to u16.
+	if (v4l2_rect->top > U16_MAX || v4l2_rect->top < 0 ||
+	    v4l2_rect->left > U16_MAX || v4l2_rect->left < 0 ||
+	    v4l2_rect->height > U16_MAX || v4l2_rect->height == 0 ||
+	    v4l2_rect->width > U16_MAX || v4l2_rect->width == 0 ||
+	    v4l2_rect->height + v4l2_rect->top - 1 > U16_MAX ||
+	    v4l2_rect->width + v4l2_rect->left - 1 > U16_MAX)
+		return -ERANGE;
+
+	uvc_rect->top = v4l2_rect->top;
+	uvc_rect->left = v4l2_rect->left;
+	uvc_rect->bottom = v4l2_rect->height + v4l2_rect->top - 1;
+	uvc_rect->right = v4l2_rect->width + v4l2_rect->left - 1;
+	return 0;
+}
+
 /* Extract the bit string specified by mapping->offset and mapping->size
  * from the little-endian data stored at 'data' and return the result as
  * a signed 32bit integer. Sign extension will be performed if the mapping
@@ -963,11 +1019,23 @@ static s32 __uvc_ctrl_get_value(struct uvc_control_mapping *mapping,
 	return value;
 }
 
+static void __uvc_ctrl_get_v4l2_rect(struct uvc_control *ctrl,
+				     struct uvc_control_mapping *mapping,
+				     u32 id,
+				     struct v4l2_rect *rect)
+{
+	struct uvc_rect *uvc_rect =
+		(struct uvc_rect *)(uvc_ctrl_data(ctrl, id)
+					+ mapping->offset / 8);
+	uvc_to_v4l2_rect(rect, uvc_rect);
+}
+
 static int __uvc_ctrl_get(struct uvc_video_chain *chain,
 	struct uvc_control *ctrl, struct uvc_control_mapping *mapping,
-	s32 *value)
+	struct v4l2_ext_control *xctrl)
 {
 	int ret;
+	struct v4l2_rect v4l2_rect = {};
 
 	if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
 		return -EACCES;
@@ -993,8 +1061,19 @@ static int __uvc_ctrl_get(struct uvc_video_chain *chain,
 		ctrl->loaded = 1;
 	}
 
-	*value = __uvc_ctrl_get_value(mapping,
+	xctrl->id = mapping->id;
+	switch (mapping->v4l2_type) {
+	case V4L2_CTRL_TYPE_RECT:
+		__uvc_ctrl_get_v4l2_rect(ctrl, mapping, UVC_CTRL_DATA_CURRENT,
+					 &v4l2_rect);
+		xctrl->size = sizeof(v4l2_rect);
+		return copy_to_user(xctrl->p_rect, &v4l2_rect,
+				sizeof(v4l2_rect));
+
+	default:
+		xctrl->value = __uvc_ctrl_get_value(mapping,
 				uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
+	}
 
 	return 0;
 }
@@ -1104,13 +1183,14 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
 		__uvc_find_control(ctrl->entity, mapping->master_id,
 				   &master_map, &master_ctrl, 0);
 	if (master_ctrl && (master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) {
-		s32 val;
-		int ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val);
+		struct v4l2_ext_control xctrl;
+		int ret =
+			__uvc_ctrl_get(chain, master_ctrl, master_map, &xctrl);
 		if (ret < 0)
 			return ret;
 
-		if (val != mapping->master_manual)
-				v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
+		if (xctrl.value != mapping->master_manual)
+			v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
 	}
 
 	if (!ctrl->cached) {
@@ -1344,16 +1424,16 @@ static void uvc_ctrl_send_slave_event(struct uvc_video_chain *chain,
 	struct uvc_control_mapping *mapping = NULL;
 	struct uvc_control *ctrl = NULL;
 	u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
-	s32 val = 0;
+	struct v4l2_ext_control xctrl;
 
 	__uvc_find_control(master->entity, slave_id, &mapping, &ctrl, 0);
 	if (ctrl == NULL)
 		return;
 
-	if (__uvc_ctrl_get(chain, ctrl, mapping, &val) == 0)
+	if (__uvc_ctrl_get(chain, ctrl, mapping, &xctrl) == 0)
 		changes |= V4L2_EVENT_CTRL_CH_VALUE;
 
-	uvc_ctrl_send_event(chain, handle, ctrl, mapping, val, changes);
+	uvc_ctrl_send_event(chain, handle, ctrl, mapping, xctrl.value, changes);
 }
 
 void uvc_ctrl_status_event(struct uvc_video_chain *chain,
@@ -1515,13 +1595,13 @@ static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned elems)
 	if (sev->flags & V4L2_EVENT_SUB_FL_SEND_INITIAL) {
 		struct v4l2_event ev;
 		u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
-		s32 val = 0;
+		struct v4l2_ext_control xctrl;
 
-		if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
+		if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &xctrl) == 0)
 			changes |= V4L2_EVENT_CTRL_CH_VALUE;
 
-		uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, val,
-				    changes);
+		uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping,
+				xctrl.value, changes);
 		/* Mark the queue as active, allowing this initial
 		   event to be accepted. */
 		sev->elems = elems;
@@ -1682,10 +1762,14 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
 }
 
 int uvc_ctrl_get(struct uvc_video_chain *chain,
-	struct v4l2_ext_control *xctrl)
+	struct v4l2_ext_control *xctrl, u32 v4l2_which)
 {
 	struct uvc_control *ctrl;
 	struct uvc_control_mapping *mapping;
+	int ret;
+	u32 flag;
+	u32 id;
+	u8 query;
 
 	if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
 		return -EACCES;
@@ -1694,7 +1778,36 @@ int uvc_ctrl_get(struct uvc_video_chain *chain,
 	if (ctrl == NULL)
 		return -EINVAL;
 
-	return __uvc_ctrl_get(chain, ctrl, mapping, &xctrl->value);
+	switch (v4l2_which) {
+	case V4L2_CTRL_WHICH_DEF_VAL:
+		flag = UVC_CTRL_FLAG_GET_DEF;
+		id = UVC_CTRL_DATA_DEF;
+		query = UVC_GET_DEF;
+		break;
+	case V4L2_CTRL_WHICH_CUR_VAL:
+	default:
+		return __uvc_ctrl_get(chain, ctrl, mapping, xctrl);
+	}
+
+	if (!ctrl->cached) {
+		ret = uvc_ctrl_populate_cache(chain, ctrl);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (!(ctrl->info.flags & flag))
+		return -EACCES;
+
+	if (mapping->v4l2_type == V4L2_CTRL_TYPE_RECT) {
+		struct v4l2_rect rect;
+
+		__uvc_ctrl_get_v4l2_rect(ctrl, mapping, id, &rect);
+		return copy_to_user(xctrl->p_rect, &rect, sizeof(rect));
+	}
+
+	xctrl->value = mapping->get(mapping, query, uvc_ctrl_data(ctrl, id));
+
+	return 0;
 }
 
 int uvc_ctrl_set(struct uvc_fh *handle,
@@ -1703,6 +1816,8 @@ int uvc_ctrl_set(struct uvc_fh *handle,
 	struct uvc_video_chain *chain = handle->chain;
 	struct uvc_control *ctrl;
 	struct uvc_control_mapping *mapping;
+	struct v4l2_rect v4l2_rect;
+	struct uvc_rect uvc_rect;
 	s32 value;
 	u32 step;
 	s32 min;
@@ -1774,6 +1889,16 @@ int uvc_ctrl_set(struct uvc_fh *handle,
 
 		break;
 
+	case V4L2_CTRL_TYPE_RECT:
+		ret = copy_from_user(&v4l2_rect, xctrl->p_rect,
+				sizeof(v4l2_rect));
+		if (ret < 0)
+			return ret;
+		ret = v4l2_to_uvc_rect(&uvc_rect, &v4l2_rect);
+		if (ret < 0)
+			return ret;
+		break;
+
 	default:
 		value = xctrl->value;
 		break;
@@ -1807,8 +1932,16 @@ int uvc_ctrl_set(struct uvc_fh *handle,
 		       ctrl->info.size);
 	}
 
-	mapping->set(mapping, value,
-		uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
+	switch (mapping->data_type) {
+	case UVC_CTRL_DATA_TYPE_RECT:
+		memcpy(uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT)
+			+ mapping->offset / 8,
+			&uvc_rect, sizeof(uvc_rect));
+		break;
+	default:
+		mapping->set(mapping, value,
+			uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
+	}
 
 	if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
 		ctrl->handle = handle;
@@ -2366,6 +2499,40 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev,
 	}
 }
 
+static int uvc_ctrl_init_roi(struct uvc_device *dev, struct uvc_control *ctrl)
+{
+	int ret;
+
+	ret = uvc_query_ctrl(dev, UVC_GET_DEF, ctrl->entity->id, dev->intfnum,
+			     UVC_CT_REGION_OF_INTEREST_CONTROL,
+			     uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
+			     ctrl->info.size);
+	if (ret)
+		goto out;
+
+	/*
+	 * Most firmwares have wrong GET_CURRENT configuration. E.g. it's
+	 * below GET_MIN, or have rectangle coordinates mixed up. This
+	 * causes problems sometimes, because we are unable to set
+	 * auto-controls value without first setting ROI rectangle to
+	 * valid configuration.
+	 *
+	 * We expect that default configuration is always correct and
+	 * is within the GET_MIN / GET_MAX boundaries.
+	 *
+	 * Set current ROI configuration to GET_DEF, so that we will
+	 * always have properly configured ROI.
+	 */
+	ret = uvc_query_ctrl(dev, UVC_SET_CUR, 1, dev->intfnum,
+			     UVC_CT_REGION_OF_INTEREST_CONTROL,
+			     uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
+			     ctrl->info.size);
+out:
+	if (ret)
+		dev_err(&dev->udev->dev, "Failed to fixup ROI (%d).\n", ret);
+	return ret;
+}
+
 /*
  * Add control information and hardcoded stock control mappings to the given
  * device.
@@ -2378,6 +2545,7 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 	const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
 	const struct uvc_control_mapping *mend =
 		mapping + ARRAY_SIZE(uvc_ctrl_mappings);
+	const u8 camera_entity[16] = UVC_GUID_UVC_CAMERA;
 
 	/* XU controls initialization requires querying the device for control
 	 * information. As some buggy UVC devices will crash when queried
@@ -2398,6 +2566,11 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 			 * GET_INFO on standard controls.
 			 */
 			uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
+
+			if (ctrl->info.selector ==
+				UVC_CT_REGION_OF_INTEREST_CONTROL &&
+			    uvc_entity_match_guid(ctrl->entity, camera_entity))
+				uvc_ctrl_init_roi(chain->dev, ctrl);
 			break;
 		 }
 	}
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 711556d13d03..040511da1005 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1040,15 +1040,11 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
 
 	if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
 		for (i = 0; i < ctrls->count; ++ctrl, ++i) {
-			struct v4l2_queryctrl qc = { .id = ctrl->id };
-
-			ret = uvc_query_v4l2_ctrl(chain, &qc);
+			ret = uvc_ctrl_get(chain, ctrl, ctrls->which);
 			if (ret < 0) {
 				ctrls->error_idx = i;
 				return ret;
 			}
-
-			ctrl->value = qc.default_value;
 		}
 
 		return 0;
@@ -1059,7 +1055,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
 		return ret;
 
 	for (i = 0; i < ctrls->count; ++ctrl, ++i) {
-		ret = uvc_ctrl_get(chain, ctrl);
+		ret = uvc_ctrl_get(chain, ctrl, ctrls->which);
 		if (ret < 0) {
 			uvc_ctrl_rollback(handle);
 			ctrls->error_idx = i;
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 143230b3275b..f414ad7d57b2 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -424,6 +424,13 @@ struct uvc_streaming_header {
 	u8 bTriggerUsage;
 };
 
+struct uvc_rect {
+	u16 top;
+	u16 left;
+	u16 bottom;
+	u16 right;
+} __packed;
+
 enum uvc_buffer_state {
 	UVC_BUF_STATE_IDLE	= 0,
 	UVC_BUF_STATE_QUEUED	= 1,
@@ -897,7 +904,8 @@ static inline int uvc_ctrl_rollback(struct uvc_fh *handle)
 	return __uvc_ctrl_commit(handle, 1, NULL);
 }
 
-int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl);
+int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl,
+		 u32 v4l2_which);
 int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl);
 int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
 			   bool read);
diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
index bfdae12cdacf..9076a444758a 100644
--- a/include/uapi/linux/usb/video.h
+++ b/include/uapi/linux/usb/video.h
@@ -104,6 +104,7 @@
 #define UVC_CT_ROLL_ABSOLUTE_CONTROL			0x0f
 #define UVC_CT_ROLL_RELATIVE_CONTROL			0x10
 #define UVC_CT_PRIVACY_CONTROL				0x11
+#define UVC_CT_REGION_OF_INTEREST_CONTROL		0x14
 
 /* A.9.5. Processing Unit Control Selectors */
 #define UVC_PU_CONTROL_UNDEFINED			0x00
diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h
index 8288137387c0..b16e5d373f3f 100644
--- a/include/uapi/linux/uvcvideo.h
+++ b/include/uapi/linux/uvcvideo.h
@@ -16,6 +16,7 @@
 #define UVC_CTRL_DATA_TYPE_BOOLEAN	3
 #define UVC_CTRL_DATA_TYPE_ENUM		4
 #define UVC_CTRL_DATA_TYPE_BITMASK	5
+#define UVC_CTRL_DATA_TYPE_RECT		6
 
 /* Control flags */
 #define UVC_CTRL_FLAG_SET_CUR		(1 << 0)
@@ -36,6 +37,18 @@
 	 UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES | \
 	 UVC_CTRL_FLAG_GET_DEF)
 
+/* V4L2 private controls */
+#define V4L2_CID_UVC_REGION_OF_INTEREST_RECT	(V4L2_CID_CAMERA_UVC_BASE+1)
+#define V4L2_CID_UVC_REGION_OF_INTEREST_AUTO	(V4L2_CID_CAMERA_UVC_BASE+2)
+#define V4L2_UVC_REGION_OF_INTEREST_AUTO_EXPOSURE		(1 << 0)
+#define V4L2_UVC_REGION_OF_INTEREST_AUTO_IRIS			(1 << 1)
+#define V4L2_UVC_REGION_OF_INTEREST_AUTO_WHITE_BALANCE		(1 << 2)
+#define V4L2_UVC_REGION_OF_INTEREST_AUTO_FOCUS			(1 << 3)
+#define V4L2_UVC_REGION_OF_INTEREST_AUTO_FACE_DETECT		(1 << 4)
+#define V4L2_UVC_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK	(1 << 5)
+#define V4L2_UVC_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION	(1 << 6)
+#define V4L2_UVC_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY		(1 << 7)
+
 struct uvc_menu_info {
 	__u32 value;
 	__u8 name[32];
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index bb40129446d4..48d12782e7e4 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1008,6 +1008,14 @@ enum v4l2_auto_focus_range {
 
 #define V4L2_CID_CAMERA_SENSOR_ROTATION		(V4L2_CID_CAMERA_CLASS_BASE+35)
 
+/* CAMERA-class private control IDs */
+
+/*
+ * The base for the uvc driver controls. See linux/uvcvideo.h for the list
+ * of controls. We reserve 16 controls for this driver.
+ */
+#define V4L2_CID_CAMERA_UVC_BASE		(V4L2_CID_CAMERA_CLASS_BASE + 0x1000)
+
 /* FM Modulator class control IDs */
 
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
-- 
2.36.1.124.g0e6072fb45-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 4/5] media: uvcvideo: support V4L2_CTRL_WHICH_MIN/MAX_VAL
  2022-05-26  5:07 [PATCH v5 0/5] media: Implement UVC v1.5 ROI Yunke Cao
                   ` (2 preceding siblings ...)
  2022-05-26  5:07 ` [PATCH v5 3/5] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
@ 2022-05-26  5:07 ` Yunke Cao
  2022-05-26  5:07 ` [PATCH v5 5/5] media: uvcvideo: document UVC v1.5 ROI Yunke Cao
  4 siblings, 0 replies; 11+ messages in thread
From: Yunke Cao @ 2022-05-26  5:07 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne
  Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao

Add support for V4L2_CTRL_WHICH_MIN/MAX_VAL in uvc driver.
It is useful for the V4L2_CID_UVC_REGION_OF_INTEREST_RECT control.

Signed-off-by: Yunke Cao <yunkec@google.com>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 10 ++++++++++
 drivers/media/usb/uvc/uvc_v4l2.c |  4 +++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c470861e408a..add82db4f6f8 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1784,6 +1784,16 @@ int uvc_ctrl_get(struct uvc_video_chain *chain,
 		id = UVC_CTRL_DATA_DEF;
 		query = UVC_GET_DEF;
 		break;
+	case V4L2_CTRL_WHICH_MIN_VAL:
+		flag = UVC_CTRL_FLAG_GET_MIN;
+		id = UVC_CTRL_DATA_MIN;
+		query = UVC_GET_MIN;
+		break;
+	case V4L2_CTRL_WHICH_MAX_VAL:
+		flag = UVC_CTRL_FLAG_GET_MAX;
+		id = UVC_CTRL_DATA_MAX;
+		query = UVC_GET_MAX;
+		break;
 	case V4L2_CTRL_WHICH_CUR_VAL:
 	default:
 		return __uvc_ctrl_get(chain, ctrl, mapping, xctrl);
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 040511da1005..a88d3fe6de93 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1038,7 +1038,9 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
 	if (ret < 0)
 		return ret;
 
-	if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
+	if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL ||
+	    ctrls->which == V4L2_CTRL_WHICH_MIN_VAL ||
+	    ctrls->which == V4L2_CTRL_WHICH_MAX_VAL) {
 		for (i = 0; i < ctrls->count; ++ctrl, ++i) {
 			ret = uvc_ctrl_get(chain, ctrl, ctrls->which);
 			if (ret < 0) {
-- 
2.36.1.124.g0e6072fb45-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 5/5] media: uvcvideo: document UVC v1.5 ROI
  2022-05-26  5:07 [PATCH v5 0/5] media: Implement UVC v1.5 ROI Yunke Cao
                   ` (3 preceding siblings ...)
  2022-05-26  5:07 ` [PATCH v5 4/5] media: uvcvideo: support V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
@ 2022-05-26  5:07 ` Yunke Cao
  4 siblings, 0 replies; 11+ messages in thread
From: Yunke Cao @ 2022-05-26  5:07 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne
  Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao

Added documentation of V4L2_CID_UVC_REGION_OF_INTEREST_RECT and
V4L2_CID_UVC_REGION_OF_INTEREST_AUTO.

Signed-off-by: Yunke Cao <yunkec@google.com>
---
Changelog since v4:
-Reword the documentation.
-Mention ROI is in global sensor coordinates.

 .../userspace-api/media/drivers/uvcvideo.rst  | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst
index e5fd8fad333c..6f28d816d24a 100644
--- a/Documentation/userspace-api/media/drivers/uvcvideo.rst
+++ b/Documentation/userspace-api/media/drivers/uvcvideo.rst
@@ -181,6 +181,7 @@ Argument: struct uvc_xu_control_mapping
 	UVC_CTRL_DATA_TYPE_BOOLEAN	Boolean
 	UVC_CTRL_DATA_TYPE_ENUM		Enumeration
 	UVC_CTRL_DATA_TYPE_BITMASK	Bitmask
+	UVC_CTRL_DATA_TYPE_RECT		Rectangular area
 
 
 UVCIOC_CTRL_QUERY - Query a UVC XU control
@@ -255,3 +256,63 @@ Argument: struct uvc_xu_control_query
 	__u8	query		Request code to send to the device
 	__u16	size		Control data size (in bytes)
 	__u8	*data		Control value
+
+Private V4L2 controls
+---------------------
+
+A few UVC specific V4L2 control IDs are listed below.
+
+``V4L2_CID_UVC_REGION_OF_INTEREST_RECT (struct)``
+	This control determines the region of interest (ROI). ROI is an
+	rectangular area represented by a struct :c:type:`v4l2_rect`. The
+	rectangle is in global sensor coordinates and pixel units. It is
+	independent of the field of view, not impacted by any cropping or
+	scaling.
+
+	Use ``V4L2_CTRL_WHICH_MIN_VAL`` and ``V4L2_CTRL_WHICH_MAX_VAL`` to query
+	the range of rectangle sizes. For example, a device can have a minimum
+	ROI rectangle of 1x1@0x0 and a maximum of 640x480@0x0.
+
+	Setting a ROI allows the camera to optimize the capture for the region.
+	The value of ``V4L2_CID_REGION_OF_INTEREST_AUTO`` control determines
+	the detailed behavior.
+
+
+``V4L2_CID_UVC_REGION_OF_INTEREST_AUTO (bitmask)``
+	This determines which, if any, on board features should track to the
+	Region of Interest specified by the current value of
+	``V4L2_CID_UVD__REGION_OF_INTEREST_RECT``.
+
+	Max value is a mask indicating all supported Auto
+	Controls.
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - ``V4L2_REGION_OF_INTEREST_AUTO_EXPOSURE``
+      - Setting this to true enables automatic exposure time for the specified
+	region.
+    * - ``V4L2_REGION_OF_INTEREST_AUTO_IRIS``
+      - Setting this to true enables automatic iris aperture for the specified
+	region.
+    * - ``V4L2_REGION_OF_INTEREST_AUTO_WHITE_BALANCE``
+      - Setting this to true enables automatic white balance adjustment for the
+	specified region.
+    * - ``V4L2_REGION_OF_INTEREST_AUTO_FOCUS``
+      - Setting this to true enables automatic focus adjustment for the
+	specified region.
+    * - ``V4L2_REGION_OF_INTEREST_AUTO_FACE_DETECT``
+      - Setting this to true enables automatic face detection for the
+	specified region.
+    * - ``V4L2_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK``
+      - Setting this to true enables automatic face detection and tracking. The
+	current value of ``V4L2_CID_REGION_OF_INTEREST_RECT`` may be updated by
+	the driver.
+    * - ``V4L2_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION``
+      - Setting this to true enables automatic image stabilization. The
+	current value of ``V4L2_CID_REGION_OF_INTEREST_RECT`` may be updated by
+	the driver.
+    * - ``V4L2_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
+      - Setting this to true enables automatically capture the specified region
+	with higher quality if possible.
-- 
2.36.1.124.g0e6072fb45-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/5] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT
  2022-05-26  5:07 ` [PATCH v5 1/5] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
@ 2022-05-26 14:15   ` Ricardo Ribalda
  2022-05-30  1:35   ` Sergey Senozhatsky
  1 sibling, 0 replies; 11+ messages in thread
From: Ricardo Ribalda @ 2022-05-26 14:15 UTC (permalink / raw)
  To: Yunke Cao
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, linux-media

On Thu, 26 May 2022 at 07:08, Yunke Cao <yunkec@google.com> wrote:
>
> Add p_rect to struct v4l2_ext_control with basic support in
> v4l2-ctrls.
>
> Signed-off-by: Yunke Cao <yunkec@google.com>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> Changelog since v4:
> - Fix typo.
>  .../media/v4l/vidioc-g-ext-ctrls.rst          |  4 ++++
>  .../media/videodev2.h.rst.exceptions          |  1 +
>  drivers/media/v4l2-core/v4l2-ctrls-core.c     | 20 +++++++++++++++++++
>  include/media/v4l2-ctrls.h                    |  2 ++
>  include/uapi/linux/videodev2.h                |  2 ++
>  5 files changed, 29 insertions(+)
>
> 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 29971a45a2d4..7473baa4e977 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> @@ -189,6 +189,10 @@ still cause this situation.
>        - ``p_area``
>        - A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
>          of type ``V4L2_CTRL_TYPE_AREA``.
> +    * - struct :c:type:`v4l2_rect` *
> +      - ``p_rect``
> +      - A pointer to a struct :c:type:`v4l2_rect`. Valid if this control is
> +        of type ``V4L2_CTRL_TYPE_RECT``.
>      * - struct :c:type:`v4l2_ctrl_h264_sps` *
>        - ``p_h264_sps``
>        - A pointer to a struct :c:type:`v4l2_ctrl_h264_sps`. Valid if this control is
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index 9cbb7a0c354a..7b423475281d 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -147,6 +147,7 @@ replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
> +replace symbol V4L2_CTRL_TYPE_RECT :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_FWHT_PARAMS :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_VP8_FRAME :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR :c:type:`v4l2_ctrl_type`
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> index 8968cec8454e..384d12a9638b 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -84,6 +84,11 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
>                 return ptr1.p_u16[idx] == ptr2.p_u16[idx];
>         case V4L2_CTRL_TYPE_U32:
>                 return ptr1.p_u32[idx] == ptr2.p_u32[idx];
> +       case V4L2_CTRL_TYPE_RECT:
> +               return ptr1.p_rect->top == ptr2.p_rect->top &&
> +                      ptr1.p_rect->left == ptr2.p_rect->left &&
> +                      ptr1.p_rect->height == ptr2.p_rect->height &&
> +                      ptr1.p_rect->width == ptr2.p_rect->width;
>         default:
>                 if (ctrl->is_int)
>                         return ptr1.p_s32[idx] == ptr2.p_s32[idx];
> @@ -307,6 +312,11 @@ static void std_log(const struct v4l2_ctrl *ctrl)
>         case V4L2_CTRL_TYPE_VP9_FRAME:
>                 pr_cont("VP9_FRAME");
>                 break;
> +       case V4L2_CTRL_TYPE_RECT:
> +               pr_cont("%ux%u@%dx%d",
> +                       ptr.p_rect->width, ptr.p_rect->height,
> +                       ptr.p_rect->left, ptr.p_rect->top);
> +               break;
>         default:
>                 pr_cont("unknown type %d", ctrl->type);
>                 break;
> @@ -525,6 +535,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>         struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
>         struct v4l2_ctrl_hevc_decode_params *p_hevc_decode_params;
>         struct v4l2_area *area;
> +       struct v4l2_rect *rect;
>         void *p = ptr.p + idx * ctrl->elem_size;
>         unsigned int i;
>
> @@ -888,6 +899,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>                         return -EINVAL;
>                 break;
>
> +       case V4L2_CTRL_TYPE_RECT:
> +               rect = p;
> +               if (!rect->width || !rect->height)
> +                       return -EINVAL;
> +               break;
> +
>         default:
>                 return -EINVAL;
>         }
> @@ -1456,6 +1473,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
>         case V4L2_CTRL_TYPE_AREA:
>                 elem_size = sizeof(struct v4l2_area);
>                 break;
> +       case V4L2_CTRL_TYPE_RECT:
> +               elem_size = sizeof(struct v4l2_rect);
> +               break;
>         default:
>                 if (type < V4L2_CTRL_COMPOUND_TYPES)
>                         elem_size = sizeof(s32);
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index b3ce438f1329..919e104de50b 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -58,6 +58,7 @@ struct video_device;
>   * @p_hdr10_cll:               Pointer to an HDR10 Content Light Level structure.
>   * @p_hdr10_mastering:         Pointer to an HDR10 Mastering Display structure.
>   * @p_area:                    Pointer to an area.
> + * @p_rect:                    Pointer to a rectangle.
>   * @p:                         Pointer to a compound value.
>   * @p_const:                   Pointer to a constant compound value.
>   */
> @@ -87,6 +88,7 @@ union v4l2_ctrl_ptr {
>         struct v4l2_ctrl_hdr10_cll_info *p_hdr10_cll;
>         struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
>         struct v4l2_area *p_area;
> +       struct v4l2_rect *p_rect;
>         void *p;
>         const void *p_const;
>  };
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 3768a0a80830..b712412cf763 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1751,6 +1751,7 @@ struct v4l2_ext_control {
>                 __u16 __user *p_u16;
>                 __u32 __user *p_u32;
>                 struct v4l2_area __user *p_area;
> +               struct v4l2_rect __user *p_rect;
>                 struct v4l2_ctrl_h264_sps __user *p_h264_sps;
>                 struct v4l2_ctrl_h264_pps *p_h264_pps;
>                 struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scaling_matrix;
> @@ -1810,6 +1811,7 @@ enum v4l2_ctrl_type {
>         V4L2_CTRL_TYPE_U16           = 0x0101,
>         V4L2_CTRL_TYPE_U32           = 0x0102,
>         V4L2_CTRL_TYPE_AREA          = 0x0106,
> +       V4L2_CTRL_TYPE_RECT          = 0x0107,
>
>         V4L2_CTRL_TYPE_HDR10_CLL_INFO           = 0x0110,
>         V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY  = 0x0111,
> --
> 2.36.1.124.g0e6072fb45-goog
>


-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 3/5] media: uvcvideo: implement UVC v1.5 ROI
  2022-05-26  5:07 ` [PATCH v5 3/5] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
@ 2022-05-26 14:35   ` Ricardo Ribalda
  2022-05-30  5:46     ` Yunke Cao
  2022-05-30  1:59   ` Sergey Senozhatsky
  1 sibling, 1 reply; 11+ messages in thread
From: Ricardo Ribalda @ 2022-05-26 14:35 UTC (permalink / raw)
  To: Yunke Cao
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, linux-media

Hi Yunke,

thanks for your patch

On Thu, 26 May 2022 at 07:08, Yunke Cao <yunkec@google.com> wrote:
>
> Implement support for ROI as described in UVC 1.5:
> 4.2.2.1.20 Digital Region of Interest (ROI) Control
>
> ROI control is implemented using V4L2 control API as
> two uvc-specific controls:
> V4L2_CID_UVC_REGION_OF_INTEREST_RECT and
> V4L2_CID_UVC_REGION_OF_INTEREST_AUTO.
>
> Signed-off-by: Yunke Cao <yunkec@google.com>
> ---
> References a rejected attempt that uses v4l2 selection API:
> https://lore.kernel.org/lkml/20210501082001.100533-2-senozhatsky@chromium.org
> Changelog since v4:
> -Check boundary condition: width or height == 0.
> -Populate xctrl->id and xctrl->size.
> -Split code for V4L2_CTRL_WHICH_MIN/MAX_VAL to patch 4/5.
>
>  drivers/media/usb/uvc/uvc_ctrl.c   | 207 ++++++++++++++++++++++++++---
>  drivers/media/usb/uvc/uvc_v4l2.c   |   8 +-
>  drivers/media/usb/uvc/uvcvideo.h   |  10 +-
>  include/uapi/linux/usb/video.h     |   1 +
>  include/uapi/linux/uvcvideo.h      |  13 ++
>  include/uapi/linux/v4l2-controls.h |   8 ++
>  6 files changed, 223 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index b4f6edf968bc..c470861e408a 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -355,6 +355,16 @@ static const struct uvc_control_info uvc_ctrls[] = {
>                 .flags          = UVC_CTRL_FLAG_GET_CUR
>                                 | UVC_CTRL_FLAG_AUTO_UPDATE,
>         },
> +       {
> +               .entity         = UVC_GUID_UVC_CAMERA,
> +               .selector       = UVC_CT_REGION_OF_INTEREST_CONTROL,
> +               .index          = 21,
> +               .size           = 10,
> +               .flags          = UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_CUR
> +                               | UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
> +                               | UVC_CTRL_FLAG_GET_DEF
> +                               | UVC_CTRL_FLAG_AUTO_UPDATE,
> +       },
>  };
>
>  static const u32 uvc_control_classes[] = {
> @@ -728,6 +738,24 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
>                 .v4l2_type      = V4L2_CTRL_TYPE_BOOLEAN,
>                 .data_type      = UVC_CTRL_DATA_TYPE_BOOLEAN,
>         },
> +       {
> +               .id             = V4L2_CID_UVC_REGION_OF_INTEREST_RECT,
> +               .entity         = UVC_GUID_UVC_CAMERA,
> +               .selector       = UVC_CT_REGION_OF_INTEREST_CONTROL,
> +               .size           = 64,
> +               .offset         = 0,
> +               .v4l2_type      = V4L2_CTRL_TYPE_RECT,
> +               .data_type      = UVC_CTRL_DATA_TYPE_RECT,
> +       },
> +       {
> +               .id             = V4L2_CID_UVC_REGION_OF_INTEREST_AUTO,
> +               .entity         = UVC_GUID_UVC_CAMERA,
> +               .selector       = UVC_CT_REGION_OF_INTEREST_CONTROL,
> +               .size           = 16,
> +               .offset         = 64,
> +               .v4l2_type      = V4L2_CTRL_TYPE_BITMASK,
> +               .data_type      = UVC_CTRL_DATA_TYPE_BITMASK,
> +       },
>  };
>
>  /* ------------------------------------------------------------------------
> @@ -749,6 +777,34 @@ static inline void uvc_clear_bit(u8 *data, int bit)
>         data[bit >> 3] &= ~(1 << (bit & 7));
>  }
>
> +static void uvc_to_v4l2_rect(struct v4l2_rect *v4l2_rect,
> +       const struct uvc_rect *uvc_rect)
> +{
> +       v4l2_rect->top = uvc_rect->top;
> +       v4l2_rect->left = uvc_rect->left;
> +       v4l2_rect->height = uvc_rect->bottom - uvc_rect->top + 1;
> +       v4l2_rect->width = uvc_rect->right - uvc_rect->left + 1;
> +}
> +
> +static int v4l2_to_uvc_rect(struct uvc_rect *uvc_rect,
> +       const struct v4l2_rect *v4l2_rect)
> +{
> +       // Safely converts s32 and u32 to u16.
> +       if (v4l2_rect->top > U16_MAX || v4l2_rect->top < 0 ||
> +           v4l2_rect->left > U16_MAX || v4l2_rect->left < 0 ||
> +           v4l2_rect->height > U16_MAX || v4l2_rect->height == 0 ||
> +           v4l2_rect->width > U16_MAX || v4l2_rect->width == 0 ||
> +           v4l2_rect->height + v4l2_rect->top - 1 > U16_MAX ||
> +           v4l2_rect->width + v4l2_rect->left - 1 > U16_MAX)
> +               return -ERANGE;
> +
> +       uvc_rect->top = v4l2_rect->top;
> +       uvc_rect->left = v4l2_rect->left;
> +       uvc_rect->bottom = v4l2_rect->height + v4l2_rect->top - 1;
> +       uvc_rect->right = v4l2_rect->width + v4l2_rect->left - 1;
> +       return 0;
> +}
> +
>  /* Extract the bit string specified by mapping->offset and mapping->size
>   * from the little-endian data stored at 'data' and return the result as
>   * a signed 32bit integer. Sign extension will be performed if the mapping
> @@ -963,11 +1019,23 @@ static s32 __uvc_ctrl_get_value(struct uvc_control_mapping *mapping,
>         return value;
>  }
>
> +static void __uvc_ctrl_get_v4l2_rect(struct uvc_control *ctrl,
> +                                    struct uvc_control_mapping *mapping,
> +                                    u32 id,
> +                                    struct v4l2_rect *rect)
> +{
> +       struct uvc_rect *uvc_rect =
> +               (struct uvc_rect *)(uvc_ctrl_data(ctrl, id)
> +                                       + mapping->offset / 8);
> +       uvc_to_v4l2_rect(rect, uvc_rect);
> +}
> +
>  static int __uvc_ctrl_get(struct uvc_video_chain *chain,
>         struct uvc_control *ctrl, struct uvc_control_mapping *mapping,
> -       s32 *value)
> +       struct v4l2_ext_control *xctrl)
>  {

We need to be a bit careful here. Now we need that xctrl->p_rect
points to a correct location and that is not true in other parts of
the code.

Maybe you want to want to make

__uvc_ctrl_get-> return -EINVAL for for V4L2_CTRL_TYPE_RECT

and a new

__uvc_ctrl_get_p_rect_to_user (and tha way me make explicit the namespace)

>         int ret;
> +       struct v4l2_rect v4l2_rect = {};
>
>         if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
>                 return -EACCES;
> @@ -993,8 +1061,19 @@ static int __uvc_ctrl_get(struct uvc_video_chain *chain,
>                 ctrl->loaded = 1;
>         }
>
> -       *value = __uvc_ctrl_get_value(mapping,
> +       xctrl->id = mapping->id;
> +       switch (mapping->v4l2_type) {
> +       case V4L2_CTRL_TYPE_RECT:
> +               __uvc_ctrl_get_v4l2_rect(ctrl, mapping, UVC_CTRL_DATA_CURRENT,
> +                                        &v4l2_rect);
> +               xctrl->size = sizeof(v4l2_rect);
> +               return copy_to_user(xctrl->p_rect, &v4l2_rect,
> +                               sizeof(v4l2_rect));
> +
> +       default:
> +               xctrl->value = __uvc_ctrl_get_value(mapping,
>                                 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
> +       }
>
>         return 0;
>  }
> @@ -1104,13 +1183,14 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
>                 __uvc_find_control(ctrl->entity, mapping->master_id,
>                                    &master_map, &master_ctrl, 0);
>         if (master_ctrl && (master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) {
> -               s32 val;
> -               int ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val);
> +               struct v4l2_ext_control xctrl;
> +               int ret =
> +                       __uvc_ctrl_get(chain, master_ctrl, master_map, &xctrl);
>                 if (ret < 0)
>                         return ret;
>
> -               if (val != mapping->master_manual)
> -                               v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
> +               if (xctrl.value != mapping->master_manual)
> +                       v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
>         }
>
>         if (!ctrl->cached) {
> @@ -1344,16 +1424,16 @@ static void uvc_ctrl_send_slave_event(struct uvc_video_chain *chain,
>         struct uvc_control_mapping *mapping = NULL;
>         struct uvc_control *ctrl = NULL;
>         u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
> -       s32 val = 0;
> +       struct v4l2_ext_control xctrl;
>
>         __uvc_find_control(master->entity, slave_id, &mapping, &ctrl, 0);
>         if (ctrl == NULL)
>                 return;
>
> -       if (__uvc_ctrl_get(chain, ctrl, mapping, &val) == 0)
> +       if (__uvc_ctrl_get(chain, ctrl, mapping, &xctrl) == 0)
>                 changes |= V4L2_EVENT_CTRL_CH_VALUE;
>
> -       uvc_ctrl_send_event(chain, handle, ctrl, mapping, val, changes);
> +       uvc_ctrl_send_event(chain, handle, ctrl, mapping, xctrl.value, changes);
>  }
>
>  void uvc_ctrl_status_event(struct uvc_video_chain *chain,
> @@ -1515,13 +1595,13 @@ static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned elems)
>         if (sev->flags & V4L2_EVENT_SUB_FL_SEND_INITIAL) {
>                 struct v4l2_event ev;
>                 u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
> -               s32 val = 0;
> +               struct v4l2_ext_control xctrl;
>
> -               if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
> +               if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &xctrl) == 0)
>                         changes |= V4L2_EVENT_CTRL_CH_VALUE;
>
> -               uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, val,
> -                                   changes);
> +               uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping,
> +                               xctrl.value, changes);
>                 /* Mark the queue as active, allowing this initial
>                    event to be accepted. */
>                 sev->elems = elems;
> @@ -1682,10 +1762,14 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
>  }
>

Please move this change to the other patch
>  int uvc_ctrl_get(struct uvc_video_chain *chain,
> -       struct v4l2_ext_control *xctrl)
> +       struct v4l2_ext_control *xctrl, u32 v4l2_which)
>  {
>         struct uvc_control *ctrl;
>         struct uvc_control_mapping *mapping;
> +       int ret;
> +       u32 flag;
> +       u32 id;
> +       u8 query;
>
>         if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
>                 return -EACCES;
> @@ -1694,7 +1778,36 @@ int uvc_ctrl_get(struct uvc_video_chain *chain,
>         if (ctrl == NULL)
>                 return -EINVAL;
>
> -       return __uvc_ctrl_get(chain, ctrl, mapping, &xctrl->value);
> +       switch (v4l2_which) {
> +       case V4L2_CTRL_WHICH_DEF_VAL:
> +               flag = UVC_CTRL_FLAG_GET_DEF;
> +               id = UVC_CTRL_DATA_DEF;
> +               query = UVC_GET_DEF;
> +               break;
> +       case V4L2_CTRL_WHICH_CUR_VAL:
> +       default:
> +               return __uvc_ctrl_get(chain, ctrl, mapping, xctrl);
> +       }
> +
> +       if (!ctrl->cached) {
> +               ret = uvc_ctrl_populate_cache(chain, ctrl);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       if (!(ctrl->info.flags & flag))
> +               return -EACCES;
> +
> +       if (mapping->v4l2_type == V4L2_CTRL_TYPE_RECT) {
> +               struct v4l2_rect rect;
> +
> +               __uvc_ctrl_get_v4l2_rect(ctrl, mapping, id, &rect);
> +               return copy_to_user(xctrl->p_rect, &rect, sizeof(rect));
> +       }
> +
> +       xctrl->value = mapping->get(mapping, query, uvc_ctrl_data(ctrl, id));
> +
> +       return 0;
>  }
>
>  int uvc_ctrl_set(struct uvc_fh *handle,
> @@ -1703,6 +1816,8 @@ int uvc_ctrl_set(struct uvc_fh *handle,
>         struct uvc_video_chain *chain = handle->chain;
>         struct uvc_control *ctrl;
>         struct uvc_control_mapping *mapping;
> +       struct v4l2_rect v4l2_rect;
> +       struct uvc_rect uvc_rect;
>         s32 value;
>         u32 step;
>         s32 min;
> @@ -1774,6 +1889,16 @@ int uvc_ctrl_set(struct uvc_fh *handle,
>
>                 break;
>
> +       case V4L2_CTRL_TYPE_RECT:
> +               ret = copy_from_user(&v4l2_rect, xctrl->p_rect,
> +                               sizeof(v4l2_rect));
> +               if (ret < 0)
> +                       return ret;
> +               ret = v4l2_to_uvc_rect(&uvc_rect, &v4l2_rect);
> +               if (ret < 0)
> +                       return ret;
> +               break;
> +
>         default:
>                 value = xctrl->value;
>                 break;
> @@ -1807,8 +1932,16 @@ int uvc_ctrl_set(struct uvc_fh *handle,
>                        ctrl->info.size);
>         }
>
> -       mapping->set(mapping, value,
> -               uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
> +       switch (mapping->data_type) {
> +       case UVC_CTRL_DATA_TYPE_RECT:
> +               memcpy(uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT)
> +                       + mapping->offset / 8,
> +                       &uvc_rect, sizeof(uvc_rect));
> +               break;
> +       default:
> +               mapping->set(mapping, value,
> +                       uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
> +       }
>
>         if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
>                 ctrl->handle = handle;
> @@ -2366,6 +2499,40 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev,
>         }
>  }
>
> +static int uvc_ctrl_init_roi(struct uvc_device *dev, struct uvc_control *ctrl)
> +{
> +       int ret;
> +
> +       ret = uvc_query_ctrl(dev, UVC_GET_DEF, ctrl->entity->id, dev->intfnum,
> +                            UVC_CT_REGION_OF_INTEREST_CONTROL,
> +                            uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
> +                            ctrl->info.size);
> +       if (ret)
> +               goto out;
> +
> +       /*
> +        * Most firmwares have wrong GET_CURRENT configuration. E.g. it's
> +        * below GET_MIN, or have rectangle coordinates mixed up. This
> +        * causes problems sometimes, because we are unable to set
> +        * auto-controls value without first setting ROI rectangle to
> +        * valid configuration.
> +        *
> +        * We expect that default configuration is always correct and
> +        * is within the GET_MIN / GET_MAX boundaries.
> +        *
> +        * Set current ROI configuration to GET_DEF, so that we will
> +        * always have properly configured ROI.
> +        */
> +       ret = uvc_query_ctrl(dev, UVC_SET_CUR, 1, dev->intfnum,
> +                            UVC_CT_REGION_OF_INTEREST_CONTROL,
> +                            uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
> +                            ctrl->info.size);
> +out:
> +       if (ret)
> +               dev_err(&dev->udev->dev, "Failed to fixup ROI (%d).\n", ret);
> +       return ret;
> +}
> +
>  /*
>   * Add control information and hardcoded stock control mappings to the given
>   * device.
> @@ -2378,6 +2545,7 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>         const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
>         const struct uvc_control_mapping *mend =
>                 mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> +       const u8 camera_entity[16] = UVC_GUID_UVC_CAMERA;
>
>         /* XU controls initialization requires querying the device for control
>          * information. As some buggy UVC devices will crash when queried
> @@ -2398,6 +2566,11 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>                          * GET_INFO on standard controls.
>                          */
>                         uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
> +
> +                       if (ctrl->info.selector ==
> +                               UVC_CT_REGION_OF_INTEREST_CONTROL &&
> +                           uvc_entity_match_guid(ctrl->entity, camera_entity))
> +                               uvc_ctrl_init_roi(chain->dev, ctrl);
>                         break;
>                  }
>         }
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 711556d13d03..040511da1005 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -1040,15 +1040,11 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
>
>         if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
>                 for (i = 0; i < ctrls->count; ++ctrl, ++i) {
> -                       struct v4l2_queryctrl qc = { .id = ctrl->id };
> -
> -                       ret = uvc_query_v4l2_ctrl(chain, &qc);
> +                       ret = uvc_ctrl_get(chain, ctrl, ctrls->which);
>                         if (ret < 0) {
>                                 ctrls->error_idx = i;
>                                 return ret;
>                         }
> -
> -                       ctrl->value = qc.default_value;
>                 }
>
>                 return 0;
> @@ -1059,7 +1055,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
>                 return ret;
>
>         for (i = 0; i < ctrls->count; ++ctrl, ++i) {
> -               ret = uvc_ctrl_get(chain, ctrl);
> +               ret = uvc_ctrl_get(chain, ctrl, ctrls->which);
>                 if (ret < 0) {
>                         uvc_ctrl_rollback(handle);
>                         ctrls->error_idx = i;
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 143230b3275b..f414ad7d57b2 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -424,6 +424,13 @@ struct uvc_streaming_header {
>         u8 bTriggerUsage;
>  };
>
> +struct uvc_rect {
> +       u16 top;
> +       u16 left;
> +       u16 bottom;
> +       u16 right;
> +} __packed;
> +
>  enum uvc_buffer_state {
>         UVC_BUF_STATE_IDLE      = 0,
>         UVC_BUF_STATE_QUEUED    = 1,
> @@ -897,7 +904,8 @@ static inline int uvc_ctrl_rollback(struct uvc_fh *handle)
>         return __uvc_ctrl_commit(handle, 1, NULL);
>  }
>
> -int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl);
> +int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl,
> +                u32 v4l2_which);
>  int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl);
>  int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
>                            bool read);
> diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
> index bfdae12cdacf..9076a444758a 100644
> --- a/include/uapi/linux/usb/video.h
> +++ b/include/uapi/linux/usb/video.h
> @@ -104,6 +104,7 @@
>  #define UVC_CT_ROLL_ABSOLUTE_CONTROL                   0x0f
>  #define UVC_CT_ROLL_RELATIVE_CONTROL                   0x10
>  #define UVC_CT_PRIVACY_CONTROL                         0x11
> +#define UVC_CT_REGION_OF_INTEREST_CONTROL              0x14
>
>  /* A.9.5. Processing Unit Control Selectors */
>  #define UVC_PU_CONTROL_UNDEFINED                       0x00
> diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h
> index 8288137387c0..b16e5d373f3f 100644
> --- a/include/uapi/linux/uvcvideo.h
> +++ b/include/uapi/linux/uvcvideo.h
> @@ -16,6 +16,7 @@
>  #define UVC_CTRL_DATA_TYPE_BOOLEAN     3
>  #define UVC_CTRL_DATA_TYPE_ENUM                4
>  #define UVC_CTRL_DATA_TYPE_BITMASK     5
> +#define UVC_CTRL_DATA_TYPE_RECT                6
>
>  /* Control flags */
>  #define UVC_CTRL_FLAG_SET_CUR          (1 << 0)
> @@ -36,6 +37,18 @@
>          UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES | \
>          UVC_CTRL_FLAG_GET_DEF)
>
> +/* V4L2 private controls */
> +#define V4L2_CID_UVC_REGION_OF_INTEREST_RECT   (V4L2_CID_CAMERA_UVC_BASE+1)
> +#define V4L2_CID_UVC_REGION_OF_INTEREST_AUTO   (V4L2_CID_CAMERA_UVC_BASE+2)
> +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_EXPOSURE              (1 << 0)
> +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_IRIS                  (1 << 1)
> +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_WHITE_BALANCE         (1 << 2)
> +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_FOCUS                 (1 << 3)
> +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_FACE_DETECT           (1 << 4)
> +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK      (1 << 5)
> +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION   (1 << 6)
> +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY                (1 << 7)
> +
>  struct uvc_menu_info {
>         __u32 value;
>         __u8 name[32];
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index bb40129446d4..48d12782e7e4 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1008,6 +1008,14 @@ enum v4l2_auto_focus_range {
>
>  #define V4L2_CID_CAMERA_SENSOR_ROTATION                (V4L2_CID_CAMERA_CLASS_BASE+35)
>
> +/* CAMERA-class private control IDs */
> +
> +/*
> + * The base for the uvc driver controls. See linux/uvcvideo.h for the list
> + * of controls. We reserve 16 controls for this driver.
> + */
> +#define V4L2_CID_CAMERA_UVC_BASE               (V4L2_CID_CAMERA_CLASS_BASE + 0x1000)
> +
>  /* FM Modulator class control IDs */
>
>  #define V4L2_CID_FM_TX_CLASS_BASE              (V4L2_CTRL_CLASS_FM_TX | 0x900)
> --
> 2.36.1.124.g0e6072fb45-goog
>


-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/5] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT
  2022-05-26  5:07 ` [PATCH v5 1/5] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
  2022-05-26 14:15   ` Ricardo Ribalda
@ 2022-05-30  1:35   ` Sergey Senozhatsky
  1 sibling, 0 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2022-05-30  1:35 UTC (permalink / raw)
  To: Yunke Cao
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, Ricardo Ribalda, linux-media

On (22/05/26 14:07), Yunke Cao wrote:
> @@ -888,6 +899,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>  			return -EINVAL;
>  		break;
>  
> +	case V4L2_CTRL_TYPE_RECT:
> +		rect = p;
> +		if (!rect->width || !rect->height)
> +			return -EINVAL;
> +		break;

Should we allow (0,0,0,0) rectangles or not? From UVC point of view,
I assume that anything that is within GET_MIN/GET_MAX is OK. Is GET_MIN
always guaranteed to be at least (0,0,1,1) or do some firmwares permit
and use (0,0,0,0)? As a "disable ROI" type of thing, for instance.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 3/5] media: uvcvideo: implement UVC v1.5 ROI
  2022-05-26  5:07 ` [PATCH v5 3/5] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
  2022-05-26 14:35   ` Ricardo Ribalda
@ 2022-05-30  1:59   ` Sergey Senozhatsky
  1 sibling, 0 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2022-05-30  1:59 UTC (permalink / raw)
  To: Yunke Cao
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, Ricardo Ribalda, linux-media

On (22/05/26 14:07), Yunke Cao wrote:
> 
> Implement support for ROI as described in UVC 1.5:
> 4.2.2.1.20 Digital Region of Interest (ROI) Control
> 
> ROI control is implemented using V4L2 control API as
> two uvc-specific controls:
> V4L2_CID_UVC_REGION_OF_INTEREST_RECT and
> V4L2_CID_UVC_REGION_OF_INTEREST_AUTO.

Just for information, several nitpicks from checkpatch


CHECK: Alignment should match open parenthesis
#75: FILE: drivers/media/usb/uvc/uvc_ctrl.c:781:
+static void uvc_to_v4l2_rect(struct v4l2_rect *v4l2_rect,
+	const struct uvc_rect *uvc_rect)

CHECK: Alignment should match open parenthesis
#84: FILE: drivers/media/usb/uvc/uvc_ctrl.c:790:
+static int v4l2_to_uvc_rect(struct uvc_rect *uvc_rect,
+	const struct v4l2_rect *v4l2_rect)

CHECK: Alignment should match open parenthesis
#146: FILE: drivers/media/usb/uvc/uvc_ctrl.c:1075:
+		xctrl->value = __uvc_ctrl_get_value(mapping,
 				uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));

CHECK: Alignment should match open parenthesis
#204: FILE: drivers/media/usb/uvc/uvc_ctrl.c:1604:
+		uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping,
+				xctrl.value, changes);

CHECK: Alignment should match open parenthesis
#277: FILE: drivers/media/usb/uvc/uvc_ctrl.c:1894:
+		ret = copy_from_user(&v4l2_rect, xctrl->p_rect,
+				sizeof(v4l2_rect));

CHECK: spaces preferred around that '+' (ctx:VxV)
#455: FILE: include/uapi/linux/uvcvideo.h:41:
+#define V4L2_CID_UVC_REGION_OF_INTEREST_RECT	(V4L2_CID_CAMERA_UVC_BASE+1)
                                             	                         ^

CHECK: spaces preferred around that '+' (ctx:VxV)
#456: FILE: include/uapi/linux/uvcvideo.h:42:
+#define V4L2_CID_UVC_REGION_OF_INTEREST_AUTO	(V4L2_CID_CAMERA_UVC_BASE+2)
                                             	                         ^


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 3/5] media: uvcvideo: implement UVC v1.5 ROI
  2022-05-26 14:35   ` Ricardo Ribalda
@ 2022-05-30  5:46     ` Yunke Cao
  0 siblings, 0 replies; 11+ messages in thread
From: Yunke Cao @ 2022-05-30  5:46 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, linux-media

Thanks for the review.

On Thu, May 26, 2022 at 11:36 PM Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Hi Yunke,
>
> thanks for your patch
>
> On Thu, 26 May 2022 at 07:08, Yunke Cao <yunkec@google.com> wrote:
> >
> > Implement support for ROI as described in UVC 1.5:
> > 4.2.2.1.20 Digital Region of Interest (ROI) Control
> >
> > ROI control is implemented using V4L2 control API as
> > two uvc-specific controls:
> > V4L2_CID_UVC_REGION_OF_INTEREST_RECT and
> > V4L2_CID_UVC_REGION_OF_INTEREST_AUTO.
> >
> > Signed-off-by: Yunke Cao <yunkec@google.com>
> > ---
> > References a rejected attempt that uses v4l2 selection API:
> > https://lore.kernel.org/lkml/20210501082001.100533-2-senozhatsky@chromium.org
> > Changelog since v4:
> > -Check boundary condition: width or height == 0.
> > -Populate xctrl->id and xctrl->size.
> > -Split code for V4L2_CTRL_WHICH_MIN/MAX_VAL to patch 4/5.
> >
> >  drivers/media/usb/uvc/uvc_ctrl.c   | 207 ++++++++++++++++++++++++++---
> >  drivers/media/usb/uvc/uvc_v4l2.c   |   8 +-
> >  drivers/media/usb/uvc/uvcvideo.h   |  10 +-
> >  include/uapi/linux/usb/video.h     |   1 +
> >  include/uapi/linux/uvcvideo.h      |  13 ++
> >  include/uapi/linux/v4l2-controls.h |   8 ++
> >  6 files changed, 223 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index b4f6edf968bc..c470861e408a 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -355,6 +355,16 @@ static const struct uvc_control_info uvc_ctrls[] = {
> >                 .flags          = UVC_CTRL_FLAG_GET_CUR
> >                                 | UVC_CTRL_FLAG_AUTO_UPDATE,
> >         },
> > +       {
> > +               .entity         = UVC_GUID_UVC_CAMERA,
> > +               .selector       = UVC_CT_REGION_OF_INTEREST_CONTROL,
> > +               .index          = 21,
> > +               .size           = 10,
> > +               .flags          = UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_CUR
> > +                               | UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
> > +                               | UVC_CTRL_FLAG_GET_DEF
> > +                               | UVC_CTRL_FLAG_AUTO_UPDATE,
> > +       },
> >  };
> >
> >  static const u32 uvc_control_classes[] = {
> > @@ -728,6 +738,24 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> >                 .v4l2_type      = V4L2_CTRL_TYPE_BOOLEAN,
> >                 .data_type      = UVC_CTRL_DATA_TYPE_BOOLEAN,
> >         },
> > +       {
> > +               .id             = V4L2_CID_UVC_REGION_OF_INTEREST_RECT,
> > +               .entity         = UVC_GUID_UVC_CAMERA,
> > +               .selector       = UVC_CT_REGION_OF_INTEREST_CONTROL,
> > +               .size           = 64,
> > +               .offset         = 0,
> > +               .v4l2_type      = V4L2_CTRL_TYPE_RECT,
> > +               .data_type      = UVC_CTRL_DATA_TYPE_RECT,
> > +       },
> > +       {
> > +               .id             = V4L2_CID_UVC_REGION_OF_INTEREST_AUTO,
> > +               .entity         = UVC_GUID_UVC_CAMERA,
> > +               .selector       = UVC_CT_REGION_OF_INTEREST_CONTROL,
> > +               .size           = 16,
> > +               .offset         = 64,
> > +               .v4l2_type      = V4L2_CTRL_TYPE_BITMASK,
> > +               .data_type      = UVC_CTRL_DATA_TYPE_BITMASK,
> > +       },
> >  };
> >
> >  /* ------------------------------------------------------------------------
> > @@ -749,6 +777,34 @@ static inline void uvc_clear_bit(u8 *data, int bit)
> >         data[bit >> 3] &= ~(1 << (bit & 7));
> >  }
> >
> > +static void uvc_to_v4l2_rect(struct v4l2_rect *v4l2_rect,
> > +       const struct uvc_rect *uvc_rect)
> > +{
> > +       v4l2_rect->top = uvc_rect->top;
> > +       v4l2_rect->left = uvc_rect->left;
> > +       v4l2_rect->height = uvc_rect->bottom - uvc_rect->top + 1;
> > +       v4l2_rect->width = uvc_rect->right - uvc_rect->left + 1;
> > +}
> > +
> > +static int v4l2_to_uvc_rect(struct uvc_rect *uvc_rect,
> > +       const struct v4l2_rect *v4l2_rect)
> > +{
> > +       // Safely converts s32 and u32 to u16.
> > +       if (v4l2_rect->top > U16_MAX || v4l2_rect->top < 0 ||
> > +           v4l2_rect->left > U16_MAX || v4l2_rect->left < 0 ||
> > +           v4l2_rect->height > U16_MAX || v4l2_rect->height == 0 ||
> > +           v4l2_rect->width > U16_MAX || v4l2_rect->width == 0 ||
> > +           v4l2_rect->height + v4l2_rect->top - 1 > U16_MAX ||
> > +           v4l2_rect->width + v4l2_rect->left - 1 > U16_MAX)
> > +               return -ERANGE;
> > +
> > +       uvc_rect->top = v4l2_rect->top;
> > +       uvc_rect->left = v4l2_rect->left;
> > +       uvc_rect->bottom = v4l2_rect->height + v4l2_rect->top - 1;
> > +       uvc_rect->right = v4l2_rect->width + v4l2_rect->left - 1;
> > +       return 0;
> > +}
> > +
> >  /* Extract the bit string specified by mapping->offset and mapping->size
> >   * from the little-endian data stored at 'data' and return the result as
> >   * a signed 32bit integer. Sign extension will be performed if the mapping
> > @@ -963,11 +1019,23 @@ static s32 __uvc_ctrl_get_value(struct uvc_control_mapping *mapping,
> >         return value;
> >  }
> >
> > +static void __uvc_ctrl_get_v4l2_rect(struct uvc_control *ctrl,
> > +                                    struct uvc_control_mapping *mapping,
> > +                                    u32 id,
> > +                                    struct v4l2_rect *rect)
> > +{
> > +       struct uvc_rect *uvc_rect =
> > +               (struct uvc_rect *)(uvc_ctrl_data(ctrl, id)
> > +                                       + mapping->offset / 8);
> > +       uvc_to_v4l2_rect(rect, uvc_rect);
> > +}
> > +
> >  static int __uvc_ctrl_get(struct uvc_video_chain *chain,
> >         struct uvc_control *ctrl, struct uvc_control_mapping *mapping,
> > -       s32 *value)
> > +       struct v4l2_ext_control *xctrl)
> >  {
>
> We need to be a bit careful here. Now we need that xctrl->p_rect
> points to a correct location and that is not true in other parts of
> the code.
>
> Maybe you want to want to make
>
> __uvc_ctrl_get-> return -EINVAL for for V4L2_CTRL_TYPE_RECT
>
> and a new
>
> __uvc_ctrl_get_p_rect_to_user (and tha way me make explicit the namespace)
>
Ok. Will do that in V6.
> >         int ret;
> > +       struct v4l2_rect v4l2_rect = {};
> >
> >         if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
> >                 return -EACCES;
> > @@ -993,8 +1061,19 @@ static int __uvc_ctrl_get(struct uvc_video_chain *chain,
> >                 ctrl->loaded = 1;
> >         }
> >
> > -       *value = __uvc_ctrl_get_value(mapping,
> > +       xctrl->id = mapping->id;
> > +       switch (mapping->v4l2_type) {
> > +       case V4L2_CTRL_TYPE_RECT:
> > +               __uvc_ctrl_get_v4l2_rect(ctrl, mapping, UVC_CTRL_DATA_CURRENT,
> > +                                        &v4l2_rect);
> > +               xctrl->size = sizeof(v4l2_rect);
> > +               return copy_to_user(xctrl->p_rect, &v4l2_rect,
> > +                               sizeof(v4l2_rect));
> > +
> > +       default:
> > +               xctrl->value = __uvc_ctrl_get_value(mapping,
> >                                 uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
> > +       }
> >
> >         return 0;
> >  }
> > @@ -1104,13 +1183,14 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> >                 __uvc_find_control(ctrl->entity, mapping->master_id,
> >                                    &master_map, &master_ctrl, 0);
> >         if (master_ctrl && (master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) {
> > -               s32 val;
> > -               int ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val);
> > +               struct v4l2_ext_control xctrl;
> > +               int ret =
> > +                       __uvc_ctrl_get(chain, master_ctrl, master_map, &xctrl);
> >                 if (ret < 0)
> >                         return ret;
> >
> > -               if (val != mapping->master_manual)
> > -                               v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
> > +               if (xctrl.value != mapping->master_manual)
> > +                       v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
> >         }
> >
> >         if (!ctrl->cached) {
> > @@ -1344,16 +1424,16 @@ static void uvc_ctrl_send_slave_event(struct uvc_video_chain *chain,
> >         struct uvc_control_mapping *mapping = NULL;
> >         struct uvc_control *ctrl = NULL;
> >         u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
> > -       s32 val = 0;
> > +       struct v4l2_ext_control xctrl;
> >
> >         __uvc_find_control(master->entity, slave_id, &mapping, &ctrl, 0);
> >         if (ctrl == NULL)
> >                 return;
> >
> > -       if (__uvc_ctrl_get(chain, ctrl, mapping, &val) == 0)
> > +       if (__uvc_ctrl_get(chain, ctrl, mapping, &xctrl) == 0)
> >                 changes |= V4L2_EVENT_CTRL_CH_VALUE;
> >
> > -       uvc_ctrl_send_event(chain, handle, ctrl, mapping, val, changes);
> > +       uvc_ctrl_send_event(chain, handle, ctrl, mapping, xctrl.value, changes);
> >  }
> >
> >  void uvc_ctrl_status_event(struct uvc_video_chain *chain,
> > @@ -1515,13 +1595,13 @@ static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned elems)
> >         if (sev->flags & V4L2_EVENT_SUB_FL_SEND_INITIAL) {
> >                 struct v4l2_event ev;
> >                 u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
> > -               s32 val = 0;
> > +               struct v4l2_ext_control xctrl;
> >
> > -               if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
> > +               if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &xctrl) == 0)
> >                         changes |= V4L2_EVENT_CTRL_CH_VALUE;
> >
> > -               uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, val,
> > -                                   changes);
> > +               uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping,
> > +                               xctrl.value, changes);
> >                 /* Mark the queue as active, allowing this initial
> >                    event to be accepted. */
> >                 sev->elems = elems;
> > @@ -1682,10 +1762,14 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
> >  }
> >
>
> Please move this change to the other patch
I'm not sure we can do that.
Before this patch, uvc_ctrl_get only handled CUR, DEF was handled by
uvc_query_v4l2_ctrl.
Now I changed to uvc_ctrl_get to handle CUR and DEF (+ MIN and MAX in 4/5).
This way it's easier because we can't pass ptr controls easily in a
v4l2_queryctrl.
We add v4l2_which as a parameter here to support V4L2_CTRL_WHICH_DEF_VAL.

> >  int uvc_ctrl_get(struct uvc_video_chain *chain,
> > -       struct v4l2_ext_control *xctrl)
> > +       struct v4l2_ext_control *xctrl, u32 v4l2_which)
> >  {
> >         struct uvc_control *ctrl;
> >         struct uvc_control_mapping *mapping;
> > +       int ret;
> > +       u32 flag;
> > +       u32 id;
> > +       u8 query;
> >
> >         if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
> >                 return -EACCES;
> > @@ -1694,7 +1778,36 @@ int uvc_ctrl_get(struct uvc_video_chain *chain,
> >         if (ctrl == NULL)
> >                 return -EINVAL;
> >
> > -       return __uvc_ctrl_get(chain, ctrl, mapping, &xctrl->value);
> > +       switch (v4l2_which) {
> > +       case V4L2_CTRL_WHICH_DEF_VAL:
> > +               flag = UVC_CTRL_FLAG_GET_DEF;
> > +               id = UVC_CTRL_DATA_DEF;
> > +               query = UVC_GET_DEF;
> > +               break;
> > +       case V4L2_CTRL_WHICH_CUR_VAL:
> > +       default:
> > +               return __uvc_ctrl_get(chain, ctrl, mapping, xctrl);
> > +       }
> > +
> > +       if (!ctrl->cached) {
> > +               ret = uvc_ctrl_populate_cache(chain, ctrl);
> > +               if (ret < 0)
> > +                       return ret;
> > +       }
> > +
> > +       if (!(ctrl->info.flags & flag))
> > +               return -EACCES;
> > +
> > +       if (mapping->v4l2_type == V4L2_CTRL_TYPE_RECT) {
> > +               struct v4l2_rect rect;
> > +
> > +               __uvc_ctrl_get_v4l2_rect(ctrl, mapping, id, &rect);
> > +               return copy_to_user(xctrl->p_rect, &rect, sizeof(rect));
> > +       }
> > +
> > +       xctrl->value = mapping->get(mapping, query, uvc_ctrl_data(ctrl, id));
> > +
> > +       return 0;
> >  }
> >
> >  int uvc_ctrl_set(struct uvc_fh *handle,
> > @@ -1703,6 +1816,8 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> >         struct uvc_video_chain *chain = handle->chain;
> >         struct uvc_control *ctrl;
> >         struct uvc_control_mapping *mapping;
> > +       struct v4l2_rect v4l2_rect;
> > +       struct uvc_rect uvc_rect;
> >         s32 value;
> >         u32 step;
> >         s32 min;
> > @@ -1774,6 +1889,16 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> >
> >                 break;
> >
> > +       case V4L2_CTRL_TYPE_RECT:
> > +               ret = copy_from_user(&v4l2_rect, xctrl->p_rect,
> > +                               sizeof(v4l2_rect));
> > +               if (ret < 0)
> > +                       return ret;
> > +               ret = v4l2_to_uvc_rect(&uvc_rect, &v4l2_rect);
> > +               if (ret < 0)
> > +                       return ret;
> > +               break;
> > +
> >         default:
> >                 value = xctrl->value;
> >                 break;
> > @@ -1807,8 +1932,16 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> >                        ctrl->info.size);
> >         }
> >
> > -       mapping->set(mapping, value,
> > -               uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
> > +       switch (mapping->data_type) {
> > +       case UVC_CTRL_DATA_TYPE_RECT:
> > +               memcpy(uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT)
> > +                       + mapping->offset / 8,
> > +                       &uvc_rect, sizeof(uvc_rect));
> > +               break;
> > +       default:
> > +               mapping->set(mapping, value,
> > +                       uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
> > +       }
> >
> >         if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
> >                 ctrl->handle = handle;
> > @@ -2366,6 +2499,40 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev,
> >         }
> >  }
> >
> > +static int uvc_ctrl_init_roi(struct uvc_device *dev, struct uvc_control *ctrl)
> > +{
> > +       int ret;
> > +
> > +       ret = uvc_query_ctrl(dev, UVC_GET_DEF, ctrl->entity->id, dev->intfnum,
> > +                            UVC_CT_REGION_OF_INTEREST_CONTROL,
> > +                            uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
> > +                            ctrl->info.size);
> > +       if (ret)
> > +               goto out;
> > +
> > +       /*
> > +        * Most firmwares have wrong GET_CURRENT configuration. E.g. it's
> > +        * below GET_MIN, or have rectangle coordinates mixed up. This
> > +        * causes problems sometimes, because we are unable to set
> > +        * auto-controls value without first setting ROI rectangle to
> > +        * valid configuration.
> > +        *
> > +        * We expect that default configuration is always correct and
> > +        * is within the GET_MIN / GET_MAX boundaries.
> > +        *
> > +        * Set current ROI configuration to GET_DEF, so that we will
> > +        * always have properly configured ROI.
> > +        */
> > +       ret = uvc_query_ctrl(dev, UVC_SET_CUR, 1, dev->intfnum,
> > +                            UVC_CT_REGION_OF_INTEREST_CONTROL,
> > +                            uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
> > +                            ctrl->info.size);
> > +out:
> > +       if (ret)
> > +               dev_err(&dev->udev->dev, "Failed to fixup ROI (%d).\n", ret);
> > +       return ret;
> > +}
> > +
> >  /*
> >   * Add control information and hardcoded stock control mappings to the given
> >   * device.
> > @@ -2378,6 +2545,7 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> >         const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
> >         const struct uvc_control_mapping *mend =
> >                 mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> > +       const u8 camera_entity[16] = UVC_GUID_UVC_CAMERA;
> >
> >         /* XU controls initialization requires querying the device for control
> >          * information. As some buggy UVC devices will crash when queried
> > @@ -2398,6 +2566,11 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> >                          * GET_INFO on standard controls.
> >                          */
> >                         uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
> > +
> > +                       if (ctrl->info.selector ==
> > +                               UVC_CT_REGION_OF_INTEREST_CONTROL &&
> > +                           uvc_entity_match_guid(ctrl->entity, camera_entity))
> > +                               uvc_ctrl_init_roi(chain->dev, ctrl);
> >                         break;
> >                  }
> >         }
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> > index 711556d13d03..040511da1005 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -1040,15 +1040,11 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
> >
> >         if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
> >                 for (i = 0; i < ctrls->count; ++ctrl, ++i) {
> > -                       struct v4l2_queryctrl qc = { .id = ctrl->id };
> > -
> > -                       ret = uvc_query_v4l2_ctrl(chain, &qc);
> > +                       ret = uvc_ctrl_get(chain, ctrl, ctrls->which);
> >                         if (ret < 0) {
> >                                 ctrls->error_idx = i;
> >                                 return ret;
> >                         }
> > -
> > -                       ctrl->value = qc.default_value;
> >                 }
> >
> >                 return 0;
> > @@ -1059,7 +1055,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
> >                 return ret;
> >
> >         for (i = 0; i < ctrls->count; ++ctrl, ++i) {
> > -               ret = uvc_ctrl_get(chain, ctrl);
> > +               ret = uvc_ctrl_get(chain, ctrl, ctrls->which);
> >                 if (ret < 0) {
> >                         uvc_ctrl_rollback(handle);
> >                         ctrls->error_idx = i;
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index 143230b3275b..f414ad7d57b2 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -424,6 +424,13 @@ struct uvc_streaming_header {
> >         u8 bTriggerUsage;
> >  };
> >
> > +struct uvc_rect {
> > +       u16 top;
> > +       u16 left;
> > +       u16 bottom;
> > +       u16 right;
> > +} __packed;
> > +
> >  enum uvc_buffer_state {
> >         UVC_BUF_STATE_IDLE      = 0,
> >         UVC_BUF_STATE_QUEUED    = 1,
> > @@ -897,7 +904,8 @@ static inline int uvc_ctrl_rollback(struct uvc_fh *handle)
> >         return __uvc_ctrl_commit(handle, 1, NULL);
> >  }
> >
> > -int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl);
> > +int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl,
> > +                u32 v4l2_which);
> >  int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl);
> >  int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
> >                            bool read);
> > diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
> > index bfdae12cdacf..9076a444758a 100644
> > --- a/include/uapi/linux/usb/video.h
> > +++ b/include/uapi/linux/usb/video.h
> > @@ -104,6 +104,7 @@
> >  #define UVC_CT_ROLL_ABSOLUTE_CONTROL                   0x0f
> >  #define UVC_CT_ROLL_RELATIVE_CONTROL                   0x10
> >  #define UVC_CT_PRIVACY_CONTROL                         0x11
> > +#define UVC_CT_REGION_OF_INTEREST_CONTROL              0x14
> >
> >  /* A.9.5. Processing Unit Control Selectors */
> >  #define UVC_PU_CONTROL_UNDEFINED                       0x00
> > diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h
> > index 8288137387c0..b16e5d373f3f 100644
> > --- a/include/uapi/linux/uvcvideo.h
> > +++ b/include/uapi/linux/uvcvideo.h
> > @@ -16,6 +16,7 @@
> >  #define UVC_CTRL_DATA_TYPE_BOOLEAN     3
> >  #define UVC_CTRL_DATA_TYPE_ENUM                4
> >  #define UVC_CTRL_DATA_TYPE_BITMASK     5
> > +#define UVC_CTRL_DATA_TYPE_RECT                6
> >
> >  /* Control flags */
> >  #define UVC_CTRL_FLAG_SET_CUR          (1 << 0)
> > @@ -36,6 +37,18 @@
> >          UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES | \
> >          UVC_CTRL_FLAG_GET_DEF)
> >
> > +/* V4L2 private controls */
> > +#define V4L2_CID_UVC_REGION_OF_INTEREST_RECT   (V4L2_CID_CAMERA_UVC_BASE+1)
> > +#define V4L2_CID_UVC_REGION_OF_INTEREST_AUTO   (V4L2_CID_CAMERA_UVC_BASE+2)
> > +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_EXPOSURE              (1 << 0)
> > +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_IRIS                  (1 << 1)
> > +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_WHITE_BALANCE         (1 << 2)
> > +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_FOCUS                 (1 << 3)
> > +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_FACE_DETECT           (1 << 4)
> > +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK      (1 << 5)
> > +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION   (1 << 6)
> > +#define V4L2_UVC_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY                (1 << 7)
> > +
> >  struct uvc_menu_info {
> >         __u32 value;
> >         __u8 name[32];
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index bb40129446d4..48d12782e7e4 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -1008,6 +1008,14 @@ enum v4l2_auto_focus_range {
> >
> >  #define V4L2_CID_CAMERA_SENSOR_ROTATION                (V4L2_CID_CAMERA_CLASS_BASE+35)
> >
> > +/* CAMERA-class private control IDs */
> > +
> > +/*
> > + * The base for the uvc driver controls. See linux/uvcvideo.h for the list
> > + * of controls. We reserve 16 controls for this driver.
> > + */
> > +#define V4L2_CID_CAMERA_UVC_BASE               (V4L2_CID_CAMERA_CLASS_BASE + 0x1000)
> > +
> >  /* FM Modulator class control IDs */
> >
> >  #define V4L2_CID_FM_TX_CLASS_BASE              (V4L2_CTRL_CLASS_FM_TX | 0x900)
> > --
> > 2.36.1.124.g0e6072fb45-goog
> >
>
>
> --
> Ricardo Ribalda

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-05-30  5:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-26  5:07 [PATCH v5 0/5] media: Implement UVC v1.5 ROI Yunke Cao
2022-05-26  5:07 ` [PATCH v5 1/5] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
2022-05-26 14:15   ` Ricardo Ribalda
2022-05-30  1:35   ` Sergey Senozhatsky
2022-05-26  5:07 ` [PATCH v5 2/5] v4l2-ctrls: add support for V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
2022-05-26  5:07 ` [PATCH v5 3/5] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
2022-05-26 14:35   ` Ricardo Ribalda
2022-05-30  5:46     ` Yunke Cao
2022-05-30  1:59   ` Sergey Senozhatsky
2022-05-26  5:07 ` [PATCH v5 4/5] media: uvcvideo: support V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
2022-05-26  5:07 ` [PATCH v5 5/5] media: uvcvideo: document UVC v1.5 ROI Yunke Cao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.