[PATCHv5 0/5] media: uvcvideo: implement UVC 1.5 ROI

[PATCHv5 0/5] media: uvcvideo: implement UVC 1.5 ROI

[Sergey Senozhatsky]

Hello,

	This patch set implements UVC 1.5 ROI using v4l2_selection API.

v5:
-- fixed UAPI typo: STABILIXATION
-- moved V4L2_CID_REGION_OF_INTEREST_AUTO to V4L2_CID_CAMERA_CLASS_BASE+36
-- added more comments (Ricardo)
-- added V4L2_CID_REGION_OF_INTEREST_AUTO to v4l2_ctrl_get_name() (Ricardo)

Sergey Senozhatsky (5):
  media: v4l UAPI: add ROI selection targets
  media: v4l UAPI: document ROI selection targets
  media: uvcvideo: add ROI auto controls
  media: v4l UAPI: document ROI auto_controls
  media: uvcvideo: add UVC 1.5 ROI control

 .../media/v4l/ext-ctrls-camera.rst            |  23 +++
 .../media/v4l/selection-api-configuration.rst |  22 ++
 .../media/v4l/selection-api-examples.rst      |  27 +++
 .../media/v4l/v4l2-selection-targets.rst      |  24 +++
 drivers/media/usb/uvc/uvc_ctrl.c              |  19 ++
 drivers/media/usb/uvc/uvc_v4l2.c              | 189 +++++++++++++++++-
 drivers/media/v4l2-core/v4l2-ctrls.c          |   1 +
 include/uapi/linux/usb/video.h                |   1 +
 include/uapi/linux/v4l2-common.h              |   8 +
 include/uapi/linux/v4l2-controls.h            |  10 +
 10 files changed, 321 insertions(+), 3 deletions(-)

-- 
2.31.1.527.g47e6f16901-goog

[PATCHv5 1/5] media: v4l UAPI: add ROI selection targets

[Sergey Senozhatsky]

UVC 1.5 requires Region Of Interest control to implement
GET_CUR, GET_DEF, GET_MIN and GET_MAX requests. This patch
adds new V4L2 selection API targets that will implement
those ROI requests.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 include/uapi/linux/v4l2-common.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
index 7d21c1634b4d..3651ebb8cb23 100644
--- a/include/uapi/linux/v4l2-common.h
+++ b/include/uapi/linux/v4l2-common.h
@@ -78,6 +78,14 @@
 #define V4L2_SEL_TGT_COMPOSE_BOUNDS	0x0102
 /* Current composing area plus all padding pixels */
 #define V4L2_SEL_TGT_COMPOSE_PADDED	0x0103
+/* Current Region of Interest area */
+#define V4L2_SEL_TGT_ROI		0x0200
+/* Default Region of Interest area */
+#define V4L2_SEL_TGT_ROI_DEFAULT	0x0201
+/* Region of Interest minimum values */
+#define V4L2_SEL_TGT_ROI_BOUNDS_MIN	0x0202
+/* Region of Interest maximum values */
+#define V4L2_SEL_TGT_ROI_BOUNDS_MAX	0x0203
 
 /* Selection flags */
 #define V4L2_SEL_FLAG_GE		(1 << 0)
-- 
2.31.1.527.g47e6f16901-goog

[PATCHv5 2/5] media: v4l UAPI: document ROI selection targets

[Sergey Senozhatsky]

Document V4L2 selection targets that will be used to ROI
implementation.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 .../media/v4l/selection-api-configuration.rst | 22 +++++++++++++++
 .../media/v4l/selection-api-examples.rst      | 27 +++++++++++++++++++
 .../media/v4l/v4l2-selection-targets.rst      | 24 +++++++++++++++++
 3 files changed, 73 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/selection-api-configuration.rst b/Documentation/userspace-api/media/v4l/selection-api-configuration.rst
