Re: [PATCH] platform/x86/amd: pmf: Use meta + L for screen lock command

[Mario Limonciello <mario.limonciello@amd.com>]

On 3/21/2025 5:25 PM, Mario Limonciello wrote:
> 
> 
> On 3/21/25 16:16, Armin Wolf wrote:
>> Am 21.03.25 um 20:30 schrieb Mario Limonciello:
>>
>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> In practice userspace software doesn't react to KEY_SCREENLOCK by
>>> default.  So any time that the PMF policies would suggest to lock
>>> the screen (for example from an HPD sensor event) userspace isn't
>>> configured to do it properly.
>>>
>>> However userspace is configured for meta + L as this is the default
>>> in the ecosystem. Adjust the PMF driver to send meta + L.
>>
>> Hi,
>>
>> KEY_SCREENLOCK is used by other drivers too, so it would make sense
>> to instead add support for KEY_SCREENLOCK to the userspace software
>> instead of having this workaround inside the driver.
> 
> Right; that's actually that's the first thing I looked at when I came to 
> this issue.
> 
> I had "expected" GNOME for example to work with KEY_SCREENLOCK, but even 
> when you program it to do so it doesn't work.
> 
> https://gitlab.gnome.org/GNOME/mutter/-/issues/3990
> 
> The ecosystem has moved to META + L.  My last employer (Dell) I remember 
> there was a FN + F key that would issue a screen lock.  It had a 
> silkscreen of a lock symbol.
> How did it work?  Not KEY_SCREENLOCK - it emulated META + L.
> 
> This is what works in Windows, GNOME and KDE.  So I am of the opinion 
> that KEY_SCREENLOCK is likely a dinosaur that doesn't really exist anymore.
> 

FWIW, I found an aftermarket keyboard (Logitech Ergo K860 [1]) that has 
a "lock" key.

It also emits a KEY_LEFTMETA combination when this key is pressed and 
works by default in GNOME as well with no changes.

-event11  DEVICE_ADDED            Logitech ERGO K860 
seat0 default group7  cap:kp left scroll-nat scroll-button

-event11  KEYBOARD_KEY            +4.191s       KEY_LEFTMETA (125) pressed
  event11  KEYBOARD_KEY            +4.231s       *** (-1) pressed
  event11  KEYBOARD_KEY            +4.374s       *** (-1) released
  event11  KEYBOARD_KEY            +4.412s       KEY_LEFTMETA (125) released

[1] https://www.logitech.com/en-us/shop/p/k860-split-ergonomic.920-009166

>>
>> Also please add a comment explaining what meta + L is supposed to 
>> achieve.
>>
> 
> Sure if we can align on doing this I will spin a V2 with a comment 
> better explaining the situation.
> 
>> Thanks,
>> Armin Wolf
>>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>>   drivers/platform/x86/amd/pmf/tee-if.c | 11 +++++++++--
>>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/ 
>>> platform/ x86/amd/pmf/tee-if.c
>>> index 8c88769ea1d87..2c00f2baeec7b 100644
>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>> @@ -151,7 +151,13 @@ static void amd_pmf_apply_policies(struct 
>>> amd_pmf_dev *dev, struct ta_pmf_enact_
>>>                   amd_pmf_update_uevents(dev, KEY_SUSPEND);
>>>                   break;
>>>               case 2:
>>> -                amd_pmf_update_uevents(dev, KEY_SCREENLOCK);
>>> +                input_report_key(dev->pmf_idev, KEY_LEFTMETA, 1);
>>> +                input_report_key(dev->pmf_idev, KEY_L, 1);
>>> +                input_sync(dev->pmf_idev);
>>> +                input_report_key(dev->pmf_idev, KEY_L, 0);
>>> +                input_sync(dev->pmf_idev);
>>> +                input_report_key(dev->pmf_idev, KEY_LEFTMETA, 0);
>>> +                input_sync(dev->pmf_idev);
>>>                   break;
>>>               default:
>>>                   dev_err(dev->dev, "Invalid PMF policy system state: 
>>> %d\n", val);
>>> @@ -422,8 +428,9 @@ static int amd_pmf_register_input_device(struct 
>>> amd_pmf_dev *dev)
>>>       dev->pmf_idev->phys = "amd-pmf/input0";
>>>
>>>       input_set_capability(dev->pmf_idev, EV_KEY, KEY_SLEEP);
>>> -    input_set_capability(dev->pmf_idev, EV_KEY, KEY_SCREENLOCK);
>>>       input_set_capability(dev->pmf_idev, EV_KEY, KEY_SUSPEND);
>>> +    input_set_capability(dev->pmf_idev, EV_KEY, KEY_L);
>>> +    input_set_capability(dev->pmf_idev, EV_KEY, KEY_LEFTMETA);
>>>
>>>       err = input_register_device(dev->pmf_idev);
>>>       if (err) {
>