Hi Ricardo, On Wed, Oct 30, 2024 at 09:47:17PM +0100, Ricardo Ribalda wrote: > Hi Sakari > > On Wed, 30 Oct 2024 at 16:04, Ricardo Ribalda <ribalda@xxxxxxxxxxxx> wrote: > > > > Hi Sakari > > > > On Wed, 30 Oct 2024 at 15:25, Sakari Ailus <sakari.ailus@xxxxxx> wrote: > > > > > > Hi Ricardo, > > > > > > On Wed, Oct 30, 2024 at 02:03:18PM +0100, Ricardo Ribalda wrote: > > > > Hi Hans (de Goede, but others are welcome as well :) ) > > > > > > > > Some notebooks have a button to disable the camera (not to be mistaken > > > > with the mechanical cover). This is a standard GPIO linked to the > > > > camera via the ACPI table. > > > > > > > > 4 years ago we added support for this button in UVC via the Privacy control. > > > > This has two issues: > > > > - If the camera has its own privacy control, it will be masked > > > > - We need to power-up the camera to read the privacy control gpio. > > > > > > > > We tried to fix the power-up issues implementing "granular power > > > > saving" but it has been more complicated than anticipated.... > > > > > > > > Last year, we proposed a patchset to implement the privacy gpio as a > > > > subdevice https://lore.kernel.org/linux-media/20230111-uvc_privacy_subdev-v1-0-f859ac9a01e3@xxxxxxxxxxxx/ > > > > > > > > I think it is a pretty clean solution and makes sense to use a > > > > subdevice for something that is a sub device of the camera :). > > > > > > > > Before moving forward, Laurent and I would like to hear your opinion. > > > > > > I remember we discussed this and I wasn't very enthusiastic about the > > > proposal but thinking about it again, there are situations where this is > > > the only feasible solution, including on raw cameras with a privacy GPIO, > > > besides the first issue you brought up above. > > > > > > Regarding the second one, why would you need to power on the camera to get > > > the GPIO's value? > > > > In order to read the control, you need to open the device, and once > > you open the device the device is powered up: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/uvc/uvc_v4l2.c#n621 > > > > In a perfect world it would only be powered up during streamon(), but > > uvc is a complicated monster: > > - We have delayed controls > > - We have buttons > > If we only power up the device during streamon those things would not > > work properly. > > > > I think I have a solution for delayed controls...assuming the device > > implements the standard properly. > > I have no solution to support buttons :(. Luckily not that many > > cameras have that today > > > > Regards! > > > > > > > > > > I'll review the set. > > If you have time to review something, please take a look instead at: > https://gitlab.freedesktop.org/linux-media/users/ribalda/-/tree/uvc-subdevice?ref_type=heads > > Once I test that on real hardware is what I plan to send to the ML. > > It has support for the ancillary link and it is a bit cleaner. Could you post that to the list, with a note it's not been tested? -- Regards, Sakari Ailus