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 12:42 PM, Rong Zhang wrote:
> Hi Mark,
>
> On Tue, 2026-09-22 at 23:58 +0800, Rong Zhang wrote:
>> Hi Mark,
>> 
>> On Tue, 2026-09-22 at 11:52 -0400, Mark Pearson wrote:
>> > 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 C 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?
>> 
>> Thanks for the information. It's useful! I will cross-check with the DSL
>> code of the correlated WMI method.
>
> I just checked a DSDT dump from 83JR [1], and it does implement
> 0x00200000!
>
> In detail, LENOVO_CAPABILITY_DATA_00 exposes 0x00200000 as
> valid+read+write conditionally, while LENOVO_OTHER_METHOD can be used to
> get/set the camera shutter.
>
> Still, I have three questions. Could you kindly check the internal
> specification and shed some light?
>
> What's the difference between "CV DSP Camera Shutter" and the "Normal"
> one? I guess the former is a firmware switch while the latter is a
> hardware switch, am I correct?
>
The Normal ID is new and was just added in the latest version of the spec.
It's possible it is on a not-released platform yet.
As a note - the 'Normal" entry is get only.

Afraid my hooks into the Yoga and Legion team are still quite new and we're figuring out some of the pieces needed at the moment :)
The WMI spec seems to cover a lot of different platforms that I've not had much experience with too.

> The device does not implement LENOVO_CAPABILITY_DATA_01 at all. Is it
> possible to determine its existence programmatically using
> LENOVO_CAPABILITY_DATA_00? Missing the former is fatal in the current
> lenovo-wmi-capdata and lenovo-wmi-other implementations, resulting in
> both driver non-functional. We need a programmatic way to tolerate its
> non-existence so that we can bind the two drivers via the component
> framework in any case.
>
It should be under CAPABILITY DATA_00, at least from what I'm seeing (pasting a table in text....):

CV DSP Camera Shutter
uint32 IDs //0x00200000
uint32 Capability // 7 :by project
bit 2: 0: not support SetFeatureValue(), 1: support SetFeatureValue()
bit 1: 0: not support GetFeatureValue(), 1: support GetFeatureValue()
bit 0: 0: not support CV DSP Camera Shutter, 1: support CV DSP Camera Shutter
uint32 DefaultValue // 0 : by project

Normal Camera Shutter Status
uint32 IDs //0x00201000
uint32 Capability // 3 :by project
bit 2: 0: not support SetFeatureValue(), 1: support SetFeatureValue()
bit 1: 0: not support GetFeatureValue(), 1: support GetFeatureValue()
bit 0: 0: not support Normal Camera Shutter Status, 1: support Normal Camera Shutter Status
uint32 DefaultValue // 1 : by project

> There is an WMI query in the DSDT dump that looks like
> LENOVO_CAPABILITY_DATA_03. What is it?
>
No idea - my spec sheet doesn't have 03 defined.
Scanning through my doc I'm not spotting anything that looks like a match for WQC either

Mark


> [1]: I never have such a device. I got the dump when fixing
> https://bugzilla.kernel.org/show_bug.cgi?id=221065
>
> Appendix (DSDT snippet):
>
>                     Device (GZFD)
>                     {
>                         Name (_HID, "PNP0C14" /* Windows Management 
> Instrumentation Device */)  // _HID: Hardware ID
>                         Name (_UID, "GMZN")  // _UID: Unique ID
> [..]
> /* This is LENOVO_OTHER_METHOD. */
>                         Method (WMAE, 3, NotSerialized)
>                         {
>                             Local0 = Arg1
>                             CreateDWordField (Arg2, Zero, SIDS)
>                             CreateDWordField (Arg2, 0x04, SVAL)
> [..]
>                             If ((Arg1 == 0x11))
>                             {
> [..]
> /* Get 0x00200000. */
>                                 If ((SIDS == 0x00200000))
>                                 {
>                                     Local0 = ^^PCI0.LPC0.EC0.DCSS /* 
> \_SB_.PCI0.LPC0.EC0_.DCSS */
>                                     Return (Local0)
>                                 }
> [..]
>                             }
>
>                             If ((Arg1 == 0x12))
>                             {
> [..]
> /* Set 0x00200000. */
>                                 If ((SIDS == 0x00200000))
>                                 {
>                                     Local0 = ToInteger (SVAL)
>                                     ^^PCI0.LPC0.EC0.DCSS = Local0
>                                     Return (Zero)
>                                 }
>                             }
> [..]
>                         }
> /* The device has no LENOVO_CAPABILITY_DATA_01. */
>
> /* Is this something like LENOVO_CAPABILITY_DATA_03? */
>                         Method (WQC3, 1, NotSerialized)
>                         {
>                             If ((Arg0 == Zero))
>                             {
>                                 Return (Buffer (0x10)
>                                 {
>                                     /* 0000 */  0x01, 0x00, 0x00, 0x00, 
> 0x01, 0x00, 0x00, 0x00,  // ........
>                                     /* 0008 */  0x10, 0x27, 0x00, 0x00, 
> 0xDC, 0x05, 0x00, 0x00   // .'......
>                                 })
>                             }
>                         }
> [..]
> /* This is LENOVO_CAPABILITY_DATA_00. */
>                         Name (CPD0, Buffer (0x0C)
>                         {
>                              0x00                                       
>       // .
>                         })
>                         CreateDWordField (CPD0, Zero, C0ID)
>                         CreateDWordField (CPD0, 0x04, C0CP)
>                         CreateDWordField (CPD0, 0x08, C0DV)
>                         Method (WQC0, 1, NotSerialized)
>                         {
>                             If ((Arg0 == Zero))
>                             {
>                                 Return (Buffer (0x0C)
>                                 {
>                                     /* 0000 */  0x00, 0x00, 0x05, 0x04, 
> 0x07, 0x00, 0x00, 0x00,  // ........
>                                     /* 0008 */  0x00, 0x00, 0x00, 0x00  
>                          // ....
>                                 })
>                             }
>
>                             If ((Arg0 == One))
>                             {
> /* 0x00200000 is exposed conditionally. */
>                                 C0ID = 0x00200000
>                                 C0DV = Zero
>                                 If ((^^PCI0.LPC0.EC0.CVSP == One))
>                                 {
>                                     C0CP = 0x07
>                                 }
>                                 Else
>                                 {
>                                     C0CP = Zero
>                                 }
>                             }
>
>                             Return (CPD0) /* \_SB_.GZFD.CPD0 */
>                         }
> [..]
>                     }
>
> Thanks,
> Rong
>
>> 
>> > 
>> > 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.
>> 
>> Take your time and don't worry :-)
>> 
>> Thanks,
>> Rong
>> 
>> > 
>> > Mark