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

[Mario Limonciello <superm1@kernel.org>]

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.

> 
> 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) {