mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Hans de Goede <hansg@kernel.org>
Cc: Michael Jordan <jordan.mymail@gmail.com>,
	Ricardo Ribalda <ribalda@chromium.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil+cisco@kernel.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/3] media: uvcvideo: live pan/tilt/zoom readback on the OBSBOT Tiny 2
Date: Mon, 28 Sep 2026 14:49:00 +0300	[thread overview]
Message-ID: <20260928114900.GC157191@killaraus.ideasonboard.com> (raw)
In-Reply-To: <f8134f74-9f32-4c98-96b3-ceeb978abb11@kernel.org>

On Mon, Sep 28, 2026 at 01:01:11PM +0200, Hans de Goede wrote:
> Hi All,
> 
> On 2-Sep-26 02:25, Michael Jordan wrote:
> > The OBSBOT Tiny 2's GET_INFO strips the AUTOUPDATE bit from its pan,
> > tilt and zoom controls, so uvcvideo caches them and userspace can never
> > observe the actuator's live state. This series reports AUTO_UPDATE
> > controls as volatile to userspace, generalises the existing XU flags
> > fixup table to all controls, and adds entries for the three affected
> > controls on this camera.
> > 
> > Changes in v2 (following Ricardo's review of v1 [1]):
> > 
> > - Patch 1: unchanged; picked up Ricardo's Reviewed-by.
> > - Patch 2: uvc_ctrl_fixup_flags() now returns bool and runs at the
> >   start of uvc_ctrl_get_flags(), before the allocation, skipping the
> >   GET_INFO query entirely for controls the table covers (Ricardo).
> > - Patch 3: as asked, I checked whether the camera's other
> >   AUTO_UPDATE-flagged controls suffer the same bug. Two more do:
> >   CT_PANTILT_RELATIVE and CT_ZOOM_ABSOLUTE, both verified on hardware
> >   to change autonomously and report live values on GET_CUR; entries
> >   added for both. Exposure, white balance and focus turned out to be
> >   write-only on this firmware (their autos work, but GET_CUR echoes the
> >   last SET_CUR), so they gain nothing from AUTO_UPDATE and were left
> >   alone; details in the commit message. The commit message also no
> >   longer claims the capability byte is the same for every control --
> >   probing every control showed it is computed per control, just wrong
> >   for the PTZ ones. Dropped Ricardo's Reviewed-by since the patch
> >   changed materially.
> > 
> > The full lsusb -v output was posted in reply to v1's patch 3, in the
> > thread at [1].
> > 
> > [1] https://lore.kernel.org/linux-media/20260828152557.653475-1-jordan.mymail@gmail.com/
> 
> Michael, thank you for the patch. Patches 1/2 look good to me:
> 
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> 
> Ricardo (and Michael, I wonder, in the light of Michael already having
> found a second camera with the same issue and also in the light of your
> "media: uvcvideo: Automatically handle invalid uvc_versions" series
> if it would not be better to try to fix this up automatically instead
> of relying on device quirks?
> 
> Specifically the UVC_CTRL_FLAG_AUTO_UPDATE flag is already there
> in the default flags for these controls (and a bunch of others)
> in uvc_ctrls[].

The UVC_CTRL_FLAG_AUTO_UPDATE flag in uvc_ctrls indicates which control
*may* be auto-update controls. In many devices, lots of those will not
be auto-update controls. For instance, UVC_PU_HUE_CONTROL may only
auto-update when auto-hue is enabled. Devices that implement manual hue
only will not update this control.

We use the flags in uvc_ctrls as a fallback if getting control info from
the device fails (in uvc_ctrl_get_flags()). I think that's the right
thing to do.

> I wonder if we should simply always honor UVC_CTRL_FLAG_AUTO_UPDATE
> from uvc_ctrls[] even when we do get a valid GET_INFO request and
> simply or in the UVC_CTRL_FLAG_AUTO_UPDATE from uvc_ctrls[] if it
> is there?
> 
> That avoids needing to adding more and more quirks for this.
> 
> Regards,
> 
> Hans
> 
> > Michael Jordan (3):
> >   media: uvcvideo: report AUTO_UPDATE controls as volatile
> >   media: uvcvideo: generalise the XU flags fixup to all controls
> >   media: uvcvideo: fix up missing AUTO_UPDATE on the OBSBOT Tiny 2
> > 
> >  drivers/media/usb/uvc/uvc_ctrl.c | 122 ++++++++++++++++++++-----------
> >  1 file changed, 81 insertions(+), 41 deletions(-)

-- 
Regards,

Laurent Pinchart

      parent reply	other threads:[~2026-09-28 11:49 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02  0:25 Michael Jordan
2026-09-02  0:25 ` [PATCH v2 1/3] media: uvcvideo: report AUTO_UPDATE controls as volatile Michael Jordan
2026-09-02  0:25 ` [PATCH v2 2/3] media: uvcvideo: generalise the XU flags fixup to all controls Michael Jordan
2026-09-02  6:38   ` Ricardo Ribalda
2026-09-28 11:44   ` Laurent Pinchart
2026-09-28 14:34     ` Michael Jordan
2026-09-02  0:25 ` [PATCH v2 3/3] media: uvcvideo: fix up missing AUTO_UPDATE on the OBSBOT Tiny 2 Michael Jordan
2026-09-02  6:34   ` Ricardo Ribalda
2026-09-27 22:06     ` Michael Jordan
2026-09-28  6:57       ` Ricardo Ribalda
2026-09-28 11:18         ` Ricardo Ribalda
2026-09-28 14:34           ` Michael Jordan
2026-09-28 11:57   ` Laurent Pinchart
2026-09-28 14:34     ` Michael Jordan
2026-09-28 14:55       ` Hans de Goede
2026-09-28 18:53       ` Laurent Pinchart
2026-09-28 11:01 ` [PATCH v2 0/3] media: uvcvideo: live pan/tilt/zoom readback " Hans de Goede
2026-09-28 11:12   ` Ricardo Ribalda
2026-09-28 11:14     ` Hans de Goede
2026-09-28 11:49   ` Laurent Pinchart [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260928114900.GC157191@killaraus.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=hansg@kernel.org \
    --cc=hverkuil+cisco@kernel.org \
    --cc=jordan.mymail@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=ribalda@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®