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

[Rong Zhang <i@rong.moe>]

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 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.

There is an WMI query in the DSDT dump that looks like
LENOVO_CAPABILITY_DATA_03. What is it?

[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