All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] media: Implement UVC v1.5 ROI
@ 2022-05-24  4:45 Yunke Cao
  2022-05-24  4:45 ` [PATCH v4 1/4] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Yunke Cao @ 2022-05-24  4:45 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 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.

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

 .../userspace-api/media/drivers/uvcvideo.rst  |  60 +++++
 .../media/v4l/vidioc-g-ext-ctrls.rst          |  12 +-
 .../media/videodev2.h.rst.exceptions          |   3 +
 drivers/media/i2c/imx214.c                    |   4 +-
 drivers/media/usb/uvc/uvc_ctrl.c              | 214 ++++++++++++++++--
 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                    |  32 ++-
 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 +
 15 files changed, 531 insertions(+), 52 deletions(-)

-- 
2.36.1.124.g0e6072fb45-goog


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

* [PATCH v4 1/4] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT
  2022-05-24  4:45 [PATCH v4 0/4] media: Implement UVC v1.5 ROI Yunke Cao
@ 2022-05-24  4:45 ` Yunke Cao
  2022-05-24  9:31   ` Ricardo Ribalda
  2022-05-24  4:45 ` [PATCH v4 2/4] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Yunke Cao @ 2022-05-24  4:45 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>
---
 .../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..f4e205ead0a2 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_area``
+      - 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] 13+ messages in thread

* [PATCH v4 2/4] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL
  2022-05-24  4:45 [PATCH v4 0/4] media: Implement UVC v1.5 ROI Yunke Cao
  2022-05-24  4:45 ` [PATCH v4 1/4] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
@ 2022-05-24  4:45 ` Yunke Cao
  2022-05-24  9:37   ` Ricardo Ribalda
  2022-05-25 20:26   ` kernel test robot
  2022-05-24  4:45 ` [PATCH v4 3/4] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
  2022-05-24  4:45 ` [PATCH v4 4/4] media: uvcvideo: document " Yunke Cao
  3 siblings, 2 replies; 13+ messages in thread
From: Yunke Cao @ 2022-05-24  4:45 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne
  Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao

Added V4L2_CTRL_WHICH_MIN/MAX_VAL and basic support in v4l2-core.

Mostly reusing a previous attempt:
https://lore.kernel.org/all/20191220134843.25977-1-m.tretter@pengutronix.de/T/#m5c83970af8b774a4b1ea5f2dca4c0a534da4ccbe

Signed-off-by: Yunke Cao <yunkec@google.com>
---
 .../media/v4l/vidioc-g-ext-ctrls.rst          |   8 +-
 .../media/videodev2.h.rst.exceptions          |   2 +
 drivers/media/i2c/imx214.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                    |  30 +++-
 include/uapi/linux/videodev2.h                |   2 +
 8 files changed, 209 insertions(+), 27 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 f4e205ead0a2..a89577726efa 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,18 @@ 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..1541a81dcd46 100644
--- a/drivers/media/i2c/imx214.c
+++ b/drivers/media/i2c/imx214.c
@@ -1037,7 +1037,9 @@ 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/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..25378304b2fb 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 default value.
+ * @p_max:     The control's default 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.
+ * 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
+ * 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] 13+ messages in thread

* [PATCH v4 3/4] media: uvcvideo: implement UVC v1.5 ROI
  2022-05-24  4:45 [PATCH v4 0/4] media: Implement UVC v1.5 ROI Yunke Cao
  2022-05-24  4:45 ` [PATCH v4 1/4] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
  2022-05-24  4:45 ` [PATCH v4 2/4] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
@ 2022-05-24  4:45 ` Yunke Cao
  2022-05-24 11:00   ` Ricardo Ribalda
  2022-05-24  4:45 ` [PATCH v4 4/4] media: uvcvideo: document " Yunke Cao
  3 siblings, 1 reply; 13+ messages in thread
From: Yunke Cao @ 2022-05-24  4:45 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne
  Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao

Supports GET_CUR, GET_DEF, GET_MIN and GET_MAX requests for UVC v1.5
using V4L2 control API.

References a rejected attempt that uses v4l2 selection API:
https://lore.kernel.org/lkml/20210501082001.100533-2-senozhatsky@chromium.org

For the devices we tested with, firmwares have wrong initial
ROI configuration. Initialize roi by setting to default value.

Signed-off-by: Yunke Cao <yunkec@google.com>
---
 drivers/media/usb/uvc/uvc_ctrl.c   | 214 ++++++++++++++++++++++++++---
 drivers/media/usb/uvc/uvc_v4l2.c   |  12 +-
 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, 233 insertions(+), 25 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index b4f6edf968bc..5c57cf688296 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,33 @@ 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->width > U16_MAX ||
+	    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 +1018,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 +1060,17 @@ static int __uvc_ctrl_get(struct uvc_video_chain *chain,
 		ctrl->loaded = 1;
 	}
 
-	*value = __uvc_ctrl_get_value(mapping,
+	switch (mapping->v4l2_type) {
+	case V4L2_CTRL_TYPE_RECT:
+		__uvc_ctrl_get_v4l2_rect(ctrl, mapping, UVC_CTRL_DATA_CURRENT,
+					 &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 +1180,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 +1421,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 +1592,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 +1759,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 +1775,46 @@ 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_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);
+	}
+
+	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 +1823,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 +1896,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 +1939,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 +2506,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;
+
+	/*
+	 * Some 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 +2552,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 +2573,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..a88d3fe6de93 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1038,17 +1038,15 @@ 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) {
-			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 +1057,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] 13+ messages in thread

* [PATCH v4 4/4] media: uvcvideo: document UVC v1.5 ROI
  2022-05-24  4:45 [PATCH v4 0/4] media: Implement UVC v1.5 ROI Yunke Cao
                   ` (2 preceding siblings ...)
  2022-05-24  4:45 ` [PATCH v4 3/4] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
@ 2022-05-24  4:45 ` Yunke Cao
  2022-05-24 11:09   ` Ricardo Ribalda
  3 siblings, 1 reply; 13+ messages in thread
From: Yunke Cao @ 2022-05-24  4:45 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>
---
 .../userspace-api/media/drivers/uvcvideo.rst  | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst
index e5fd8fad333c..9b96c8b90b7a 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,62 @@ 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. Region of interest is an
+	rectangular area represented by a struct v4l2_rect. The rectangle is in
+	pixel units and global coordinates. 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, a maximum of 640x480@0x0.
+
+	Setting a region of interest 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] 13+ messages in thread

* Re: [PATCH v4 1/4] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT
  2022-05-24  4:45 ` [PATCH v4 1/4] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
