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. The point is that we need to know the purpose of the
button/event.
Thanks,
Armin Wolf
