Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 17 Mar 2021 18:18:11 +0900
From:   Sergey Senozhatsky <senozhatsky@chromium.org>
To:     Tomasz Figa <tfiga@chromium.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>
Cc:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
        Sergey Senozhatsky <senozhatsky@chromium.org>,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: Re: [PATCHv2 2/3] media: uvcvideo: add ROI auto controls
Message-ID: <YFHJUyNA53J72bVA@google.com>
References: <20210208051749.1785246-1-sergey.senozhatsky@gmail.com>
 <20210208051749.1785246-3-sergey.senozhatsky@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210208051749.1785246-3-sergey.senozhatsky@gmail.com>
Precedence: bulk

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.