@ 2022-05-24  9:31   ` Ricardo Ribalda
  2022-05-26  1:48     ` Yunke Cao
  0 siblings, 1 reply; 13+ messages in thread
From: Ricardo Ribalda @ 2022-05-24  9:31 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.

Just a minor comment

On Tue, 24 May 2022 at 06:45, 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>
> ---
>  .../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..f4e205ead0a2 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_area``

I guess you mean: 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] 13+ messages in thread

* Re: [PATCH v4 2/4] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL
  2022-05-24  4:45 ` [PATCH v4 2/4] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
@ 2022-05-24  9:37   ` Ricardo Ribalda
  2022-05-26  1:51     ` Yunke Cao
  2022-05-25 20:26   ` kernel test robot
  1 sibling, 1 reply; 13+ messages in thread
From: Ricardo Ribalda @ 2022-05-24  9:37 UTC (permalink / raw)
  To: Yunke Cao
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, linux-media

Hi Yunke

On Tue, 24 May 2022 at 06:45, Yunke Cao <yunkec@google.com> wrote:
>
> Added V4L2_CTRL_WHICH_MIN/MAX_VAL and basic support in v4l2-core.
>
> Mostly reusing a previous attempt:
> https://lore.kernel.org/all/20191220134843.25977-1-m.tretter@pengutronix.de/T/#m5c83970af8b774a4b1ea5f2dca4c0a534da4ccbe
maybe: https://lore.kernel.org/all/20191119113457.57833-3-hverkuil-cisco@xs4all.nl/


>
> Signed-off-by: Yunke Cao <yunkec@google.com>
I think it is not official, but I like the tag:
Credit-to:

Instead of sending your version. Could you cherry-pick Hans patch?

You could add after the
====
how did you solve the conflicts

(if it is too different from Hans, please disregard this message)

> ---
>  .../media/v4l/vidioc-g-ext-ctrls.rst          |   8 +-
>  .../media/videodev2.h.rst.exceptions          |   2 +
>  drivers/media/i2c/imx214.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                    |  30 +++-
>  include/uapi/linux/videodev2.h                |   2 +
>  8 files changed, 209 insertions(+), 27 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 f4e205ead0a2..a89577726efa 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,18 @@ 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..1541a81dcd46 100644
> --- a/drivers/media/i2c/imx214.c
> +++ b/drivers/media/i2c/imx214.c
> @@ -1037,7 +1037,9 @@ 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/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..25378304b2fb 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 default value.
> + * @p_max:     The control's default 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.
> + * 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
> + * 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
>


-- 
Ricardo Ribalda

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

