LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
@ 2018-05-03  3:04 Chris Chiu
  2018-05-07 14:10 ` Andy Shevchenko
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Chiu @ 2018-05-03  3:04 UTC (permalink / raw)
  To: corentin.chary, dvhart, andy
  Cc: acpi4asus-user, platform-driver-x86, linux-kernel, linux,
	Chris Chiu, Jian-Hong Pan

Some Asus laptops like UX550GE has hotkey (Fn+F7) for keyboard
backlight toggle. In this UX550GE, the hotkey incremet the level
of brightness for each keypress from 1 to 3, and then switch it
off when the brightness has been the max. This commit interprets
the code 0xc7 generated from hotkey to KEY_KBDILLUMUP to increment
the brightness, then pass KEY_KBDILLUMTOGGLE to user space after
the brightness max been reached for switching the led off.

Signed-off-by: Chris Chiu <chiu@endlessm.com>
Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
Tested-by: Jian-Hong Pan <jian-hong@endlessm.com>
---

Notes:
    v2:
    - Remove redundant 'else' branch logic

 drivers/platform/x86/asus-nb-wmi.c | 1 +
 drivers/platform/x86/asus-wmi.c    | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index 136ff2b4cce5..14c502e18579 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -496,6 +496,7 @@ static const struct key_entry asus_nb_wmi_keymap[] = {
 	{ KE_KEY, 0xC4, { KEY_KBDILLUMUP } },
 	{ KE_KEY, 0xC5, { KEY_KBDILLUMDOWN } },
 	{ KE_IGNORE, 0xC6, },  /* Ambient Light Sensor notification */
+	{ KE_KEY, 0xC7, { KEY_KBDILLUMTOGGLE } },
 	{ KE_END, 0},
 };
 
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 1f6e68f0b646..4a3ba575c9ce 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -67,6 +67,7 @@ MODULE_LICENSE("GPL");
 #define NOTIFY_BRNDOWN_MAX		0x2e
 #define NOTIFY_KBD_BRTUP		0xc4
 #define NOTIFY_KBD_BRTDWN		0xc5
+#define NOTIFY_KBD_BRTTOGGLE		0xc7
 
 /* WMI Methods */
 #define ASUS_WMI_METHODID_SPEC	        0x43455053 /* BIOS SPECification */
@@ -1745,6 +1746,10 @@ static void asus_wmi_notify(u32 value, void *context)
 		}
 	}
 
+	if (code == NOTIFY_KBD_BRTTOGGLE)
+		if (asus->kbd_led_wk < asus->kbd_led.max_brightness)
+			code = NOTIFY_KBD_BRTUP;
+
 	if (is_display_toggle(code) &&
 	    asus->driver->quirks->no_display_toggle)
 		goto exit;
-- 
2.14.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-03  3:04 [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support Chris Chiu
@ 2018-05-07 14:10 ` Andy Shevchenko
  2018-05-07 14:46   ` Daniel Drake
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2018-05-07 14:10 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Corentin Chary, Darren Hart, Andy Shevchenko, acpi4asus-user,
	Platform Driver, Linux Kernel Mailing List,
	Linux Upstreaming Team, Jian-Hong Pan

On Thu, May 3, 2018 at 6:04 AM, Chris Chiu <chiu@endlessm.com> wrote:
> Some Asus laptops like UX550GE has hotkey (Fn+F7) for keyboard
> backlight toggle. In this UX550GE, the hotkey incremet the level
> of brightness for each keypress from 1 to 3, and then switch it
> off when the brightness has been the max. This commit interprets
> the code 0xc7 generated from hotkey to KEY_KBDILLUMUP to increment
> the brightness, then pass KEY_KBDILLUMTOGGLE to user space after
> the brightness max been reached for switching the led off.
>

Pushed to my review and testing queue, thanks!

> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Tested-by: Jian-Hong Pan <jian-hong@endlessm.com>
> ---
>
> Notes:
>     v2:
>     - Remove redundant 'else' branch logic
>
>  drivers/platform/x86/asus-nb-wmi.c | 1 +
>  drivers/platform/x86/asus-wmi.c    | 5 +++++
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index 136ff2b4cce5..14c502e18579 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -496,6 +496,7 @@ static const struct key_entry asus_nb_wmi_keymap[] = {
>         { KE_KEY, 0xC4, { KEY_KBDILLUMUP } },
>         { KE_KEY, 0xC5, { KEY_KBDILLUMDOWN } },
>         { KE_IGNORE, 0xC6, },  /* Ambient Light Sensor notification */
> +       { KE_KEY, 0xC7, { KEY_KBDILLUMTOGGLE } },
>         { KE_END, 0},
>  };
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 1f6e68f0b646..4a3ba575c9ce 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -67,6 +67,7 @@ MODULE_LICENSE("GPL");
>  #define NOTIFY_BRNDOWN_MAX             0x2e
>  #define NOTIFY_KBD_BRTUP               0xc4
>  #define NOTIFY_KBD_BRTDWN              0xc5
> +#define NOTIFY_KBD_BRTTOGGLE           0xc7
>
>  /* WMI Methods */
>  #define ASUS_WMI_METHODID_SPEC         0x43455053 /* BIOS SPECification */
> @@ -1745,6 +1746,10 @@ static void asus_wmi_notify(u32 value, void *context)
>                 }
>         }
>
> +       if (code == NOTIFY_KBD_BRTTOGGLE)
> +               if (asus->kbd_led_wk < asus->kbd_led.max_brightness)
> +                       code = NOTIFY_KBD_BRTUP;
> +
>         if (is_display_toggle(code) &&
>             asus->driver->quirks->no_display_toggle)
>                 goto exit;
> --
> 2.14.1
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-07 14:10 ` Andy Shevchenko
@ 2018-05-07 14:46   ` Daniel Drake
  2018-05-15  0:25     ` Daniel Drake
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Drake @ 2018-05-07 14:46 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Chris Chiu, Corentin Chary, Darren Hart, Andy Shevchenko,
	acpi4asus-user, Platform Driver, Linux Kernel Mailing List,
	Linux Upstreaming Team, Jian-Hong Pan, bberg

