Re: Lenovo WMI camera driver, switching to SW_CAMERA_LENS_COVER ?

[Hans de Goede <hdegoede@redhat.com>]

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.

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@redhat.com/
>>
>> 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