* Re: [PATCH v4 3/4] media: uvcvideo: implement UVC v1.5 ROI
  2022-05-24  4:45 ` [PATCH v4 3/4] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
@ 2022-05-24 11:00   ` Ricardo Ribalda
  2022-05-26  3:50     ` Yunke Cao
  0 siblings, 1 reply; 13+ messages in thread
From: Ricardo Ribalda @ 2022-05-24 11:00 UTC (permalink / raw)
  To: Yunke Cao
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, linux-media

Hi Yunke

Thanks for the patch

On Tue, 24 May 2022 at 06:45, Yunke Cao <yunkec@google.com> wrote:
>
Might be a good idea to reference the standard?. Something like:

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

> Supports GET_CUR, GET_DEF, GET_MIN and GET_MAX requests for UVC v1.5
> using V4L2 control API.

nit:

>
> References a rejected attempt that uses v4l2 selection API:
> https://lore.kernel.org/lkml/20210501082001.100533-2-senozhatsky@chromium.org
I think this comment belongs to after the ---

>
> For the devices we tested with, firmwares have wrong initial
> ROI configuration. Initialize roi by setting to default value.

nit: I think this comment should be inlined in the code, not in the
subject, but might be a taste




>
> Signed-off-by: Yunke Cao <yunkec@google.com>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c   | 214 ++++++++++++++++++++++++++---
>  drivers/media/usb/uvc/uvc_v4l2.c   |  12 +-
>  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, 233 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index b4f6edf968bc..5c57cf688296 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,

uvc says that:

The ROI must be within the current
Digital Window as specified by the CT_WINDOW control.

Have you tried changing size/cropping? and then reading back the control?


> +       },
>  };
>
>  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,33 @@ 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->width > U16_MAX ||
> +           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;
if width == 0 and left ==0 -> right = -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 +1018,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;
      struct v4l2_rect v4l2_rect = {};
>
>         if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
>                 return -EACCES;
> @@ -993,8 +1060,17 @@ static int __uvc_ctrl_get(struct uvc_video_chain *chain,
>                 ctrl->loaded = 1;
>         }
>
          xctrl->id = mapping->id;
> -       *value = __uvc_ctrl_get_value(mapping,
> +       switch (mapping->v4l2_type) {
> +       case V4L2_CTRL_TYPE_RECT:
> +               __uvc_ctrl_get_v4l2_rect(ctrl, mapping, UVC_CTRL_DATA_CURRENT,
> +                                        &v4l2_rect);
                    xctrl->size =
> +               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 +1180,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 +1421,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 +1592,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 +1759,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)
>  {
If the user asks for
uvc_ctrl_get(chain, GAIN, V4L2_CTRL_WHICH_MAX_VAL)

is this code working?


Also
I think you need to split the code for
V4L2_CTRL_WHICH_MAX_VAL
and
V4L2_CTRL_WHICH_MIN_VAL

into a different patch.

>         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 +1775,46 @@ 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_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);
> +       }
> +
> +       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 +1823,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 +1896,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 +1939,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 +2506,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;
> +
> +       /*
> +        * Some firmwares have wrong GET_CURRENT configuration. E.g. it's
* Most firmwares
> +        * 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 +2552,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 +2573,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..a88d3fe6de93 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -1038,17 +1038,15 @@ 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) {
> -                       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 +1057,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] 13+ messages in thread

* Re: [PATCH v4 4/4] media: uvcvideo: document UVC v1.5 ROI
  2022-05-24  4:45 ` [PATCH v4 4/4] media: uvcvideo: document " Yunke Cao
@ 2022-05-24 11:09   ` Ricardo Ribalda
  0 siblings, 0 replies; 13+ messages in thread
From: Ricardo Ribalda @ 2022-05-24 11:09 UTC (permalink / raw)
  To: Yunke Cao
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, linux-media

Hi Yunke

On Tue, 24 May 2022 at 06:45, Yunke Cao <yunkec@google.com> wrote:
>
> 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>
> ---
>  .../userspace-api/media/drivers/uvcvideo.rst  | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst
> index e5fd8fad333c..9b96c8b90b7a 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,62 @@ 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. Region of interest is an
This control determines the Region Of Interest (ROI). ROI is a
> +       rectangular area represented by a struct v4l2_rect. The rectangle is in
 struct :c:type:`v4l2_rect`.
> +       pixel units and global coordinates. 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, a maximum of 640x480@0x0.
 rectangle of 1x1@0x0 and a maximum of 640x480@0x0.
> +
> +       Setting a region of interest allows the camera to optimize the capture for
ROI
> +       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
>


-- 
Ricardo Ribalda

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

* Re: [PATCH v4 2/4] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL
  2022-05-24  4:45 ` [PATCH v4 2/4] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
  2022-05-24  9:37   ` Ricardo Ribalda
@ 2022-05-25 20:26   ` kernel test robot
  1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2022-05-25 20:26 UTC (permalink / raw)
  To: Yunke Cao, Hans Verkuil, Laurent Pinchart, Nicolas Dufresne
  Cc: llvm, kbuild-all, Tomasz Figa, Sergey Senozhatsky,
	Ricardo Ribalda, linux-media, Yunke Cao

Hi Yunke,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linus/master v5.18 next-20220525]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Yunke-Cao/media-Implement-UVC-v1-5-ROI/20220524-124718
base:   git://linuxtv.org/media_tree.git master
config: hexagon-randconfig-r041-20220524 (https://download.01.org/0day-ci/archive/20220526/202205260458.5bfyANVz-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d52a6e75b0c402c7f3b42a2b1b2873f151220947)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/25bdce0e26cdd486b08cbca11412dbfc5a1469eb
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yunke-Cao/media-Implement-UVC-v1-5-ROI/20220524-124718
        git checkout 25bdce0e26cdd486b08cbca11412dbfc5a1469eb
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/media/platform/qcom/venus/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/media/platform/qcom/venus/venc_ctrls.c:582:34: error: too few arguments to function call, expected 6, have 4
                                      v4l2_ctrl_ptr_create(NULL));
                                                                ^
   include/media/v4l2-ctrls.h:726:19: note: 'v4l2_ctrl_new_std_compound' declared here
   struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl,
                     ^
   drivers/media/platform/qcom/venus/venc_ctrls.c:586:34: error: too few arguments to function call, expected 6, have 4
                                      v4l2_ctrl_ptr_create(NULL));
                                                                ^
   include/media/v4l2-ctrls.h:726:19: note: 'v4l2_ctrl_new_std_compound' declared here
   struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl,
                     ^
   2 errors generated.