Hi Andy,

On Mon, May 7, 2018 at 8:10 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, May 3, 2018 at 6:04 AM, Chris Chiu <chiu@endlessm.com> wrote:
> > Some Asus laptops like UX550GE has hotkey (Fn+F7) for keyboard
> > backlight toggle. In this UX550GE, the hotkey incremet the level
> > of brightness for each keypress from 1 to 3, and then switch it
> > off when the brightness has been the max. This commit interprets
> > the code 0xc7 generated from hotkey to KEY_KBDILLUMUP to increment
> > the brightness, then pass KEY_KBDILLUMTOGGLE to user space after
> > the brightness max been reached for switching the led off.
> >
>
> Pushed to my review and testing queue, thanks!

We found that GNOME's handling of the toggle key is somewhat imperfect
and it will need modifying before we achieve the
Up-Up-Up-off-Up-Up-Up-off... cycle that we are looking for.

https://gitlab.gnome.org/GNOME/gnome-settings-daemon/issues/41

In that discussion an alternative perspective was raised:

Is it right for the kernel to modify the key sent to userspace, when
it is then relying on the specific userspace action of it changing the
brightness to the next expected level? (and this userspace behaviour
is not even working right in the GNOME case)

Instead, would it make sense for the kernel to always report TOGGLE in
this case, and for GNOME to interpret toggle as simply "cycle through
all the available brightness levels"?

We'd be interested in your thoughts.

Thanks
Daniel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-07 14:46   ` Daniel Drake
@ 2018-05-15  0:25     ` Daniel Drake
  2018-05-15  7:52       ` Andy Shevchenko
  2018-05-15 13:41       ` Benjamin Berg
  0 siblings, 2 replies; 12+ messages in thread
From: Daniel Drake @ 2018-05-15  0:25 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Chris Chiu, Corentin Chary, Darren Hart, Andy Shevchenko,
	acpi4asus-user, Platform Driver, Linux Kernel Mailing List,
	Linux Upstreaming Team, Jian-Hong Pan, bberg

Hi Andy,

