diff mbox series

[PATCHv2,2/3] media: uvcvideo: add ROI auto controls

Message ID 20210208051749.1785246-3-sergey.senozhatsky@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add UVC 1.5 Region Of Interest control to uvcvideo | expand

Commit Message

Sergey Senozhatsky Feb. 8, 2021, 5:17 a.m. UTC
From: Sergey Senozhatsky <senozhatsky@chromium.org>

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_MENU
b) ROI rectangle (SET_CUR, GET_CUR, GET_DEF) handling is implemented
   separately, by the means of selection API.

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

Comments

Ricardo Ribalda Delgado March 16, 2021, 6:29 p.m. UTC | #1
Hi Sergey

Thanks for the patch

On Mon, Feb 8, 2021 at 6:24 AM Sergey Senozhatsky
<sergey.senozhatsky@gmail.com> wrote:
>
> From: Sergey Senozhatsky <senozhatsky@chromium.org>
>
> 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_MENU
> b) ROI rectangle (SET_CUR, GET_CUR, GET_DEF) handling is implemented
>    separately, by the means of selection API.

Maybe a reference to the uvc doc would be a good thing to add here.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
>  .../media/v4l/ext-ctrls-camera.rst            | 25 +++++++++++++++++++
>  drivers/media/usb/uvc/uvc_ctrl.c              | 19 ++++++++++++++
>  include/uapi/linux/usb/video.h                |  1 +
>  include/uapi/linux/v4l2-controls.h            |  9 +++++++
>  4 files changed, 54 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> index c05a2d2c675d..1593c999c8e2 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> @@ -653,6 +653,31 @@ enum v4l2_scene_mode -
>                            |                    |
>                            +--------------------+
>
> +``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_STABILIXATION``
> +      - Image Stabilization.
> +    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
> +      - Higher Quality.
> +
>
Nit: Same as before splitting doc and code.