vim +582 drivers/media/platform/qcom/venus/venc_ctrls.c

aaaa93eda64b00 Stanimir Varbanov   2017-06-15  354  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  355  int venc_ctrl_init(struct venus_inst *inst)
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  356  {
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  357  	int ret;
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  358  
bfee75f73c37a2 Mansur Alisha Shaik 2021-07-29  359  	ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 58);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  360  	if (ret)
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  361  		return ret;
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  362  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  363  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  364  		V4L2_CID_MPEG_VIDEO_BITRATE_MODE,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  365  		V4L2_MPEG_VIDEO_BITRATE_MODE_CBR,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  366  		~((1 << V4L2_MPEG_VIDEO_BITRATE_MODE_VBR) |
69ff4b2caa95e1 Stanimir Varbanov   2020-06-16  367  		  (1 << V4L2_MPEG_VIDEO_BITRATE_MODE_CBR) |
69ff4b2caa95e1 Stanimir Varbanov   2020-06-16  368  		  (1 << V4L2_MPEG_VIDEO_BITRATE_MODE_CQ)),
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  369  		V4L2_MPEG_VIDEO_BITRATE_MODE_VBR);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  370  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  371  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  372  		V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  373  		V4L2_MPEG_VIDEO_H264_ENTROPY_MODE_CABAC,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  374  		0, V4L2_MPEG_VIDEO_H264_ENTROPY_MODE_CAVLC);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  375  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  376  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  377  		V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  378  		V4L2_MPEG_VIDEO_MPEG4_PROFILE_ADVANCED_CODING_EFFICIENCY,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  379  		~((1 << V4L2_MPEG_VIDEO_MPEG4_PROFILE_SIMPLE) |
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  380  		  (1 << V4L2_MPEG_VIDEO_MPEG4_PROFILE_ADVANCED_SIMPLE)),
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  381  		V4L2_MPEG_VIDEO_MPEG4_PROFILE_SIMPLE);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  382  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  383  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  384  		V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  385  		V4L2_MPEG_VIDEO_MPEG4_LEVEL_5,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  386  		0, V4L2_MPEG_VIDEO_MPEG4_LEVEL_0);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  387  
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  388  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  389  		V4L2_CID_MPEG_VIDEO_HEVC_PROFILE,
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  390  		V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10,
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  391  		~((1 << V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN) |
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  392  		  (1 << V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE) |
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  393  		  (1 << V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10)),
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  394  		V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN);
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  395  
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  396  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  397  		V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  398  		V4L2_MPEG_VIDEO_HEVC_LEVEL_6_2,
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  399  		0, V4L2_MPEG_VIDEO_HEVC_LEVEL_1);
cd396c8cbfcdd7 Kelvin Lawson       2018-12-10  400  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  401  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  402  		V4L2_CID_MPEG_VIDEO_H264_PROFILE,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  403  		V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  404  		~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  405  		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE) |
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  406  		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  407  		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_HIGH) |
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  408  		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH) |
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  409  		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH)),
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  410  		V4L2_MPEG_VIDEO_H264_PROFILE_HIGH);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  411  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  412  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  413  		V4L2_CID_MPEG_VIDEO_H264_LEVEL,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  414  		V4L2_MPEG_VIDEO_H264_LEVEL_5_1,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  415  		0, V4L2_MPEG_VIDEO_H264_LEVEL_1_0);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  416  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  417  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  418  		V4L2_CID_MPEG_VIDEO_H264_LOOP_FILTER_MODE,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  419  		AT_SLICE_BOUNDARY,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  420  		0, V4L2_MPEG_VIDEO_H264_LOOP_FILTER_MODE_DISABLED);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  421  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  422  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  423  		V4L2_CID_MPEG_VIDEO_HEADER_MODE,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  424  		V4L2_MPEG_VIDEO_HEADER_MODE_JOINED_WITH_1ST_FRAME,
002c22bd360e07 Dikshita Agarwal    2021-01-08  425  		~((1 << V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE) |
002c22bd360e07 Dikshita Agarwal    2021-01-08  426  		(1 << V4L2_MPEG_VIDEO_HEADER_MODE_JOINED_WITH_1ST_FRAME)),
39a6b9185d305d Stanimir Varbanov   2021-03-06  427  		V4L2_MPEG_VIDEO_HEADER_MODE_JOINED_WITH_1ST_FRAME);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  428  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  429  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  430  		V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE,
4914425e28fb90 Hans Verkuil        2019-04-24  431  		V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_MAX_BYTES,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  432  		0, V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_SINGLE);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  433  
5520b9467a39d5 Keiichi Watanabe    2018-06-18  434  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
5520b9467a39d5 Keiichi Watanabe    2018-06-18  435  		V4L2_CID_MPEG_VIDEO_VP8_PROFILE,
5520b9467a39d5 Keiichi Watanabe    2018-06-18  436  		V4L2_MPEG_VIDEO_VP8_PROFILE_3,
5520b9467a39d5 Keiichi Watanabe    2018-06-18  437  		0, V4L2_MPEG_VIDEO_VP8_PROFILE_0);
5520b9467a39d5 Keiichi Watanabe    2018-06-18  438  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  439  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  440  		V4L2_CID_MPEG_VIDEO_BITRATE, BITRATE_MIN, BITRATE_MAX,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  441  		BITRATE_STEP, BITRATE_DEFAULT);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  442  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  443  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  444  		V4L2_CID_MPEG_VIDEO_BITRATE_PEAK, BITRATE_MIN, BITRATE_MAX,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  445  		BITRATE_STEP, BITRATE_DEFAULT_PEAK);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  446  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  447  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  448  			  V4L2_CID_MPEG_VIDEO_H264_I_FRAME_QP, 1, 51, 1, 26);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  449  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  450  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  451  			  V4L2_CID_MPEG_VIDEO_H264_P_FRAME_QP, 1, 51, 1, 28);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  452  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  453  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  454  			  V4L2_CID_MPEG_VIDEO_H264_B_FRAME_QP, 1, 51, 1, 30);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  455  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  456  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  457  			  V4L2_CID_MPEG_VIDEO_H264_MIN_QP, 1, 51, 1, 1);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  458  
74c895974fd356 Dikshita Agarwal    2020-12-24  459  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  460  			  V4L2_CID_MPEG_VIDEO_H264_I_FRAME_MIN_QP, 1, 51, 1, 1);
74c895974fd356 Dikshita Agarwal    2020-12-24  461  
bfee75f73c37a2 Mansur Alisha Shaik 2021-07-29  462  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
61b3317dd424a3 Stanimir Varbanov   2022-02-08  463  			  V4L2_CID_MPEG_VIDEO_H264_8X8_TRANSFORM, 0, 1, 1, 1);
bfee75f73c37a2 Mansur Alisha Shaik 2021-07-29  464  
74c895974fd356 Dikshita Agarwal    2020-12-24  465  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  466  			  V4L2_CID_MPEG_VIDEO_H264_P_FRAME_MIN_QP, 1, 51, 1, 1);
74c895974fd356 Dikshita Agarwal    2020-12-24  467  
74c895974fd356 Dikshita Agarwal    2020-12-24  468  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  469  			  V4L2_CID_MPEG_VIDEO_H264_B_FRAME_MIN_QP, 1, 51, 1, 1);
74c895974fd356 Dikshita Agarwal    2020-12-24  470  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  471  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  472  			  V4L2_CID_MPEG_VIDEO_H264_MAX_QP, 1, 51, 1, 51);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  473  
74c895974fd356 Dikshita Agarwal    2020-12-24  474  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  475  			  V4L2_CID_MPEG_VIDEO_H264_I_FRAME_MAX_QP, 1, 51, 1, 51);
74c895974fd356 Dikshita Agarwal    2020-12-24  476  
74c895974fd356 Dikshita Agarwal    2020-12-24  477  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  478  			  V4L2_CID_MPEG_VIDEO_H264_P_FRAME_MAX_QP, 1, 51, 1, 51);
74c895974fd356 Dikshita Agarwal    2020-12-24  479  
74c895974fd356 Dikshita Agarwal    2020-12-24  480  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  481  			  V4L2_CID_MPEG_VIDEO_H264_B_FRAME_MAX_QP, 1, 51, 1, 51);
74c895974fd356 Dikshita Agarwal    2020-12-24  482  
74c895974fd356 Dikshita Agarwal    2020-12-24  483  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  484  			  V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP, 1, 63, 1, 26);
74c895974fd356 Dikshita Agarwal    2020-12-24  485  
74c895974fd356 Dikshita Agarwal    2020-12-24  486  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  487  			  V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP, 1, 63, 1, 28);
74c895974fd356 Dikshita Agarwal    2020-12-24  488  
74c895974fd356 Dikshita Agarwal    2020-12-24  489  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  490  			  V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP, 1, 63, 1, 30);
74c895974fd356 Dikshita Agarwal    2020-12-24  491  
74c895974fd356 Dikshita Agarwal    2020-12-24  492  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  493  			  V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP, 1, 63, 1, 1);
74c895974fd356 Dikshita Agarwal    2020-12-24  494  
74c895974fd356 Dikshita Agarwal    2020-12-24  495  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  496  			  V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_MIN_QP, 1, 63, 1, 1);
74c895974fd356 Dikshita Agarwal    2020-12-24  497  
74c895974fd356 Dikshita Agarwal    2020-12-24  498  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  499  			  V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_MIN_QP, 1, 63, 1, 1);
74c895974fd356 Dikshita Agarwal    2020-12-24  500  
74c895974fd356 Dikshita Agarwal    2020-12-24  501  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  502  			  V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_MIN_QP, 1, 63, 1, 1);
74c895974fd356 Dikshita Agarwal    2020-12-24  503  
74c895974fd356 Dikshita Agarwal    2020-12-24  504  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  505  			  V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP, 1, 63, 1, 63);
74c895974fd356 Dikshita Agarwal    2020-12-24  506  
74c895974fd356 Dikshita Agarwal    2020-12-24  507  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  508  			  V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_MAX_QP, 1, 63, 1, 63);
74c895974fd356 Dikshita Agarwal    2020-12-24  509  
74c895974fd356 Dikshita Agarwal    2020-12-24  510  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  511  			  V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_MAX_QP, 1, 63, 1, 63);
74c895974fd356 Dikshita Agarwal    2020-12-24  512  
74c895974fd356 Dikshita Agarwal    2020-12-24  513  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
74c895974fd356 Dikshita Agarwal    2020-12-24  514  			  V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_MAX_QP, 1, 63, 1, 63);
74c895974fd356 Dikshita Agarwal    2020-12-24  515  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  516  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  517  		V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_BYTES, SLICE_BYTE_SIZE_MIN,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  518  		SLICE_BYTE_SIZE_MAX, 1, SLICE_BYTE_SIZE_MIN);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  519  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  520  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  521  		V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_MB, 1,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  522  		SLICE_MB_SIZE_MAX, 1, 1);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  523  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  524  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  525  		V4L2_CID_MPEG_VIDEO_H264_LOOP_FILTER_ALPHA, -6, 6, 1, 0);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  526  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  527  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  528  		V4L2_CID_MPEG_VIDEO_H264_LOOP_FILTER_BETA, -6, 6, 1, 0);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  529  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  530  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
f8e5b2f3903ce1 Malathi Gottam      2018-11-12  531  		V4L2_CID_MPEG_VIDEO_GOP_SIZE, 0, (1 << 16) - 1, 1, 30);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  532  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  533  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  534  		V4L2_CID_MPEG_VIDEO_VPX_MIN_QP, 1, 128, 1, 1);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  535  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  536  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  537  		V4L2_CID_MPEG_VIDEO_VPX_MAX_QP, 1, 128, 1, 128);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  538  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  539  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  540  		V4L2_CID_MPEG_VIDEO_B_FRAMES, 0, 4, 1, 0);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  541  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  542  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  543  		V4L2_CID_MPEG_VIDEO_H264_I_PERIOD, 0, (1 << 16) - 1, 1, 0);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  544  
c35f0b16537c15 Malathi Gottam      2018-11-02  545  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
c35f0b16537c15 Malathi Gottam      2018-11-02  546  			  V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME, 0, 0, 0, 0);
c35f0b16537c15 Malathi Gottam      2018-11-02  547  
6f704b2fbbde0c Jeffrey Kardatzke   2020-02-22  548  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
6f704b2fbbde0c Jeffrey Kardatzke   2020-02-22  549  			  V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE, 0, 1, 1, 1);
6f704b2fbbde0c Jeffrey Kardatzke   2020-02-22  550  
69ff4b2caa95e1 Stanimir Varbanov   2020-06-16  551  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
69ff4b2caa95e1 Stanimir Varbanov   2020-06-16  552  			  V4L2_CID_MPEG_VIDEO_CONSTANT_QUALITY, 0, 100, 1, 0);
69ff4b2caa95e1 Stanimir Varbanov   2020-06-16  553  
94dfb1689c25ed Stanimir Varbanov   2020-07-05  554  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
94dfb1689c25ed Stanimir Varbanov   2020-07-05  555  			       V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE,
94dfb1689c25ed Stanimir Varbanov   2020-07-05  556  			       V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT,
94dfb1689c25ed Stanimir Varbanov   2020-07-05  557  			       ~((1 << V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED) |
94dfb1689c25ed Stanimir Varbanov   2020-07-05  558  			       (1 << V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT)),
94dfb1689c25ed Stanimir Varbanov   2020-07-05  559  			       V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED);
94dfb1689c25ed Stanimir Varbanov   2020-07-05  560  
e98ce77b57530a Dikshita Agarwal    2021-01-04  561  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
e98ce77b57530a Dikshita Agarwal    2021-01-04  562  			  V4L2_CID_MPEG_VIDEO_BASELAYER_PRIORITY_ID, 0,
e98ce77b57530a Dikshita Agarwal    2021-01-04  563  			  6, 1, 0);
e98ce77b57530a Dikshita Agarwal    2021-01-04  564  
08c06797627935 Stanimir Varbanov   2020-11-24  565  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
08c06797627935 Stanimir Varbanov   2020-11-24  566  			  V4L2_CID_MPEG_VIDEO_AU_DELIMITER, 0, 1, 1, 0);
08c06797627935 Stanimir Varbanov   2020-11-24  567  
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  568  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  569  			  V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES, 0,
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  570  			  ((1 << MAX_LTR_FRAME_COUNT) - 1), 0, 0);
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  571  
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  572  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  573  			  V4L2_CID_MPEG_VIDEO_LTR_COUNT, 0,
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  574  			  MAX_LTR_FRAME_COUNT, 1, 0);
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  575  
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  576  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  577  			  V4L2_CID_MPEG_VIDEO_FRAME_LTR_INDEX, 0,
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  578  			  (MAX_LTR_FRAME_COUNT - 1), 1, 0);
f2fb3f02abe2e6 Dikshita Agarwal    2021-03-24  579  
9172652d72f8e9 Stanimir Varbanov   2020-04-23  580  	v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops,
9172652d72f8e9 Stanimir Varbanov   2020-04-23  581  				   V4L2_CID_COLORIMETRY_HDR10_CLL_INFO,
9172652d72f8e9 Stanimir Varbanov   2020-04-23 @582  				   v4l2_ctrl_ptr_create(NULL));
9172652d72f8e9 Stanimir Varbanov   2020-04-23  583  
9172652d72f8e9 Stanimir Varbanov   2020-04-23  584  	v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops,
9172652d72f8e9 Stanimir Varbanov   2020-04-23  585  				   V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY,
9172652d72f8e9 Stanimir Varbanov   2020-04-23  586  				   v4l2_ctrl_ptr_create(NULL));
9172652d72f8e9 Stanimir Varbanov   2020-04-23  587  
fc503c8a3f12af Dikshita Agarwal    2022-04-19  588  	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
fc503c8a3f12af Dikshita Agarwal    2022-04-19  589  			       V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD_TYPE,
fc503c8a3f12af Dikshita Agarwal    2022-04-19  590  			       V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD_TYPE_CYCLIC,
fc503c8a3f12af Dikshita Agarwal    2022-04-19  591  			       0, V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD_TYPE_RANDOM);
fc503c8a3f12af Dikshita Agarwal    2022-04-19  592  
f7a3d3dc5831df Stanimir Varbanov   2021-06-22  593  	v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
f7a3d3dc5831df Stanimir Varbanov   2021-06-22  594  			  V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0,
f7a3d3dc5831df Stanimir Varbanov   2021-06-22  595  			  ((4096 * 2304) >> 8), 1, 0);
f7a3d3dc5831df Stanimir Varbanov   2021-06-22  596  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  597  	ret = inst->ctrl_handler.error;
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  598  	if (ret)
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  599  		goto err;
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  600  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  601  	ret = v4l2_ctrl_handler_setup(&inst->ctrl_handler);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  602  	if (ret)
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  603  		goto err;
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  604  
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  605  	return 0;
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  606  err:
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  607  	v4l2_ctrl_handler_free(&inst->ctrl_handler);
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  608  	return ret;
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  609  }
aaaa93eda64b00 Stanimir Varbanov   2017-06-15  610  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [PATCH v4 1/4] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT
  2022-05-24  9:31   ` Ricardo Ribalda
@ 2022-05-26  1:48     ` Yunke Cao
  0 siblings, 0 replies; 13+ messages in thread