On Mon, May 7, 2018 at 8:46 AM, Daniel Drake <drake@endlessm.com> wrote:
> > > Some Asus laptops like UX550GE has hotkey (Fn+F7) for keyboard
> > > backlight toggle. In this UX550GE, the hotkey incremet the level
> > > of brightness for each keypress from 1 to 3, and then switch it
> > > off when the brightness has been the max. This commit interprets
> > > the code 0xc7 generated from hotkey to KEY_KBDILLUMUP to increment
> > > the brightness, then pass KEY_KBDILLUMTOGGLE to user space after
> > > the brightness max been reached for switching the led off.
> > >
> >
> > Pushed to my review and testing queue, thanks!
>
> We found that GNOME's handling of the toggle key is somewhat imperfect
> and it will need modifying before we achieve the
> Up-Up-Up-off-Up-Up-Up-off... cycle that we are looking for.
>
> https://gitlab.gnome.org/GNOME/gnome-settings-daemon/issues/41
>
> In that discussion an alternative perspective was raised:
>
> Is it right for the kernel to modify the key sent to userspace, when
> it is then relying on the specific userspace action of it changing the
> brightness to the next expected level? (and this userspace behaviour
> is not even working right in the GNOME case)
>
> Instead, would it make sense for the kernel to always report TOGGLE in
> this case, and for GNOME to interpret toggle as simply "cycle through
> all the available brightness levels"?

Any comments on this? I am tempted to send a patch to just make this
key always emit TOGGLE from the kernel given that we have a tentative
agreement on implementing the brightness cycle within GNOME.

