Re: [PATCH v2 0/6] media: uvcvideo: Implement the Privacy GPIO as a subdevice

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 25-Nov-24 1:56 PM, Laurent Pinchart wrote:
> On Mon, Nov 25, 2024 at 01:31:49PM +0100, Hans de Goede wrote:
>> Hi Ricardo,
>>
>> On 10-Nov-24 5:04 PM, Ricardo Ribalda wrote:
>>> On Sun, 10 Nov 2024 at 16:14, Laurent Pinchart wrote:
>>
>> <snip>
>>
>>>>> Can we start powering up the device during try/set fmt and then
>>>>> implement the format caching as an improvement?
>>>>
>>>> This sounds worth trying. We'll need to test it on a wide range of
>>>> devices though, both internal and external.
>>
>> Ack, as mentioned in the other mail which I just send I think
>> this is worth trying.
>>
>>> We still need a plan for asynchronous controls.
>>
>> As I mentioned in that other email I think we can do the same there.
>>
>> So basically delay powering up the camera from /dev/video# open till
>> the first moment we actually need to communicate to the camera and
>> track per file-handle if we did a usb_autopm_get_interface() for
>> that file-handle and if yes, then do the put-interface on file-handle
>> close.
>>
>>> And we have to decide if we stop supporting the uvc button (maybe we
>>> can start by moving USB_VIDEO_CLASS_INPUT_EVDEV to staging and see
>>> what happens?)
>>
>> As I mentioned in other threads I do not think that the button
>> only working changing from:
>>
>> "only works when /dev/video# is open"
>>
>> to:
>>
>> "only works when streaming from /dev/video#"
>>
>> (or actually only works when some action on the camera which
>> requires it to be powered-on has been done).
>>
>> is a big deal, since most apps which open /dev/video# for
>> a longer time will almost always do so to actually do something
>> with the camera, at which point the button will work just as
>> before.
>>
>> And for apps which only do a short-lived open of /dev/video#
>> the button does not work with the current code either.
>>
>> TL;DR: IMHO it is fine if the button only works when streaming.
> 
> I'm fine with that, we can reconsider if people complain. It would be
> painful though, as it could mean reverting everything we'll build
> related to power management from now on until someone notices the new
> behaviour, which could easily take a year. The risk is low, but the
> consequences serious.

I think that if some users complain we can just add a default off module
option to restore the old behavior for use-cases which somehow depend
on that.

Doing an extra usb_autopm_get_interface() + usb_autopm_put_interface()
at open/close() time if the option is set is easy, and that will just
render out other get() and put() calls into no-ops.

So we always have that route as an escape-hatch.

Regards,

Hans