>  .. [#f1]
>     This control may be changed to a menu control in the future, if more
> 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,
Are

> +               .data_type      = UVC_CTRL_DATA_TYPE_BITMASK,
> +       },
>  };
>
>  /* ------------------------------------------------------------------------
> diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
> index d854cb19c42c..c87624962896 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 039c0d7add1b..6a3dac481cb4 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -976,6 +976,15 @@ enum v4l2_auto_focus_range {
>
>  #define V4L2_CID_PAN_SPEED                     (V4L2_CID_CAMERA_CLASS_BASE+32)
>  #define V4L2_CID_TILT_SPEED                    (V4L2_CID_CAMERA_CLASS_BASE+33)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO       (V4L2_CID_CAMERA_CLASS_BASE+34)
> +#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_STABILIXATION   (1 << 6)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY        (1 << 7)
>
>  #define V4L2_CID_CAMERA_ORIENTATION            (V4L2_CID_CAMERA_CLASS_BASE+34)
>  #define V4L2_CAMERA_ORIENTATION_FRONT          0
> --
> 2.30.0
>

I think we have to add the CID to v4l2_ctrl_get_name()
Sergey Senozhatsky March 17, 2021, 1:34 a.m. UTC | #2
On (21/03/16 19:29), Ricardo Ribalda Delgado wrote:
> > 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_MENU
> > b) ROI rectangle (SET_CUR, GET_CUR, GET_DEF) handling is implemented
> >    separately, by the means of selection API.
> 
> Maybe a reference to the uvc doc would be a good thing to add here.

OK. What should be referenced there?

> > +    * - ``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_STABILIXATION``
> > +      - Image Stabilization.
> > +    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
> > +      - Higher Quality.
> > +
> >
> Nit: Same as before splitting doc and code.

OK, I guess I can split.

> >  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,
> Are

Are?

> >  #define V4L2_CID_PAN_SPEED                     (V4L2_CID_CAMERA_CLASS_BASE+32)
> >  #define V4L2_CID_TILT_SPEED                    (V4L2_CID_CAMERA_CLASS_BASE+33)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO       (V4L2_CID_CAMERA_CLASS_BASE+34)
> > +#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_STABILIXATION   (1 << 6)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY        (1 << 7)
> >
> >  #define V4L2_CID_CAMERA_ORIENTATION            (V4L2_CID_CAMERA_CLASS_BASE+34)
> >  #define V4L2_CAMERA_ORIENTATION_FRONT          0
> > --
> > 2.30.0
> >
> 
> I think we have to add the CID to v4l2_ctrl_get_name()

Sounds good. Only this one - V4L2_CID_REGION_OF_INTEREST_AUTO, right?
Ricardo Ribalda Delgado March 17, 2021, 8:08 a.m. UTC | #3
On Wed, Mar 17, 2021 at 2:34 AM Sergey Senozhatsky
<sergey.senozhatsky.work@gmail.com> wrote:
>
> On (21/03/16 19:29), Ricardo Ribalda Delgado wrote:
> > > 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_MENU
> > > b) ROI rectangle (SET_CUR, GET_CUR, GET_DEF) handling is implemented
> > >    separately, by the means of selection API.
> >
> > Maybe a reference to the uvc doc would be a good thing to add here.
>
> OK. What should be referenced there?

Nevermind, I thought there was a non-pdf version of the standard :(
https://www.usb.org/document-library/video-class-v15-document-set

>
> > > +    * - ``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_STABILIXATION``
> > > +      - Image Stabilization.
> > > +    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
> > > +      - Higher Quality.
> > > +
> > >
> > Nit: Same as before splitting doc and code.
>
> OK, I guess I can split.
>
> > >  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,
> > Are
>
> Are?

Aye!
You are not a good kernel reviewer if you do not talk pirate :P.

I wanted to make sure that V4L2_CTRL_TYPE_BITMASK was handled by the
uvc driver, and I think that it will work fine.
Sorry for the noise.

>
> > >  #define V4L2_CID_PAN_SPEED                     (V4L2_CID_CAMERA_CLASS_BASE+32)
> > >  #define V4L2_CID_TILT_SPEED                    (V4L2_CID_CAMERA_CLASS_BASE+33)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO       (V4L2_CID_CAMERA_CLASS_BASE+34)
> > > +#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_STABILIXATION   (1 << 6)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY        (1 << 7)
> > >
> > >  #define V4L2_CID_CAMERA_ORIENTATION            (V4L2_CID_CAMERA_CLASS_BASE+34)
> > >  #define V4L2_CAMERA_ORIENTATION_FRONT          0
> > > --
> > > 2.30.0
> > >
> >
> > I think we have to add the CID to v4l2_ctrl_get_name()
>
> Sounds good. Only this one - V4L2_CID_REGION_OF_INTEREST_AUTO, right?

I think so :)

Thanks!
Sergey Senozhatsky March 17, 2021, 8:12 a.m. UTC | #4
Fixed Tomasz's email

On (21/03/17 09:08), Ricardo Ribalda Delgado wrote:
[..]
> > > > +               .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,
> > > Are
> >
> > Are?
> 
> Aye!
> You are not a good kernel reviewer if you do not talk pirate :P.

Arr, Matey!

	-ss
Sergey Senozhatsky March 17, 2021, 9:18 a.m. UTC | #5
On (21/02/08 14:17), Sergey Senozhatsky wrote:
> 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_MENU
> b) ROI rectangle (SET_CUR, GET_CUR, GET_DEF) handling is implemented
>    separately, by the means of selection API.

This approach is "no go".

I just figured out (am still debugging tho) that this patch set works on
some devices and doesn't work on other. The root cause seems to be the
fact that some firmwares error out all ROI requests when sizeof() of the
ROI data is not 5 * __u16.

So those devices are not happy if we set/get ROI rectangle 4 * __u16
and auto-controls __u16 separately; they want to set/get rect and
rectanles in one shot.

This fixes ROI on those devices.

---

+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1190,6 +1190,7 @@ struct uvc_roi_rect {
        __u16                   left;
        __u16                   bottom;
        __u16                   right;
+       __u16                   auto_controls;
 };

---

Back to base 1.
Sergey Senozhatsky March 17, 2021, 9:27 a.m. UTC | #6
On (21/03/17 18:18), Sergey Senozhatsky wrote:
> On (21/02/08 14:17), Sergey Senozhatsky wrote:
> > 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_MENU
> > b) ROI rectangle (SET_CUR, GET_CUR, GET_DEF) handling is implemented
> >    separately, by the means of selection API.
> 
> This approach is "no go".
> 
> I just figured out (am still debugging tho) that this patch set works on
> some devices and doesn't work on other. The root cause seems to be the
> fact that some firmwares error out all ROI requests when sizeof() of the
> ROI data is not 5 * __u16.
> 
> So those devices are not happy if we set/get ROI rectangle 4 * __u16
> and auto-controls __u16 separately; they want to set/get rect and
> rectanles in one shot.
> 
> This fixes ROI on those devices.
> 
> ---
> 
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -1190,6 +1190,7 @@ struct uvc_roi_rect {
>         __u16                   left;
>         __u16                   bottom;
>         __u16                   right;
> +       __u16                   auto_controls;
>  };

Maybe I can drop the separate auto-control and just use v4l2_selection::flags
to set the roi auto-controls value.
diff mbox series

Patch

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
index c05a2d2c675d..1593c999c8e2 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
@@ -653,6 +653,31 @@  enum v4l2_scene_mode -
                           |                    |
                           +--------------------+
 
+``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_STABILIXATION``
+      - Image Stabilization.
+    * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
+      - Higher Quality.
+
 
 .. [#f1]
    This control may be changed to a menu control in the future, if more
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/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
index d854cb19c42c..c87624962896 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 039c0d7add1b..6a3dac481cb4 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -976,6 +976,15 @@  enum v4l2_auto_focus_range {
 
 #define V4L2_CID_PAN_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+32)
 #define V4L2_CID_TILT_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+33)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO	(V4L2_CID_CAMERA_CLASS_BASE+34)
+#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_STABILIXATION	(1 << 6)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY	(1 << 7)
 
 #define V4L2_CID_CAMERA_ORIENTATION		(V4L2_CID_CAMERA_CLASS_BASE+34)
 #define V4L2_CAMERA_ORIENTATION_FRONT		0