Re: [PATCH] Fixed ideapad-laptop driver to support Yoga 9 2 in 1 14imh9 unknown keys

[Hans de Goede <hdegoede@redhat.com>]

Hi Gašper,

On 14-Mar-25 12:35 PM, Gašper Nemgar wrote:
> Thanks for the feedback, 
> 
> I will go over your notes here and explain things.
> The Eye button on windows triggeres "Eye Care mode" https://download.lenovo.com/consumer/mobiles_pub/yoga_9i_2-in-1_14_9_ug_en.pdf#page=50 <https://download.lenovo.com/consumer/mobiles_pub/yoga_9i_2-in-1_14_9_ug_en.pdf#page=50>,  does linux have a keycode as something like this, i didn't found any, should we jus assign it to one of the programmable keys?

Yes that sounds like the best solution.

> I will handle the performance as you have suggested, thanks

You're welcome.

> The shift + prtSc i have made a mistake here, i meant the Fn + prtSc. 
> Just PrtSc is handled by a different device ("AT Translated Set 2 keyboard") and is not managed by this module. evtest told me it sends SysRq code which is intended and it triggers the selective screenshot.

Right, e.g. GNOME does not really differentiate between sysrq/printscreen
and KEY_SELECTIVE_SCREENSHOT and always uses the dialog instead of making
printscreen directly take a full screen screenshot. Still making this
the Fn + printscreen comboe send KEY_SELECTIVE_SCREENSHOT is the right thing
todo and is also done on other laptop models with the same icon.

> If I assign the Fn + PrtSc to KEY_SELECTIVE_SCREENSHOT the evtest shows
me it triggers code 634 and code name is "?". Is this correct behaviour?

This just means that your evtest is a bit old and does not know about
KEY_SELECTIVE_SCREENSHOT yet.

Regards,

Hans



> On Fri, 14 Mar 2025 at 10:34, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
> 
>     Hi Gašper,
> 
>     Thank you for your patch.
> 
>     First if all a few generic notes:
> 
>     1. When sending out v2 of the patch please add
>        platform-driver-x86@vger.kernel.org <mailto:platform-driver-x86@vger.kernel.org> to the Cc
> 
>     2. The patch subject (first line of commit message) should have
>        a prefix describing the subsystem + driver, e.g. use:
>        "platform/x86: ideapad-laptop: Add a few new keymap entries"
> 
>     3. Your patch is missing a signed-off-by, see:
>     https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin <https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin>
> 
>     4. Your commit message / patch should also have a body, e.g.
>        the entirety of the commit message could look something like this:
> 
>     -- begin --
>     platform/x86: ideapad-laptop: Add a few new keymap entries
> 
>     The Yoga 9 2 in 1 14imh9 introduces 4 new hotkeys which are not
>     yet in ideapad_keymap[], add entries to map these keys.
> 
>     Signed-off-by: Gašper Nemgar <your-email-here>
>     -- end --
> 
>     A few more specific remarks below based on looking at
>     this picture of the keyboard:
> 
>     https://ardes.bg/uploads/original/lenovo-yoga-9-2-in-1-14-g9-550178.jpg <https://ardes.bg/uploads/original/lenovo-yoga-9-2-in-1-14-g9-550178.jpg>
> 
>     On 13-Mar-25 4:17 PM, Gašper Nemgar wrote:
>     > diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
>     > index 30bd366d7..af124aafe 100644
>     > --- a/drivers/platform/x86/ideapad-laptop.c
>     > +++ b/drivers/platform/x86/ideapad-laptop.c
>     > @@ -1308,6 +1308,14 @@ static const struct key_entry ideapad_keymap[] = {
>     >       /* Specific to some newer models */
>     >       { KE_KEY,       0x3e | IDEAPAD_WMI_KEY, { KEY_MICMUTE } },
>     >       { KE_KEY,       0x3f | IDEAPAD_WMI_KEY, { KEY_RFKILL } },
>     > +     /*Star- (User Asignable Key)*/
>     > +     { KE_KEY,       0x44 | IDEAPAD_WMI_KEY, { KEY_PROG1 } },
> 
>     Ack.
> 
>     > +     /*Eye*/
>     > +     { KE_KEY,       0x45 | IDEAPAD_WMI_KEY, { KEY_DISPLAYTOGGLE } },
> 
>     It looks like the laptop already does display-toggle as Fn-F7 although
>     it like sends super + P for this (AKA meta + P).
> 
>     So mapping this to KEY_DISPLAYTOGGLE seems wrong, what does this
>     do under Windows?
> 
>     Maybe KEY_ZOOM ?
> 
>     > +     /*Performance*/
>     > +     { KE_KEY,       0x3d | IDEAPAD_WMI_KEY, { KEY_SPORT } },
> 
>     I think that instead of mapping this it should be handled specially
>     and call platform_profile_cycle() instead of sending a key-press
>     to userspace
> 
>     > +     /*shift + prtsc*/
>     > +     { KE_KEY,       0x2d | IDEAPAD_WMI_KEY, { KEY_PROG3 } },
> 
>     Looking a the symbol on the keyboard this should send
>     KEY_SELECTIVE_SCREENSHOT
> 
>     > 
>     >       { KE_END },
>     >  };
>     >
> 
>     Regards,
> 
>     Hans
> 
>