index fee49bf1a1c0..b5fdd765e2db 100644
--- a/Documentation/userspace-api/media/v4l/selection-api-configuration.rst
+++ b/Documentation/userspace-api/media/v4l/selection-api-configuration.rst
@@ -135,3 +135,25 @@ and the height of rectangles obtained using ``V4L2_SEL_TGT_CROP`` and
 ``V4L2_SEL_TGT_COMPOSE`` targets. If these are not equal then the
 scaling is applied. The application can compute the scaling ratios using
 these values.
+
+Configuration of Region of Interest (ROI)
+=========================================
+
+The range of auto-controls values and of coordinates of the top left
+corner, width and height of areas that can be ROI is given by the
+``V4L2_SEL_TGT_ROI_BOUNDS_MIN`` and ``V4L2_SEL_TGT_ROI_BOUNDS_MAX``
+targets. It is recommended for the driver developers to put the top/left
+corner at position ``(0,0)``.
+
+The top left corner, width and height of the Region of Interest area
+and auto-controls currently being employed by the device are given by
+the ``V4L2_SEL_TGT_ROI`` target. It uses the same coordinate system
+as ``V4L2_SEL_TGT_ROI_BOUNDS_MIN`` and ``V4L2_SEL_TGT_ROI_BOUNDS_MAX``.
+
+In order to change active ROI top left, width and height coordinates
+and ROI auto-controls use ``V4L2_SEL_TGT_ROI`` target.
+
+Each capture device has a default ROI rectangle and auto-controls
+value given by the ``V4L2_SEL_TGT_ROI_DEFAULT`` target. Drivers shall
+set the ROI rectangle to the default when the driver is first loaded,
+but not later.
diff --git a/Documentation/userspace-api/media/v4l/selection-api-examples.rst b/Documentation/userspace-api/media/v4l/selection-api-examples.rst
index 5f8e8a1f59d7..39aba98d55f1 100644
--- a/Documentation/userspace-api/media/v4l/selection-api-examples.rst
+++ b/Documentation/userspace-api/media/v4l/selection-api-examples.rst
@@ -82,3 +82,30 @@ Example: Querying for scaling factors
 	/* computing scaling factors */
 	hscale = (double)compose.r.width / crop.r.width;
 	vscale = (double)compose.r.height / crop.r.height;
+
+Setting Region Of Interest area to half of the default value
+
+Example: Simple ROI
+===========================
+
+.. code-block:: c
+
+	struct v4l2_selection roi = {
+	    .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+	    .target = V4L2_SEL_TGT_ROI_DEFAULT,
+	};
+	struct v4l2_rect r;
+
+	ret = ioctl(fd, VIDIOC_G_SELECTION, &roi);
+	if (ret)
+	    exit(-1);
+	/* setting smaller ROI rectangle */
+	r.width = roi.r.width / 2;
+	r.height = roi.r.height / 2;
+	r.left = roi.r.width / 4;
+	r.top = roi.r.height / 4;
+	roi.r = r;
+	roi.target = V4L2_SEL_TGT_ROI;
+	ret = ioctl(fd, VIDIOC_S_SELECTION, &roi);
+	if (ret)
+	    exit(-1);
diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst
index b46bae984f35..d1dc9c50eb05 100644
--- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst
+++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst
@@ -75,6 +75,30 @@ of the two interfaces they are used.
 	modified by hardware.
       - Yes
       - No
+    * - ``V4L2_SEL_TGT_ROI``
+      - 0x0200
+      - Current Region of Interest rectangle and auto-controls value.
+      - Yes
+      - No
+    * - ``V4L2_SEL_TGT_ROI_DEFAULT``
+      - 0x0201
+      - Suggested Region of Interest rectangle and auto-controls value.
+      - Yes
+      - No
+    * - ``V4L2_SEL_TGT_ROI_BOUNDS_MIN``
+      - 0x0202
+      - Minimum bounds of the Region of Interest rectangle and minimum
+	auto-controls value. All valid ROI rectangles and auto-controls
+	should be within minimum-maximum range.
+      - Yes
+      - No
+    * - ``V4L2_SEL_TGT_ROI_BOUNDS_MAX``
+      - 0x0203
+      - Maximum bounds of the Region of Interest rectangle and maximum
+	auto-controls value. All valid ROI rectangles and auto-controls
+	should be within minimum-maximum range.
+      - Yes
+      - No
 
 .. raw:: latex
 
