Hi Armin, On 18-Nov-24 7:34 PM, Armin Wolf wrote: > Am 18.11.24 um 16:02 schrieb Hans de Goede: > >> Hi Armin, ai, >> >> I'm answering to both your replies in one go here since they >> are related. This also re-adds the list for Ai's reply which >> dropped the list from the Cc. >> >> On 12-Nov-24 4:16 AM, 艾超 wrote: >>> We have discussed this question(initial state of the camera) with the >>> >>> hardware enginner about SW_CAMERA_LENS_COVER. >>> >>> Driver need a WMI events to report current camera state to desktop-environment, >>> >>> and no need to get the old camera state. Whether the camera lens cover on/off , the camera >>> streaming is normal. >> Ok, so this means that if lenovo-wmi switch gets moved to use SW_CAMERA_LENS_COVER >> it will need to do the same thing as which hp-wmi is doing and delay registering >> the input device with the SW_CAMERA_LENS_COVER capability until the first idea. >> >> Do I understand correctly that the GPIO triggering the WMI events is attached to >> the mechanical lens cover and basically reports the location of the cover ? >> >> In that case switching to SW_CAMERA_LENS_COVER would definitely be the right >> thing to do. > > I believe that the GPIO is connected to a general "Enable/Disable cameras" button, but if the GPIO > is indeed connected to some sort of lens cover then i agree with sending "SW_CAMERA_LENS_COVER". > >> On 11-Nov-24 7:37 PM, Armin Wolf wrote: >>> Am 11.11.24 um 15:33 schrieb Hans de Goede: >>> >>>> Hi Ai, >>>> >>>> I have been looking into all the different way the kernel is >>>> currently communicating to userspace that a camera is disabled >>>> by some form of on/off switch / privacy control, see: >>>> >>>> https://lore.kernel.org/linuxa-media/a644fed4-aff5-4514-8e35-d6cab642d3dd@xxxxxxxxxx/ >>>> >>>> As I mention there my proposal is to standardize on >>>> SW_CAMERA_LENS_COVER. This assumes that the button >>>> which generates the WMI events actually enables / >>>> disables the camera the hardware level. >>> Hi, >>> >>> i think SW_CAMERA_LENS_COVER is misleading here, since AFAIK the camera completely disappears >>> from the USB bus. With a proper lens cover however, the camera would still be accessible for >>> querying things like supported resolutions, etc. >> As Ai mentions above the camera keeps streaming normally looking ay >> the original commit message for the driver which says: >> >> """ >> Add lenovo WMI camera button driver to support camera button. >> The Camera button is a GPIO device. This driver receives ACPI notifications >> when the camera button is switched on/off. This driver is used in >> Lenovo A70, it is a Computer integrated machine. >> """ >> >> We are talking about a GPIO attached to some sort of on/off contact >> here not normal keypresses. And this is for a so called All-In-One >> device (a monitor with a PC builtin) e.g. something like this: >> >> https://c1.neweggimages.com/ProductImageCompressAll1280/83-265-019-01.jpg >> >> Notice how there is a camera-cover which the user can slide over the cam, >> I presume the GPIO reports the position of the camera.\ >> >> Ai can you confirm this? >> >> Also note that these AIO devices use normal keyboards which typically do >> not have any camera on/off key. So I really believe that this WMI driver >> is for a physical switch co-located with the camera itself and thus >> reporting SW_CAMERA_LENS_COVER is the right thing to do. >> >> Armin, You are right that there are cases where the camera completely >> drops of the USB bus when some laptop camera hotkey gets pressed on >> laptop keyboards. But I do not believe that that is the case here. >> >>> Also i believe that the intent of the buttons handled by this driver is to disable access to >>> all cameras connected to the system, so KEY_CAMERA_ACCESS_ENABLE / KEY_CAMERA_ACCESS_DISABLE >>> is necessary here if external USB cameras are connected. >> As you say on laptops these keys sometimes drop the camera completely >> from the USB bus, so the key clearly works on the internal camera >> not all cameras. >> >> What to do in those cases is a bit offtopic here, but even there >> I would like to see us move to SW_CAMERA_LENS_COVER especially >> since sometimes there is no info when we get the key press event >> if the camera was added or removed. So what I would like to do >> there is have some in kernel helper to look for an internal USB >> camera and if there is none report SW_CAMERA_LENS_COVER=1 and >> then when the camera comes back report SW_CAMERA_LENS_COVER=0 >> at the moment on these laptops we just report "KEY_CAMERA" which >> really is not helpful for userspace. >> >> Userspace could do the look for internal camera thing itself >> but I would rather solve this once in the kernel and use >> SW_CAMERA_LENS_COVER consistently everywhere. I know dropping >> the camera from the bus is not technically a lens-cover but >> the important thing is to let userspace camera using apps >> know that they need to ask the user to enable the camera. >> >> ATM this is a bit of a mess mixing SW_CAMERA_LENS_COVER + >> KEY_CAMERA + KEY_CAMERA_ACCESS_[DIS|EN]ABLE and I would like >> to try fixing this before userspace starts relying on the >> current broken status quo. >> >> Regards, >> >> Hans > > I agree with you here regarding the "dropping the camera of the bus" type of devices. > > However we might need to differentiate between buttons which are designed to: > > - disable all cameras (like some sort of configuration shortcut) > > - notify that the camera was disabled (and possible dropped of the bus) > > In the first case i would continue to use KEY_CAMERA_ACCESS_[DIS|EN]ABLE, while in the second > case we could indeed use SW_CAMERA_LENS_COVER. I agree. > The point is that we need to know the purpose of the > button/event. For ACPI / WMI / EC drivers it will pretty much always be about the device's internal camera. I would only expect "disable all cameras (like some sort of configuration shortcut)" to be seen on some external keyboards, not on laptop builtin keyboards. On some of the laptops they camera on/off hotkey even has a little LED in there to indicate when the camera is turned off just like the mute LED sometimes found in mic / speaker mute hotkeys. Regards, Hans
