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

Hans de Goede
2024-11-11T12:59:43Z

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.