-- 
2.31.1.527.g47e6f16901-goog

[PATCHv5 3/5] media: uvcvideo: add ROI auto controls

[Sergey Senozhatsky]

This patch adds support for Region of Interest bmAutoControls.

ROI control is a compound data type:
  Control Selector     CT_REGION_OF_INTEREST_CONTROL
  Mandatory Requests   SET_CUR, GET_CUR, GET_MIN, GET_MAX, GET_DEF
  wLength 10
  Offset   Field            Size
  0        wROI_Top         2
  2        wROI_Left        2
  4        wROI_Bottom      2
  6        wROI_Right       2
  8        bmAutoControls   2       (Bitmap)

uvc_control_mapping, however, can handle only s32 data type at the
moment: ->get() returns s32 value, ->set() accepts s32 value; while
v4l2_ctrl maximum/minimum/default_value can hold only s64 values.

Hence ROI control handling is split into two patches:
a) bmAutoControls is handled via uvc_control_mapping as V4L2_CTRL_TYPE_BITMASK
b) ROI rectangle (SET_CUR, GET_CUR, GET_DEF, etc.) handling is implemented
   separately, by the means of selection API.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c     | 19 +++++++++++++++++++
 drivers/media/v4l2-core/v4l2-ctrls.c |  1 +
 include/uapi/linux/usb/video.h       |  1 +
 include/uapi/linux/v4l2-controls.h   | 10 ++++++++++
 4 files changed, 31 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index b3dde98499f4..5502fe540519 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -355,6 +355,15 @@ 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
+	},
 };
 
 static const struct uvc_menu_info power_line_frequency_controls[] = {
@@ -753,6 +762,16 @@ 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_REGION_OF_INTEREST_AUTO,
+		.name		= "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,
+	},
 };
 
 /* ------------------------------------------------------------------------
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 0d7fe1bd975a..8c5816f3f7d4 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -832,6 +832,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:	return "Min Number of Output Buffers";
 	case V4L2_CID_ALPHA_COMPONENT:		return "Alpha Component";
 	case V4L2_CID_COLORFX_CBCR:		return "Color Effects, CbCr";
+	case V4L2_CID_REGION_OF_INTEREST_AUTO:	return "Region Of Interest Auto Controls";
 
 	/* Codec controls */
 	/* The MPEG controls are applicable to all codec controls
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/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index d43bec5f1afd..97f11b7a3a7c 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -993,6 +993,16 @@ enum v4l2_auto_focus_range {
 
 #define V4L2_CID_CAMERA_SENSOR_ROTATION		(V4L2_CID_CAMERA_CLASS_BASE+35)
 
+#define V4L2_CID_REGION_OF_INTEREST_AUTO	(V4L2_CID_CAMERA_CLASS_BASE+36)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE		(1 << 0)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS			(1 << 1)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE		(1 << 2)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS			(1 << 3)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT		(1 << 4)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK	(1 << 5)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION	(1 << 6)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY	(1 << 7)
+
 /* FM Modulator class control IDs */
 
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
-- 
2.31.1.527.g47e6f16901-goog

[PATCHv5 4/5] media: v4l UAPI: document ROI auto_controls

[Sergey Senozhatsky]

UVC 1.5 defines the following Region of Interest auto_controls