Daniel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-15  0:25     ` Daniel Drake
@ 2018-05-15  7:52       ` Andy Shevchenko
  2018-05-15 13:41       ` Benjamin Berg
  1 sibling, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2018-05-15  7:52 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Chris Chiu, Corentin Chary, Darren Hart, Andy Shevchenko,
	acpi4asus-user, Platform Driver, Linux Kernel Mailing List,
	Linux Upstreaming Team, Jian-Hong Pan, Benjamin Berg

On Tue, May 15, 2018 at 3:25 AM, Daniel Drake <drake@endlessm.com> wrote:
> Hi Andy,
>
> On Mon, May 7, 2018 at 8:46 AM, Daniel Drake <drake@endlessm.com> wrote:
>> > > Some Asus laptops like UX550GE has hotkey (Fn+F7) for keyboard
>> > > backlight toggle. In this UX550GE, the hotkey incremet the level
>> > > of brightness for each keypress from 1 to 3, and then switch it
>> > > off when the brightness has been the max. This commit interprets
>> > > the code 0xc7 generated from hotkey to KEY_KBDILLUMUP to increment
>> > > the brightness, then pass KEY_KBDILLUMTOGGLE to user space after
>> > > the brightness max been reached for switching the led off.
>> > >
>> >
>> > Pushed to my review and testing queue, thanks!
>>
>> We found that GNOME's handling of the toggle key is somewhat imperfect
>> and it will need modifying before we achieve the
>> Up-Up-Up-off-Up-Up-Up-off... cycle that we are looking for.
>>
>> https://gitlab.gnome.org/GNOME/gnome-settings-daemon/issues/41
>>
>> In that discussion an alternative perspective was raised:
>>
>> Is it right for the kernel to modify the key sent to userspace, when
>> it is then relying on the specific userspace action of it changing the
>> brightness to the next expected level? (and this userspace behaviour
>> is not even working right in the GNOME case)
>>
>> Instead, would it make sense for the kernel to always report TOGGLE in
>> this case, and for GNOME to interpret toggle as simply "cycle through
>> all the available brightness levels"?
>
> Any comments on this? I am tempted to send a patch to just make this
> key always emit TOGGLE from the kernel given that we have a tentative
> agreement on implementing the brightness cycle within GNOME.

The patch is slipped to our for-next queue. We have about 1-2 weeks to
fix things there if the initial approach is not what you want.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-15  0:25     ` Daniel Drake
  2018-05-15  7:52       ` Andy Shevchenko
@ 2018-05-15 13:41       ` Benjamin Berg
  2018-05-22  9:18         ` Andy Shevchenko
  1 sibling, 1 reply; 12+ messages in thread
From: Benjamin Berg @ 2018-05-15 13:41 UTC (permalink / raw)
  To: Daniel Drake, Andy Shevchenko
  Cc: Chris Chiu, Corentin Chary, Darren Hart, Andy Shevchenko,
	acpi4asus-user, Platform Driver, Linux Kernel Mailing List,
	Linux Upstreaming Team, Jian-Hong Pan

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hi Daniel,

I had a quick chat with Bastien about this. The conclusion was that
reusing the TOGGLE key may be problematic for gnome-settings-daemon.
And the alternative of a new CYCLE key also has some caveats.

The most straight forward solution is likely to simply handle the
brightness change in the kernel and not report the key to userspace at
all. This should work just fine and at least GNOME will show an on
screen display in response to the brightness change.

Do you think that approach would work well?

Benjamin


On Mon, 2018-05-14 at 18:25 -0600, Daniel Drake wrote:
> Hi Andy,
> 
> On Mon, May 7, 2018 at 8:46 AM, Daniel Drake <drake@endlessm.com>
> wrote:
> > > > Some Asus laptops like UX550GE has hotkey (Fn+F7) for keyboard
> > > > backlight toggle. In this UX550GE, the hotkey incremet the
> > > > level
> > > > of brightness for each keypress from 1 to 3, and then switch it
> > > > off when the brightness has been the max. This commit
> > > > interprets
> > > > the code 0xc7 generated from hotkey to KEY_KBDILLUMUP to
> > > > increment
> > > > the brightness, then pass KEY_KBDILLUMTOGGLE to user space
> > > > after
> > > > the brightness max been reached for switching the led off.
> > > > 
> > > 
> > > Pushed to my review and testing queue, thanks!
> > 
> > We found that GNOME's handling of the toggle key is somewhat
> > imperfect
> > and it will need modifying before we achieve the
> > Up-Up-Up-off-Up-Up-Up-off... cycle that we are looking for.
> > 
> > https://gitlab.gnome.org/GNOME/gnome-settings-daemon/issues/41
> > 
> > In that discussion an alternative perspective was raised:
> > 
> > Is it right for the kernel to modify the key sent to userspace,
> > when
> > it is then relying on the specific userspace action of it changing
> > the
> > brightness to the next expected level? (and this userspace
> > behaviour
> > is not even working right in the GNOME case)
> > 
> > Instead, would it make sense for the kernel to always report TOGGLE
> > in
> > this case, and for GNOME to interpret toggle as simply "cycle
> > through
> > all the available brightness levels"?
> 
> Any comments on this? I am tempted to send a patch to just make this
> key always emit TOGGLE from the kernel given that we have a tentative
> agreement on implementing the brightness cycle within GNOME.
> 
> Daniel
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEED2NO4vMS33W8E4AFq6ZWhpmFY3AFAlr6434ACgkQq6ZWhpmF
Y3D97A/9Hqi/xvHU5jcgtKQ2AH3pN445whjXrnUPpcN3I6zgeo/2kXHxn+DEFzD/
xsBLRy2hzfvw5HXWsclf8u31TajmP9OOihHTMB+Ng7ZJohIEgwPjIMr/eaHiiLYr
W9aamm/5DJF5cwYXmiQ46+y+Wbc5ZJbu0wSgiMDY2uEhlph/9J3NGRadb6xEWJX+
Rrp2te5lqwIvHKv4tF/HOrXev6R77AdpHy+WYrA1IKdG996kFDAZcLErk5EL/J0U
+p+pkGbul/JHKcwU77L/Sg8daM1GMQfxWKbR9k21Fow27iaJRMn9Vn9s8HEmHJ+5
S8cBIo71EjNHXO2oDaSzI9Ng1q+Zy7XjHtbK8/MmauQ/3n/z3tt8ZkW4/FumdMkm
ZZwmS9OxOTc+G9hc3SLkggRF1ygbaCgsZaynyEiBHSq37V0G9WBL6BaDW9P6jEIV
7aPJN6SEBLqXStBZiwNXn/yr1AC7duhnIvzxYZj/SUdCk18aDS/tGIehGTqTumGV
2jhyHQek2fp7J0AZH8oOnU9rAIdKtPb3vf5DJuCOD2TtB/yMZDJVj1Uc1g565IPs
KcVdMdiqTy7m65BAwjJyrFu04GIi02SBFFEsdKGOkAcuPPWpG+4EnUwGpArwxwYv
SaUnUD4DRvsoyZHeXETsa7ePh9aBZwmNR+AGSoLj8UY+vbh3/dg=
=szZs
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-15 13:41       ` Benjamin Berg
@ 2018-05-22  9:18         ` Andy Shevchenko
  2018-05-22 10:11           ` Chris Chiu
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2018-05-22  9:18 UTC (permalink / raw)
  To: Benjamin Berg
  Cc: Daniel Drake, Chris Chiu, Corentin Chary, Darren Hart,
	Andy Shevchenko, acpi4asus-user, Platform Driver,
	Linux Kernel Mailing List, Linux Upstreaming Team, Jian-Hong Pan

