Re: [PATCH 1/2] Input: Add a helper for reporting a screen lock key sequence

[Armin Wolf]

Am 16.04.25 um 22:43 schrieb Mario Limonciello:

> On 4/16/2025 3:39 PM, Armin Wolf wrote:
> > Am 07.04.25 um 17:27 schrieb Mario Limonciello:
> >
> > > From: Mario Limonciello <mario.limonciello@xxxxxxx>
> > >
> > > In the PC industry KEY_SCREENLOCK isn't used as frequently as it used
> > > to be. Modern versions of Windows [1], GNOME and KDE support "META" 
> > > + "L"
> > > to lock the screen. Modern hardware [2] also sends this sequence of
> > > events for keys with a silkscreen for screen lock.
> > >
> > > Introduced a helper input_report_lock_sequence() for drivers to utilize
> > > if they want to send this sequence.
> > >
> > > Link: 
> > > https://support.microsoft.com/en-us/windows/keyboard-shortcuts- 
> > > in-windows-dcc61a57-8ff0-cffe-9796-cb9706c75eec [1]
> > > Link: https://www.logitech.com/en-us/shop/p/k860-split- 
> > > ergonomic.920-009166 [2]
> > > Suggested-by: Armin Wolf <W_Armin@xxxxxx>
> > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> > > ---
> > >   drivers/input/input.c | 20 ++++++++++++++++++++
> > >   include/linux/input.h |  2 ++
> > >   2 files changed, 22 insertions(+)
> > >
> > > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > > index ec4346f20efdd..dfeace85c4710 100644
> > > --- a/drivers/input/input.c
> > > +++ b/drivers/input/input.c
> > > @@ -508,6 +508,26 @@ void input_copy_abs(struct input_dev *dst, 
> > > unsigned int dst_axis,
> > >   }
> > >   EXPORT_SYMBOL(input_copy_abs);
> > > +/**
> > > + * input_report_lock_sequence - Report key combination to lock the 
> > > screen
> > > + * @dev: input device
> > > + *
> > > + * Key combination used in the PC industry since Windows 7 for 
> > > locking display
> > > + * is META + L. This is also used in GNOME and KDE by default.
> > > + * See https://support.microsoft.com/en-us/windows/keyboard- 
> > > shortcuts-in-windows-dcc61a57-8ff0-cffe-9796-cb9706c75eec
> > > + */
> >
> > Hi,
> >
> > maybe it would make more sense to have the input subsystem 
> > transparently translate between KEY_SCREENLOCK and this
> > special screen lock sequence. This way there would be no room for 
> > regressions as people could enable/disable this
> > behavior via Kconfig.
>
> Wouldn't all drivers that emit KEY_SCREENLOCK still need to be 
> modified to have support to emit KEY_LEFTMETA and KEY_L?
>
> Wouldn't that mean conditional code in every single driver based on 
> the Kconfig?
>
> Is that worth it?
This translation code could live inside the input code itself. This way all drivers would benefit from it.

Thanks,
Armin Wolf

> > Thanks,
> > Armin Wolf
> >
> > > +void input_report_lock_sequence(struct input_dev *dev)
> > > +{
> > > +    input_report_key(dev, KEY_LEFTMETA, 1);
> > > +    input_report_key(dev, KEY_L, 1);
> > > +    input_sync(dev);
> > > +    input_report_key(dev, KEY_L, 0);
> > > +    input_sync(dev);
> > > +    input_report_key(dev, KEY_LEFTMETA, 0);
> > > +    input_sync(dev);
> > > +}
> > > +EXPORT_SYMBOL(input_report_lock_sequence);
> > > +
> > >   /**
> > >    * input_grab_device - grabs device for exclusive use
> > >    * @handle: input handle that wants to own the device
> > > diff --git a/include/linux/input.h b/include/linux/input.h
> > > index 7d7cb0593a63e..16f7bef12f1c1 100644
> > > --- a/include/linux/input.h
> > > +++ b/include/linux/input.h
> > > @@ -492,6 +492,8 @@ void input_set_abs_params(struct input_dev *dev, 
> > > unsigned int axis,
> > >   void input_copy_abs(struct input_dev *dst, unsigned int dst_axis,
> > >               const struct input_dev *src, unsigned int src_axis);
> > > +void input_report_lock_sequence(struct input_dev *dev);
> > > +
> > >   #define INPUT_GENERATE_ABS_ACCESSORS(_suffix, _item)            \
> > >   static inline int input_abs_get_##_suffix(struct input_dev *dev,    \
> > >                         unsigned int axis)        \