|D0: Auto Exposure
|D1: Auto Iris
|D2: Auto White Balance
|D3: Auto Focus
|D4: Auto Face Detect
|D5: Auto Detect and Track
|D6: Image Stabilization
|D7: Higher Quality
|D8 – D15: Reserved, set to zero

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 .../media/v4l/ext-ctrls-camera.rst            | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
index 4c5061aa9cd4..f59cb4d06697 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
@@ -217,7 +217,30 @@ enum v4l2_auto_focus_range -
     * - ``V4L2_AUTO_FOCUS_RANGE_INFINITY``
       - The lens is set to focus on an object at infinite distance.
 
+``V4L2_CID_REGION_OF_INTEREST_AUTO (bitmask)``
+    This determines which, if any, on board features should track to the
+    Region of Interest.
 
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE``
+      - Auto Exposure.
+    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS``
+      - Auto Iris.
+    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE``
+      - Auto White Balance.
+    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS``
+      - Auto Focus.
+    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT``
+      - Auto Face Detect.
+    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK``
+      - Auto Detect and Track.
+    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION``
+      - Image Stabilization.
+    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
+      - Higher Quality.
 
 ``V4L2_CID_ZOOM_ABSOLUTE (integer)``
     Specify the objective lens focal length as an absolute value. The
-- 
2.31.1.527.g47e6f16901-goog

[PATCHv5 5/5] media: uvcvideo: add UVC 1.5 ROI control

[Sergey Senozhatsky]

This patch implements UVC 1.5 Region of Interest (ROI) control.

Note that, UVC 1.5 defines CT_DIGITAL_WINDOW_CONTROL controls
and mentions that ROI rectangle coordinates "must be within
the current Digital Window as specified by the CT_WINDOW control."
(4.2.2.1.20 Digital Region of Interest (ROI) Control).

It's is not entirely clear if we need to implement WINDOW_CONTROL.
ROI is naturally limited by GET_MIN and GET_MAX rectangles.

Another thing to note is that ROI support is implemented as
V4L2 selection target: selection rectangle represents ROI
rectangle.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/media/usb/uvc/uvc_v4l2.c | 189 ++++++++++++++++++++++++++++++-
 1 file changed, 186 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 252136cc885c..fc2ccc144cdd 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1139,14 +1139,74 @@ static int uvc_ioctl_querymenu(struct file *file, void *fh,
 	return uvc_query_v4l2_menu(chain, qm);
 }
 