On Tue, May 15, 2018 at 4:41 PM, Benjamin Berg <bberg@redhat.com> wrote:

> I had a quick chat with Bastien about this. The conclusion was that
> reusing the TOGGLE key may be problematic for gnome-settings-daemon.
> And the alternative of a new CYCLE key also has some caveats.
>
> The most straight forward solution is likely to simply handle the
> brightness change in the kernel and not report the key to userspace at
> all. This should work just fine and at least GNOME will show an on
> screen display in response to the brightness change.
>
> Do you think that approach would work well?

Am I right we are waiting for Daniel's answer?

Btw, I mistakenly thought that patch in the queue for-next, while it's
not. So, I'm going to drop it now.
Feel free to re-submit a new (better) version.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-22  9:18         ` Andy Shevchenko
@ 2018-05-22 10:11           ` Chris Chiu
  2018-05-22 13:48             ` Daniel Drake
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Chiu @ 2018-05-22 10:11 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Benjamin Berg, Daniel Drake, Corentin Chary, Darren Hart,
	Andy Shevchenko, acpi4asus-user, Platform Driver,
	Linux Kernel Mailing List, Linux Upstreaming Team, Jian-Hong Pan

On Tue, May 22, 2018 at 5:18 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, May 15, 2018 at 4:41 PM, Benjamin Berg <bberg@redhat.com> wrote:
>
>> I had a quick chat with Bastien about this. The conclusion was that
>> reusing the TOGGLE key may be problematic for gnome-settings-daemon.
>> And the alternative of a new CYCLE key also has some caveats.
>>
>> The most straight forward solution is likely to simply handle the
>> brightness change in the kernel and not report the key to userspace at
>> all. This should work just fine and at least GNOME will show an on
>> screen display in response to the brightness change.
>>
>> Do you think that approach would work well?
>
> Am I right we are waiting for Daniel's answer?
>
> Btw, I mistakenly thought that patch in the queue for-next, while it's
> not. So, I'm going to drop it now.
> Feel free to re-submit a new (better) version.
>
> --
> With Best Regards,
> Andy Shevchenko

So it's better to handle NOTIFY_KBD_BRTUP, BRTDOWN, BRTTOGGLE
(c4, c5, c7) in the driver and no need to pass it up to userspace? I'll work
out a patch for this.

Chris

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-22 10:11           ` Chris Chiu
@ 2018-05-22 13:48             ` Daniel Drake
  2018-05-24  8:33               ` Chris Chiu
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Drake @ 2018-05-22 13:48 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Andy Shevchenko, Benjamin Berg, Corentin Chary, Darren Hart,
	Andy Shevchenko, acpi4asus-user, Platform Driver,
	Linux Kernel Mailing List, Linux Upstreaming Team, Jian-Hong Pan

On Tue, May 22, 2018 at 4:11 AM, Chris Chiu <chiu@endlessm.com> wrote:
> On Tue, May 22, 2018 at 5:18 PM, Andy Shevchenko
>> Btw, I mistakenly thought that patch in the queue for-next, while it's
>> not. So, I'm going to drop it now.
>> Feel free to re-submit a new (better) version.
>
> So it's better to handle NOTIFY_KBD_BRTUP, BRTDOWN, BRTTOGGLE
> (c4, c5, c7) in the driver and no need to pass it up to userspace? I'll work
> out a patch for this.

Yes, let's explore this approach before submitting another patch to Andy.

Thanks
Daniel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-22 13:48             ` Daniel Drake
@ 2018-05-24  8:33               ` Chris Chiu
  2018-06-04  9:27                 ` Benjamin Berg
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Chiu @ 2018-05-24  8:33 UTC (permalink / raw)
  To: Daniel Drake
  Cc: Andy Shevchenko, Benjamin Berg, Corentin Chary, Darren Hart,
	Andy Shevchenko, acpi4asus-user, Platform Driver,
	Linux Kernel Mailing List, Linux Upstreaming Team, Jian-Hong Pan

On Tue, May 22, 2018 at 9:48 PM, Daniel Drake <drake@endlessm.com> wrote:
> On Tue, May 22, 2018 at 4:11 AM, Chris Chiu <chiu@endlessm.com> wrote:
>> On Tue, May 22, 2018 at 5:18 PM, Andy Shevchenko
>>> Btw, I mistakenly thought that patch in the queue for-next, while it's
>>> not. So, I'm going to drop it now.
>>> Feel free to re-submit a new (better) version.
>>
>> So it's better to handle NOTIFY_KBD_BRTUP, BRTDOWN, BRTTOGGLE
>> (c4, c5, c7) in the driver and no need to pass it up to userspace? I'll work
>> out a patch for this.
>
> Yes, let's explore this approach before submitting another patch to Andy.
>
> Thanks
> Daniel

Hi Andy,
    I've made my change to set the brightness level directly in the
driver, but the
OSD doesn't show correctly correspond to the level value. The brightness shows
OK in /sys/class/led/xxxx/brighness but the OSD always shows level 0. I thought
GNOME should read the brightness from /sys before showing OSD?

Chris

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-05-24  8:33               ` Chris Chiu
@ 2018-06-04  9:27                 ` Benjamin Berg
  2018-06-05  1:59                   ` Darren Hart
  0 siblings, 1 reply; 12+ messages in thread
