Re: [ibm-acpi-devel] [PATCH] platform/x86: Add Lenovo ThinkPad PrivacyGuard.
On Thursday 22 August 2019 13:48:33 Alexander Schremmer wrote:
> This feature is found optionally in T480s, T490, T490s.
>
> The feature is called lcdshadow and visible via
> /proc/acpi/ibm/lcdshadow.
>
> The ACPI methods \_SB.PCI0.LPCB.EC.HKEY.{GSSS,,TSSS,CSSS} are
> available in these machines. They get, set, toggle or change the state
> apparently.
>
> The patch was tested on a 5.0 series kernel on a T480s.
>
> Signed-off-by: Alexander Schremmer
> ---
> .../admin-guide/laptops/thinkpad-acpi.rst | 23
> drivers/platform/x86/thinkpad_acpi.c | 112 ++
> 2 files changed, 135 insertions(+)
>
> diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> index adea0bf2acc5..822907dcc845 100644
> --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> @@ -49,6 +49,7 @@ detailed description):
> - Fan control and monitoring: fan speed, fan enable/disable
> - WAN enable and disable
> - UWB enable and disable
> + - LCD Shadow (PrivacyGuard) enable and disable
>
> A compatibility table by model and feature is maintained on the web
> site, http://ibm-acpi.sf.net/. I appreciate any success or failure
> @@ -1409,6 +1410,28 @@ Sysfs notes
> Documentation/driver-api/rfkill.rst for details.
>
>
> +LCD Shadow control
> +--
> +
> +procfs: /proc/acpi/ibm/lcdshadow
> +
> +Some newer T480s and T490s ThinkPads provide a feature called
> +PrivacyGuard. By turning this feature on, the usable vertical and
> +horizontal viewing angles of the LCD can be limited (as if some privacy
> +screen was applied manually in front of the display).
> +
> +procfs notes
> +
> +
> +The available commands are::
> +
> + echo '0' >/proc/acpi/ibm/lcdshadow
> + echo '1' >/proc/acpi/ibm/lcdshadow
> +
> +The first command ensures the best viewing angle and the latter one turns
> +on the feature, restricting the viewing angles.
> +
> +
> EXPERIMENTAL: UWB
> -----
Hello! Is not the whole /proc/apci/ibm API for new things obsoleted or
deprecated?
And should not rather it use platform driver in /sys/ (class?) namespace?
--
Pali Rohár
pali.ro...@gmail.com
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH 0/6] Introduce audio-mute LED trigger (and conversions to it)
On Wednesday 28 November 2018 21:34:10 Pavel Machek wrote: > Hi! > > > >>>> Looks good... except one detail: you have "tpacpi::micmute" and > > >>>> "dell::micmute". I know it follows "tradition", but we are trying to > > >>>> fix that at the moment. Laptop micmute button is a laptop micmute > > >>>> button, and userspace should not need to know what prefix to use > > >>>> depending on vendor. > > >>>> > > >>>> I'd suggest using "sys::micmute". > > >>> > > >>> I can imagine that in future some devices like keyboards would have also > > >>> mute led. We already have keyboards with mute key, so it is something > > >>> not unrealistic. What should be name convention for these mute leds? > > >>> > > >>> Is not "sys::" prefix too generic? > > >> > > >> Good point. I thought of "laptop::" but it's not always laptop. > > >> "builtin::"? Doesn't sound great, either. > > >> > > >> A nice godfather is required here... > > > > > > Just use sys:: :-). > > > > > > laptop:: would work for me, too. (It is always laptop in the cases we > > > are handling now, right?) > > > > > > When we get a keyboard with mute led, we'll have to decide if it > > > should be input6::mute -- because it is on keyboard, or if it is > > > sys::mute -- because the key is expected to mute whole system. > > > > drivers/input/input-leds.c seems to already support mute LED. > > It will be exposed as inputN::mute. > > > > Documentation/leds/leds-class.txt defines LED naming pattern > > to and "sys" does not look as > > something resembling device name. > > So what is your suggestion? I guess we should follow documentation. Or update documentation if it does not make sense to follow it. > I don't care much as long as it is same in tpacpi and dell > case. (Neither are device names, btw :-). > > Actually "::mute" would make sense, too. "::mute" is not a good idea due to name uniqueness. In case you would have two drivers which both provides "mute" led, then they need to have different name. Reason also why generic name "sys" is not a good idea. input subsystem seems to solved this problem by appending number after "input" word. I think that driver name or subsystem name would be usable together with number. Userspace application would be probably interested to distinguish between "mute led which is part of laptop" and "mute led which is available on external USB keyboard". If external USB keyboard is identified as "input7" device, then "input7::mute" is a good name for mute key. But "sys::mute" does not say anything to which device or hardware it belongs nor does not solve problem that which device/driver/subsystem should have privilege to take this "sys" name. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: PGP signature ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH 0/6] Introduce audio-mute LED trigger (and conversions to it)
On Tuesday 27 November 2018 09:44:18 Pavel Machek wrote: > Looks good... except one detail: you have "tpacpi::micmute" and > "dell::micmute". I know it follows "tradition", but we are trying to > fix that at the moment. Laptop micmute button is a laptop micmute > button, and userspace should not need to know what prefix to use > depending on vendor. > > I'd suggest using "sys::micmute". I can imagine that in future some devices like keyboards would have also mute led. We already have keyboards with mute key, so it is something not unrealistic. What should be name convention for these mute leds? Is not "sys::" prefix too generic? -- Pali Rohár pali.ro...@gmail.com ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH 0/6] Introduce audio-mute LED trigger (and conversions to it)
On Monday 26 November 2018 18:11:20 Takashi Iwai wrote: > Hi, > > this is patch series I've hacked after useful conversation with > Pavel. Basically this adds a new LED trigger audio-mute and > audio-micmute, and convert the HD-audio driver and the platform > drivers to use the LED trigger instead of the ugly direct dynamic > symbol binding. > > The latest version of patches are found in topic/leds-trigger branch > in my sound git tree. > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git > > As these are cross-tree patches, the branch above is based cleanly on > v4.20-rc3, so that it can be merged well to multiple trees. > > Once after getting the ACK's, I'll add tags and fixate for merges. > > This patch series don't include huawei-wmi stuff; so Huawei patches > need rework. I already have some piece of changes for huawei-wmi, so > please ping me if needed. > > I checked briefly on my Dell laptop, and a Thinkpad model. > Wider tests are appreciated, of course. Hi! Thanks for patch series. It is really improvement in current mute led situation. I quickly looked at patches and they look good. So you can add my Acked-By: Pali Rohár . -- Pali Rohár pali.ro...@gmail.com ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Thursday 21 June 2018 13:35:21 Damjan Georgievski wrote: > > I applied them to Debian's kernel (with slightly modification) and they > > are working fine. Via "alsamixer -c 0" I can change "Mic Mute-LED Mode" > > to "Follow Capture" and then led is ON only when microphone is ON. > > Recording from microphone is working in both configurations. > > hm this is the opposite of how it works on my T450s and X1, where the > led is ON when microphone is muted. Purpose of this patch it to allow user to configure that LED. As I wrote above, when I *changed* "Mic Mute-LED Mode" to "Follow Capture" in alsamixer, then led is ON when mic is ON. Default configuration (prior changing anything) in alsamixer is "Follow Mute" when led is ON when mic is OFF. That patch allows you to change how LED behave. > (I do wish the led would be ON when the microphone is not muted ie. listening) -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Monday 18 June 2018 12:11:44 Pali Rohár wrote: > On Friday 15 June 2018 20:36:28 Henrique de Moraes Holschuh wrote: > > On Fri, 15 Jun 2018, Pali Rohár wrote: > > > means set. What is SHDA doing, I have not figured out. It does not > > > > Look at the HDA mixer state, SHDA is likely to be directly messing with > > it. > > Now I found following email thread between you and Alex with patch which > configures HDA mixer via that SHDA method: > > https://www.spinics.net/lists/platform-driver-x86/msg03206.html > > But looks like that patch was never included into mainline kernel... > > Seems it would be needed in some part (to not break older Thinkpads) for > that LED support in alsa mixer about which Takashi Iwai wrote. Hi Alex, do you have other details or documentation for your patch "thinkpad_acpi: added BIOS mute interfaces for volume" (linked above) which you sent to platform-driver-x86 list 6 years ago? It calls SHDA thinkpad ACPI function and I would like to know more what is this function doing... -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Monday 18 June 2018 17:35:18 Takashi Iwai wrote: > On Mon, 18 Jun 2018 13:26:18 +0200, > Pali Rohár wrote: > > > > On Monday 18 June 2018 13:21:21 Pali Rohár wrote: > > > Now I see that in Debian backports is some 4.16.12 kernel version. I can > > > try this one (and later compile one from Linus tree). > > > > Tested, this version is working fine and leds works as expected. And now > > also snd_hda_codec_realtek is loaded. > > Good to hear :) > > Below are test patches to allow user choosing the mic mute LED > behavior on Thinkpad, too. Note: totally untested! > > Let me know if these work for you. I applied them to Debian's kernel (with slightly modification) and they are working fine. Via "alsamixer -c 0" I can change "Mic Mute-LED Mode" to "Follow Capture" and then led is ON only when microphone is ON. Recording from microphone is working in both configurations. If testing with Debian's kernel is enough, you can add my: Tested-by: Pali Rohár Anyway, (this question is also for Henrique), what can we do with MUTE led to work? Support for controlling it was there, but was revered in commit 3530febb5c7636f6b26d15637f68296804d26491. -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Tuesday 19 June 2018 10:42:12 Takashi Iwai wrote: > On Tue, 19 Jun 2018 10:37:12 +0200, > Pali Rohár wrote: > > Interesting is this part: > > > > $ head /proc/asound/card0/codec#0 > > Codec: Realtek ALC257 > > Address: 0 > > AFG Function Id: 0x1 (unsol 1) > > Vendor Id: 0x10ec0257 > > Subsystem Id: 0x17aa2258 > > Revision Id: 0x11 > > No Modem Function Group found > > Default PCM: > > rates [0x560]: 44100 48000 96000 192000 > > bits [0xe]: 16 20 24 > > > > It uses vendor id 0x10ec0257 and support for it was added into > > snd-hda-codec-realtek in commit f429e7e494 which was introduced in > > version v4.15. > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f429e7e494afaded76e62c6f98211a635aa03098 > > > > I applied this patch to Debian's 4.9 kernel and mute & mic mute leds > > started working. This one patch was enough. > > > > Takashi, it is possible to backport this patch into -stable trees? > > It has already a Cc-to-stable, so it should have been backported. > Maybe it wasn't applicable at that time due to lack of other dependent > patches... > > In anyway, feel free to ping Greg for including it to 4.9.x if it > works for you now. I sent email to Greg, but it was eaten by "email-bot of Greg Kroah-Hartman's inbox". So I sent another patch email to stable ML. -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Monday 18 June 2018 13:21:21 Pali Rohár wrote: > On Monday 18 June 2018 12:36:31 Takashi Iwai wrote: > > On Mon, 18 Jun 2018 12:28:06 +0200, > > Pali Rohár wrote: > > > > > > On Saturday 16 June 2018 18:02:14 Takashi Iwai wrote: > > > > On Sat, 16 Jun 2018 17:43:09 +0200, > > > > Pali Rohár wrote: > > > > > > > > > > On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote: > > > > > > On Fri, 15 Jun 2018 21:09:59 +0200, > > > > > > Pali Rohár wrote: > > > > > > > > > > > > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote: > > > > > > > > On Fri, 08 Jun 2018 13:10:57 +0200, > > > > > > > > Pali Rohár wrote: > > > > > > > > > > > > > > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s > > > > > > > > > I'm seeing > > > > > > > > > a strange behavior of LEDs which are integrated into mic mute > > > > > > > > > (Fn+F4) > > > > > > > > > and mute (Fn+F1) keys. > > > > > > > > > > > > > > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working > > > > > > > > > fine. When > > > > > > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set > > > > > > > > > 2 keyboard > > > > > > > > > input device and also turn on/of mute led. But when micmute > > > > > > > > > key is > > > > > > > > > pressed then, nothing happen. No key event is reported and > > > > > > > > > also led is > > > > > > > > > not turned on/off. > > > > > > > > > > > > > > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both > > > > > > > > > buttons > > > > > > > > > mute and micmute correctly generates input events; mute via > > > > > > > > > AT keyboard > > > > > > > > > and micmute via ThinkPad Extra Buttons. But led is not > > > > > > > > > changed. When > > > > > > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and > > > > > > > > > micmute) and > > > > > > > > > leds after pressing any of those buttons, leds are not turned > > > > > > > > > on again. > > > > > > > > > > > > > > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button > > > > > > > > > again start > > > > > > > > > switching led on/off. > > > > > > > > > > > > > > > > > > So it seems that some init sequence of thinkpad_acpi.ko > > > > > > > > > breaks mute led. > > > > > > > > > And fini sequence of thinkpad_acpi.ko makes mute led working > > > > > > > > > again. > > > > > > > > > > > > > > > > Usually the mute LED on Thinkpad is triggered from HD-audio > > > > > > > > driver > > > > > > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via > > > > > > > > symbol_request(tpacpi_led_set). I thought thinkpad_acpi is > > > > > > > > auto-loaded when the module gets bound. > > > > > > > > > > > > > > > > A possible explanation would be that TPT480s has neither > > > > > > > > IBM0068, > > > > > > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not > > > > > > > > auto-loaded. > > > > > > > > > > > > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 > > > > > > > alias for > > > > > > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded > > > > > > > automatically. But I > > > > > > > have put thinkpad_acpi into /etc/modules and it is now > > > > > > > automatically > > > > > > > loaded at boot. > > > > > > > > > > > > That's odd. It's exposed via > > > > > > MODULE_DEVICE_TABL
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Monday 18 June 2018 13:21:21 Pali Rohár wrote: > Now I see that in Debian backports is some 4.16.12 kernel version. I can > try this one (and later compile one from Linus tree). Tested, this version is working fine and leds works as expected. And now also snd_hda_codec_realtek is loaded. -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Monday 18 June 2018 12:36:31 Takashi Iwai wrote: > On Mon, 18 Jun 2018 12:28:06 +0200, > Pali Rohár wrote: > > > > On Saturday 16 June 2018 18:02:14 Takashi Iwai wrote: > > > On Sat, 16 Jun 2018 17:43:09 +0200, > > > Pali Rohár wrote: > > > > > > > > On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote: > > > > > On Fri, 15 Jun 2018 21:09:59 +0200, > > > > > Pali Rohár wrote: > > > > > > > > > > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote: > > > > > > > On Fri, 08 Jun 2018 13:10:57 +0200, > > > > > > > Pali Rohár wrote: > > > > > > > > > > > > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s > > > > > > > > I'm seeing > > > > > > > > a strange behavior of LEDs which are integrated into mic mute > > > > > > > > (Fn+F4) > > > > > > > > and mute (Fn+F1) keys. > > > > > > > > > > > > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working > > > > > > > > fine. When > > > > > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 > > > > > > > > keyboard > > > > > > > > input device and also turn on/of mute led. But when micmute key > > > > > > > > is > > > > > > > > pressed then, nothing happen. No key event is reported and also > > > > > > > > led is > > > > > > > > not turned on/off. > > > > > > > > > > > > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both > > > > > > > > buttons > > > > > > > > mute and micmute correctly generates input events; mute via AT > > > > > > > > keyboard > > > > > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. > > > > > > > > When > > > > > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and > > > > > > > > micmute) and > > > > > > > > leds after pressing any of those buttons, leds are not turned > > > > > > > > on again. > > > > > > > > > > > > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button > > > > > > > > again start > > > > > > > > switching led on/off. > > > > > > > > > > > > > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks > > > > > > > > mute led. > > > > > > > > And fini sequence of thinkpad_acpi.ko makes mute led working > > > > > > > > again. > > > > > > > > > > > > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver > > > > > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via > > > > > > > symbol_request(tpacpi_led_set). I thought thinkpad_acpi is > > > > > > > auto-loaded when the module gets bound. > > > > > > > > > > > > > > A possible explanation would be that TPT480s has neither IBM0068, > > > > > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not > > > > > > > auto-loaded. > > > > > > > > > > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 > > > > > > alias for > > > > > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. > > > > > > But I > > > > > > have put thinkpad_acpi into /etc/modules and it is now automatically > > > > > > loaded at boot. > > > > > > > > > > That's odd. It's exposed via > > > > > MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids); > > > > > > > > > > It's been already in 4.9. At this point, something is fishy. > > > > > > > > $ /sbin/modinfo thinkpad_acpi | grep alias > > > > alias: dmi:bvnIBM:bvrI[MU]ET??WW* > > > > alias: tpacpi > > > > alias: acpi*:LEN0068:* > > > > alias: acpi*:IBM0068:* > > > > > > > > No there is no LEN0268 on 4.9. > &
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Saturday 16 June 2018 18:02:14 Takashi Iwai wrote:
> On Sat, 16 Jun 2018 17:43:09 +0200,
> Pali Rohár wrote:
> >
> > On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote:
> > > On Fri, 15 Jun 2018 21:09:59 +0200,
> > > Pali Rohár wrote:
> > > >
> > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote:
> > > > > On Fri, 08 Jun 2018 13:10:57 +0200,
> > > > > Pali Rohár wrote:
> > > > > >
> > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm
> > > > > > seeing
> > > > > > a strange behavior of LEDs which are integrated into mic mute
> > > > > > (Fn+F4)
> > > > > > and mute (Fn+F1) keys.
> > > > > >
> > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine.
> > > > > > When
> > > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2
> > > > > > keyboard
> > > > > > input device and also turn on/of mute led. But when micmute key is
> > > > > > pressed then, nothing happen. No key event is reported and also led
> > > > > > is
> > > > > > not turned on/off.
> > > > > >
> > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both
> > > > > > buttons
> > > > > > mute and micmute correctly generates input events; mute via AT
> > > > > > keyboard
> > > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When
> > > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute)
> > > > > > and
> > > > > > leds after pressing any of those buttons, leds are not turned on
> > > > > > again.
> > > > > >
> > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again
> > > > > > start
> > > > > > switching led on/off.
> > > > > >
> > > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute
> > > > > > led.
> > > > > > And fini sequence of thinkpad_acpi.ko makes mute led working again.
> > > > >
> > > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver
> > > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via
> > > > > symbol_request(tpacpi_led_set). I thought thinkpad_acpi is
> > > > > auto-loaded when the module gets bound.
> > > > >
> > > > > A possible explanation would be that TPT480s has neither IBM0068,
> > > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded.
> > > >
> > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for
> > > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I
> > > > have put thinkpad_acpi into /etc/modules and it is now automatically
> > > > loaded at boot.
> > >
> > > That's odd. It's exposed via
> > > MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids);
> > >
> > > It's been already in 4.9. At this point, something is fishy.
> >
> > $ /sbin/modinfo thinkpad_acpi | grep alias
> > alias: dmi:bvnIBM:bvrI[MU]ET??WW*
> > alias: tpacpi
> > alias: acpi*:LEN0068:*
> > alias: acpi*:IBM0068:*
> >
> > No there is no LEN0268 on 4.9.
>
> OK, that's the cause. It's really old.
>
> The commit a3c42a467a25 ("platform/x86: thinkpad_acpi: Adding new
> hotkey ID for Lenovo thinkpad") has to be backported.
This commit just autoloads thinkpad_acpi driver, right? So manual
modprobe (for now) is OK too?
> Also, in the HD-audio side, the commit 2ecb704a1290 ("ALSA: hda - add
> a new condition to check if it is thinkpad") is needed, too.
This commit was introduced in 4.9 and Debian kernel has it. I looked
into Debian source code and there is check for LEN0268 in file
sound/pci/hda/thinkpad_helper.c
> > > > I also compiled upstream version of thinkpad_acpi.ko, loaded it in
> > > > Stretch kernel, but it behaves in same way.
> > > >
> > > > Maybe... there could be a problem that thinkpad_acpi.ko must be already
> > > > loaded when sound subsystem is doing initialization? If yes, this could
> > > > explain it as /etc/modules is loaded at later stage and manuall
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Friday 15 June 2018 20:36:28 Henrique de Moraes Holschuh wrote: > On Fri, 15 Jun 2018, Pali Rohár wrote: > > means set. What is SHDA doing, I have not figured out. It does not > > Look at the HDA mixer state, SHDA is likely to be directly messing with > it. Now I found following email thread between you and Alex with patch which configures HDA mixer via that SHDA method: https://www.spinics.net/lists/platform-driver-x86/msg03206.html But looks like that patch was never included into mainline kernel... Seems it would be needed in some part (to not break older Thinkpads) for that LED support in alsa mixer about which Takashi Iwai wrote. -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Saturday 16 June 2018 09:05:41 Takashi Iwai wrote: > On Fri, 15 Jun 2018 21:09:59 +0200, > Pali Rohár wrote: > > > > On Friday 15 June 2018 14:51:47 Takashi Iwai wrote: > > > On Fri, 08 Jun 2018 13:10:57 +0200, > > > Pali Rohár wrote: > > > > > > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing > > > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4) > > > > and mute (Fn+F1) keys. > > > > > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When > > > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard > > > > input device and also turn on/of mute led. But when micmute key is > > > > pressed then, nothing happen. No key event is reported and also led is > > > > not turned on/off. > > > > > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons > > > > mute and micmute correctly generates input events; mute via AT keyboard > > > > and micmute via ThinkPad Extra Buttons. But led is not changed. When > > > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and > > > > leds after pressing any of those buttons, leds are not turned on again. > > > > > > > > When thinkpad_acpi.ko is unloaded, then pressing mute button again start > > > > switching led on/off. > > > > > > > > So it seems that some init sequence of thinkpad_acpi.ko breaks mute led. > > > > And fini sequence of thinkpad_acpi.ko makes mute led working again. > > > > > > Usually the mute LED on Thinkpad is triggered from HD-audio driver > > > (sound/pci/hda/thinkpad_helper.c), and it's a soft-bound via > > > symbol_request(tpacpi_led_set). I thought thinkpad_acpi is > > > auto-loaded when the module gets bound. > > > > > > A possible explanation would be that TPT480s has neither IBM0068, > > > LEN0068 nor LEN0268 ACPI HIDs, hence the driver is not auto-loaded. > > > > I have Debian Stretch kernel (4.9) which does not have LEN0268 alias for > > thinkpad_acpi.ko. So thinkpad_acpi.ko is not loaded automatically. But I > > have put thinkpad_acpi into /etc/modules and it is now automatically > > loaded at boot. > > That's odd. It's exposed via > MODULE_DEVICE_TABLE(acpi, ibm_htk_device_ids); > > It's been already in 4.9. At this point, something is fishy. $ /sbin/modinfo thinkpad_acpi | grep alias alias: dmi:bvnIBM:bvrI[MU]ET??WW* alias: tpacpi alias: acpi*:LEN0068:* alias: acpi*:IBM0068:* No there is no LEN0268 on 4.9. > > I also compiled upstream version of thinkpad_acpi.ko, loaded it in > > Stretch kernel, but it behaves in same way. > > > > Maybe... there could be a problem that thinkpad_acpi.ko must be already > > loaded when sound subsystem is doing initialization? If yes, this could > > explain it as /etc/modules is loaded at later stage and manually loading > > of new version of thinkpad_acpi.ko at runtime does not help when sound > > subsystem is already running. > > Not really. The HD-audio driver tries to bind with tpacpi_led_set() > via symbol_request(). i.e. if it's not present, it tries to load a > module. > > Check whether hda_fixup_thinkpad_acpi() is called and the symbol gets > loaded or not. > > But, I don't think it's worth to debug such an old kernel primarily. It is default one used by the last released Debian stable version. > Could you test the latest Linus tree or 4.17.x at least as a test > basis? Ok, will do that later. > > > (In HD-audio driver side, the ACPI ID is checked and the mute LED > > > control is applied only to these three IDs, too.) > > > Meanwhile, when you load thinkpad_acpi, it does still recognize some > > > device and initialize it. By the initialization, it goes out of BIOS > > > control, and the OS control is expected... This is my wild guess. > > > > > > > > > BTW, the reason we have no LED class for these is that we don't want > > > to confuse users by providing multiple ways to access to the single > > > stuff. We've had already the mute LED control from the audio driver > > > since long time ago, we don't want to drop and enforce the user-space > > > solution (that is anyway flakier than in kernel in most cases). > > > > If possible... I would prefer swapped LED behavior: turn led on when > > microphone is turned on AND turn led off when microphone is turned off. > > Becau
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Friday 15 June 2018 09:30:07 Henrique de Moraes Holschuh wrote:
> On Fri, 15 Jun 2018, Pali Rohár wrote:
> > Henrique, any idea why there are no exported led classes for mute and
> > micmute? And how are suppose to be controlled?
>
> I have to look into the code, it was contributed by someone who had
> access to the proper hardware to test it.
>
> But the way *I* would like it to work is this:
>
> 1. When implemented in *hardware* or *EC*, let the hardware and EC take
>full control, and never allow the operating system to mess with it.
>So, it becomes much harder for that LED to "lie".
This means that kernel should not export any led class device. Or when
exported, then "set" operation should always fail.
> 2. Otherwise implement it in-kernel, so that userspace cannot unmute
>when the human has activated the "mute" switch, and the LED cannot be
>controlled by userspace to lie (report mute when it is not mute).
This looks like a good candidate to use led "trigger" interface. Create
a mute trigger and attach it to that led device.
I hope that this is what Pavel means.
> It might, or might not be possible to achieve the above.
>
> > > With thinkpad_acpi.ko unloaded, hardware drives the LEDs, so nothing
> > > for us to do...
> >
> > So somehow tell thinkpad_acpi.ko to let hardware control those LEDs when
> > thinkpad_acpi.ko is loaded?
>
> I.e. look into the DSDT and XSDT, to find out what it is doing. It will
> be there: it is very rare for the thinkpad EC itself to implement these
> behavior changes.
DSDT helped. Thanks!
Function with "EC.LED" name is self explaining and also "EC.HKEY". I
used acpi_call.ko for debugging and here are results how to control
these two leds on ThinkPad T480s:
Scope (\_SB.PCI0.LPCB.EC.HKEY)
{
Method (MMTG, 0, NotSerialized)
{
Local0 = 0x0101
If (HDMC)
{
Local0 |= 0x0001
}
Return (Local0)
}
Method (MMTS, 1, NotSerialized)
{
If (HDMC)
{
Noop
}
ElseIf (Arg0 == 0x02)
{
\_SB.PCI0.LPCB.EC.LED (0x0E, 0x80)
}
ElseIf (Arg0 == 0x03)
{
\_SB.PCI0.LPCB.EC.LED (0x0E, 0xC0)
}
Else
{
\_SB.PCI0.LPCB.EC.LED (0x0E, 0x00)
}
}
}
Scope (\_SB.PCI0.LPCB.EC.HKEY)
{
Method (GSMS, 1, NotSerialized)
{
Return (\AUDC (0x00, 0x00))
}
Method (SSMS, 1, NotSerialized)
{
Return (\AUDC (0x01, (Arg0 & 0x01)))
}
Method (SHDA, 1, NotSerialized)
{
Local0 = Arg0
If ((OSYS >= 0x07DF) && (Local0 == 0x01))
{
Local0 = 0x02
}
Return (\AUDC (0x02, (Local0 & 0x03)))
}
}
Method (AUDC, 2, NotSerialized)
{
Return (SMI (0x14, 0x07, Arg0, Arg1, 0x00))
}
MMTS controls mic mute led. When Arg0 is 0x02 then mic mute led is
turned on. When it is 0x03 then it starts blinking. And otherwise is
turned off. MM in name probably means MicMute and S as set. I guess that
MMTG (G as get) would return capabilities as it returns constant.
blink: echo "\_SB.PCI0.LPCB.EC.HKEY.MMTS 3" > /proc/acpi/call
on:echo "\_SB.PCI0.LPCB.EC.HKEY.MMTS 2" > /proc/acpi/call
off: echo "\_SB.PCI0.LPCB.EC.HKEY.MMTS 0" > /proc/acpi/call
SSMS controls mute led. It calls some SMI method via AUDC. When Arg0 has
first bit set to one then mute led is turned on. When set to zero then
is turned off. GSMS takes one parameter, but ignores it. When mute led
is turned off then it returns 0x100. When mute led is turned on then it
return 0x101. So looks like that "G" in GSMS means "get" and "S" in SSMS
means set. What is SHDA doing, I have not figured out. It does not
change GSMS result nor led status. When called with argument 1..10 then
it returns always returned 0x0 except for 3 and 7 it returned
0x8000. There is no blinking support (or at least I have not figured
out).
on:echo "\_SB.PCI0.LPCB.EC.HKEY.SSMS 1" > /proc/acpi/call
off: echo "\_SB.PCI0.LPCB.EC.HKEY.SSMS 0" > /proc/acpi/call
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
On Friday 15 June 2018 13:26:06 Pavel Machek wrote: > Hi! > > > Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing > > a strange behavior of LEDs which are integrated into mic mute (Fn+F4) > > and mute (Fn+F1) keys. > > > > When thinkpad_acpi.ko is not loaded, then mute key is working fine. When > > pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard > > input device and also turn on/of mute led. But when micmute key is > > pressed then, nothing happen. No key event is reported and also led is > > not turned on/off. > > > > On the other hand, when thinkpad_acpi.ko is loaded, then both buttons > > mute and micmute correctly generates input events; mute via AT keyboard > > and micmute via ThinkPad Extra Buttons. But led is not changed. When > > thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and > > leds after pressing any of those buttons, leds are not turned on again. > > With thinkpad_acpi.ko loaded, kernel should handle the LEDs, right? I suppose. But I would be happy even with working "hardware" controlling (which is working fine when thinkpad_acpi.ko is unloaded). > Do we have a support for that? In thinkpad_acpi.c there are some TPACPI_LED_MUTE and TPACPI_LED_MICMUTE keywords. And also function static int mute_led_on_off(). So some kind of support there is. > Do they have directories in /sys/class/leds? What are the triggers there? No. $ ll /sys/class/leds total 0 drwxr-xr-x 2 root root 0 Jun 15 13:27 ./ drwxr-xr-x 51 root root 0 Jun 15 13:27 ../ lrwxrwxrwx 1 root root 0 Jun 15 13:27 input0::capslock -> ../../devices/platform/i8042/serio0/input/input0/input0::capslock/ lrwxrwxrwx 1 root root 0 Jun 15 13:27 input0::numlock -> ../../devices/platform/i8042/serio0/input/input0/input0::numlock/ lrwxrwxrwx 1 root root 0 Jun 15 13:27 input0::scrolllock -> ../../devices/platform/i8042/serio0/input/input0/input0::scrolllock/ lrwxrwxrwx 1 root root 0 Jun 15 13:27 phy0-led -> ../../devices/pci:00/:00:1c.6/:3d:00.0/leds/phy0-led/ lrwxrwxrwx 1 root root 0 Jun 15 13:27 tpacpi::kbd_backlight -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::kbd_backlight/ lrwxrwxrwx 1 root root 0 Jun 15 13:27 tpacpi::power -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::power/ lrwxrwxrwx 1 root root 0 Jun 15 13:27 tpacpi::standby -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::standby/ lrwxrwxrwx 1 root root 0 Jun 15 13:27 tpacpi::thinklight -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::thinklight/ lrwxrwxrwx 1 root root 0 Jun 15 13:27 tpacpi::thinkvantage -> ../../devices/platform/thinkpad_acpi/leds/tpacpi::thinkvantage/ But looks like that thinkpad_acpi.c does not define any struct led_classdev nor struct tpacpi_led_classdev for MUTE or MICMUTE. Henrique, any idea why there are no exported led classes for mute and micmute? And how are suppose to be controlled? > With thinkpad_acpi.ko unloaded, hardware drives the LEDs, so nothing > for us to do... So somehow tell thinkpad_acpi.ko to let hardware control those LEDs when thinkpad_acpi.ko is loaded? -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
[ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE
Hi! With up-to-date thinkpad_acpi.ko driver on ThinkPad T480s I'm seeing a strange behavior of LEDs which are integrated into mic mute (Fn+F4) and mute (Fn+F1) keys. When thinkpad_acpi.ko is not loaded, then mute key is working fine. When pressed, it correctly generates KEY_MUTE on AT Translated Set 2 keyboard input device and also turn on/of mute led. But when micmute key is pressed then, nothing happen. No key event is reported and also led is not turned on/off. On the other hand, when thinkpad_acpi.ko is loaded, then both buttons mute and micmute correctly generates input events; mute via AT keyboard and micmute via ThinkPad Extra Buttons. But led is not changed. When thinkpad_acpi.ko is loaded it turn off both leds (mute and micmute) and leds after pressing any of those buttons, leds are not turned on again. When thinkpad_acpi.ko is unloaded, then pressing mute button again start switching led on/off. So it seems that some init sequence of thinkpad_acpi.ko breaks mute led. And fini sequence of thinkpad_acpi.ko makes mute led working again. -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] ThinkPad T480s & thinkpad_acpi.ko
On Friday 08 June 2018 12:40:32 Andy Shevchenko wrote: > On Fri, Jun 8, 2018 at 11:12 AM, Pali Rohár wrote: > > Hi! I just found out that thinkpad_acpi.ko is not automatically loaded > > on new ThinkPad T480s. Is there any reason for it? Manually calling > > modprobe thinkpad_acpi is working fine, but I would expect that it > > should be loaded automatically. > > > > Here is dmesg output after loading it manually: > > > > thinkpad_acpi: ThinkPad ACPI Extras v0.25 > > thinkpad_acpi: http://ibm-acpi.sf.net/ > > thinkpad_acpi: ThinkPad BIOS N22ET34W (1.11 ), EC unknown > > thinkpad_acpi: Lenovo ThinkPad T480s, model 20L7S03000 > > thinkpad_acpi: radio switch found; radios are enabled > > thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness > > control, supported by the ACPI video driver > > thinkpad_acpi: Disabling thinkpad-acpi brightness events by default... > > thinkpad_acpi: rfkill switch tpacpi_bluetooth_sw: radio is unblocked > > thinkpad_acpi: Standard ACPI backlight interface available, not loading > > native one > > Did you run depmod -a? > Is it present in the output? > > There are three ways of enumeration as far as I can see from the driver's > code: > - by short driver name "tpacpi" (not likely our case) > - by DMI matching (only for old BIOS'es) > - by ACPI ID > > So, does the ACPI contain one of the listed IDs? > > #define TPACPI_ACPI_IBM_HKEY_HID"IBM0068" > #define TPACPI_ACPI_LENOVO_HKEY_HID "LEN0068" > #define TPACPI_ACPI_LENOVO_HKEY_V2_HID "LEN0268" > ? DSDT contains only LEN0268. And so I found reason. I'm using Debian which has only 4.9 kernel and LEN0268 identifier was added in git commit a3c42a467a254a17236ab817d5c7c6bc054e4f84 for 4.10. -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
[ibm-acpi-devel] ThinkPad T480s & thinkpad_acpi.ko
Hi! I just found out that thinkpad_acpi.ko is not automatically loaded on new ThinkPad T480s. Is there any reason for it? Manually calling modprobe thinkpad_acpi is working fine, but I would expect that it should be loaded automatically. Here is dmesg output after loading it manually: thinkpad_acpi: ThinkPad ACPI Extras v0.25 thinkpad_acpi: http://ibm-acpi.sf.net/ thinkpad_acpi: ThinkPad BIOS N22ET34W (1.11 ), EC unknown thinkpad_acpi: Lenovo ThinkPad T480s, model 20L7S03000 thinkpad_acpi: radio switch found; radios are enabled thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver thinkpad_acpi: Disabling thinkpad-acpi brightness events by default... thinkpad_acpi: rfkill switch tpacpi_bluetooth_sw: radio is unblocked thinkpad_acpi: Standard ACPI backlight interface available, not loading native one -- Pali Rohár pali.ro...@gmail.com -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger
On Friday 25 November 2016 12:14:56 Hans de Goede wrote: > Hi, > > On 25-11-16 11:01, Pavel Machek wrote: > > Hi! > > > >> In view of the above we could report hw brightness changes with > >> POLLPRI on brightness file, but unfortunately we can't because it > >> is impossible to guarantee that readout of brightness file will > >> return the brightness the POLLPRI was meant to notify about. > > > > Agreed here. > > > >> That's why a separate read only file seems to be the only proper > >> solution. > > > > Yes please. And lets make self-changing leds into a trigger, as > > proposed, and as Hans' patch should be already doing. > > > >> Moreover, the file should return the brightness from the time > >> of last POLLPRI. > > > > Not sure I agree here. Normally, kernel returns current state for > > variables, does not track "old" state. > > Agreed, storing the last state just unnecessarily complicates things. > > So do we have a consensus on implementing a new hw_brightness_change > sysfs attribute now, which only some LEDs will have, can be polled > to detect changed done autonomously by the hardware and returns > the current / actual LED brightness when read ? > > As for the modeling how the hotkey controls the LED as a trigger, > although I do like this from one pov, I can see Jacek's point that > this is confusing as there really is nothing to configure here, > where as normally a user could do "echo none > trigger" to break > the link. So I think that is best (cleanest /minimal non confusing > API) with just the hw_brightness_change sysfs-attribute and not > model this as a trigger. I can accept with this solution (no trigger, event on new sysfs file which returns current/actual brightness state, new sysfs file only for devices which can report brightness state). But I'm not sure if it is really fixing that original problem with high power usage... As wrote in some previous emails, consider "actual_brightness" sysfs name which is already used for this purpose by backlight subsystem -- because for consistency. backlight devices have: actual_brightness, brightness, max_brightness. > That, or fall back to my latest patch-set as posted, I still like > that one the most. > > Regards, > > Hans -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger
On Thursday 24 November 2016 22:35:52 Jacek Anaszewski wrote: > > I understood that we cannot notify about changes done by CPU > > trigger due to high power usage... Or not? > > Exactly. So in this case exporting any new sysfs file (or using existing) which report POLLPRI events for LED devices with active trigger is not accepted. This means that trigger itself is problematic and trigger code should tell to led subsystem if it should send POLLPRI events to userspace or not. E.g. cpu trigger should tell led subsystem that events must not be sent and e.g. rfkill trigger could tell that events can be sent... -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger
On Thursday 24 November 2016 17:21:19 Jacek Anaszewski wrote: > On 11/24/2016 04:36 PM, Pali Rohár wrote: > > On Thursday 24 November 2016 16:32:06 Jacek Anaszewski wrote: > >> Since it has been reported that POLLPRI notifications on > >> brightness file can lead to increased power consumption, and > >> having my above statement I don't think that it is a good idea to > >> use brightness file for this. > > > > How is brightness file different from others that it cannot issue > > POLLPRI notification? > > > > I understood that problem is there in case that LED level is > > changed too many times per second (like by CPU trigger). > > > > If this is not that problem can you describe real issue, why we > > cannot use POLLPRI for brightness file? > > It would be inconsistent not to notify all brightness changes on > brightness file. We should notify all of them or none. I understood that we cannot notify about changes done by CPU trigger due to high power usage... Or not? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger
On Thursday 24 November 2016 16:32:06 Jacek Anaszewski wrote: > Since it has been reported that POLLPRI notifications on brightness > file can lead to increased power consumption, and having my above > statement I don't think that it is a good idea to use brightness > file for this. How is brightness file different from others that it cannot issue POLLPRI notification? I understood that problem is there in case that LED level is changed too many times per second (like by CPU trigger). If this is not that problem can you describe real issue, why we cannot use POLLPRI for brightness file? > >Yes, how triggers interact with brightness file, what happen when you > >write 0 on active trigger, > > There is already a patch in linux-next adding the following: > > > + Writing 0 to this file clears active trigger. > + > + Writing non-zero to this file while trigger is active changes > the > + top brightness trigger is going to use. > + Great! > >what happen when you read brightness file > >with active trigger / without trigger. > > > > Yes, this needs to be covered too. -- Pali Rohár pali.ro...@gmail.com -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger
On Thursday 24 November 2016 15:21:36 Jacek Anaszewski wrote:
> On 11/24/2016 10:15 AM, Pali Rohár wrote:
> >On Wednesday 23 November 2016 12:01:02 Jacek Anaszewski wrote:
> >>I would also appreciate your opinion on the other solution to the
> >>problem of notifying brightness changes originating from hardware,
> >>i.e. hw_brightness_change{_ro} file, that would support POLLPRI events,
> >>and reading brightness.
> >
> >Another idea:
> >
> >If no trigger is active then led subsystem will invoke POLLPRI on
> >"brightness" sysfs file.
> >
> >And if there is active trigger then only trigger code could invoke
> >POLLPRI on "brightness" file.
> >
> >This could solve problem with high CPU load and power usage when e.g.
> >cpu trigger is active (and cpu trigger will not implement any POLLPRI).
> >
> >Do not know if this is really enough for your situation, it is just and
> >another idea.
>
> This way we would be losing POLLPRI events when trigger is active,
> whereas it would be useful to have ones in some use cases.
In case it makes sense, trigger can implement that POLLPRI event. E.g.
for CPU trigger it probably does not make sense (or at least send
POLLPRI event lot of times per second).
> >But first please update documentation in ABI/testing to match current
> >situation. That is really needed.
> >
>
> I suppose that you're thinking about behaviour on brightness file
> reading? Is there anything else you'd like to have clarified in the doc?
Yes, how triggers interact with brightness file, what happen when you
write 0 on active trigger, what happen when you read brightness file
with active trigger / without trigger.
--
Pali Rohár
pali.ro...@gmail.com
--
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger
On Wednesday 23 November 2016 12:01:02 Jacek Anaszewski wrote:
> I would also appreciate your opinion on the other solution to the
> problem of notifying brightness changes originating from hardware,
> i.e. hw_brightness_change{_ro} file, that would support POLLPRI events,
> and reading brightness.
Another idea:
If no trigger is active then led subsystem will invoke POLLPRI on
"brightness" sysfs file.
And if there is active trigger then only trigger code could invoke
POLLPRI on "brightness" file.
This could solve problem with high CPU load and power usage when e.g.
cpu trigger is active (and cpu trigger will not implement any POLLPRI).
Do not know if this is really enough for your situation, it is just and
another idea.
But first please update documentation in ABI/testing to match current
situation. That is really needed.
--
Pali Rohár
pali.ro...@gmail.com
--
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger
On Monday 21 November 2016 14:29:00 Jacek Anaszewski wrote: > Let's wait until every involved part agrees (Pavel, Pali). Ok, I read that discussion on linux-leds ML and finally understand motivation and results. Personally I still do not like current approach and big problem what I see is that I was not able to understand *why* introduction of current_brightness is needed and how userspace application should use it, before I read whole that linux-leds discussion. For people who already understand situation it is probably OK, but when I first time saw this patch series I just said WTF and description in Documentation files nor in commit messages did not help me. I would suggest to properly document *current* behaviour of LED sysfs files in Documentation/ABI before doing any decision how to solve current problem. Without correct documentation how sysfs LED interface behave in different situations (trigger is active; zero is written to brightness; brightness is read when blinking is active; etc) it is really hard to discuss about those problems. As many people (me included) first looked at those documentation files and think that info written here is correct and handle everything... Adding trigger for LED devices which belongs to keyboard backlight has only disadvantage: You cannot set other kernel trigger to control level of keyboard backlight. I can imagine that trigger "key pressed" or "mouse moved" can be really useful for controlling keyboard backlight level. But that new keyboard backlight trigger just disallow it. -- Pali Rohár pali.ro...@gmail.com -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger
On Monday 21 November 2016 10:31:33 Hans de Goede wrote: > Pali, I'm sorry that you don't like the LED side design, but there > has been a long discussion about this (which you apparently missed) > and this really is the best way forward. Yea, I thought that I should have missed something as I was not able to find all needed information about it in my mailbox. Is that discussion somewhere logged/available? That could help to describe and understand all problems hidden behind. > Have you looked at what the new design means for the platform/x86 > patches ? Gone is the ugly dell_laptop_notifier as the event > forwarding between dell-wmi and dell-laptop is now handles by > the led-trigger subsys. Yes, I saw that ugly dell_lpatop notifier is not there. If there is some more information about decision and pro/coins about this approach I would like to read it before. -- Pali Rohár pali.ro...@gmail.com -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 1/6] leds: triggers: Add current_brightness trigger parameter
On Monday 21 November 2016 00:48:42 Pavel Machek wrote: > On Mon 2016-11-21 00:07:35, Pali Rohár wrote: > > On Thursday 17 November 2016 23:24:36 Hans de Goede wrote: > > > In some cases it may be desirable for userspace to be notified when > > > a trigger event happens. This commit adds support for a poll-able > > > current_brightness trigger specific sysfs attribute which triggers > > > may register: > > > > > > What: /sys/class/leds//current_brightness > > > > What about name actual_brightness? That name is already used by > > /sys/class/backlight/... > > Is that a problem? Is not better to have same name? -- Pali Rohár pali.ro...@gmail.com -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 1/6] leds: triggers: Add current_brightness trigger parameter
On Thursday 17 November 2016 23:24:36 Hans de Goede wrote: > In some cases it may be desirable for userspace to be notified when > a trigger event happens. This commit adds support for a poll-able > current_brightness trigger specific sysfs attribute which triggers > may register: > > What: /sys/class/leds//current_brightness What about name actual_brightness? That name is already used by /sys/class/backlight/... -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 1/6] leds: triggers: Add current_brightness trigger parameter
On Sunday 20 November 2016 19:45:40 Hans de Goede wrote: > Hi, > > On 20-11-16 15:59, Pali Rohár wrote: > > On Thursday 17 November 2016 23:24:36 Hans de Goede wrote: > >> In some cases it may be desirable for userspace to be notified > >> when a trigger event happens. This commit adds support for a > >> poll-able current_brightness trigger specific sysfs attribute > >> which triggers may register: > >> > >> What: /sys/class/leds//current_brightness > >> Date: November 2016 > >> KernelVersion: 4.10 > >> > >> Description: > >>Triggers which support it may register a current_brightness > >>file. This file supports poll() to detect when the trigger > >>modifies the brightness of the LED. > >>Reading this file will always return the current brightness > >>of the LED. > >>Writing this file sets the current brightness of the LED, > >>without influencing the trigger. > > > > Personally I do not like this new sysfs attribute... > > > > Now when somebody look at /sys/class/leds//, the first > > thing which say would be: > > > > "What the hell, why there are two files (brightness and > > current_brightness) for changing LED level? And which should I > > use?" > > > > If I understood correctly we need to handle two things: > > > > 1) Provide poll() for userspace when LED level is changed (either > > by HW > > > >or other user call) > > > > 2) Deal with fact that on _some_ hardware, special key is hardwired > > to > > > >change LED level > > > > So why for 1) we cannot use existing sysfs file "brightness"? I do > > not see any problem with it. > > That was our first attempt at this, but because the brightness may > also be changed by triggers / blink-timers, we need to wakeup poll() > in those cases too (anything else would be inconsistent) and doing > such a wakeup in that case has turned out to cause too much overhead > in some cases (even if userspace is not listening), specifically the > idle power uses on some systems got multiplied by a factor of 5 or > more. > > So this approach was rejected. But approach with exporting new sysfs file with name current_brightness with existence of old brightness sysfs file is not good and in my opinion it is even worst as current situation (= without poll support). What happen in next 5 years? Somebody point that sysfs file "brightness" and sysfs file "current_brightness" is still not good and invent "really_current_brightness" sysfs with new logic? No this is really not a way... I understand that extending current "brightness" sysfs is complicated, but it is not a reason to not doing it and inventing new crippling sysfs file which duplicate existing one (with some modifications). Anyway, I cannot believe that power usage is increased by factor 5 with exporting poll support. If we are going to change brightness level (either by trigger or timer) we still need to do wakeup. And we can do some optimizations, e.g. do not send poll event when nobody is listening or postpone event so we do not send too many events in 1s interval (or choose longer interval). Polling support is not in mainline kernel yet, so we can change its behaviour without breaking ABI. And we can define (if it help us!) that evens can be send to userspace with some delay (e.g. 1-3s). -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v5 1/6] leds: triggers: Add current_brightness trigger parameter
On Thursday 17 November 2016 23:24:36 Hans de Goede wrote: > In some cases it may be desirable for userspace to be notified when > a trigger event happens. This commit adds support for a poll-able > current_brightness trigger specific sysfs attribute which triggers > may register: > > What: /sys/class/leds//current_brightness > Date: November 2016 > KernelVersion:4.10 > Description: > Triggers which support it may register a current_brightness > file. This file supports poll() to detect when the trigger > modifies the brightness of the LED. > Reading this file will always return the current brightness > of the LED. > Writing this file sets the current brightness of the LED, > without influencing the trigger. Personally I do not like this new sysfs attribute... Now when somebody look at /sys/class/leds//, the first thing which say would be: "What the hell, why there are two files (brightness and current_brightness) for changing LED level? And which should I use?" If I understood correctly we need to handle two things: 1) Provide poll() for userspace when LED level is changed (either by HW or other user call) 2) Deal with fact that on _some_ hardware, special key is hardwired to change LED level So why for 1) we cannot use existing sysfs file "brightness"? I do not see any problem with it. And for 2) we even do not know on which machines is such hardwired feature enabled. Yes, on _some_ (not *all*) Dell machines there is Fn key combination which changes level of one LED device. But kernel does not know if hardware on which is running is doing such thing or not. Some machines do not have to have key for such action and we do not know it. And what about situation when somebody wants to configure e.g. mouse movement (or keypress) trigger to enable/disable LED device (which belongs to keyboard brightness)? In this case user explicitly know that his Fn+Space change level of LED device, so can be careful to not press it. With your read-only trigger you basically disable such (I think useful) feature. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v4 2/4] platform: x86: thinkpad: Call led_notify_brightness_change on kbd brightness change
On Friday 11 November 2016 19:40:51 Kevin Locke wrote: > On Fri, 2016-11-11 at 15:33 +0100, Hans de Goede wrote: > > On 11-11-16 15:12, Pali Rohár wrote: > >> My question remains. Is this for Thinklight or keyboard backlight? > >> Because Thinklinght has led device "tpacpi_led_thinklight" and > >> keyboard backlight has led device "tpacpi_led_kbdlight". > > > > I would say both, this matches with the pre-existing > > TP_ACPI_HKEY_THNKLGHT_MASK (they have a 1:1 mapping), > > keep in mind that there are no thinkpads with both > > a thinklight and a backlit keyboard, as those both > > serve the same purpose so it looks like the re-used > > the scancode. > > That's not entirely correct. The ThinkPad T430 has both a ThinkLight > and a keyboard backlight. Pressing Fn-Space toggles between 4 > states: > > Both Off -> Backlight Low -> Backlight High -> ThinkLight On (BL Off) > > I tested out your patch and observed the following behavior (printing > brightness initially and on POLLPRI): > > # Initial state with both lights off. > tpacpi::kbd_backlight/brightness: 0 > tpacpi::thinklight/brightness: 0 > # Press Fn-Space. KBD Backlight comes on low. > tpacpi::kbd_backlight/brightness: 1 > # Press Fn-Space. KBD Backlight brightens to high. > tpacpi::kbd_backlight/brightness: 2 > # Press Fn-Space. KBD Backlight turns off. ThinkLight turns on. > tpacpi::kbd_backlight/brightness: 0 > # Press Fn-Space. ThinkLight turns off. > tpacpi::kbd_backlight/brightness: 0 > > It works, but the behavior is not quite what I would hope for. There > are no poll events for tpacpi::thinklight/brightness Yes, this is what I already pointed... That we should send event also for tpacpi::thinklight. > and when the > ThinkLight turns off it triggers an unnecessary > tpacpi::kbd_backlight/brightness poll event. It is problem? Or are forced to send event only if brightness level is really changed? I think that bigger problem is when brightness changes, but we do not send event. Should not be event treated as "hey, brightness level was probably changed, read file to get current status"? > If there is anything else I can do to assist, let me know. Great, this is really useful to see somebody who can test patches on those machines with both Thinklight and keyboard backlight... -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v4 2/4] platform: x86: thinkpad: Call led_notify_brightness_change on kbd brightness change
On Tuesday 01 November 2016 14:37:46 Hans de Goede wrote:
> Make thinkpad_acpi call led_notify_brightness_change on the kbd_led
> led_classdev registered by thinkpad_acpi when the kbd backlight
> brightness changes.
>
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> ---
> Changes in v3:
> -This is a new patch in v3 of this patch-set
> Changes in v4:
> -No Changes
> ---
> drivers/platform/x86/thinkpad_acpi.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c index b65ce75..5dcd7d8b
> 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -162,6 +162,7 @@ enum tpacpi_hkey_event_t {
> TP_HKEY_EV_HOTKEY_BASE = 0x1001, /* first hotkey (FN+F1) */
> TP_HKEY_EV_BRGHT_UP = 0x1010, /* Brightness up */
> TP_HKEY_EV_BRGHT_DOWN = 0x1011, /* Brightness down */
> + TP_HKEY_EV_THINKLIGHT = 0x1012, /* Thinklight/kbd backlight */
My question remains. Is this for Thinklight or keyboard backlight?
Because Thinklinght has led device "tpacpi_led_thinklight" and keyboard
backlight has led device "tpacpi_led_kbdlight".
> TP_HKEY_EV_VOL_UP = 0x1015, /* Volume up or unmute */
> TP_HKEY_EV_VOL_DOWN = 0x1016, /* Volume down or unmute */
> TP_HKEY_EV_VOL_MUTE = 0x1017, /* Mixer output mute */
> @@ -5167,6 +5168,8 @@ static int __init kbdlight_init(struct
> ibm_init_struct *iibm) return rc;
> }
>
> + tpacpi_hotkey_driver_mask_set(hotkey_driver_mask |
> + TP_ACPI_HKEY_THNKLGHT_MASK);
> return 0;
> }
>
> @@ -9114,6 +9117,8 @@ static void tpacpi_driver_event(const unsigned
> int hkey_event) volume_alsa_notify_change();
> }
> }
> + if (tp_features.kbdlight && hkey_event == TP_HKEY_EV_THINKLIGHT)
> + led_notify_brightness_change(_led_kbdlight.led_classdev);
This looks incorrect. You are trying to inform tpacpi_led_kbdlight when
tpacpi_led_thinklight change led status?
> }
>
> static void hotkey_driver_event(const unsigned int scancode)
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v3 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]
On Thursday 27 October 2016 13:46:33 Hans de Goede wrote: > Hi, > > On 27-10-16 12:38, Pali Rohár wrote: > >On Wednesday 26 October 2016 19:41:18 Hans de Goede wrote: > >>Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider > >>events, replace dell_rbtn_notifier_register() / > >>dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() used > >>by dell-laptop to decide whether or not to use the i8042 filter and used > >>by dell-rbtn to auto-remove its rfkill interface when called. > >> > >>This results in a nice cleanup, downside is that the rfkill interface > >>of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, this > >>now requires rmmod + insmod of dell-rbtn. But people who do not want > >>dell-laptop for some reason will have it blacklisted anyways, so this > >>is not an issue and there is a work-around. > >> > >>Signed-off-by: Hans de Goede <hdego...@redhat.com> > >>--- > >>Changes in v2: > >>-This is a new patch in v2 of my platform/x86/dell-* notifier set intended > >> to show how dell_smbios*notifier can be used to improve the dell-rbtn > >> integration too > >>Changes in v3: > >>-Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so that the > >> dynamic symbol dance we do to allow loading without dell-rbtn actually > >> works. > >>--- > >> drivers/platform/x86/Kconfig | 1 + > >> drivers/platform/x86/dell-laptop.c | 53 +++- > >> drivers/platform/x86/dell-rbtn.c | 63 > >> +- > >> drivers/platform/x86/dell-rbtn.h | 5 +-- > >> drivers/platform/x86/dell-smbios.h | 1 + > >> 5 files changed, 29 insertions(+), 94 deletions(-) > > > >Looks like that for preventing sending event that rfkill switch was > >changed by hardware slider we must always drop atk i8042 keycode... > > > >Needs to check if key is really send by both dell-rbtn and also by atk > >i8042 keyboard driver and if yes then i8042_install_filter() is always > >needed (if rbtn is there or not)... > > But this is not related to this patch, right? This patch does not change > any behavior. Other then your concerns about where to put the notifier, > do you like the approach of this patch? It is not related, but if above is truth, then another rewrite (also of those your changes!) is needed. So maybe it would be better to postpone (or drop this patch from your patch series) so we do not invest time for something which will be again rewritten... -- Pali Rohár pali.ro...@gmail.com -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v3 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]
On Wednesday 26 October 2016 19:41:18 Hans de Goede wrote: > Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider > events, replace dell_rbtn_notifier_register() / > dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() used > by dell-laptop to decide whether or not to use the i8042 filter and used > by dell-rbtn to auto-remove its rfkill interface when called. > > This results in a nice cleanup, downside is that the rfkill interface > of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, this > now requires rmmod + insmod of dell-rbtn. But people who do not want > dell-laptop for some reason will have it blacklisted anyways, so this > is not an issue and there is a work-around. > > Signed-off-by: Hans de Goede <hdego...@redhat.com> > --- > Changes in v2: > -This is a new patch in v2 of my platform/x86/dell-* notifier set intended > to show how dell_smbios*notifier can be used to improve the dell-rbtn > integration too > Changes in v3: > -Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so that the > dynamic symbol dance we do to allow loading without dell-rbtn actually works. > --- > drivers/platform/x86/Kconfig | 1 + > drivers/platform/x86/dell-laptop.c | 53 +++- > drivers/platform/x86/dell-rbtn.c | 63 > +- > drivers/platform/x86/dell-rbtn.h | 5 +-- > drivers/platform/x86/dell-smbios.h | 1 + > 5 files changed, 29 insertions(+), 94 deletions(-) Looks like that for preventing sending event that rfkill switch was changed by hardware slider we must always drop atk i8042 keycode... Needs to check if key is really send by both dell-rbtn and also by atk i8042 keyboard driver and if yes then i8042_install_filter() is always needed (if rbtn is there or not)... -- Pali Rohár pali.ro...@gmail.com -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v3 2/5] platform: x86: thinkpad: Call led_notify_brightness_change on kbd brightness change
On Wednesday 26 October 2016 19:41:15 Hans de Goede wrote: > + TP_HKEY_EV_THINKLIGHT = 0x1012, /* Thinklight/kbd backlight */ Hi! Do you have some documentation for this number? When it is send? When keyboard baklight level is changed or when thinklight is turned on/off? Or in both cases? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v4 2/4] platform: x86: thinkpad: Call led_notify_brightness_change on kbd brightness change
On Friday 11 November 2016 15:33:48 Hans de Goede wrote:
> Hi,
>
> On 11-11-16 15:12, Pali Rohár wrote:
> > On Tuesday 01 November 2016 14:37:46 Hans de Goede wrote:
> >> Make thinkpad_acpi call led_notify_brightness_change on the
> >> kbd_led led_classdev registered by thinkpad_acpi when the kbd
> >> backlight brightness changes.
> >>
> >> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> >> ---
> >> Changes in v3:
> >> -This is a new patch in v3 of this patch-set
> >> Changes in v4:
> >> -No Changes
> >> ---
> >>
> >> drivers/platform/x86/thinkpad_acpi.c | 5 +
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> >> b/drivers/platform/x86/thinkpad_acpi.c index b65ce75..5dcd7d8b
> >> 100644
> >> --- a/drivers/platform/x86/thinkpad_acpi.c
> >> +++ b/drivers/platform/x86/thinkpad_acpi.c
> >> @@ -162,6 +162,7 @@ enum tpacpi_hkey_event_t {
> >>
> >>TP_HKEY_EV_HOTKEY_BASE = 0x1001, /* first hotkey (FN+F1) */
> >>TP_HKEY_EV_BRGHT_UP = 0x1010, /* Brightness up */
> >>TP_HKEY_EV_BRGHT_DOWN = 0x1011, /* Brightness down */
> >>
> >> + TP_HKEY_EV_THINKLIGHT = 0x1012, /* Thinklight/kbd backlight
*/
> >
> > My question remains. Is this for Thinklight or keyboard backlight?
> > Because Thinklinght has led device "tpacpi_led_thinklight" and
> > keyboard backlight has led device "tpacpi_led_kbdlight".
>
> I would say both, this matches with the pre-existing
> TP_ACPI_HKEY_THNKLGHT_MASK (they have a 1:1 mapping),
> keep in mind that there are no thinkpads with both
> a thinklight and a backlit keyboard
That is not truth. X230 is in variant with both Thinklight (light above
display) and keyboard backlight. Plus in BIOS it is possible to
configure if you want to enable only Thinkligh, only keyboard backlight
or both. Probably T430 and other 30s models can be found with both. But
I do not own those modules...
> as those both
> serve the same purpose so it looks like the re-used
> the scancode.
So I could guess that it reports event when one of those leds are
changed...
> I've tried this code on both a W541 and a P50 and both
> emit TP_ACPI_HOTKEYSCAN_FNPAGEUP for the backlight
> keyboard hotkey (fn + space), I'm naming the event for
> this TP_HKEY_EV_THINKLIGHT for consistency with the
> existing TP_ACPI_HKEY_THNKLGHT_MASK which corresponds
> to TP_ACPI_HOTKEYSCAN_FNPAGEUP.
>
> >>TP_HKEY_EV_VOL_UP = 0x1015, /* Volume up or unmute */
> >>TP_HKEY_EV_VOL_DOWN = 0x1016, /* Volume down or unmute */
> >>TP_HKEY_EV_VOL_MUTE = 0x1017, /* Mixer output mute */
> >>
> >> @@ -5167,6 +5168,8 @@ static int __init kbdlight_init(struct
> >> ibm_init_struct *iibm) return rc;
> >>
> >>}
> >>
> >> + tpacpi_hotkey_driver_mask_set(hotkey_driver_mask |
> >> +TP_ACPI_HKEY_THNKLGHT_MASK);
> >>
> >>return 0;
> >>
> >> }
> >>
> >> @@ -9114,6 +9117,8 @@ static void tpacpi_driver_event(const
> >> unsigned int hkey_event) volume_alsa_notify_change();
> >>
> >>}
> >>
> >>}
> >>
> >> + if (tp_features.kbdlight && hkey_event == TP_HKEY_EV_THINKLIGHT)
> >> +
> >> led_notify_brightness_change(_led_kbdlight.led_classdev)
> >> ;
> >
> > This looks incorrect. You are trying to inform tpacpi_led_kbdlight
> > when tpacpi_led_thinklight change led status?
>
> No as said the scancode / event is shared, notice the check for
> tp_features.kbdlight, so the notify will only happen on laptops which
> actually have a backlit keyboard.
If that event is send when either Thinklight or keyb-backlight is
changed, should not we inform both led devices about level change? Do
you have a chance to test it on notebook with Thinklight?
And funny question is what would happen on those machines which have
both Thinklight and keyboard backlight?
Anyway, I would suggest different name, not TP_HKEY_EV_THINKLIGHT as it
is not thinklight-only and could be misleading in keyboard backlight
context...
> Regards,
>
> Hans
>
> >> }
> >>
> >> static void hotkey_driver_event(const unsigned int scancode)
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v4 3/4] platform: x86: dell-*: Add a generic dell-laptop notifier chain
On Tuesday 01 November 2016 14:37:47 Hans de Goede wrote:
> @@ -43,4 +45,13 @@ void dell_smbios_release_buffer(void);
> void dell_smbios_send_request(int class, int select);
>
> struct calling_interface_token *dell_smbios_find_token(int tokenid);
> +
> +enum dell_laptop_notifier_actions {
> + dell_laptop_kbd_backlight_brightness_changed,
> +};
Should not be upper case? Looks like that Documentation/CodingStyle also
suggest it:
Names of macros defining constants and labels in enums are capitalized.
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: Add support for keyboard backlight
On Wednesday 13 January 2016 20:10:30 Pavel Machek wrote:
> On Wed 2016-01-13 20:07:36, Pavel Machek wrote:
> > On Wed 2016-01-13 09:54:55, Pali Rohár wrote:
> > > On Tuesday 12 January 2016 22:58:04 Pavel Machek wrote:
> > > > Hi!
> > > >
>
> > > > Next question is.. apparently there are some keyboards that have
> > > > per-key RGB backlight... but maybe we can just call that "weird
> > > > enough" and ignore...
> > >
> > > First we need to defines stable kernel ABI for keyboard backlight. And I
> > > suggest to use existing convention used by upower/console-kit and other
> > > userspace apps...
> >
> > Hmm... I'm not sure that can be done. What were the masks used by
> > upower again? Will upower write to all 6 leds if we present them?
>
> Got it...
>
> It has function up_kbd_backlight_find() which do:
>
> /* find a led device that is a keyboard device */
> while ((filename = g_dir_read_name (dir)) != NULL) {
> if (g_strstr_len (filename, -1,
> "kbd_backlight") != NULL) {
> dir_path = g_build_filename
> ("/sys/class/leds",
> filename,
>
> NULL);
> break;
>
> That suggests that it stops at the first matching device. Adding new
> "virtual" led controlling 6 physical leds would be ugly. So... new
> interface should be done.
>
> Pavel
Yes, it would be ugly, but lp5523 is already ugly... it can accept
numeric value, trigger and also program in bytecode either via sysfs or
via request_firmware... Another virtual led control should not be
problem for this :-)
--
Pali Rohár
pali.ro...@gmail.com
--
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v2] thinkpad_acpi: Add support for keyboard backlight
On Monday 04 January 2016 21:04:25 Darren Hart wrote: > On Wed, Dec 30, 2015 at 11:27:41PM +0100, Pali Rohár wrote: > > This patch adds support for controlling keyboard backlight via > > standard linux led class interface (::kbd_backlight). It uses ACPI > > HKEY device with MLCG and MLCS methods. > > Which laptops is this intended to support? Thinkpad ??30 series and new which have backlight keyboard. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v2] thinkpad_acpi: Add support for keyboard backlight
On Monday 04 January 2016 21:40:20 Darren Hart wrote: > On Mon, Jan 04, 2016 at 09:26:19PM +0100, Pali Rohár wrote: > > On Monday 04 January 2016 21:04:25 Darren Hart wrote: > > > On Wed, Dec 30, 2015 at 11:27:41PM +0100, Pali Rohár wrote: > > > > This patch adds support for controlling keyboard backlight via > > > > standard linux led class interface (::kbd_backlight). It uses > > > > ACPI HKEY device with MLCG and MLCS methods. > > > > > > Which laptops is this intended to support? > > > > Thinkpad ??30 series and new which have backlight keyboard. > > Thanks, we should include that in the commit message as well as the > comments surrounding the driver section. ??30 is probably not good characteristic, but I mean all those Thinkpad laptops like T430, x230, E430, X1 (1st) and their successors (T440, T450, X1 3rd, ...) All those which are from Ivy Bridge processor generation (and new). But basically it cover all Thinkpad laptops which have backlight keyboard. Older Thinkpad laptops had only light in bezel. So if you have better idea for commit message, feel free to change it. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: Add support for keyboard backlight
On Monday 04 January 2016 21:12:31 Pavel Machek wrote: > Hi1 > > > This patch adds support for controlling keyboard backlight via > > standard linux led class interface (::kbd_backlight). It uses ACPI > > HKEY device with MLCG and MLCS methods. > > > > Signed-off-by: Pali Rohár <pali.ro...@gmail.com> > > Tested-by: Fabio D'Urso <fabiodu...@hotmail.it> > > On my thinkpad, keyboard light is controlled by > > /sys/class/leds/tpacpi\:\:thinklight/brightness > > (that's a bad name). Hi! That is light in upper case of bezel/display, right? Thinklight is probably official marketing name for that by IBM/Lenovo. My patch adds support for keyboard backlight (light under the keyboard). > On n900, it is .../leds/kb0..kb6. Now we'd have kbd_backlight. I > guess we should standartize on one name for this light, so that > userspace has the chance to handle it automatically... Looks like userspace already uses /sys/class/leds/*::kbd_backlight for keyboard backlight (light under the keyboard). At least other drivers uses this name and my KDE desktop recognized "dell::kbd_backlight" (from dell-laptop.ko) and "tpacpi::kbd_backlight" too. So really for keyboard backlight use *::kbd_backlight it is already handled by existing userspace applications. > Also, neccessity of workqueues for LED setting is slowly being > removed from the kernel, see LED mailing list for details. My patch uses kbd_backlight LED in same as as other LEDs in thinkpad acpi driver. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part. -- Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: Add support for keyboard backlight
On Tuesday 12 January 2016 22:58:04 Pavel Machek wrote: > Hi! > > On Mon 2016-01-11 21:03:01, Pali Rohár wrote: > > On Monday 11 January 2016 20:28:00 Henrique de Moraes Holschuh wrote > > > The two features are not the same (and are handled differently by the > > > firmware, for whatever reason), although they do serve the same > > > purpose. I don't think a thinkpad will ever have both features at > > > the same time, so I have no idea why they changed the firmware > > > interface. > > > > Maybe we should decide if ::kbd_backlight LED suffix could be used also > > for other LED devices and not only for those which are physically under > > the keyboard. > > Another problem is that N900 has _6_ backlight LEDs. Named > lp5523::kb1..6. ... Which does means desktop software will probably > not pick them up :-(. > > I guess we could have "/sys/class/kbd_light/brightness" that would > control all of them with one write. Probably... But there is problem that lp5523 is not ordinary on/off light, it can be programmed to execute own "light" application. > Next question is.. apparently there are some keyboards that have > per-key RGB backlight... but maybe we can just call that "weird > enough" and ignore... First we need to defines stable kernel ABI for keyboard backlight. And I suggest to use existing convention used by upower/console-kit and other userspace apps... -- Pali Rohár pali.ro...@gmail.com -- Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140 ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: Add support for keyboard backlight
On Wednesday 30 December 2015 23:28:48 Pali Rohár wrote: > On Monday 28 December 2015 15:48:14 Pali Rohár wrote: > > > Also, is it working properly across suspend+resume? > > > > When doing resume from suspend or hibernate BIOS turning keyboard > > backlight automatically off. > > Fixed in v2. > Now I see that BIOS try to be too intelligent and automatically turn of keyboard backlight when LID is closed. When LID is open again then keyboard backlight stay turned off. Sysfs show correct state (brightness is zero). Should thinkpad acpi driver do something? Or let BIOS do that job? -- Pali Rohár pali.ro...@gmail.com -- Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140 ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
