platform/x86: ideapad-laptop: support Fn+R dual-function key

Message ID 0cdbc0e6eb65e160384ae0ed152e7de3ded1d9d5.1707604991.git.soyer@irl.hu
State New
Headers
Series platform/x86: ideapad-laptop: support Fn+R dual-function key |

Commit Message

Gergo Koteles Feb. 10, 2024, 10:51 p.m. UTC
  According to the manual, Fn+R adjusts the display refresh rate.
Map Fn+R to KEY_DISPLAYTOGGLE.

Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
 drivers/platform/x86/ideapad-laptop.c | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Ilpo Järvinen Feb. 13, 2024, 12:01 p.m. UTC | #1
On Sat, 10 Feb 2024 23:51:57 +0100, Gergo Koteles wrote:

> According to the manual, Fn+R adjusts the display refresh rate.
> Map Fn+R to KEY_DISPLAYTOGGLE.
> 
> 


Thank you for your contribution, it has been applied to my local
review-ilpo branch. Note it will show up in the public
platform-drivers-x86/review-ilpo branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/1] platform/x86: ideapad-laptop: support Fn+R dual-function key
      commit: c5211eacf3326538fbf31b612e5ea546ca8a3425

--
 i.
  
Ike Panhc Feb. 19, 2024, 2:39 a.m. UTC | #2
On 2/11/24 06:51, Gergo Koteles wrote:
> According to the manual, Fn+R adjusts the display refresh rate.
> Map Fn+R to KEY_DISPLAYTOGGLE.
> 
> Signed-off-by: Gergo Koteles <soyer@irl.hu>
> ---
>  drivers/platform/x86/ideapad-laptop.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index 88eefccb6ed2..4c130957f80d 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -1091,6 +1091,8 @@ static const struct key_entry ideapad_keymap[] = {
>  	{ KE_KEY,	0x07 | IDEAPAD_WMI_KEY, { KEY_HELP } },
>  	{ KE_KEY,	0x0e | IDEAPAD_WMI_KEY, { KEY_PICKUP_PHONE } },
>  	{ KE_KEY,	0x0f | IDEAPAD_WMI_KEY, { KEY_HANGUP_PHONE } },
> +	/* Refresh Rate Toggle (Fn+R) */
> +	{ KE_KEY,	0x10 | IDEAPAD_WMI_KEY, { KEY_DISPLAYTOGGLE } },
>  	/* Dark mode toggle */
>  	{ KE_KEY,	0x13 | IDEAPAD_WMI_KEY, { KEY_PROG1 } },
>  	/* Sound profile switch */

Acked-by: Ike Panhc <ike.pan@canonical.com>

BTW on which ideapad we need this patch?

Thanks.

--
Ike
  
Gergo Koteles Feb. 19, 2024, 12:32 p.m. UTC | #3
Hi Ike,

On Mon, 2024-02-19 at 10:39 +0800, Ike Panhc wrote:
> On 2/11/24 06:51, Gergo Koteles wrote:
> > According to the manual, Fn+R adjusts the display refresh rate.
> > Map Fn+R to KEY_DISPLAYTOGGLE.
> > 
> > Signed-off-by: Gergo Koteles <soyer@irl.hu>
> > ---
> >  drivers/platform/x86/ideapad-laptop.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> > index 88eefccb6ed2..4c130957f80d 100644
> > --- a/drivers/platform/x86/ideapad-laptop.c
> > +++ b/drivers/platform/x86/ideapad-laptop.c
> > @@ -1091,6 +1091,8 @@ static const struct key_entry ideapad_keymap[] = {
> >  	{ KE_KEY,	0x07 | IDEAPAD_WMI_KEY, { KEY_HELP } },
> >  	{ KE_KEY,	0x0e | IDEAPAD_WMI_KEY, { KEY_PICKUP_PHONE } },
> >  	{ KE_KEY,	0x0f | IDEAPAD_WMI_KEY, { KEY_HANGUP_PHONE } },
> > +	/* Refresh Rate Toggle (Fn+R) */
> > +	{ KE_KEY,	0x10 | IDEAPAD_WMI_KEY, { KEY_DISPLAYTOGGLE } },
> >  	/* Dark mode toggle */
> >  	{ KE_KEY,	0x13 | IDEAPAD_WMI_KEY, { KEY_PROG1 } },
> >  	/* Sound profile switch */
> 
> Acked-by: Ike Panhc <ike.pan@canonical.com>
> 
> BTW on which ideapad we need this patch?
> 

Oh, I somehow missed that. I found it on a Yoga 7 14ARB7.

Newer Yogas and Legions with 60Hz/90Hz displays have this refresh rate
toggle feature.

I'm wondering if this would be worth a new KEY_REFRESH_RATE_TOGGLE
event code? 

KEY_DISPLAYTOGGLE is used to toggle the LCD on/off in other drivers.

What do you think?

Thanks,
Gergo

> Thanks.
> 
> --
> Ike
  
Ilpo Järvinen Feb. 19, 2024, 12:51 p.m. UTC | #4
On Mon, 19 Feb 2024, Gergo Koteles wrote:
> On Mon, 2024-02-19 at 10:39 +0800, Ike Panhc wrote:
> > On 2/11/24 06:51, Gergo Koteles wrote:
> > > According to the manual, Fn+R adjusts the display refresh rate.
> > > Map Fn+R to KEY_DISPLAYTOGGLE.
> > > 
> > > Signed-off-by: Gergo Koteles <soyer@irl.hu>

> > > +++ b/drivers/platform/x86/ideapad-laptop.c
> > > @@ -1091,6 +1091,8 @@ static const struct key_entry ideapad_keymap[] = {
> > >  	{ KE_KEY,	0x07 | IDEAPAD_WMI_KEY, { KEY_HELP } },
> > >  	{ KE_KEY,	0x0e | IDEAPAD_WMI_KEY, { KEY_PICKUP_PHONE } },
> > >  	{ KE_KEY,	0x0f | IDEAPAD_WMI_KEY, { KEY_HANGUP_PHONE } },
> > > +	/* Refresh Rate Toggle (Fn+R) */
> > > +	{ KE_KEY,	0x10 | IDEAPAD_WMI_KEY, { KEY_DISPLAYTOGGLE } },
> > >  	/* Dark mode toggle */
> > >  	{ KE_KEY,	0x13 | IDEAPAD_WMI_KEY, { KEY_PROG1 } },
> > >  	/* Sound profile switch */
> > 
> > Acked-by: Ike Panhc <ike.pan@canonical.com>
> > 
> > BTW on which ideapad we need this patch?
> > 
> 
> Oh, I somehow missed that. I found it on a Yoga 7 14ARB7.
> 
> Newer Yogas and Legions with 60Hz/90Hz displays have this refresh rate
> toggle feature.
> 
> I'm wondering if this would be worth a new KEY_REFRESH_RATE_TOGGLE
> event code? 
> 
> KEY_DISPLAYTOGGLE is used to toggle the LCD on/off in other drivers.
> 
> What do you think?

At least to me it felt like an abuse of KEY_DISPLAYTOGGLE because it's 
obviously different. But since there was existing, similar use for it 
already, I didn't push back on it but took your patch.
  

Patch

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 88eefccb6ed2..4c130957f80d 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1091,6 +1091,8 @@  static const struct key_entry ideapad_keymap[] = {
 	{ KE_KEY,	0x07 | IDEAPAD_WMI_KEY, { KEY_HELP } },
 	{ KE_KEY,	0x0e | IDEAPAD_WMI_KEY, { KEY_PICKUP_PHONE } },
 	{ KE_KEY,	0x0f | IDEAPAD_WMI_KEY, { KEY_HANGUP_PHONE } },
+	/* Refresh Rate Toggle (Fn+R) */
+	{ KE_KEY,	0x10 | IDEAPAD_WMI_KEY, { KEY_DISPLAYTOGGLE } },
 	/* Dark mode toggle */
 	{ KE_KEY,	0x13 | IDEAPAD_WMI_KEY, { KEY_PROG1 } },
 	/* Sound profile switch */