-static int uvc_ioctl_g_selection(struct file *file, void *fh,
-				 struct v4l2_selection *sel)
+/* UVC 1.5 ROI rectangle is half the size of v4l2_rect */
+struct uvc_roi_rect {
+	__u16			top;
+	__u16			left;
+	__u16			bottom;
+	__u16			right;
+	__u16			auto_controls;
+} __packed;
+
+static int uvc_ioctl_g_roi_target(struct file *file, void *fh,
+				  struct v4l2_selection *sel)
 {
 	struct uvc_fh *handle = fh;
 	struct uvc_streaming *stream = handle->stream;
+	struct uvc_video_chain *chain = handle->chain;
+	struct uvc_roi_rect *roi;
+	u8 query;
+	int ret;
 
-	if (sel->type != stream->type)
+	switch (sel->target) {
+	case V4L2_SEL_TGT_ROI:
+		query = UVC_GET_CUR;
+		break;
+	case V4L2_SEL_TGT_ROI_DEFAULT:
+		query = UVC_GET_DEF;
+		break;
+	case V4L2_SEL_TGT_ROI_BOUNDS_MIN:
+		query = UVC_GET_MAX;
+		break;
+	case V4L2_SEL_TGT_ROI_BOUNDS_MAX:
+		query = UVC_GET_MAX;
+		break;
+	default:
 		return -EINVAL;
+	}
+
+	/* hcd requires transfer buffer to be DMA capable */
+	roi = kzalloc(sizeof(struct uvc_roi_rect), GFP_KERNEL);
+	if (!roi)
+		return -ENOMEM;
+
+	/*
+	 * Synchronize with uvc_ioctl_query_ext_ctrl() that can set
+	 * ROI auto_controls concurrently.
+	 */
+	mutex_lock(&chain->ctrl_mutex);
+
+	ret = uvc_query_ctrl(stream->dev, query, 1, stream->dev->intfnum,
+			     UVC_CT_REGION_OF_INTEREST_CONTROL, roi,
+			     sizeof(struct uvc_roi_rect));
+	if (!ret) {
+		/* ROI left, top, right, bottom are global coordinates. */
+		sel->r.left	= roi->left;
+		sel->r.top	= roi->top;
+		sel->r.width	= roi->right - roi->left + 1;
+		sel->r.height	= roi->bottom - roi->top + 1;
+	}
+
+	mutex_unlock(&chain->ctrl_mutex);
+	kfree(roi);
+	return ret;
+}
+
+static int uvc_ioctl_g_sel_target(struct file *file, void *fh,
+				  struct v4l2_selection *sel)
+{
+	struct uvc_fh *handle = fh;
+	struct uvc_streaming *stream = handle->stream;
 
 	switch (sel->target) {
 	case V4L2_SEL_TGT_CROP_DEFAULT:
@@ -1173,6 +1233,128 @@ static int uvc_ioctl_g_selection(struct file *file, void *fh,
 	return 0;
 }
 
+static int uvc_ioctl_g_selection(struct file *file, void *fh,
+				 struct v4l2_selection *sel)
+{
+	struct uvc_fh *handle = fh;
+	struct uvc_streaming *stream = handle->stream;
+
+	if (sel->type != stream->type)
+		return -EINVAL;
+
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		return uvc_ioctl_g_sel_target(file, fh, sel);
+	case V4L2_SEL_TGT_ROI:
+	case V4L2_SEL_TGT_ROI_DEFAULT:
+	case V4L2_SEL_TGT_ROI_BOUNDS_MIN:
+	case V4L2_SEL_TGT_ROI_BOUNDS_MAX:
+		return uvc_ioctl_g_roi_target(file, fh, sel);
+	}
+
+	return -EINVAL;
+}
+
+static void validate_roi_bounds(struct uvc_streaming *stream,
+				struct v4l2_selection *sel)
+{
+	lockdep_assert_held(&stream->mutex);
+
+	if (sel->r.left > USHRT_MAX)
+		sel->r.left = 0;
+
+	if (sel->r.top > USHRT_MAX)
+		sel->r.top = 0;
+
+	if (sel->r.width + sel->r.left > USHRT_MAX || !sel->r.width) {
+		sel->r.left = 0;
+		sel->r.width = stream->cur_frame->wWidth;
+	}
+
+	if (sel->r.height + sel->r.top > USHRT_MAX || !sel->r.height) {
+		sel->r.top = 0;
+		sel->r.height = stream->cur_frame->wHeight;
+	}
+}
+
+static int uvc_ioctl_s_roi(struct file *file, void *fh,
+			   struct v4l2_selection *sel)
+{
+	struct uvc_fh *handle = fh;
+	struct uvc_streaming *stream = handle->stream;
+	struct uvc_video_chain *chain = handle->chain;
+	struct uvc_roi_rect *roi;
+	int ret;
+
+	/* hcd requires transfer buffer to be DMA capable */
+	roi = kzalloc(sizeof(struct uvc_roi_rect), GFP_KERNEL);
+	if (!roi)
+		return -ENOMEM;
+
+	/*
+	 * Synchronize with uvc_ioctl_query_ext_ctrl() that can set
+	 * ROI auto_controls concurrently.
+	 */
+	mutex_lock(&chain->ctrl_mutex);
+
+	/*
+	 * Get current ROI configuration. We are especially interested in
+	 * ->auto_controls, because we will use GET_CUR ->auto_controls
+	 * value for SET_CUR. Some firmwares require sizeof(uvc_roi_rect)
+	 * to be 5 * sizeof(__u16) so we need to set correct rectangle
+	 * dimensions and correct auto_controls value.
+	 */
+	ret = uvc_query_ctrl(stream->dev, UVC_GET_CUR, 1, stream->dev->intfnum,
+			     UVC_CT_REGION_OF_INTEREST_CONTROL, roi,
+			     sizeof(struct uvc_roi_rect));
+	if (ret)
+		goto out;
+
+	mutex_lock(&stream->mutex);
+
+	validate_roi_bounds(stream, sel);
+
+	/*
+	 * ROI left, top, right, bottom are global coordinates.
+	 * Note that we use ->auto_controls value which we read earlier.
+	 */
+	roi->left		= sel->r.left;
+	roi->top		= sel->r.top;
+	roi->right		= sel->r.width + sel->r.left - 1;
+	roi->bottom		= sel->r.height + sel->r.top - 1;
+
+	ret = uvc_query_ctrl(stream->dev, UVC_SET_CUR, 1, stream->dev->intfnum,
+			     UVC_CT_REGION_OF_INTEREST_CONTROL, roi,
+			     sizeof(struct uvc_roi_rect));
+
+	mutex_unlock(&stream->mutex);
+
+out:
+	mutex_unlock(&chain->ctrl_mutex);
+	kfree(roi);
+	return ret;
+}
+
+static int uvc_ioctl_s_selection(struct file *file, void *fh,
+				 struct v4l2_selection *sel)
+{
+	struct uvc_fh *handle = fh;
+	struct uvc_streaming *stream = handle->stream;
+
+	if (sel->type != stream->type)
+		return -EINVAL;
+
+	switch (sel->target) {
+	case V4L2_SEL_TGT_ROI:
+		return uvc_ioctl_s_roi(file, fh, sel);
+	}
+
+	return -EINVAL;
+}
+
 static int uvc_ioctl_g_parm(struct file *file, void *fh,
 			    struct v4l2_streamparm *parm)
 {
@@ -1533,6 +1715,7 @@ const struct v4l2_ioctl_ops uvc_ioctl_ops = {
 	.vidioc_try_ext_ctrls = uvc_ioctl_try_ext_ctrls,
 	.vidioc_querymenu = uvc_ioctl_querymenu,
 	.vidioc_g_selection = uvc_ioctl_g_selection,
+	.vidioc_s_selection = uvc_ioctl_s_selection,
 	.vidioc_g_parm = uvc_ioctl_g_parm,
 	.vidioc_s_parm = uvc_ioctl_s_parm,
 	.vidioc_enum_framesizes = uvc_ioctl_enum_framesizes,
-- 
2.31.1.527.g47e6f16901-goog

Re: [PATCHv5 0/5] media: uvcvideo: implement UVC 1.5 ROI

[Hans Verkuil]

Hi Sergey,

On 01/05/2021 10:19, Sergey Senozhatsky wrote:
> Hello,
> 
> 	This patch set implements UVC 1.5 ROI using v4l2_selection API.
> 
> v5:
> -- fixed UAPI typo: STABILIXATION
> -- moved V4L2_CID_REGION_OF_INTEREST_AUTO to V4L2_CID_CAMERA_CLASS_BASE+36
> -- added more comments (Ricardo)
> -- added V4L2_CID_REGION_OF_INTEREST_AUTO to v4l2_ctrl_get_name() (Ricardo)
> 
> Sergey Senozhatsky (5):
>   media: v4l UAPI: add ROI selection targets

As mentioned in my reply to v4 0/5, I am quite strongly opposed to using the
Selection API for this as opposed to using controls. Unless someone can provide
strong arguments for using the Selection API, I am inclined to reject this.

Sorry about that, I probably should have been reviewing this series sooner.

Regards,

	Hans

>   media: v4l UAPI: document ROI selection targets
>   media: uvcvideo: add ROI auto controls
>   media: v4l UAPI: document ROI auto_controls
>   media: uvcvideo: add UVC 1.5 ROI control
> 
>  .../media/v4l/ext-ctrls-camera.rst            |  23 +++
>  .../media/v4l/selection-api-configuration.rst |  22 ++
>  .../media/v4l/selection-api-examples.rst      |  27 +++
>  .../media/v4l/v4l2-selection-targets.rst      |  24 +++
>  drivers/media/usb/uvc/uvc_ctrl.c              |  19 ++
>  drivers/media/usb/uvc/uvc_v4l2.c              | 189 +++++++++++++++++-
>  drivers/media/v4l2-core/v4l2-ctrls.c          |   1 +
>  include/uapi/linux/usb/video.h                |   1 +
>  include/uapi/linux/v4l2-common.h              |   8 +
>  include/uapi/linux/v4l2-controls.h            |  10 +
>  10 files changed, 321 insertions(+), 3 deletions(-)
> 

Re: [PATCHv5 0/5] media: uvcvideo: implement UVC 1.5 ROI

[Sergey Senozhatsky]

On (21/05/26 12:38), Hans Verkuil wrote:
> Hi Sergey,
> 
> On 01/05/2021 10:19, Sergey Senozhatsky wrote:
> > Hello,
> > 
> > 	This patch set implements UVC 1.5 ROI using v4l2_selection API.
> > 
> > v5:
> > -- fixed UAPI typo: STABILIXATION
> > -- moved V4L2_CID_REGION_OF_INTEREST_AUTO to V4L2_CID_CAMERA_CLASS_BASE+36
> > -- added more comments (Ricardo)
> > -- added V4L2_CID_REGION_OF_INTEREST_AUTO to v4l2_ctrl_get_name() (Ricardo)
> > 
> > Sergey Senozhatsky (5):
> >   media: v4l UAPI: add ROI selection targets
> 
> As mentioned in my reply to v4 0/5, I am quite strongly opposed to using the
> Selection API for this as opposed to using controls. Unless someone can provide
> strong arguments for using the Selection API, I am inclined to reject this.
> 
> Sorry about that, I probably should have been reviewing this series sooner.

Hans, any suggestions regarding the UVCs GET_CUR, GET_DEF, GET_MIN/MAX
requests handling?

Re: [PATCHv5 0/5] media: uvcvideo: implement UVC 1.5 ROI

[Hans Verkuil]

On 01/06/2021 10:46, Sergey Senozhatsky wrote:
> On (21/05/26 12:38), Hans Verkuil wrote:
>> Hi Sergey,
>>
>> On 01/05/2021 10:19, Sergey Senozhatsky wrote:
>>> Hello,
>>>
>>> 	This patch set implements UVC 1.5 ROI using v4l2_selection API.
>>>
>>> v5:
>>> -- fixed UAPI typo: STABILIXATION
>>> -- moved V4L2_CID_REGION_OF_INTEREST_AUTO to V4L2_CID_CAMERA_CLASS_BASE+36
>>> -- added more comments (Ricardo)
>>> -- added V4L2_CID_REGION_OF_INTEREST_AUTO to v4l2_ctrl_get_name() (Ricardo)
>>>
>>> Sergey Senozhatsky (5):
>>>   media: v4l UAPI: add ROI selection targets
>>
>> As mentioned in my reply to v4 0/5, I am quite strongly opposed to using the
>> Selection API for this as opposed to using controls. Unless someone can provide
>> strong arguments for using the Selection API, I am inclined to reject this.
>>
>> Sorry about that, I probably should have been reviewing this series sooner.
> 
> Hans, any suggestions regarding the UVCs GET_CUR, GET_DEF, GET_MIN/MAX
> requests handling?
> 

Support for current value and default value is already present in the control
framework (V4L2_CTRL_WHICH_CUR_VAL and V4L2_CTRL_WHICH_DEF_VAL). MIN/MAX support
would have to be added by creating V4L2_CTRL_WHICH_MIN/MAX_VAL defines and
implementing this in the control framework, pretty much along the same lines
as the DEF_VAL support. Should be quite easy.

Regards,

	Hans