From: Benjamin Berg @ 2018-06-04  9:27 UTC (permalink / raw)
  To: Chris Chiu, Daniel Drake, Hans de Goede
  Cc: Andy Shevchenko, Corentin Chary, Darren Hart, Andy Shevchenko,
	acpi4asus-user, Platform Driver, Linux Kernel Mailing List,
	Linux Upstreaming Team, Jian-Hong Pan

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

Hi,

On Thu, 2018-05-24 at 16:33 +0800, Chris Chiu wrote:
> I've made my change to set the brightness level directly in the
> driver, but the
> OSD doesn't show correctly correspond to the level value. The brightness shows
> OK in /sys/class/led/xxxx/brighness but the OSD always shows level 0. I thought
> GNOME should read the brightness from /sys before showing OSD?

Sorry for the late response.

There is a special mechanism to report that the HW changed the
brightness. This works using the "brightness_hw_changed" sysfs
attribute. So you will need to set the LED_BRIGHT_HW_CHANGED flag on
the LED and then call led_classdev_notify_brightness_hw_changed to make
it work.

Userspace should correctly show the OSD when this is done.

Benjamin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support
  2018-06-04  9:27                 ` Benjamin Berg
@ 2018-06-05  1:59                   ` Darren Hart
  0 siblings, 0 replies; 12+ messages in thread
From: Darren Hart @ 2018-06-05  1:59 UTC (permalink / raw)
  To: Benjamin Berg
  Cc: Chris Chiu, Daniel Drake, Hans de Goede, Andy Shevchenko,
	Corentin Chary, Andy Shevchenko, acpi4asus-user, Platform Driver,
	Linux Kernel Mailing List, Linux Upstreaming Team, Jian-Hong Pan

On Mon, Jun 04, 2018 at 11:27:43AM +0200, Benjamin Berg wrote:
> Hi,
> 
> On Thu, 2018-05-24 at 16:33 +0800, Chris Chiu wrote:
> > I've made my change to set the brightness level directly in the
> > driver, but the
> > OSD doesn't show correctly correspond to the level value. The brightness shows
> > OK in /sys/class/led/xxxx/brighness but the OSD always shows level 0. I thought
> > GNOME should read the brightness from /sys before showing OSD?
> 
> Sorry for the late response.
> 
> There is a special mechanism to report that the HW changed the
> brightness. This works using the "brightness_hw_changed" sysfs
> attribute. So you will need to set the LED_BRIGHT_HW_CHANGED flag on
> the LED and then call led_classdev_notify_brightness_hw_changed to make
> it work.
> 
> Userspace should correctly show the OSD when this is done.

This makes sense. Userspace can't be aware of every platforms sys files,
so there needs to be a common mechanism. LED_BRIGHT_HW_CHANGED makes
sense.

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-06-05  1:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-03  3:04 [PATCH v2] platform/x86: asus-wmi: Add keyboard backlight toggle support Chris Chiu
2018-05-07 14:10 ` Andy Shevchenko
2018-05-07 14:46   ` Daniel Drake
2018-05-15  0:25     ` Daniel Drake
2018-05-15  7:52       ` Andy Shevchenko
2018-05-15 13:41       ` Benjamin Berg
2018-05-22  9:18         ` Andy Shevchenko
2018-05-22 10:11           ` Chris Chiu
2018-05-22 13:48             ` Daniel Drake
2018-05-24  8:33               ` Chris Chiu
2018-06-04  9:27                 ` Benjamin Berg
2018-06-05  1:59                   ` Darren Hart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).