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

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

Hi Ricardo,

On 10-Nov-24 5:07 PM, Ricardo Ribalda wrote:
> On Sun, 10 Nov 2024 at 16:14, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:

<snip>

>>>> Here is what I have in mind for this:
>>>>
>>>> 1. Assume that the results of trying a specific fmt do not change over time.
>>>>
>>>> 2. Only allow userspace to request fmts which match one of the enum-fmts ->
>>>>    enum-frame-sizes -> enum-frame-rates tripplet results
>>>>    (constrain what userspace requests to these)
>>>>
>>>> 3. Run the equivalent of tryfmt on all possible combinations (so the usaul
>>>>    3 levels nested loop for this) on probe() and cache the results
>>>>
>>>> 4. Make try_fmt / set_fmt not poweron the device but instead constrain
>>>>    the requested fmt to one from our cached fmts
>>>>
>>>> 5. On stream-on do the actual power-on + set-fmt + verify that we get
>>>>    what we expect based on the cache, and otherwise return -EIO.
>>>
>>> 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.
> 
> For what is worth, we have been running something similar to
> https://lore.kernel.org/linux-media/20220920-resend-powersave-v5-2-692e6df6c1e2@chromium.org/
> in ChromeOS and it has worked fine....
> 
> But I am pretty sure that it has issues with async controls :S

Interesting that is actually a lot more aggressive (as in doing a
usb_autopm_put_interface() often) then what I expected when you said:

"Can we start powering up the device during try/set fmt"

As I mentioned in my other emails what I think would worth nicely
is delay the initial usb_autopm_get_interface() till we need it
and then just leave the camera on till /dev/video# gets closed.

That idea is based on dividing apps in 2 groups:

1. Apps just temporarily opening /dev/video# nodes for discovery,
where we ideally would not power-up the camera.

2. Apps (could even be the same app) opening /dev/video# for
a longer time because it actually want to use the camera
at the moment of opening and which close the /dev/video# node
when done with the camera.

Your code seems to also covers a 3th group of apps:

3. Apps opening /dev/video# for a long time, while not using
it all the time.

Although it would be nice to also cover those, IMHO those are
not well behaved apps and I'm not sure if we need to cover those.

Although looking back at the libcamera uvc pipeline handler issue
I fixed recently, that was actually a case of 3.

Regards,

Hans