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 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 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.
