p.s. On 11-Nov-24 1:59 PM, Hans de Goede wrote: > Hi Ricardo, Et al., > > On 8-Nov-24 9:25 PM, Ricardo Ribalda wrote: >> 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. > > Thinking more about this I think we need to start with looking at the userspace > API for privacy controls, define how we want that to look and then go from > there. > > The reason I'm writing this is because due to my work in drivers/platform/x86 > (pdx86) on EC / ACPI / WMI drivers for non chromebooks I am aware of at least > 4 different methods camera on/off (aka privacy) toggles are being reported > to userspace at the moment. Adding a v4l2-ctrl on a subdev instead of directly > on /dev/video# would be adding a 5th method which seems highly undesirable. > > Instead I would like to first focus on fixing these userspace API > inconsistencies agreeing on a single API we want to use everywhere > going forward. We don't need to fix all drivers at once, but IMHO we > should agree on what the API should look like and document that and > any future drivers implementing camera privacy control related code > then must use the new API. > > Lets start with the 3 APIs I'm currently aware of: > > 1. uvcvideo driver exporting V4L2_CID_PRIVACY on /dev/video# > uvcvideo seems to be the only user of this CID (i) > > 2. pdx86 drivers exporting an input evdev with EV_SW, > SW_CAMERA_LENS_COVER. This is somewhat of a special case > for some Dell laptops with an electro-mechanical shutter > operated by the EC. But this is not also used by > hp-wmi.c where it does not necessarily indicate the s/not/now/ > status of a mechanical cover, but also possibly simply > disconnecting the camera from the USB bus. > > 3. pdx86 drivers exporting an input evdev with EV_KEY, > KEY_CAMERA_ACCESS_ENABLE, KEY_CAMERA_ACCESS_DISABLE > These KEY codes are based on offical the HUTRR72 HID/HUT > extension and as such may also be send by USB/I2C/BT HID > devices. > > The only user outside of hid-input.c is the recently added > drivers/platform/x86/lenovo-wmi-camera.c driver and I'm > wondering if that should not use SW_CAMERA_LENS_COVER > instead. I'll ask the driver author about how this I have send an email out about possibly switching this to SW_CAMERA_LENS_COVER : https://lore.kernel.org/platform-driver-x86/5666914c-e8c2-481d-8fdf-aff82865c228@xxxxxxxxxx/ Regards, Hans > 4. pdx86 drivers exporting an input evdev with EV_KEY, > KEY_CAMERA. Note this 4th method lacks information on if > the camera was enabled or disabled. In many cases this > is send to indicate that the EC has either dropped > a UVC camera of the bus, or added it to the bus. > Ideally we would have some helper checking for internal > UVC camera presence and turn this into 2 or 3. > > TL;DR: it a mess. > > Circling back to this patch-set, note how 3 of the 4 > currently in use variants today use in input evdev. > > I think that using an input evdev (shared with the > snapshot button if present) will give us a nice out for > the power-management issue with the V4L2_CID_PRIVACY, > while at the same time giving a nice opportunity to > standardize on a single userspace API. > > My proposal would be to standardize on SW_CAMERA_LENS_COVER > I realize that the GPIO does not always indicate a lens > cover, but the resulting black frames are the same result > as if there were a lens cover and looking at: > > https://support.hp.com/ie-en/document/ish_3960099-3335046-16 > > and then the second picture when expanding "Locate and use > the webcam privacy switch" that does look like it may be > an actual cover which reports back its state through a GPIO. > > The reason why I'm not in favor of using > KEY_CAMERA_ACCESS_ENABLE + KEY_CAMERA_ACCESS_DISABLE is that > looking at the HUTRR72 it talks about: > "Enables programmatic access to camera device" > which suggests that it is a request to the OS / desktop- > environment to block camera access at the software level, > rather then reporting back that a hw-level block is in place. > > And since these may be used by any HID device we are not of > control in how these will be used. > > Ricardo, what do you think of instead of using a v4l-subdev, > using an input evdev (shared with the existing one) reporting > SW_CAMERA_LENS_COVER ? The v4l-subdev approach will need > userspace changes anyways and if we are going to make userspace > changes we might as well use the best API available. > > One downside of going the evdev route is that it is a bit > harder for userspace to map the evdev to a camera: > > 1. For the various WMI interfaces this already is impossible, > and just to show a notification it is not necessary (using > an external cam will make things weird though). > > 2. For UVC cameras mapping the evdev to the /dev/video# > node can still be done by looking if they share a parent > USB interface. This is e.g. already done in apps like > xawtv looking at the PCI parent to pair up /dev/video# > for video capture with the ALSA interface exposed for > sound by bttv cards. > > 3. We can maybe do something at the media-controller > level to help userspace linking a camera to its evdev node. > This would also be helpful for the existing WMI interfaces. > > Regards, > > Hans > > > > i) With the exception of drivers/media/pci/intel/ivsc/mei_csi.c > which has a V4L2_CID_PRIVACY control which always reads 0, so > I guess we can / should probably drop that. > > > > p.s. > > I do plan to also get back to you on the actually powermanagement > discussion. But only so many hours in a day, so it will probably > be a couple of days. > >