From: Yunke Cao @ 2022-05-26  1:48 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, linux-media

Hi Ricardo,

Yes, thanks for catching that!

Best,
Yunke

On Tue, May 24, 2022 at 6:31 PM Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Hi Yunke
>
> Thanks for your patch.
>
> Just a minor comment
>
> On Tue, 24 May 2022 at 06:45, 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>
> > ---
> >  .../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..f4e205ead0a2 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_area``
>
> I guess you mean: 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] 13+ messages in thread

* Re: [PATCH v4 2/4] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL
  2022-05-24  9:37   ` Ricardo Ribalda
@ 2022-05-26  1:51     ` Yunke Cao
  0 siblings, 0 replies; 13+ messages in thread
From: Yunke Cao @ 2022-05-26  1:51 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Hans Verkuil, Laurent Pinchart, Nicolas Dufresne, Tomasz Figa,
	Sergey Senozhatsky, linux-media

Hi Ricardo,

Thanks for the review.

On Tue, May 24, 2022 at 6:38 PM Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Hi Yunke
>
> On Tue, 24 May 2022 at 06:45, Yunke Cao <yunkec@google.com> wrote:
> >
> > Added V4L2_CTRL_WHICH_MIN/MAX_VAL and basic support in v4l2-core.
> >
> > Mostly reusing a previous attempt:
> > https://lore.kernel.org/all/20191220134843.25977-1-m.tretter@pengutronix.de/T/#m5c83970af8b774a4b1ea5f2dca4c0a534da4ccbe
> maybe: https://lore.kernel.org/all/20191119113457.57833-3-hverkuil-cisco@xs4all.nl/
>
>
> >
> > Signed-off-by: Yunke Cao <yunkec@google.com>
> I think it is not official, but I like the tag:
> Credit-to:
>
> Instead of sending your version. Could you cherry-pick Hans patch?
>
> You could add after the
> ====
> how did you solve the conflicts
>
> (if it is too different from Hans, please disregard this message)
>
It's not different from Hans's patch at all. I will cherry-pick in v5.

