On Mon, Nov 25, 2024 at 01:39:05PM +0100, Hans de Goede wrote: > Hi Ricardo, > > On 10-Nov-24 5:07 PM, Ricardo Ribalda wrote: > > On Sun, 10 Nov 2024 at 16:14, Laurent Pinchart > > <laurent.pinchart@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxx/ > > 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. Is that right ? Isn't it better for an application to keep the device open to avoid open delays or even open failures when it wants to use the device ? > Although looking back at the libcamera uvc pipeline handler issue > I fixed recently, that was actually a case of 3. -- Regards, Laurent Pinchart
