Re: [PATCH v2] platform/x86: ideapad-laptop: Report camera switch as SW_CAMERA_LENS_COVER

[Mark Pearson]

Hi Rong

On Tue, Sep 22, 2026, at 8:18 AM, Rong Zhang wrote:
> Hi Marco,
>
> On Mon, 2026-09-21 at 21:00 +0200, Marco Giunta wrote:
>> Hi Rong,
>> 
>> Thank you so much for your detailed reply and the much improved patch.
>> 
>> Let me immediately go on record and say that, apart from a small addition
>> to the SMI driver and one line in ideapad-laptop, I am mostly active in
>> the sound system, and therefore consider myself a platform/x86 newbie.
>> So I really appreciate you sharing your knowledge here.
>> 
>> > > * I considered a couple of alternatives to the approach in this patch:
>> > > 
>> > >   1.  Simply map 0x0c and 0x0d to KEY_CAMERA_ACCESS_ENABLE/DISABLE:
>> > > 
>> > >       { KE_KEY,	0x0c | IDEAPAD_WMI_KEY, { KEY_CAMERA_ACCESS_ENABLE } },
>> > >       { KE_KEY,	0x0d | IDEAPAD_WMI_KEY, { KEY_CAMERA_ACCESS_DISABLE } },
>> > > 
>> > >       similar to the touchpad off/on hkey events (66/67) this driver emits
>> > >       after the firmware has toggled the touchpad state. However, HUTRR72
>> > >       describes these hkey events as asserting a camera access state for
>> > >       the host to apply, rather than reporting one the firmware has
>> > >       already applied.
>> > > 
>> > 
>> > Well, it really doesn't matter how the specification says; what matters
>> > is how devices in reality do: whenever the HID Usage Table doesn't define
>> > a usage for hardware/firmware to report the already-applied value but
>> > defines a usage for notifying the host to apply a specific value, a lot
>> > of devices will emerge, using the latter even when the former is
>> > intended.
>> > 
>> > For example, many USB Audio Class devices come with volume control
>> > buttons or knobs. Some of them tune the volume themselves as well as
>> > emitting Volume Increment/Decrement HID events.
>> > 
>> > In this case, the audio stack, in response to the HID events,
>> > intentionally overrides the volume set by hardware, effectively
>> > synchronizing the hardware and software volume value.
>> > 
>> > The pattern has become so common that many devices like that also
>> > implement an internal timeout mechanism. They only set the volume
>> > themselves when reaching the timeout with no UAC volume control request
>> > received.
>> > 
>> > IOW, it doesn't matter whether the hardware/firmware has done its job as
>> > long as the subsequent software control is harmless. That's why we can
>> > safely use KEY_TOUCHPAD_ON/OFF even if the firmware has toggled the
>> > touchpad state. They cause the desktop environment to mask/unmask
>> > touchpad input, which is essentially a no-op (thus a safe operation) when
>> > the firmware has already disabled/enabled the touchpad.
>> > 
>> > Fundamentally speaking, using KEY_CAMERA_ACCESS_ENABLE/DISABLE is also
>> > OK. It's just...
>> > 
>> > > Furthermore, lenovo-wmi-camera moved away from
>> > >       these hkey events in favor of SW_CAMERA_LENS_COVER in the quoted
>> > >       commit, which is arguably the closest relevant precedent.
>> > 
>> > ...relatively new and isn't widely used compared to SW_CAMERA_LENS_COVER.
>> > 
>> > Thus, I agreed that using SW_CAMERA_LENS_COVER is a better approach as it
>> > aligns with other drivers' behavior. The patch seems over-engineered
>> > though, see below.
>> 
>> This seems pretty reasonable to me, I figured that a single standard was
>> hard to enforce and also not strictly needed in practice (the "harmless
>> no-op" argument above).
>> I was mostly wondering about certain discussions I saw online while doing
>> research for this patch of people arguing that such a no-op should be
>> avoided, see e.g.:
>> https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/merge_requests/181
>
> Thanks for the context. I didn't know the behavior of GNOME as I use KDE
> Plasma, whose behavior is unmask/mask touchpad events on
> KEY_TOUCHPAD_ON/OFF.
>
>> 
>> Still, your point that userspace duplicating an action already applied by
>> the hardware is common practice, and fine as long as it is harmless, makes
>> perfect sense to me. So overall I have no objections to your reasoning here.
>> 
>> > > 
>> > >   2.  Ignore these events like the firmware-handled FnLock events:
>> > > 
>> > > 			{ KE_IGNORE,	0x0c | IDEAPAD_WMI_KEY },
>> > > 			{ KE_IGNORE,	0x0d | IDEAPAD_WMI_KEY },
>> > > 
>> > >       This fixes the KEY_UNKNOWN issue, but userspace gets no information.
>> > > 
>> > >   Overall, in the end I copied the approach of the lenovo-wmi-camera
>> > >   driver. Kindly let me know if there are other/better solutions.
>> > > 
>> > > * Unlike the touchpad, whose state this driver reads with VPCCMD_R_TOUCHPAD
>> > >   at probe, on resume and on each event,
>> > > 
>> > 
>> > At least for the input device, the synchronization on probe or on resume
>> > does not matter, as no input event is emitted due to !send_events.
>> 
>> Good point, I was conflating stuff.
>> 
>> > > the camera switch state is only
>> > >   available in EC-private fields, and the two tested laptops even use
>> > >   different ones. Hence the lenovo-wmi-camera approach: the switch appears
>> > >   on the first event, and a change made while suspended is not reported,
>> > >   leaving the state wrong until the switch is toggled again.
>> > 
>> > So the difference compared to the lenovo-wmi-camera approach is only
>> > about when and how the input device is registered.
>> > 
>> > On recent models, the firmware neither emits VPCCMD_R_TOUCHPAD nor
>> > toggles the touchpad. It leaves the job to software by emitting WMI key
>> > event 0x29, which is converted to KEY_TOUCHPAD_TOGGLE via ideapad_keymap.
>> > It'd be tidier to take that approach and reuse the existing input device.
>> > Sparse keymap supports KEY_SW, so it's viable to map the two WMI events
>> > to SW_CAMERA_LENS_COVER with appropriate switch value.
>> 
>> This makes a lot of sense to me. I appreciate the simplicity of this
>> approach.
>> 
>> Part of the reason why I initially adopted the "separate input device"
>> approach was this comment from
>> drivers/platform/x86/dell/dell-wmi-privacy.c:
>> 
>> /*
>>  * Unlike keys where only presses matter, userspace may act
>>  * on switches in both of their positions. Only register
>>  * SW_CAMERA_LENS_COVER if it is actually there.
>>  */
>
> Hmm, this makes some sense. Ideally we should prevent exposing
> SW_CAMERA_LENS_COVER unless it's there. I wonder, if the capability is
> declared in LENOVO_UTILITY_DATA? AFAIK, LENOVO_UTILITY_EVENT (8FC0DE0C-
> B4E4-43FD-B0F3-8871711C1294) shares the same ACPI device as
> LENOVO_UTILITY_DATA (CE6C0974-0407-4F50-88BA-4FC3B6559AD8). The former's
> driver is ideapad-laptop, while latter's driver is lenovo-wmi-hotkey-
> utilities.
>
>    [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"), 
> Description("Lenovo Utility Key Press Event"), 
> guid("{8fc0de0c-b4e4-43fd-b0f3-8871711c1294}")]
>    class LENOVO_UTILITY_EVENT : WMIEvent {
>      [key, read] string InstanceName;
>      [read] boolean Active;
>      [WmiDataId(1), read, Description("Lenovo Utility Press key 
> Event")] uint32 PressTypeDataVal;
>    };
>   
>    [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"), 
> Description("LENOVO_UTILITY_DATA class"), 
> guid("{ce6c0974-0407-4f50-88ba-4fc3b6559ad8}")]
>    class LENOVO_UTILITY_DATA {
>      [key, read] string InstanceName;
>      [read] boolean Active;
>   
>      [WmiMethodId(1), Implemented, Description("Utility 3.1 function is 
> Support or the function Version")] void GetIfSupportOrVersion([in, 
> Description("Control type")] uint32 datatype, [out, Description("0 is 
> not support 1,2,3...version")] uint32 Data);
>      [WmiMethodId(2), Implemented, Description("Utility 3.1 Set feature 
> function ")] void SetFeature([in, Description("Control type")] uint32 
> featuretype, [out, Description("Control result 0 is success 1,2,3...is 
> failed")] uint32 Data);
>      [WmiMethodId(3), Implemented, Description("Utility 3.2 Set feature 
> function ")] void SetFeatureEx([in] uint32 IDs, [in] uint32 Value, 
> [out, Description("result 0 is success 1,2,3...is failed")] uint32 Ret);
>    };
>
>
> I just checked several DSDT tables from different devices (they don't
> have camera switches though). The ACPI device is usually \_SB.WMIU. The
> former's WMI event is usually implemented by \_SB.WMIU._WED, while the
> latter's WMI methods are usually implemented by \_SB.WMIU.WMSK. There may
> be some capabilities defined in the first WMI method, and the WMI event
> may also reveal some details in EC query handling.
>
> Could you dump the ACPI tables of the two devices and attach the acpidump
> files in your reply?
>
> As you've said, "the camera switch state is only available in EC-private
> fields, and the two tested laptops even use different ones." Could you
> elaborate it as well? Such information may be helpful when cross-
> referencing ACPI tables.
>

I'm way behind on my emails and trying to get through my inbox a bit...so apologies if I've missed subtleties as I skim read this a bit

Reading the internal spec - there are some WMI calls I can see that return Camera shutter status:
LENOVO_OTHER_METHOD (GUID dc2a8805-3a8c-41ba-a6f7-092e0089cd3b)
Get CV DSP Camera Shutter Status - Input ID 0x00200000 - returns 0 (closed) or 1 (open)
Get Normal Camera Shutter Status - Input ID 0x00201000 - returns 0 (closed) or 1 (open)

I can't find the Utility GUIDs mentioned at all I'm afraid. Is the above useful?

When I get a chance I'll check this out on a Legion platform I have that has this switch and confirm....but it might take a little while.

Mark