Best,
Yunke
> > ---
> >  .../media/v4l/vidioc-g-ext-ctrls.rst          |   8 +-
> >  .../media/videodev2.h.rst.exceptions          |   2 +
> >  drivers/media/i2c/imx214.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                    |  30 +++-
> >  include/uapi/linux/videodev2.h                |   2 +
> >  8 files changed, 209 insertions(+), 27 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 f4e205ead0a2..a89577726efa 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,18 @@ 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..1541a81dcd46 100644
> > --- a/drivers/media/i2c/imx214.c
> > +++ b/drivers/media/i2c/imx214.c
> > @@ -1037,7 +1037,9 @@ 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/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..25378304b2fb 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 default value.
> > + * @p_max:     The control's default 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.
> > + * 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
> > + * 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
> >
>
>
> --
> Ricardo Ribalda

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

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

Hi Ricardo,

Thanks for the review.


On Tue, May 24, 2022 at 8:00 PM Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Hi Yunke
>
> Thanks for the patch
>
> On Tue, 24 May 2022 at 06:45, Yunke Cao <yunkec@google.com> wrote:
> >
> Might be a good idea to reference the standard?. Something like:
>
> Implement support for ROI as described in UVC 1.5:
> 4.2.2.1.20 Digital Region of Interest (ROI) Control
>
> > Supports GET_CUR, GET_DEF, GET_MIN and GET_MAX requests for UVC v1.5
> > using V4L2 control API.
>
> nit:
>
> >
> > References a rejected attempt that uses v4l2 selection API:
> > https://lore.kernel.org/lkml/20210501082001.100533-2-senozhatsky@chromium.org
> I think this comment belongs to after the ---
>
> >
> > For the devices we tested with, firmwares have wrong initial
> > ROI configuration. Initialize roi by setting to default value.
>
> nit: I think this comment should be inlined in the code, not in the
> subject, but might be a taste
>
>
>
>
> >
> > Signed-off-by: Yunke Cao <yunkec@google.com>
> > ---
> >  drivers/media/usb/uvc/uvc_ctrl.c   | 214 ++++++++++++++++++++++++++---
> >  drivers/media/usb/uvc/uvc_v4l2.c   |  12 +-
> >  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, 233 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index b4f6edf968bc..5c57cf688296 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,
>
> uvc says that:
>
> The ROI must be within the current
> Digital Window as specified by the CT_WINDOW control.
>
> Have you tried changing size/cropping? and then reading back the control?
>
>
I tried changing size using VIDIOC_S_FMT and reading back the control.
ROI stays unchanged. This is expected because ROI (and CT_WINDOW) are
in global sensor coordinates according to the UVC specification
(I should probably add this to the documentation).



