Em Fri, 08 Nov 2024 20:25:44 +0000 Ricardo Ribalda <ribalda@xxxxxxxxxxxx> escreveu: > 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 :). > > This is an attempt to continue with that approach. > > Tested on gimble: > gimble-rev3 ~ # v4l2-ctl --all -d /dev/v4l-subdev0 No matter if internally implemented as a subdevice or not, UVC is not a MC-centric device[1]. It means that UVC can be compiled without media controller support, and that its functionality shall be visible via /dev/video* nodes. So, whatever internal implementation it is used, it shall not require config MEDIA_CONTROLLER and the control shall be visible via /dev/video*. Moving privacy control out of /dev/video would mean that this will break support for it on existing applications, which is a big nack. Now, it would be acceptable to have this visible via V4L2 and subdev APIs. [1] https://www.kernel.org/doc/html/latest/userspace-api/media/glossary.html#term-MC-centric Regards, Mauro > Driver Info: > Driver version : 6.6.56 > Capabilities : 0x00000000 > Media Driver Info: > Driver name : uvcvideo > Model : HP 5M Camera: HP 5M Camera > Serial : 0001 > Bus info : usb-0000:00:14.0-6 > Media version : 6.6.56 > Hardware revision: 0x00009601 (38401) > Driver version : 6.6.56 > Interface Info: > ID : 0x0300001d > Type : V4L Sub-Device > Entity Info: > ID : 0x00000013 (19) > Name : GPIO > Function : Unknown sub-device (00020006) > > Camera Controls > > privacy 0x009a0910 (bool) : default=0 value=0 flags=read-only, volatile > > gimble-rev3 ~ # media-ctl -p > Media controller API version 6.6.56 > > Media device information > ------------------------ > driver uvcvideo > model HP 5M Camera: HP 5M Camera > serial 0001 > bus info usb-0000:00:14.0-6 > hw revision 0x9601 > driver version 6.6.56 > > Device topology > - entity 1: HP 5M Camera: HP 5M Camera (1 pad, 1 link) > type Node subtype V4L flags 1 > device node name /dev/video0 > pad0: Sink > <- "Extension 8":1 [ENABLED,IMMUTABLE] > > - entity 4: HP 5M Camera: HP 5M Camera (0 pad, 0 link) > type Node subtype V4L flags 0 > device node name /dev/video1 > > - entity 8: Extension 8 (2 pads, 2 links, 0 routes) > type V4L2 subdev subtype Unknown flags 0 > pad0: Sink > <- "Extension 4":1 [ENABLED,IMMUTABLE] > pad1: Source > -> "HP 5M Camera: HP 5M Camera":0 [ENABLED,IMMUTABLE] > > - entity 11: Extension 4 (2 pads, 2 links, 0 routes) > type V4L2 subdev subtype Unknown flags 0 > pad0: Sink > <- "Processing 2":1 [ENABLED,IMMUTABLE] > pad1: Source > -> "Extension 8":0 [ENABLED,IMMUTABLE] > > - entity 14: Processing 2 (2 pads, 2 links, 0 routes) > type V4L2 subdev subtype Unknown flags 0 > pad0: Sink > <- "Camera 1":0 [ENABLED,IMMUTABLE] > pad1: Source > -> "Extension 4":0 [ENABLED,IMMUTABLE] > > - entity 17: Camera 1 (1 pad, 1 link, 0 routes) > type V4L2 subdev subtype Sensor flags 0 > pad0: Source > -> "Processing 2":0 [ENABLED,IMMUTABLE] > > - entity 19: GPIO (0 pad, 0 link, 0 routes) > type V4L2 subdev subtype Decoder flags 0 > device node name /dev/v4l-subdev0 > > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx> > --- > Changes in v2: > - Rebase on top of https://patchwork.linuxtv.org/project/linux-media/patch/20241106-uvc-crashrmmod-v6-1-fbf9781c6e83@xxxxxxxxxxxx/ > - Create uvc_gpio_cleanup and uvc_gpio_deinit > - Refactor quirk: do not disable irq > - Change define number for MEDIA_ENT_F_GPIO > - Link to v1: https://lore.kernel.org/r/20241031-uvc-subdev-v1-0-a68331cedd72@xxxxxxxxxxxx > > --- > Ricardo Ribalda (5): > media: uvcvideo: Factor out gpio functions to its own file > Revert "media: uvcvideo: Allow entity-defined get_info and get_cur" > media: uvcvideo: Create ancillary link for GPIO subdevice > media: v4l2-core: Add new MEDIA_ENT_F_GPIO > media: uvcvideo: Use MEDIA_ENT_F_GPIO for the GPIO entity > > Yunke Cao (1): > media: uvcvideo: Re-implement privacy GPIO as a separate subdevice > > .../userspace-api/media/mediactl/media-types.rst | 4 + > drivers/media/usb/uvc/Makefile | 3 +- > drivers/media/usb/uvc/uvc_ctrl.c | 40 +---- > drivers/media/usb/uvc/uvc_driver.c | 123 +------------- > drivers/media/usb/uvc/uvc_entity.c | 20 ++- > drivers/media/usb/uvc/uvc_gpio.c | 187 +++++++++++++++++++++ > drivers/media/usb/uvc/uvc_video.c | 4 + > drivers/media/usb/uvc/uvcvideo.h | 34 ++-- > drivers/media/v4l2-core/v4l2-async.c | 3 +- > include/uapi/linux/media.h | 1 + > 10 files changed, 252 insertions(+), 167 deletions(-) > --- > base-commit: 4353256f5487e0c5c47e8ff764bf4f9e679fb525 > change-id: 20241030-uvc-subdev-89f4467a00b5 > > Best regards, Thanks, Mauro