> > +       },
> >  };
> >
> >  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,33 @@ 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->width > U16_MAX ||
> > +           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;
> if width == 0 and left ==0 -> right = -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 +1018,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;
>       struct v4l2_rect v4l2_rect = {};
> >
> >         if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
> >                 return -EACCES;
> > @@ -993,8 +1060,17 @@ static int __uvc_ctrl_get(struct uvc_video_chain *chain,
> >                 ctrl->loaded = 1;
> >         }
> >
>           xctrl->id = mapping->id;
> > -       *value = __uvc_ctrl_get_value(mapping,
> > +       switch (mapping->v4l2_type) {
> > +       case V4L2_CTRL_TYPE_RECT:
> > +               __uvc_ctrl_get_v4l2_rect(ctrl, mapping, UVC_CTRL_DATA_CURRENT,
> > +                                        &v4l2_rect);
>                     xctrl->size =
> > +               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 +1180,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 +1421,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 +1592,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 +1759,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)
> >  {
> If the user asks for
> uvc_ctrl_get(chain, GAIN, V4L2_CTRL_WHICH_MAX_VAL)
>
> is this code working?
GAIN doesn't seem to be supported on my device. I tried brightness and
it works fine.
>
>
> Also
> I think you need to split the code for
> V4L2_CTRL_WHICH_MAX_VAL
> and
> V4L2_CTRL_WHICH_MIN_VAL
>
> into a different patch.
Ok. Will do in V5.

Best,
Yunke
>
> >         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 +1775,46 @@ 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_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);
> > +       }
> > +
> > +       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 +1823,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 +1896,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 +1939,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 +2506,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;
> > +
> > +       /*
> > +        * Some firmwares have wrong GET_CURRENT configuration. E.g. it's
> * Most firmwares
> > +        * 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 +2552,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 +2573,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..a88d3fe6de93 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -1038,17 +1038,15 @@ 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) {
> > -                       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 +1057,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] 13+ messages in thread

end of thread, other threads:[~2022-05-26  3:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-24  4:45 [PATCH v4 0/4] media: Implement UVC v1.5 ROI Yunke Cao
2022-05-24  4:45 ` [PATCH v4 1/4] media: v4l2_ctrl: Add V4L2_CTRL_TYPE_RECT Yunke Cao
2022-05-24  9:31   ` Ricardo Ribalda
2022-05-26  1:48     ` Yunke Cao
2022-05-24  4:45 ` [PATCH v4 2/4] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
2022-05-24  9:37   ` Ricardo Ribalda
2022-05-26  1:51     ` Yunke Cao
2022-05-25 20:26   ` kernel test robot
2022-05-24  4:45 ` [PATCH v4 3/4] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
2022-05-24 11:00   ` Ricardo Ribalda
2022-05-26  3:50     ` Yunke Cao
2022-05-24  4:45 ` [PATCH v4 4/4] media: uvcvideo: document " Yunke Cao
2022-05-24 11:09   ` Ricardo Ribalda

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.