ibm-acpi-devel — ibm-acpi Linux driver development

[ibm-acpi-devel] [PATCH v2 0/4] Add 4 new keycodes and use them for 4 new hotkeys on new Lenovo Thinkpads

[Hans de G. <hde...@re...>]

Hi All,

Here is a v2 of my series to get a couple of new hotkeys, and matching
thinkpad_acpi hkey event codes seen on the last 2 generations of
Thinkpads supported.

New in this version that Lenovo got back to me and explained that the 0x1316
hkey event belongs to Fn + right Shift, so that is mapped now too.

Since these make both include/uapi/linux/input-event-codes.h and
drivers/platform/x86 changes one open question with this series is how to
merge it.

Dmitry, for v1 of this series you suggested you could merge them all
through the input tree ? Andy, one of the drivers/platform/x86 maintainers
gave his Acked-by for this:

Acked-by: Andy Shevchenko <and...@gm...>

Alternatively the entire series could be merged through the
linux-platform-drivers-x86 repo:

https://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git/shortlog/refs/heads/review-andy

Regards,

Hans

[ibm-acpi-devel] [PATCH v2 4/4] platform/x86: thinkpad_acpi: Map Clipping tool hotkey to KEY_SELECTIVE_SCREENSHOT

[Hans de G. <hde...@re...>]

Commit 696c6523ec8f ("platform/x86: thinkpad_acpi: add mapping for new
hotkeys") added support for a bunch of new hotkeys, but the
clipping/snipping tool hotkey got ignored because there was no good
key-code to map it to.

Recently a new KEY_SELECTIVE_SCREENSHOT keycode was added by commit
3b059da9835c ("Input: allocate keycode for "Selective Screenshot" key")
quoting from the commit message:

"New Chrome OS keyboards have a "snip" key that is basically a selective
screenshot (allows a user to select an area of screen to be copied).
Allocate a keycode for it."

Support for this "snip" key seems like it is also a good match for the
clipping/snipping tool hotkey, so map this hotkey to the new
KEY_SELECTIVE_SCREENSHOT key-code.

Reviewed-by: Bastien Nocera <ha...@ha...>
Acked-by: Henrique de Moraes Holschuh <hm...@hm...>
Signed-off-by: Hans de Goede <hde...@re...>
---
 drivers/platform/x86/thinkpad_acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 4b701e9a0392..47925c319d7b 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -3434,7 +3434,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 		KEY_UNKNOWN,
 
 		KEY_BOOKMARKS,			/* Favorite app, 0x311 */
-		KEY_RESERVED,			/* Clipping tool */
+		KEY_SELECTIVE_SCREENSHOT,	/* Clipping tool */
 		KEY_CALC,			/* Calculator (above numpad, P52) */
 		KEY_BLUETOOTH,			/* Bluetooth */
 		KEY_KEYBOARD,			/* Keyboard, 0x315 */
-- 
2.28.0

[ibm-acpi-devel] [PATCH v2 3/4] platform/x86: thinkpad_acpi: Add support for new hotkeys found on X1C8 / T14

[Hans de G. <hde...@re...>]

New Lenovo Thinkpad models, e.g. the X1 Carbon 8th gen and the new T14 gen1
models have 3 new symbols / shortcuts on their F9-F11 keys (and the
thinkpad_acpi driver receives 3 new hkey events for these):

F9:  Has a symbol resembling a rectangular speech balloon, the manual says
     the hotkey functions shows or hides the notification center
F10: Has a symbol of a telephone horn which has been picked up from the
     receiver, the manual says: "Answer incoming calls"
F11: Has a symbol of a telephone horn which is resting on the receiver,
     the manual says: "Decline incoming calls"

And these Thinkpad models also send a new 0x1316 hkey events when the
Fn + right Shift key-combo is pressed.

This commit adds support for these 4 new hkey events.

Acked-by: Henrique de Moraes Holschuh <hm...@hm...>
Signed-off-by: Hans de Goede <hde...@re...>
---
Changes in v2:
- Map 0x1316 to KEY_FN_RIGHT_SHIFT instead of to KEY_UNKNOWN (now that we
  know what it does)
---
 drivers/platform/x86/thinkpad_acpi.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 9c4df41687a3..4b701e9a0392 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -1913,6 +1913,10 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
 	TP_ACPI_HOTKEYSCAN_CALCULATOR,
 	TP_ACPI_HOTKEYSCAN_BLUETOOTH,
 	TP_ACPI_HOTKEYSCAN_KEYBOARD,
+	TP_ACPI_HOTKEYSCAN_FN_RIGHT_SHIFT, /* Used by "Lenovo Quick Clean" */
+	TP_ACPI_HOTKEYSCAN_NOTIFICATION_CENTER,
+	TP_ACPI_HOTKEYSCAN_PICKUP_PHONE,
+	TP_ACPI_HOTKEYSCAN_HANGUP_PHONE,
 
 	/* Hotkey keymap size */
 	TPACPI_HOTKEY_MAP_LEN
@@ -3429,11 +3433,15 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 		KEY_UNKNOWN, KEY_UNKNOWN, KEY_UNKNOWN, KEY_UNKNOWN,
 		KEY_UNKNOWN,
 
-		KEY_BOOKMARKS,       /* Favorite app, 0x311 */
-		KEY_RESERVED,        /* Clipping tool */
-		KEY_CALC,            /* Calculator (above numpad, P52) */
-		KEY_BLUETOOTH,       /* Bluetooth */
-		KEY_KEYBOARD         /* Keyboard, 0x315 */
+		KEY_BOOKMARKS,			/* Favorite app, 0x311 */
+		KEY_RESERVED,			/* Clipping tool */
+		KEY_CALC,			/* Calculator (above numpad, P52) */
+		KEY_BLUETOOTH,			/* Bluetooth */
+		KEY_KEYBOARD,			/* Keyboard, 0x315 */
+		KEY_FN_RIGHT_SHIFT,		/* Fn + right Shift */
+		KEY_NOTIFICATION_CENTER,	/* Notification Center */
+		KEY_PICKUP_PHONE,		/* Answer incoming call */
+		KEY_HANGUP_PHONE,		/* Decline incoming call */
 		},
 	};
 
-- 
2.28.0

[ibm-acpi-devel] [PATCH v2 2/4] Input: allocate keycode for Fn + right shift

[Hans de G. <hde...@re...>]

The last 2 generations of Lenovo Thinkpads send an acpi_thinkpad event when
Fn + right shift is pressed.  This is intended for use with "Lenovo Quick
Clean" software, which disables the touchpad + kbd for 2 minutes on this
key-combo so that healthcare workes can disinfect it.

But there is no silkscreen print on the right-keyboard to indicate this,
so add a KEY_FN_RIGHT_SHIFT keycode define to use for this key-combo.

Signed-off-by: Hans de Goede <hde...@re...>
---
Changes in v2:
- New patch in v2 of this patch-set
---
 include/uapi/linux/input-event-codes.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index b74821d09145..ee93428ced9a 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -545,6 +545,7 @@
 #define KEY_FN_F		0x1e2
 #define KEY_FN_S		0x1e3
 #define KEY_FN_B		0x1e4
+#define KEY_FN_RIGHT_SHIFT	0x1e5
 
 #define KEY_BRL_DOT1		0x1f1
 #define KEY_BRL_DOT2		0x1f2
-- 
2.28.0

[ibm-acpi-devel] [PATCH v2 1/4] Input: allocate keycodes for notification-center, pickup-phone and hangup-phone

[Hans de G. <hde...@re...>]

New Lenovo Thinkpad models, e.g. the X1 Carbon 8th gen and the new T14 gen1
models have 3 new symbols / shortcuts on their F9-F11 keys (and the
thinkpad_acpi driver receives 3 new "scancodes" for these):

F9:  Has a symbol resembling a rectangular speech balloon, the manual says
     the hotkey functions shows or hides the notification center
F10: Has a symbol of a telephone horn which has been picked up from the
     receiver, the manual says: "Answer incoming calls"
F11: Has a symbol of a telephone horn which is resting on the receiver,
     the manual says: "Decline incoming calls"

We have no existing keycodes which are a good match for these, so
add 3 new keycodes for these.

I noticed that we have a hole in our keycodes between 0x1ba and 0x1c0
which does not seem to be reserved for any specific purpose, so these
new 3 codes use 0x1bc - 0x1be, instead of starting at 0x27b.

Acked-by: Henrique de Moraes Holschuh <hm...@hm...>
Signed-off-by: Hans de Goede <hde...@re...>
---
 include/uapi/linux/input-event-codes.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 0c2e27d28e0a..b74821d09145 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -515,6 +515,9 @@
 #define KEY_10CHANNELSUP	0x1b8	/* 10 channels up (10+) */
 #define KEY_10CHANNELSDOWN	0x1b9	/* 10 channels down (10-) */
 #define KEY_IMAGES		0x1ba	/* AL Image Browser */
+#define KEY_NOTIFICATION_CENTER	0x1bc	/* Show/hide the notification center */
+#define KEY_PICKUP_PHONE	0x1bd	/* Answer incoming call */
+#define KEY_HANGUP_PHONE	0x1be	/* Decline incoming call */
 
 #define KEY_DEL_EOL		0x1c0
 #define KEY_DEL_EOS		0x1c1
-- 
2.28.0

[ibm-acpi-devel] Thinkpad T14: unhandled HKEY event 0x1317, 0x1318 and 0x1319

[Florian B. <me...@th...>]

Hey,

On my Thinkpad T14, I have three buttons which I think are new on that
system:

Fn + F9:  A "message" / chat symbol
Fn + F10: An "accept call" symbol
Fn + F11: A "hang up" symbol

You can see them here:
https://www.notebookcheck.net/fileadmin/_processed_/9/e/csm_IMG_20200629_014519_369088cd34.jpg
(Note that the picture is from the Intel variant - I own the AMD
variant, but it has the same keyboard)

When I press them, no X keypress event is generated, instead I get:

    thinkpad_acpi: unhandled HKEY event 0x1317
    thinkpad_acpi: please report the conditions when this event happened to ibm...@li...

The IDs I see are:

0x1317: message
0x1318: accept call
0x1319: hang up

I guess the first two would correspond to XF86Messenger and XF86Phone.
No clue about the third one.

I took a quick look at the source[1] and I'm guessing they'd need to be
added after TP_ACPI_HOTKEYSCAN_EXTENDED_START somehow, but I don't
really understand how those values are used exactly, or how (and by
what) they're mapped to X key events. I might be able to contribute a
fix (or at least test one) if someone can help me get started, though
it'll likely take me a bit of time.

[1] linux-5.9-rc3/drivers/platform/x86/thinkpad_acpi.c that is

Florian

Re: [ibm-acpi-devel] [PATCH 0/3] Add 3 new keycodes and use them for 3 new hotkeys on new Lenovo Thinkpads

[Hans de G. <hde...@re...>]

Hi Dmitry,

On 7/22/20 7:41 AM, Dmitry Torokhov wrote:
> On Sun, Jul 19, 2020 at 07:56:49PM -0300, Henrique de Moraes Holschuh wrote:
>> On Fri, 17 Jul 2020, Hans de Goede wrote:
>>> This is a simple patch-series adding support for 3 new hotkeys found
>>> on various new Lenovo Thinkpad models.
>>
>> For all three patches, pending an ack for the new keycodes by the input
>> maintainers:
>>
>> Acked-by: Henrique de Moraes Holschuh <hm...@hm...>
> 
> Do you want me to merge all 3 through input tree?

Despite Andy's acked-by:

Acked-by: Andy Shevchenko <and...@gm...>

For merging these through the input tree I'm not seeing these in 5.9-rc2 ?

Regards,

Hans

Re: [ibm-acpi-devel] [External] RE: [PATCH v2] platform/x86: thinkpad_acpi: performance mode interface

[Limonciello, M. <Mar...@de...>]

> On 8/21/2020 4:43 PM, Limonciello, Mario wrote:
> >> -----Original Message----- From: Mark Pearson
> >> <mar...@le...> Sent: Friday, August 21, 2020 15:06
> >>
> >> On 8/21/2020 4:00 PM, Limonciello, Mario wrote: <snip>
> >>>>>> + +The sysfs entry provides the ability to return the
> >>>>>> current status and to set the +desired mode. For example::
> >>>>>>  + +        echo H >
> >>>>>> /sys/devices/platform/thinkpad_acpi/dytc_perfmode + echo M
> >>>>>>> /sys/devices/platform/thinkpad_acpi/dytc_perfmode + echo
> >>>>>> L > /sys/devices/platform/thinkpad_acpi/dytc_perfmode +
> >>>>>
> >>>>> I was thinking about this some more, do you actually want
> >>>>> another mode that "disables" this feature?   IE "O" turns it
> >>>>>  off an calls DYTC_DISABLE_CQL.
> >>>>>
> >>>>> For example if a user wanted to test the recently landed code
> >>>>> in thermald 2.3 and compare performance between the two it
> >>>>> seems like this and that "might" fight. As an outsider
> >>>>> looking in - I of course may be wrong too here.
> >>>>>
> >>>>> If at some point in the future thermald does a better job
> >>>>> than this implementation you might also want an "out" to let
> >>>>>  thermald or another piece of userland turn this off if it's
> >>>>>  in the picture.
> >>>>>
> >>>> I'm still digging into this one. Right now I haven't found a
> >>>> good clean way of just disabling the firmware. Currently when
> >>>> thermald goes in and tweaks the CPU power registers it has the
> >>>>  effect of overriding the FW anyway - but I appreciate that's
> >>>> not quite the same as actually doing it explicitly.
> >>>>
> >>>
> >>> What about a modprobe parameter to disable at least?  That would
> >>>  at least make it pretty easy to make a change, reboot and
> >>> compare with thermald (or other software) without disabling the
> >>> rest of the functionality of the thinkpad_acpi driver.
> >>>
> >> The problem is I don't have a good way to disable the firmware
> >> (that I know of yet) so a modprobe parameter wouldn't really do
> >> much. I guess it could skip providing the sysfs entry points - but
> >>  the FW will still be there doing it's thing, so I'm not sure I see
> >>  the benefit of that. At least the sysfs entry point gives a bit
> >> more insight into what is going on. Let me know if I'm missing
> >> something obvious.
> >>
> >
> > Oh so it's not actually the driver loading tells the firmware it's
> > supposed to work this way.  The firmware actually detects "I'm
> > running on Linux, so I'll do this differently"?
> >
> Right. This patch is just providing a more friendly interface so user
> space can see what is going on and be able to do control of the
> different modes without the need for hotkeys.
> 
> Afraid I don't have much insight into the nitty gritty details of
> exactly how that works - I believe it's related to detecting the use of
> DPTF but the details aren't shared with me because of NDA's. As a
> thought it's quite possible (probable?) that if thermald is using DPTF
> the firmware will automatically disable itself. Proving that might be
> tricky - I'll see if the firmware team have any ideas.
> 

Right - it's a reverse engineered implementation done in thermald and there
are still some missing items.  So your comment makes perfect sense to me now,
and if/when those other things are added this may change and it will make
sense to offer a way to turn this off at that time.

[ibm-acpi-devel] System freeze - unhandled HKEY event 0x6031

[Paweł P. <re...@gm...>]

Hi all,

Reporting here as requested in /var/log/syslog

```
Aug 24 10:28:51 x1 kernel: [153517.254925] thinkpad_acpi: unhandled HKEY
event 0x6031
Aug 24 10:28:51 x1 kernel: [153517.254928] thinkpad_acpi: please report the
conditions when this event happened to ibm...@li...
```

Conditions:
* connected to external monitor via hdml port
* no usb/c thunderbolt devices plugged in
* x1 extreme rev 1
* no super heavy workload at the moment of freeze (zoom confcall with video
sharing)

-- 
pozdrawiam,
Paweł

[ibm-acpi-devel] [PATCH] Documentation: laptops: thinkpad-acpi: fix underline length build warning

[Randy D. <rd...@in...>]

From: Randy Dunlap <rd...@in...>

Fix underline length build warning in thinkpad-acpi.rst documentation:

Documentation/admin-guide/laptops/thinkpad-acpi.rst:1437: WARNING: Title underline too short.
DYTC Lapmode sensor
------------------

Fixes: acf7f4a59114 ("platform/x86: thinkpad_acpi: lap or desk mode interface")
Signed-off-by: Randy Dunlap <rd...@in...>
Cc: Nitin Joshi <nj...@le...>
Cc: Sugumaran <sla...@le...>
Cc: Bastien Nocera <bn...@re...>
Cc: Mark Pearson <mar...@le...>
Cc: Andy Shevchenko <and...@li...>
Cc: Darren Hart <dv...@in...>
Cc: Henrique de Moraes Holschuh <ibm...@hm...>
Cc: ibm...@li...
Cc: pla...@vg...
---
 Documentation/admin-guide/laptops/thinkpad-acpi.rst |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- lnx-59-rc2.orig/Documentation/admin-guide/laptops/thinkpad-acpi.rst
+++ lnx-59-rc2/Documentation/admin-guide/laptops/thinkpad-acpi.rst
@@ -1434,7 +1434,7 @@ on the feature, restricting the viewing
 
 
 DYTC Lapmode sensor
-------------------
+-------------------
 
 sysfs: dytc_lapmode
 

Re: [ibm-acpi-devel] [External] RE: [PATCH v2] platform/x86: thinkpad_acpi: performance mode interface

[Mark P. <mar...@le...>]

On 8/21/2020 4:43 PM, Limonciello, Mario wrote:
>> -----Original Message----- From: Mark Pearson 
>> <mar...@le...> Sent: Friday, August 21, 2020 15:06
>> 
>> On 8/21/2020 4:00 PM, Limonciello, Mario wrote: <snip>
>>>>>> + +The sysfs entry provides the ability to return the 
>>>>>> current status and to set the +desired mode. For example::
>>>>>>  + +        echo H > 
>>>>>> /sys/devices/platform/thinkpad_acpi/dytc_perfmode + echo M
>>>>>>> /sys/devices/platform/thinkpad_acpi/dytc_perfmode + echo
>>>>>> L > /sys/devices/platform/thinkpad_acpi/dytc_perfmode +
>>>>> 
>>>>> I was thinking about this some more, do you actually want 
>>>>> another mode that "disables" this feature?   IE "O" turns it
>>>>>  off an calls DYTC_DISABLE_CQL.
>>>>> 
>>>>> For example if a user wanted to test the recently landed code
>>>>> in thermald 2.3 and compare performance between the two it
>>>>> seems like this and that "might" fight. As an outsider 
>>>>> looking in - I of course may be wrong too here.
>>>>> 
>>>>> If at some point in the future thermald does a better job 
>>>>> than this implementation you might also want an "out" to let
>>>>>  thermald or another piece of userland turn this off if it's
>>>>>  in the picture.
>>>>> 
>>>> I'm still digging into this one. Right now I haven't found a 
>>>> good clean way of just disabling the firmware. Currently when 
>>>> thermald goes in and tweaks the CPU power registers it has the
>>>>  effect of overriding the FW anyway - but I appreciate that's 
>>>> not quite the same as actually doing it explicitly.
>>>> 
>>> 
>>> What about a modprobe parameter to disable at least?  That would
>>>  at least make it pretty easy to make a change, reboot and 
>>> compare with thermald (or other software) without disabling the 
>>> rest of the functionality of the thinkpad_acpi driver.
>>> 
>> The problem is I don't have a good way to disable the firmware 
>> (that I know of yet) so a modprobe parameter wouldn't really do 
>> much. I guess it could skip providing the sysfs entry points - but
>>  the FW will still be there doing it's thing, so I'm not sure I see
>>  the benefit of that. At least the sysfs entry point gives a bit 
>> more insight into what is going on. Let me know if I'm missing 
>> something obvious.
>> 
> 
> Oh so it's not actually the driver loading tells the firmware it's 
> supposed to work this way.  The firmware actually detects "I'm 
> running on Linux, so I'll do this differently"?
> 
Right. This patch is just providing a more friendly interface so user 
space can see what is going on and be able to do control of the 
different modes without the need for hotkeys.

Afraid I don't have much insight into the nitty gritty details of 
exactly how that works - I believe it's related to detecting the use of 
DPTF but the details aren't shared with me because of NDA's. As a 
thought it's quite possible (probable?) that if thermald is using DPTF 
the firmware will automatically disable itself. Proving that might be 
tricky - I'll see if the firmware team have any ideas.

Mark

Re: [ibm-acpi-devel] [External] RE: [PATCH v2] platform/x86: thinkpad_acpi: performance mode interface

[Limonciello, M. <Mar...@de...>]

> -----Original Message-----
> From: Mark Pearson <mar...@le...>
> Sent: Friday, August 21, 2020 15:06
> To: Limonciello, Mario
> Cc: ibm...@li...; pla...@vg...;
> ibm...@hm...; bn...@re...; Nitin Joshi
> Subject: Re: [External] RE: [PATCH v2] platform/x86: thinkpad_acpi:
> performance mode interface
> 
> 
> [EXTERNAL EMAIL]
> 
> On 8/21/2020 4:00 PM, Limonciello, Mario wrote:
> <snip>
> >>>> + +The sysfs entry provides the ability to return the current
> >>>> status and to set
> >>>> the +desired mode. For example:: + +        echo H >
> >>>> /sys/devices/platform/thinkpad_acpi/dytc_perfmode +        echo
> >>>> M > /sys/devices/platform/thinkpad_acpi/dytc_perfmode +
> >>>> echo L > /sys/devices/platform/thinkpad_acpi/dytc_perfmode +
> >>>
> >>> I was thinking about this some more, do you actually want another
> >>> mode that "disables"
> >>> this feature?   IE "O" turns it off an calls DYTC_DISABLE_CQL.
> >>>
> >>> For example if a user wanted to test the recently landed code in
> >>> thermald 2.3
> >>> and compare performance between the two it seems like this and
> >>> that "might" fight.
> >>> As an outsider looking in - I of course may be wrong too here.
> >>>
> >>> If at some point in the future thermald does a better job than
> >>> this implementation you
> >>> might also want an "out" to let thermald or another piece of
> >>> userland turn this off if it's in the picture.
> >>>
> >> I'm still digging into this one. Right now I haven't found a good
> >> clean way of just disabling the firmware. Currently when thermald
> >> goes in and tweaks the CPU power registers it has the effect of
> >> overriding the FW anyway - but I appreciate that's not quite the
> >> same as actually doing it explicitly.
> >>
> >
> > What about a modprobe parameter to disable at least?  That would at
> > least make it pretty easy to make a change, reboot and compare with
> > thermald (or other software) without disabling the rest of the
> > functionality of the thinkpad_acpi driver.
> >
> The problem is I don't have a good way to disable the firmware (that I
> know of yet) so a modprobe parameter wouldn't really do much. I guess it
> could skip providing the sysfs entry points - but the FW will still be
> there doing it's thing, so I'm not sure I see the benefit of that. At
> least the sysfs entry point gives a bit more insight into what is going on.
> Let me know if I'm missing something obvious.
> 

Oh so it's not actually the driver loading tells the firmware it's supposed
to work this way.  The firmware actually detects "I'm running on Linux, so I'll
do this differently"?

Re: [ibm-acpi-devel] [External] RE: [PATCH v2] platform/x86: thinkpad_acpi: performance mode interface

[Mark P. <mar...@le...>]

On 8/21/2020 4:00 PM, Limonciello, Mario wrote:
<snip>
>>>> + +The sysfs entry provides the ability to return the current
>>>> status and to set
>>>> the +desired mode. For example:: + +        echo H >
>>>> /sys/devices/platform/thinkpad_acpi/dytc_perfmode +        echo
>>>> M > /sys/devices/platform/thinkpad_acpi/dytc_perfmode +
>>>> echo L > /sys/devices/platform/thinkpad_acpi/dytc_perfmode +
>>> 
>>> I was thinking about this some more, do you actually want another
>>> mode that "disables"
>>> this feature?   IE "O" turns it off an calls DYTC_DISABLE_CQL.
>>> 
>>> For example if a user wanted to test the recently landed code in
>>> thermald 2.3
>>> and compare performance between the two it seems like this and
>>> that "might" fight.
>>> As an outsider looking in - I of course may be wrong too here.
>>> 
>>> If at some point in the future thermald does a better job than
>>> this implementation you
>>> might also want an "out" to let thermald or another piece of
>>> userland turn this off if it's in the picture.
>>> 
>> I'm still digging into this one. Right now I haven't found a good
>> clean way of just disabling the firmware. Currently when thermald
>> goes in and tweaks the CPU power registers it has the effect of
>> overriding the FW anyway - but I appreciate that's not quite the
>> same as actually doing it explicitly.
>> 
> 
> What about a modprobe parameter to disable at least?  That would at
> least make it pretty easy to make a change, reboot and compare with
> thermald (or other software) without disabling the rest of the
> functionality of the thinkpad_acpi driver.
> 
The problem is I don't have a good way to disable the firmware (that I 
know of yet) so a modprobe parameter wouldn't really do much. I guess it 
could skip providing the sysfs entry points - but the FW will still be 
there doing it's thing, so I'm not sure I see the benefit of that. At 
least the sysfs entry point gives a bit more insight into what is going on.
Let me know if I'm missing something obvious.

Mark

Re: [ibm-acpi-devel] [External] RE: [PATCH v2] platform/x86: thinkpad_acpi: performance mode interface

[Limonciello, M. <Mar...@de...>]

> -----Original Message-----
> From: pla...@vg... <platform-driver-x86-
> ow...@vg...> On Behalf Of Mark Pearson
> Sent: Friday, August 21, 2020 14:36
> To: Limonciello, Mario
> Cc: ibm...@li...; pla...@vg...;
> ibm...@hm...; bn...@re...; Nitin Joshi
> Subject: Re: [External] RE: [PATCH v2] platform/x86: thinkpad_acpi:
> performance mode interface
> 
> 
> [EXTERNAL EMAIL]
> 
> Hi Mario
> 
> On 8/21/2020 3:15 PM, Limonciello, Mario wrote:
> >>
> <snip>
> >
> > Thanks for adapting a lot of my feedback, this looks much better to me.
> >
> No problems and genuinely thank you for the feedback. Sorry it took so
> long to get this update out - PTO and other things got in the way :)
> 
> >>
> <snip>
> >> +
> >> +The sysfs entry provides the ability to return the current status and to
> set
> >> the
> >> +desired mode. For example::
> >> +
> >> +        echo H > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
> >> +        echo M > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
> >> +        echo L > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
> >> +
> >
> > I was thinking about this some more, do you actually want another mode that
> "disables"
> > this feature?   IE "O" turns it off an calls DYTC_DISABLE_CQL.
> >
> > For example if a user wanted to test the recently landed code in thermald
> 2.3
> > and compare performance between the two it seems like this and that "might"
> fight.
> > As an outsider looking in - I of course may be wrong too here.
> >
> > If at some point in the future thermald does a better job than this
> implementation you
> > might also want an "out" to let thermald or another piece of userland turn
> this off
> > if it's in the picture.
> >
> I'm still digging into this one. Right now I haven't found a good clean
> way of just disabling the firmware. Currently when thermald goes in and
> tweaks the CPU power registers it has the effect of overriding the FW
> anyway - but I appreciate that's not quite the same as actually doing it
> explicitly.
> 

What about a modprobe parameter to disable at least?  That would at least
make it pretty easy to make a change, reboot and compare with thermald (or
other software) without disabling the rest of the functionality of the
thinkpad_acpi driver.

> I'm still discussing with the BIOS team how to implement this - these
> conversations take time.... I suspect I'll need a new BIOS API which
> will take a while to implement and get rolled out. I didn't want to hold
> up this patch as there are benefits for users in the meantime and Lenovo
> do plan on supporting the thermal software going forwards. I'm not not
> dropping the plan to have a disable function as I can see the benefits.
> 


Yup, well aware how these types of things go.

> Thanks for the input. Let me know if you want a reviewed-by tag added. I
> think it's kinda awesome having input from a Dell engineer on a Lenovo
> related driver and I hope in the future I can provide the same help ;)
> Open source is awesome.
> 
> Mark

😊

Re: [ibm-acpi-devel] [External] RE: [PATCH v2] platform/x86: thinkpad_acpi: performance mode interface

[Mark P. <mar...@le...>]

Hi Mario

On 8/21/2020 3:15 PM, Limonciello, Mario wrote:
>>
<snip>
> 
> Thanks for adapting a lot of my feedback, this looks much better to me.
> 
No problems and genuinely thank you for the feedback. Sorry it took so 
long to get this update out - PTO and other things got in the way :)

>>
<snip>
>> +
>> +The sysfs entry provides the ability to return the current status and to set
>> the
>> +desired mode. For example::
>> +
>> +        echo H > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
>> +        echo M > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
>> +        echo L > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
>> +
> 
> I was thinking about this some more, do you actually want another mode that "disables"
> this feature?   IE "O" turns it off an calls DYTC_DISABLE_CQL.
> 
> For example if a user wanted to test the recently landed code in thermald 2.3
> and compare performance between the two it seems like this and that "might" fight.
> As an outsider looking in - I of course may be wrong too here.
> 
> If at some point in the future thermald does a better job than this implementation you
> might also want an "out" to let thermald or another piece of userland turn this off
> if it's in the picture.
> 
I'm still digging into this one. Right now I haven't found a good clean 
way of just disabling the firmware. Currently when thermald goes in and 
tweaks the CPU power registers it has the effect of overriding the FW 
anyway - but I appreciate that's not quite the same as actually doing it 
explicitly.

I'm still discussing with the BIOS team how to implement this - these 
conversations take time.... I suspect I'll need a new BIOS API which 
will take a while to implement and get rolled out. I didn't want to hold 
up this patch as there are benefits for users in the meantime and Lenovo 
do plan on supporting the thermal software going forwards. I'm not not 
dropping the plan to have a disable function as I can see the benefits.

Thanks for the input. Let me know if you want a reviewed-by tag added. I 
think it's kinda awesome having input from a Dell engineer on a Lenovo 
related driver and I hope in the future I can provide the same help ;) 
Open source is awesome.

Mark

Re: [ibm-acpi-devel] [PATCH v2] platform/x86: thinkpad_acpi: performance mode interface

[Limonciello, M. <Mar...@de...>]

> 
> Lenovo Thinkpad platforms with DYTC version 5 and newer have enhanced
> firmware to provide different performance/thermal modes.
> 
> The modes can be controlled by hotkeys (FN+H, FN+M, FN+L) to switch
> the operating mode between three different modes.
> 
> H - High performance. Maximum power is available and the temperature is
> allowed to increase to the maximum for the platform.
> M - Medium performance (aka balance). In this mode power will be limited
> and the laptop will have a lower maximum temperature.
> L - Low performance (aka quiet). In this mode power consumption is reduced
> and the device will be cooler.
> 
> High performance mode is only available when the device is in 'desk mode'.
> If the device detects that it is on a lap then it will automatically drop
> into medium mode to maintain a safer operating temperature.
> 
> This patch provides an interface to determine the current mode and to also
> allow the setting of the mode through the dytc_perfmode sysfs entry. This
> can be used by userspace for improved control.
> 
> Reviewed-by: Nitin Joshi <nj...@le...>
> Signed-off-by: Mark Pearson <mar...@le...>
> ---
> Changes in v2:
>  - Add userspace notification to hkey event handler. Note this got
>    somewhat more complicated than expected as retrieving the performance
>    mode when in lapmode generates an event itself; added a flag to
>    ignore these known events.
>  - Updated the values returned to be simpler (H/M*/M/L) as suggested.
>  - Added ABI documentation as requested. Lap and palm sensor details
>    added whilst I was doing this.
>  - Cleaned up code based on recommendations as well as feedback received
>    from other patch reviews.
>  - Based on discussion with firmware team the lapmode sensor should only
>    be made available for DYTC v5 and later. Added this to init logic.

Thanks for adapting a lot of my feedback, this looks much better to me.

> 
>  .../sysfs-devices-platform-thinkpad_acpi      |  34 +++
>  .../admin-guide/laptops/thinkpad-acpi.rst     |  35 +++
>  drivers/platform/x86/thinkpad_acpi.c          | 267 ++++++++++++++++--
>  3 files changed, 310 insertions(+), 26 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-
> thinkpad_acpi
> 
> diff --git a/Documentation/ABI/testing/sysfs-devices-platform-thinkpad_acpi
> b/Documentation/ABI/testing/sysfs-devices-platform-thinkpad_acpi
> new file mode 100644
> index 000000000000..28f07753a889
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-devices-platform-thinkpad_acpi
> @@ -0,0 +1,34 @@
> +What:		/sys/devices/platform/thinkpad_acpi/dytc_perfmode
> +Date:		August 2020
> +Contact:	Mark Pearson <mar...@le...>
> +Description:
> +		Reads return the current performance mode setting configured in
> firmware using
> +		the below nomenclature.
> +
> +		Writes configure the performance mode setting by using the below
> nomenclature.
> +
> +		H  - High performance mode. Maximum power and temperature
> available.
> +		M* - High performance mode but performance is limited to medium as
> system is
> +		     in lapmode. Power and temperature maximums reduced to a safe
> threshold.
> +		M  - Medium performance mode (aka 'balance'). Lower maximum power
> and temperatures
> +		     but better battery life.
> +		L  - Low performance mode (aka 'quiet'). Reduced power setting
> gives lower
> +		     temperatures and extended battery life. Fans run quieter.
> +
> +What:		/sys/devices/platform/thinkpad_acpi/dytc_lapmode
> +Date:		August 2020
> +Contact:	Mark Pearson <mar...@le...>
> +Description:
> +		Reads returns the current value of the lapmode sensor.
> +
> +		0 - desk mode is detected
> +		1 - lap mode is detected
> +
> +What:		/sys/devices/platform/thinkpad_acpi/psensor_state
> +Date:		August 2020
> +Contact:	Nitin Joshi <nj...@le...>
> +Description:
> +		Reads returns the current value of the palm sensor.
> +
> +		0 - palm not detected
> +		1 - palm detected
> diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> index 6b57c52d8f13..b98f0de9e063 100644
> --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> @@ -52,6 +52,7 @@ detailed description):
>  	- LCD Shadow (PrivacyGuard) enable and disable
>  	- Lap mode sensor
>          - Palm sensor (aka psensor)
> +	- Thermal mode status and control
> 
>  A compatibility table by model and feature is maintained on the web
>  site, http://ibm-acpi.sf.net/. I appreciate any success or failure
> @@ -1465,6 +1466,40 @@ Note - some platforms have a limitation whereby the EC
> firmware cannot
>  determine if the sensor is installed or not. On these platforms the psensor
>  state will always be reported as true to avoid high power being used
> incorrectly.
> 
> +DYTC Thermal mode status and control
> +------------------------------------
> +
> +sysfs: dytc_perfmode
> +
> +Lenovo Thinkpad platforms with DYTC version 5 and newer have enhanced
> firmware to
> +provide improved performance control.
> +
> +The firmware can be controlled by hotkeys (FN+H, FN+M, FN+L) to switch the
> +operating mode between three different modes. This sysfs node provides a
> better
> +interface for user space to use.
> +
> +The modes available are:
> +
> +H - High performance. Maximum power is available and the temperature is
> +allowed to increase to the maximum for the platform.
> +
> +M - Medium performance (aka balance). In this mode power will be limited and
> +the laptop will remain cooler.
> +
> +L - Low performance (aka quiet). In this mode power consumption is reduced
> and
> +the device will be cooler and quieter.
> +
> +Note: High performance mode is only available when the device is in
> 'deskmode'. If
> +the device detects that it is on a lap then it will automatically drop into
> medium
> +mode to maintain a safer operating temperature.
> +
> +The sysfs entry provides the ability to return the current status and to set
> the
> +desired mode. For example::
> +
> +        echo H > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
> +        echo M > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
> +        echo L > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
> +

I was thinking about this some more, do you actually want another mode that "disables"
this feature?   IE "O" turns it off an calls DYTC_DISABLE_CQL.

For example if a user wanted to test the recently landed code in thermald 2.3
and compare performance between the two it seems like this and that "might" fight.
As an outsider looking in - I of course may be wrong too here.

If at some point in the future thermald does a better job than this implementation you
might also want an "out" to let thermald or another piece of userland turn this off
if it's in the picture.

>  EXPERIMENTAL: UWB
>  -----------------
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index 41b75dd4755c..8fcb660aa5a2 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9817,18 +9817,43 @@ static struct ibm_struct lcdshadow_driver_data = {
>  };
> 
>  /*************************************************************************
> - * DYTC subdriver, for the Lenovo lapmode feature
> + * DYTC subdriver, for the Lenovo lap and performance mode feature
>   */
> 
> +#define DYTC_CMD_QUERY        0 /* To get DYTC status - enable/revision */
> +#define DYTC_CMD_SET          1 /* To enable/disable IC function mode */
>  #define DYTC_CMD_GET          2 /* To get current IC function and mode */
> -#define DYTC_GET_LAPMODE_BIT 17 /* Set when in lapmode */
> +#define DYTC_CMD_RESET    0x1ff /* To reset back to default */
> 
> -static bool dytc_lapmode;
> +#define DYTC_QUERY_ENABLE_BIT 8  /* Bit 8 - 0 = disabled, 1 = enabled */
> +#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revisision */
> +#define DYTC_QUERY_REV_BIT    28 /* Bits 28 - 31 - revision */
> 
> -static void dytc_lapmode_notify_change(void)
> -{
> -	sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_lapmode");
> -}
> +#define DYTC_GET_FUNCTION_BIT 8  /* Bits 8-11 - function setting */
> +#define DYTC_GET_MODE_BIT     12 /* Bits 12-15 - mode setting */
> +#define DYTC_GET_LAPMODE_BIT  17 /* Bit 17 - lapmode. Set when on lap */
> +
> +#define DYTC_SET_FUNCTION_BIT 12 /* Bits 12-15 - funct setting */
> +#define DYTC_SET_MODE_BIT     16 /* Bits 16-19 - mode setting */
> +#define DYTC_SET_VALID_BIT    20 /* Bit 20 - 1 = on, 0 = off */
> +
> +#define DYTC_FUNCTION_STD     0  /* Function = 0, standard mode */
> +#define DYTC_FUNCTION_CQL     1  /* Function = 1, lap mode */
> +#define DYTC_FUNCTION_MMC     11 /* Function = 11, desk mode */
> +
> +#define DYTC_MODE_PERFORM     2  /* High power mode aka performance */
> +#define DYTC_MODE_QUIET       3  /* low power mode aka quiet */
> +#define DYTC_MODE_BALANCE   0xF  /* default mode aka balance */
> +
> +#define DYTC_DISABLE_CQL ((DYTC_MODE_BALANCE << DYTC_SET_MODE_BIT) | \
> +		(DYTC_FUNCTION_CQL << DYTC_SET_FUNCTION_BIT) | \
> +		DYTC_CMD_SET)
> +#define DYTC_ENABLE_CQL (DYTC_DISABLE_CQL | (1 << DYTC_SET_VALID_BIT))
> +
> +static bool dytc_lapmode;
> +static int  dytc_perfmode;
> +static bool dytc_available;
> +static bool dytc_ignore_next_event;
> 
>  static int dytc_command(int command, int *output)
>  {
> @@ -9843,6 +9868,87 @@ static int dytc_command(int command, int *output)
>  	return 0;
>  }
> 
> +static int dytc_perfmode_get(int *perfmode, int *funcmode)
> +{
> +	int output, err;
> +
> +	if (!dytc_available)
> +		return -ENODEV;
> +
> +	err = dytc_command(DYTC_CMD_GET, &output);
> +	if (err)
> +		return err;
> +	*funcmode = (output >> DYTC_GET_FUNCTION_BIT) & 0xF;
> +
> +	if (*funcmode == DYTC_FUNCTION_CQL) {
> +		int dummy;
> +		/*
> +		 * We can't get the mode when in CQL mode - so we disable CQL
> +		 * mode retrieve the mode and then enable it again.
> +		 * As disabling/enabling CQL triggers an event we set a flag to
> +		 * ignore these events. This will be cleared by the event handler
> +		 */
> +		dytc_ignore_next_event = true;
> +		err = dytc_command(DYTC_DISABLE_CQL, &dummy);
> +		if (err)
> +			return err;
> +		err = dytc_command(DYTC_CMD_GET, &output);
> +		if (err)
> +			return err;
> +		/* Again ignore this event */
> +		dytc_ignore_next_event = true;
> +		err = dytc_command(DYTC_ENABLE_CQL, &dummy);
> +		if (err)
> +			return err;
> +	}
> +	*perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
> +	return 0;
> +}
> +
> +static int dytc_perfmode_set(int perfmode)
> +{
> +	int err, dytc_set;
> +	int output;
> +	int cur_perfmode, cur_funcmode;
> +
> +	if (!dytc_available)
> +		return -ENODEV;
> +
> +	if (perfmode == DYTC_MODE_BALANCE) {
> +		/* To get back to balance mode we just issue a reset command */
> +		err = dytc_command(DYTC_CMD_RESET, &output);
> +		if (err)
> +			return err;
> +	} else {
> +		/* Determine if we are in CQL mode. This alters the commands we do
> */
> +		err = dytc_perfmode_get(&cur_perfmode, &cur_funcmode);
> +		if (err)
> +			return err;
> +
> +		if (cur_funcmode == DYTC_FUNCTION_CQL) {
> +			/* To set the mode we need to disable CQL first*/
> +			dytc_ignore_next_event = true; /*ignore event*/
> +			err = dytc_command(DYTC_DISABLE_CQL, &output);
> +			if (err)
> +				return err;
> +		}
> +		dytc_set = (1 << DYTC_SET_VALID_BIT) |
> +			(DYTC_FUNCTION_MMC << DYTC_SET_FUNCTION_BIT) |
> +			(perfmode << DYTC_SET_MODE_BIT) |
> +			DYTC_CMD_SET;
> +		err = dytc_command(dytc_set, &output);
> +		if (err)
> +			return err;
> +		if (cur_funcmode == DYTC_FUNCTION_CQL) {
> +			dytc_ignore_next_event = true; /*ignore event*/
> +			err = dytc_command(DYTC_ENABLE_CQL, &output);
> +			if (err)
> +				return err;
> +		}
> +	}
> +	return 0;
> +}
> +
>  static int dytc_lapmode_get(bool *state)
>  {
>  	int output, err;
> @@ -9854,45 +9960,130 @@ static int dytc_lapmode_get(bool *state)
>  	return 0;
>  }
> 
> -static void dytc_lapmode_refresh(void)
> +static void dytc_refresh(void)
>  {
> -	bool new_state;
> +	bool lapmode;
> +	int perfmode, funcmode;
>  	int err;
> 
> -	err = dytc_lapmode_get(&new_state);
> -	if (err || (new_state == dytc_lapmode))
> +	err = dytc_lapmode_get(&lapmode);
> +	if (err)
> +		return;
> +	if (dytc_ignore_next_event) {
> +		dytc_ignore_next_event = false; /*clear setting*/
>  		return;
> +	}
> +	if (lapmode != dytc_lapmode) {
> +		dytc_lapmode = lapmode;
> +		sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_lapmode");
> +	}
> +	err = dytc_perfmode_get(&perfmode, &funcmode);
> +	if (err)
> +		return;
> +	if (perfmode != dytc_perfmode) {
> +		dytc_perfmode = perfmode;
> +		sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_perfmode");
> +	}
> +}
> +
> +/* sysfs perfmode entry */
> +static ssize_t dytc_perfmode_show(struct device *dev,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	int err;
> +	int perfmode, funcmode;
> +
> +	err = dytc_perfmode_get(&perfmode, &funcmode);
> +	if (err)
> +		return err;
> 
> -	dytc_lapmode = new_state;
> -	dytc_lapmode_notify_change();
> +	switch (perfmode) {
> +	case DYTC_MODE_PERFORM:
> +		/* High performance is only available in deskmode */
> +		if (funcmode == DYTC_FUNCTION_CQL)
> +			return sprintf(buf, "M*\n");
> +		else
> +			return sprintf(buf, "H\n");
> +	case DYTC_MODE_QUIET:
> +		return sprintf(buf, "L\n");
> +	case DYTC_MODE_BALANCE:
> +		return sprintf(buf, "M\n");
> +	default:
> +		return sprintf(buf, "Unknown (%d)\n", perfmode);
> +	}
>  }
> 
> +static ssize_t dytc_perfmode_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t count)
> +{
> +	int err;
> +
> +	switch (buf[0]) {
> +	case 'l':
> +	case 'L':
> +		err = dytc_perfmode_set(DYTC_MODE_QUIET);
> +		break;
> +	case 'm':
> +	case 'M':
> +		err = dytc_perfmode_set(DYTC_MODE_BALANCE);
> +		break;
> +	case 'h':
> +	case 'H':
> +		err = dytc_perfmode_set(DYTC_MODE_PERFORM);
> +		break;
> +	default:
> +		err = -EINVAL;
> +		pr_err("Unknown operating mode. Ignoring\n");
> +		break;
> +	}
> +	if (err)
> +		return err;
> +
> +	tpacpi_disclose_usertask(attr->attr.name,
> +				"Performance mode set to %c\n", buf[0]);
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(dytc_perfmode);
> +
> +static struct attribute *dytc_perfmode_attributes[] = {
> +	&dev_attr_dytc_perfmode.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group dytc_perf_attr_group = {
> +	.attrs = dytc_perfmode_attributes
> +};
> +
>  /* sysfs lapmode entry */
>  static ssize_t dytc_lapmode_show(struct device *dev,
>  					struct device_attribute *attr,
>  					char *buf)
>  {
> -	return snprintf(buf, PAGE_SIZE, "%d\n", dytc_lapmode);
> +	return sprintf(buf, "%d\n", dytc_lapmode);
>  }
> 
>  static DEVICE_ATTR_RO(dytc_lapmode);
> 
> -static struct attribute *dytc_attributes[] = {
> +static struct attribute *dytc_lap_attributes[] = {
>  	&dev_attr_dytc_lapmode.attr,
> -	NULL,
> +	NULL
>  };
> 
> -static const struct attribute_group dytc_attr_group = {
> -	.attrs = dytc_attributes,
> +static const struct attribute_group dytc_lap_attr_group = {
> +	.attrs = dytc_lap_attributes
>  };
> 
>  static int tpacpi_dytc_init(struct ibm_init_struct *iibm)
>  {
> -	int err;
> +	int err, output;
> 
> -	err = dytc_lapmode_get(&dytc_lapmode);
> -	/* If support isn't available (ENODEV) then don't return an error
> -	 * but just don't create the sysfs group
> +	err = dytc_command(DYTC_CMD_QUERY, &output);
> +	/*
> +	 * If support isn't available (ENODEV) then don't return an error
> +	 * just don't create the sysfs group
>  	 */
>  	if (err == -ENODEV)
>  		return 0;
> @@ -9900,14 +10091,38 @@ static int tpacpi_dytc_init(struct ibm_init_struct
> *iibm)
>  	if (err)
>  		return err;
> 
> -	/* Platform supports this feature - create the group */
> -	err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group);
> +	/* Check DYTC is enabled and supports mode setting */
> +	dytc_available = false;
> +	dytc_ignore_next_event = false;
> +	if (output & BIT(DYTC_QUERY_ENABLE_BIT)) {
> +		/* Only DYTC v5.0 and later has this feature. */
> +		int dytc_version;
> +
> +		dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
> +		if (dytc_version >= 5) {
> +			dbg_printk(TPACPI_DBG_INIT,
> +				   "DYTC version %d: thermal mode available\n",
> dytc_version);
> +			dytc_available = true;
> +			/* Platform supports this feature - create the group */
> +			err = sysfs_create_group(&tpacpi_pdev->dev.kobj,
> &dytc_perf_attr_group);
> +			if (err)
> +				return err;
> +
> +			err = dytc_lapmode_get(&dytc_lapmode);
> +			if (err)
> +				return err;
> +			err = sysfs_create_group(&tpacpi_pdev->dev.kobj,
> &dytc_lap_attr_group);
> +		}
> +	}
>  	return err;
>  }
> 
>  static void dytc_exit(void)
>  {
> -	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group);
> +	if (dytc_available) {
> +		sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_lap_attr_group);
> +		sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_perf_attr_group);
> +	}
>  }
> 
>  static struct ibm_struct dytc_driver_data = {
> @@ -10057,7 +10272,7 @@ static void tpacpi_driver_event(const unsigned int
> hkey_event)
>  	}
> 
>  	if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
> -		dytc_lapmode_refresh();
> +		dytc_refresh();
> 
>  	if ((hkey_event == TP_HKEY_EV_PALM_DETECTED) ||
>  		(hkey_event == TP_HKEY_EV_PALM_UNDETECTED))
> --
> 2.26.2

[ibm-acpi-devel] [PATCH v2] platform/x86: thinkpad_acpi: performance mode interface

[Mark P. <mar...@le...>]

Lenovo Thinkpad platforms with DYTC version 5 and newer have enhanced
firmware to provide different performance/thermal modes.

The modes can be controlled by hotkeys (FN+H, FN+M, FN+L) to switch
the operating mode between three different modes.

H - High performance. Maximum power is available and the temperature is
allowed to increase to the maximum for the platform.
M - Medium performance (aka balance). In this mode power will be limited
and the laptop will have a lower maximum temperature.
L - Low performance (aka quiet). In this mode power consumption is reduced
and the device will be cooler.

High performance mode is only available when the device is in 'desk mode'.
If the device detects that it is on a lap then it will automatically drop
into medium mode to maintain a safer operating temperature.

This patch provides an interface to determine the current mode and to also
allow the setting of the mode through the dytc_perfmode sysfs entry. This
can be used by userspace for improved control.

Reviewed-by: Nitin Joshi <nj...@le...>
Signed-off-by: Mark Pearson <mar...@le...>
---
Changes in v2:
 - Add userspace notification to hkey event handler. Note this got
   somewhat more complicated than expected as retrieving the performance
   mode when in lapmode generates an event itself; added a flag to
   ignore these known events.
 - Updated the values returned to be simpler (H/M*/M/L) as suggested.
 - Added ABI documentation as requested. Lap and palm sensor details
   added whilst I was doing this.
 - Cleaned up code based on recommendations as well as feedback received
   from other patch reviews.
 - Based on discussion with firmware team the lapmode sensor should only
   be made available for DYTC v5 and later. Added this to init logic.

 .../sysfs-devices-platform-thinkpad_acpi      |  34 +++
 .../admin-guide/laptops/thinkpad-acpi.rst     |  35 +++
 drivers/platform/x86/thinkpad_acpi.c          | 267 ++++++++++++++++--
 3 files changed, 310 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-thinkpad_acpi

diff --git a/Documentation/ABI/testing/sysfs-devices-platform-thinkpad_acpi b/Documentation/ABI/testing/sysfs-devices-platform-thinkpad_acpi
new file mode 100644
index 000000000000..28f07753a889
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-platform-thinkpad_acpi
@@ -0,0 +1,34 @@
+What:		/sys/devices/platform/thinkpad_acpi/dytc_perfmode
+Date:		August 2020
+Contact:	Mark Pearson <mar...@le...>
+Description:
+		Reads return the current performance mode setting configured in firmware using
+		the below nomenclature.
+
+		Writes configure the performance mode setting by using the below nomenclature.
+
+		H  - High performance mode. Maximum power and temperature available.
+		M* - High performance mode but performance is limited to medium as system is
+		     in lapmode. Power and temperature maximums reduced to a safe threshold.
+		M  - Medium performance mode (aka 'balance'). Lower maximum power and temperatures
+		     but better battery life.
+		L  - Low performance mode (aka 'quiet'). Reduced power setting gives lower
+		     temperatures and extended battery life. Fans run quieter.
+
+What:		/sys/devices/platform/thinkpad_acpi/dytc_lapmode
+Date:		August 2020
+Contact:	Mark Pearson <mar...@le...>
+Description:
+		Reads returns the current value of the lapmode sensor.
+
+		0 - desk mode is detected
+		1 - lap mode is detected
+
+What:		/sys/devices/platform/thinkpad_acpi/psensor_state
+Date:		August 2020
+Contact:	Nitin Joshi <nj...@le...>
+Description:
+		Reads returns the current value of the palm sensor.
+
+		0 - palm not detected
+		1 - palm detected
diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
index 6b57c52d8f13..b98f0de9e063 100644
--- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
+++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
@@ -52,6 +52,7 @@ detailed description):
 	- LCD Shadow (PrivacyGuard) enable and disable
 	- Lap mode sensor
         - Palm sensor (aka psensor)
+	- Thermal mode status and control
 
 A compatibility table by model and feature is maintained on the web
 site, http://ibm-acpi.sf.net/. I appreciate any success or failure
@@ -1465,6 +1466,40 @@ Note - some platforms have a limitation whereby the EC firmware cannot
 determine if the sensor is installed or not. On these platforms the psensor
 state will always be reported as true to avoid high power being used incorrectly.
 
+DYTC Thermal mode status and control
+------------------------------------
+
+sysfs: dytc_perfmode
+
+Lenovo Thinkpad platforms with DYTC version 5 and newer have enhanced firmware to
+provide improved performance control.
+
+The firmware can be controlled by hotkeys (FN+H, FN+M, FN+L) to switch the
+operating mode between three different modes. This sysfs node provides a better
+interface for user space to use.
+
+The modes available are:
+
+H - High performance. Maximum power is available and the temperature is
+allowed to increase to the maximum for the platform.
+
+M - Medium performance (aka balance). In this mode power will be limited and
+the laptop will remain cooler.
+
+L - Low performance (aka quiet). In this mode power consumption is reduced and
+the device will be cooler and quieter.
+
+Note: High performance mode is only available when the device is in 'deskmode'. If
+the device detects that it is on a lap then it will automatically drop into medium
+mode to maintain a safer operating temperature.
+
+The sysfs entry provides the ability to return the current status and to set the
+desired mode. For example::
+
+        echo H > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
+        echo M > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
+        echo L > /sys/devices/platform/thinkpad_acpi/dytc_perfmode
+
 EXPERIMENTAL: UWB
 -----------------
 
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 41b75dd4755c..8fcb660aa5a2 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9817,18 +9817,43 @@ static struct ibm_struct lcdshadow_driver_data = {
 };
 
 /*************************************************************************
- * DYTC subdriver, for the Lenovo lapmode feature
+ * DYTC subdriver, for the Lenovo lap and performance mode feature
  */
 
+#define DYTC_CMD_QUERY        0 /* To get DYTC status - enable/revision */
+#define DYTC_CMD_SET          1 /* To enable/disable IC function mode */
 #define DYTC_CMD_GET          2 /* To get current IC function and mode */
-#define DYTC_GET_LAPMODE_BIT 17 /* Set when in lapmode */
+#define DYTC_CMD_RESET    0x1ff /* To reset back to default */
 
-static bool dytc_lapmode;
+#define DYTC_QUERY_ENABLE_BIT 8  /* Bit 8 - 0 = disabled, 1 = enabled */
+#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revisision */
+#define DYTC_QUERY_REV_BIT    28 /* Bits 28 - 31 - revision */
 
-static void dytc_lapmode_notify_change(void)
-{
-	sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_lapmode");
-}
+#define DYTC_GET_FUNCTION_BIT 8  /* Bits 8-11 - function setting */
+#define DYTC_GET_MODE_BIT     12 /* Bits 12-15 - mode setting */
+#define DYTC_GET_LAPMODE_BIT  17 /* Bit 17 - lapmode. Set when on lap */
+
+#define DYTC_SET_FUNCTION_BIT 12 /* Bits 12-15 - funct setting */
+#define DYTC_SET_MODE_BIT     16 /* Bits 16-19 - mode setting */
+#define DYTC_SET_VALID_BIT    20 /* Bit 20 - 1 = on, 0 = off */
+
+#define DYTC_FUNCTION_STD     0  /* Function = 0, standard mode */
+#define DYTC_FUNCTION_CQL     1  /* Function = 1, lap mode */
+#define DYTC_FUNCTION_MMC     11 /* Function = 11, desk mode */
+
+#define DYTC_MODE_PERFORM     2  /* High power mode aka performance */
+#define DYTC_MODE_QUIET       3  /* low power mode aka quiet */
+#define DYTC_MODE_BALANCE   0xF  /* default mode aka balance */
+
+#define DYTC_DISABLE_CQL ((DYTC_MODE_BALANCE << DYTC_SET_MODE_BIT) | \
+		(DYTC_FUNCTION_CQL << DYTC_SET_FUNCTION_BIT) | \
+		DYTC_CMD_SET)
+#define DYTC_ENABLE_CQL (DYTC_DISABLE_CQL | (1 << DYTC_SET_VALID_BIT))
+
+static bool dytc_lapmode;
+static int  dytc_perfmode;
+static bool dytc_available;
+static bool dytc_ignore_next_event;
 
 static int dytc_command(int command, int *output)
 {
@@ -9843,6 +9868,87 @@ static int dytc_command(int command, int *output)
 	return 0;
 }
 
+static int dytc_perfmode_get(int *perfmode, int *funcmode)
+{
+	int output, err;
+
+	if (!dytc_available)
+		return -ENODEV;
+
+	err = dytc_command(DYTC_CMD_GET, &output);
+	if (err)
+		return err;
+	*funcmode = (output >> DYTC_GET_FUNCTION_BIT) & 0xF;
+
+	if (*funcmode == DYTC_FUNCTION_CQL) {
+		int dummy;
+		/*
+		 * We can't get the mode when in CQL mode - so we disable CQL
+		 * mode retrieve the mode and then enable it again.
+		 * As disabling/enabling CQL triggers an event we set a flag to
+		 * ignore these events. This will be cleared by the event handler
+		 */
+		dytc_ignore_next_event = true;
+		err = dytc_command(DYTC_DISABLE_CQL, &dummy);
+		if (err)
+			return err;
+		err = dytc_command(DYTC_CMD_GET, &output);
+		if (err)
+			return err;
+		/* Again ignore this event */
+		dytc_ignore_next_event = true;
+		err = dytc_command(DYTC_ENABLE_CQL, &dummy);
+		if (err)
+			return err;
+	}
+	*perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
+	return 0;
+}
+
+static int dytc_perfmode_set(int perfmode)
+{
+	int err, dytc_set;
+	int output;
+	int cur_perfmode, cur_funcmode;
+
+	if (!dytc_available)
+		return -ENODEV;
+
+	if (perfmode == DYTC_MODE_BALANCE) {
+		/* To get back to balance mode we just issue a reset command */
+		err = dytc_command(DYTC_CMD_RESET, &output);
+		if (err)
+			return err;
+	} else {
+		/* Determine if we are in CQL mode. This alters the commands we do */
+		err = dytc_perfmode_get(&cur_perfmode, &cur_funcmode);
+		if (err)
+			return err;
+
+		if (cur_funcmode == DYTC_FUNCTION_CQL) {
+			/* To set the mode we need to disable CQL first*/
+			dytc_ignore_next_event = true; /*ignore event*/
+			err = dytc_command(DYTC_DISABLE_CQL, &output);
+			if (err)
+				return err;
+		}
+		dytc_set = (1 << DYTC_SET_VALID_BIT) |
+			(DYTC_FUNCTION_MMC << DYTC_SET_FUNCTION_BIT) |
+			(perfmode << DYTC_SET_MODE_BIT) |
+			DYTC_CMD_SET;
+		err = dytc_command(dytc_set, &output);
+		if (err)
+			return err;
+		if (cur_funcmode == DYTC_FUNCTION_CQL) {
+			dytc_ignore_next_event = true; /*ignore event*/
+			err = dytc_command(DYTC_ENABLE_CQL, &output);
+			if (err)
+				return err;
+		}
+	}
+	return 0;
+}
+
 static int dytc_lapmode_get(bool *state)
 {
 	int output, err;
@@ -9854,45 +9960,130 @@ static int dytc_lapmode_get(bool *state)
 	return 0;
 }
 
-static void dytc_lapmode_refresh(void)
+static void dytc_refresh(void)
 {
-	bool new_state;
+	bool lapmode;
+	int perfmode, funcmode;
 	int err;
 
-	err = dytc_lapmode_get(&new_state);
-	if (err || (new_state == dytc_lapmode))
+	err = dytc_lapmode_get(&lapmode);
+	if (err)
+		return;
+	if (dytc_ignore_next_event) {
+		dytc_ignore_next_event = false; /*clear setting*/
 		return;
+	}
+	if (lapmode != dytc_lapmode) {
+		dytc_lapmode = lapmode;
+		sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_lapmode");
+	}
+	err = dytc_perfmode_get(&perfmode, &funcmode);
+	if (err)
+		return;
+	if (perfmode != dytc_perfmode) {
+		dytc_perfmode = perfmode;
+		sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_perfmode");
+	}
+}
+
+/* sysfs perfmode entry */
+static ssize_t dytc_perfmode_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	int err;
+	int perfmode, funcmode;
+
+	err = dytc_perfmode_get(&perfmode, &funcmode);
+	if (err)
+		return err;
 
-	dytc_lapmode = new_state;
-	dytc_lapmode_notify_change();
+	switch (perfmode) {
+	case DYTC_MODE_PERFORM:
+		/* High performance is only available in deskmode */
+		if (funcmode == DYTC_FUNCTION_CQL)
+			return sprintf(buf, "M*\n");
+		else
+			return sprintf(buf, "H\n");
+	case DYTC_MODE_QUIET:
+		return sprintf(buf, "L\n");
+	case DYTC_MODE_BALANCE:
+		return sprintf(buf, "M\n");
+	default:
+		return sprintf(buf, "Unknown (%d)\n", perfmode);
+	}
 }
 
+static ssize_t dytc_perfmode_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	int err;
+
+	switch (buf[0]) {
+	case 'l':
+	case 'L':
+		err = dytc_perfmode_set(DYTC_MODE_QUIET);
+		break;
+	case 'm':
+	case 'M':
+		err = dytc_perfmode_set(DYTC_MODE_BALANCE);
+		break;
+	case 'h':
+	case 'H':
+		err = dytc_perfmode_set(DYTC_MODE_PERFORM);
+		break;
+	default:
+		err = -EINVAL;
+		pr_err("Unknown operating mode. Ignoring\n");
+		break;
+	}
+	if (err)
+		return err;
+
+	tpacpi_disclose_usertask(attr->attr.name,
+				"Performance mode set to %c\n", buf[0]);
+	return count;
+}
+
+static DEVICE_ATTR_RW(dytc_perfmode);
+
+static struct attribute *dytc_perfmode_attributes[] = {
+	&dev_attr_dytc_perfmode.attr,
+	NULL
+};
+
+static const struct attribute_group dytc_perf_attr_group = {
+	.attrs = dytc_perfmode_attributes
+};
+
 /* sysfs lapmode entry */
 static ssize_t dytc_lapmode_show(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", dytc_lapmode);
+	return sprintf(buf, "%d\n", dytc_lapmode);
 }
 
 static DEVICE_ATTR_RO(dytc_lapmode);
 
-static struct attribute *dytc_attributes[] = {
+static struct attribute *dytc_lap_attributes[] = {
 	&dev_attr_dytc_lapmode.attr,
-	NULL,
+	NULL
 };
 
-static const struct attribute_group dytc_attr_group = {
-	.attrs = dytc_attributes,
+static const struct attribute_group dytc_lap_attr_group = {
+	.attrs = dytc_lap_attributes
 };
 
 static int tpacpi_dytc_init(struct ibm_init_struct *iibm)
 {
-	int err;
+	int err, output;
 
-	err = dytc_lapmode_get(&dytc_lapmode);
-	/* If support isn't available (ENODEV) then don't return an error
-	 * but just don't create the sysfs group
+	err = dytc_command(DYTC_CMD_QUERY, &output);
+	/*
+	 * If support isn't available (ENODEV) then don't return an error
+	 * just don't create the sysfs group
 	 */
 	if (err == -ENODEV)
 		return 0;
@@ -9900,14 +10091,38 @@ static int tpacpi_dytc_init(struct ibm_init_struct *iibm)
 	if (err)
 		return err;
 
-	/* Platform supports this feature - create the group */
-	err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group);
+	/* Check DYTC is enabled and supports mode setting */
+	dytc_available = false;
+	dytc_ignore_next_event = false;
+	if (output & BIT(DYTC_QUERY_ENABLE_BIT)) {
+		/* Only DYTC v5.0 and later has this feature. */
+		int dytc_version;
+
+		dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
+		if (dytc_version >= 5) {
+			dbg_printk(TPACPI_DBG_INIT,
+				   "DYTC version %d: thermal mode available\n", dytc_version);
+			dytc_available = true;
+			/* Platform supports this feature - create the group */
+			err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &dytc_perf_attr_group);
+			if (err)
+				return err;
+
+			err = dytc_lapmode_get(&dytc_lapmode);
+			if (err)
+				return err;
+			err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &dytc_lap_attr_group);
+		}
+	}
 	return err;
 }
 
 static void dytc_exit(void)
 {
-	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group);
+	if (dytc_available) {
+		sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_lap_attr_group);
+		sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_perf_attr_group);
+	}
 }
 
 static struct ibm_struct dytc_driver_data = {
@@ -10057,7 +10272,7 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
 	}
 
 	if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
-		dytc_lapmode_refresh();
+		dytc_refresh();
 
 	if ((hkey_event == TP_HKEY_EV_PALM_DETECTED) ||
 		(hkey_event == TP_HKEY_EV_PALM_UNDETECTED))
-- 
2.26.2

[ibm-acpi-devel] [PATCH v3] platform/x86: thinkpad_acpi: psensor interface

[Mark P. <mar...@le...>]

Some Lenovo Thinkpad platforms are equipped with a 'palm sensor' so as
to be able to determine if a user is physically proximate to the device.

This patch provides the ability to retrieve the psensor state via sysfs
entrypoints and will be used by userspace for WWAN functionality to
control the transmission level safely

Co-developed-by: Nitin Joshi <nj...@le...>
Signed-off-by: Nitin Joshi <nj...@le...>
Signed-off-by: Mark Pearson <mar...@le...>
---
Changes in v2:
 - addressed formatting and coding style errors identified during review
Changes in v3:
 - I had missed updating thinkpad-acpi.rst in the v2 update so adding
   that back so it is correct. 

 .../admin-guide/laptops/thinkpad-acpi.rst     |  18 +++
 drivers/platform/x86/thinkpad_acpi.c          | 107 +++++++++++++++++-
 2 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
index 5e477869df18..6b57c52d8f13 100644
--- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
+++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
@@ -51,6 +51,7 @@ detailed description):
 	- UWB enable and disable
 	- LCD Shadow (PrivacyGuard) enable and disable
 	- Lap mode sensor
+        - Palm sensor (aka psensor)
 
 A compatibility table by model and feature is maintained on the web
 site, http://ibm-acpi.sf.net/. I appreciate any success or failure
@@ -1447,6 +1448,23 @@ they differ between desk and lap mode.
 The property is read-only. If the platform doesn't have support the sysfs
 class is not created.
 
+Palm sensor
+------------------
+
+sysfs: psensor_state
+
+Certain thinkpads and mobile workstations are equipped with a palm sensor to
+detect when a user is physically near the device. This device, when present,
+is used in conjunction with the lapmode sensor to decide if WWAN transmission
+can be increased to maximum power.
+
+The property is read-only. If the platform doesn't have support the sysfs
+class is not created.
+
+Note - some platforms have a limitation whereby the EC firmware cannot
+determine if the sensor is installed or not. On these platforms the psensor
+state will always be reported as true to avoid high power being used incorrectly.
+
 EXPERIMENTAL: UWB
 -----------------
 
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 4864a5c189d4..41b75dd4755c 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4079,10 +4079,9 @@ static bool hotkey_notify_6xxx(const u32 hkey,
 
 	case TP_HKEY_EV_PALM_DETECTED:
 	case TP_HKEY_EV_PALM_UNDETECTED:
-		/* palm detected hovering the keyboard, forward to user-space
-		 * via netlink for consumption */
+		/* palm detected - pass on to event handler */
+		tpacpi_driver_event(hkey);
 		return true;
-
 	default:
 		/* report simply as unknown, no sensor dump */
 		return false;
@@ -9916,6 +9915,99 @@ static struct ibm_struct dytc_driver_data = {
 	.exit = dytc_exit,
 };
 
+/**********************************************************************
+ * Palm sensor subdriver
+ */
+
+#define PSENSOR_PRESENT_BIT 0 /* Determine if psensor present */
+#define PSENSOR_ON_BIT      1 /* psensor status */
+
+static bool psensor_state;
+
+static void psensor_notify_change(void)
+{
+	sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "psensor_state");
+}
+
+static int psensor_get(bool *state)
+{
+	acpi_handle psensor_handle;
+	int output;
+
+	if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GPSS", &psensor_handle)))
+		return -ENODEV;
+
+	if (!acpi_evalf(psensor_handle, &output, NULL, "d"))
+		return -EIO;
+
+	/* Check if sensor has a Psensor */
+	if (!(output & BIT(PSENSOR_PRESENT_BIT)))
+		return -ENODEV;
+
+	/* Return if psensor is set or not */
+	*state = output & BIT(PSENSOR_ON_BIT) ? true : false;
+	return 0;
+}
+
+static void psensor_state_refresh(void)
+{
+	bool new_state;
+	int err;
+
+	err = psensor_get(&new_state);
+	if (err || (new_state == psensor_state))
+		return;
+
+	psensor_state = new_state;
+	psensor_notify_change();
+}
+
+/* sysfs psensor entry */
+static ssize_t psensor_state_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	return sprintf(buf, "%d\n", psensor_state);
+}
+static DEVICE_ATTR_RO(psensor_state);
+
+static struct attribute *psensor_attributes[] = {
+	&dev_attr_psensor_state.attr,
+	NULL
+};
+
+static const struct attribute_group psensor_attr_group = {
+	.attrs = psensor_attributes,
+};
+
+static int tpacpi_psensor_init(struct ibm_init_struct *iibm)
+{
+	int err;
+
+	err = psensor_get(&psensor_state);
+	/*
+	 * If support isn't available (ENODEV) then don't return an error,
+	 * just don't create the sysfs group.
+	 */
+	if (err == -ENODEV)
+		return 0;
+	/* For all other errors we can flag the failure */
+	if (err)
+		return err;
+
+	return sysfs_create_group(&tpacpi_pdev->dev.kobj, &psensor_attr_group);
+}
+
+static void psensor_exit(void)
+{
+	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &psensor_attr_group);
+}
+
+static struct ibm_struct psensor_driver_data = {
+	.name = "psensor",
+	.exit = psensor_exit,
+};
+
 /****************************************************************************
  ****************************************************************************
  *
@@ -9967,6 +10059,10 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
 	if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
 		dytc_lapmode_refresh();
 
+	if ((hkey_event == TP_HKEY_EV_PALM_DETECTED) ||
+		(hkey_event == TP_HKEY_EV_PALM_UNDETECTED))
+		psensor_state_refresh();
+
 }
 
 static void hotkey_driver_event(const unsigned int scancode)
@@ -10409,6 +10505,11 @@ static struct ibm_init_struct ibms_init[] __initdata = {
 		.init = tpacpi_dytc_init,
 		.data = &dytc_driver_data,
 	},
+	{
+		.init = tpacpi_psensor_init,
+		.data = &psensor_driver_data,
+	},
+
 };
 
 static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
-- 
2.26.2

[ibm-acpi-devel] [PATCH v2] platform/x86: thinkpad_acpi: psensor interface

[Mark P. <mar...@le...>]

Some Lenovo Thinkpad platforms are equipped with a 'palm sensor' so as
to be able to determine if a user is physically proximate to the device.

This patch provides the ability to retrieve the psensor state via sysfs
entrypoints and will be used by userspace for WWAN functionality to
control the transmission level safely

Co-developed-by: Nitin Joshi <nj...@le...>
Signed-off-by: Nitin Joshi <nj...@le...>
Signed-off-by: Mark Pearson <mar...@le...>
---
Changes in v2:
 - addressed formatting and coding style errors identified during review

 drivers/platform/x86/thinkpad_acpi.c | 107 ++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 4864a5c189d4..41b75dd4755c 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4079,10 +4079,9 @@ static bool hotkey_notify_6xxx(const u32 hkey,
 
 	case TP_HKEY_EV_PALM_DETECTED:
 	case TP_HKEY_EV_PALM_UNDETECTED:
-		/* palm detected hovering the keyboard, forward to user-space
-		 * via netlink for consumption */
+		/* palm detected - pass on to event handler */
+		tpacpi_driver_event(hkey);
 		return true;
-
 	default:
 		/* report simply as unknown, no sensor dump */
 		return false;
@@ -9916,6 +9915,99 @@ static struct ibm_struct dytc_driver_data = {
 	.exit = dytc_exit,
 };
 
+/**********************************************************************
+ * Palm sensor subdriver
+ */
+
+#define PSENSOR_PRESENT_BIT 0 /* Determine if psensor present */
+#define PSENSOR_ON_BIT      1 /* psensor status */
+
+static bool psensor_state;
+
+static void psensor_notify_change(void)
+{
+	sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "psensor_state");
+}
+
+static int psensor_get(bool *state)
+{
+	acpi_handle psensor_handle;
+	int output;
+
+	if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GPSS", &psensor_handle)))
+		return -ENODEV;
+
+	if (!acpi_evalf(psensor_handle, &output, NULL, "d"))
+		return -EIO;
+
+	/* Check if sensor has a Psensor */
+	if (!(output & BIT(PSENSOR_PRESENT_BIT)))
+		return -ENODEV;
+
+	/* Return if psensor is set or not */
+	*state = output & BIT(PSENSOR_ON_BIT) ? true : false;
+	return 0;
+}
+
+static void psensor_state_refresh(void)
+{
+	bool new_state;
+	int err;
+
+	err = psensor_get(&new_state);
+	if (err || (new_state == psensor_state))
+		return;
+
+	psensor_state = new_state;
+	psensor_notify_change();
+}
+
+/* sysfs psensor entry */
+static ssize_t psensor_state_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	return sprintf(buf, "%d\n", psensor_state);
+}
+static DEVICE_ATTR_RO(psensor_state);
+
+static struct attribute *psensor_attributes[] = {
+	&dev_attr_psensor_state.attr,
+	NULL
+};
+
+static const struct attribute_group psensor_attr_group = {
+	.attrs = psensor_attributes,
+};
+
+static int tpacpi_psensor_init(struct ibm_init_struct *iibm)
+{
+	int err;
+
+	err = psensor_get(&psensor_state);
+	/*
+	 * If support isn't available (ENODEV) then don't return an error,
+	 * just don't create the sysfs group.
+	 */
+	if (err == -ENODEV)
+		return 0;
+	/* For all other errors we can flag the failure */
+	if (err)
+		return err;
+
+	return sysfs_create_group(&tpacpi_pdev->dev.kobj, &psensor_attr_group);
+}
+
+static void psensor_exit(void)
+{
+	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &psensor_attr_group);
+}
+
+static struct ibm_struct psensor_driver_data = {
+	.name = "psensor",
+	.exit = psensor_exit,
+};
+
 /****************************************************************************
  ****************************************************************************
  *
@@ -9967,6 +10059,10 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
 	if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
 		dytc_lapmode_refresh();
 
+	if ((hkey_event == TP_HKEY_EV_PALM_DETECTED) ||
+		(hkey_event == TP_HKEY_EV_PALM_UNDETECTED))
+		psensor_state_refresh();
+
 }
 
 static void hotkey_driver_event(const unsigned int scancode)
@@ -10409,6 +10505,11 @@ static struct ibm_init_struct ibms_init[] __initdata = {
 		.init = tpacpi_dytc_init,
 		.data = &dytc_driver_data,
 	},
+	{
+		.init = tpacpi_psensor_init,
+		.data = &psensor_driver_data,
+	},
+
 };
 
 static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
-- 
2.26.2

Re: [ibm-acpi-devel] [External] Re: [PATCH] platform/x86: thinkpad_acpi: psensor interface

[Mark P. <mar...@le...>]

Hi Andy,

First off apologies for my delay in replying and thanks to Nitin for 
covering for me. I took a week of PTO and then suffered the consequences 
of that for the week after - it's taken me a bit to catch-up.


On 7/27/2020 11:51 PM, Nitin Joshi1 wrote:
> Hi Andy ,
> 
>> -----Original Message-----
>> From: Andy Shevchenko <and...@gm...>
>> Sent: Monday, July 27, 2020 7:35 PM
>> On Thu, Jul 16, 2020 at 2:53 AM Mark Pearson <mar...@le...>
>> wrote:
>>>
>>>          case TP_HKEY_EV_PALM_DETECTED:
>>>          case TP_HKEY_EV_PALM_UNDETECTED:
>>> -               /* palm detected hovering the keyboard, forward to user-space
>>> -                * via netlink for consumption */
>>> +               /* palm detected - pass on to event handler */
>>> +               tpacpi_driver_event(hkey);
>>>                  return true;
>>
>> Comment here tells something about the netlink interface to user space.
>> Can you elaborate why we need sysfs now and how it's all supposed to
>> work?
> Using  netlink , we were getting proximity events like '0x60b0' and '0x60b1' but for our WWAN requirement, we need default and current
> p-sensor state too .
> Some tools like "acpi-listen" uses netlink to display events but we need default and current p-sensor state also as per our requirement.
> hence , we have added new sysfs to get current p-sensor state using 'GPSS' method from BIOS .
> This will be used for implementing "Dynamic power reduction" app which is used to control Body SAR value as per FCC requirement .
> When Body or any object is near or away from p-sensor location on thinkpad system , then sysfs will be set .
> 
I think Nitin has covered it. Let us know if any follow on questions or 
concerns here.

>>
>> ...
>>
>>> +static int psensor_get(bool *state)
>>> +{
>>> +       acpi_handle psensor_handle;
>>> +       int output;
>>> +
>>> +       if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GPSS",
>> &psensor_handle)))
>>> +               return -ENODEV;
>>> +
>>> +       if (!acpi_evalf(psensor_handle, &output, NULL, "d"))
>>> +               return -EIO;
>>> +
>>> +       /* Check if sensor has a Psensor */
>>> +       if (!(output & BIT(PSENSOR_PRESENT_BIT)))
>>> +               return -ENODEV;
>>> +
>>> +       /* Return if psensor is set or not */
>>> +       *state = output & BIT(PSENSOR_ON_BIT) ? true : false;
>>> +       return 0;
>>> +}
>>
>> It reminds me of a function you created in one of the previous changes. Can
>> you rather create a parameterized helper which will serve for both?
> 
> Ack , we will check it .
I've been looking at this and I understand where you're coming from but 
the benefits of combining them aren't working for me.

The previous change was for the lapmode sensor which is a separate piece 
of hardware. The ACPI handle and access format is different and the 
lapmode sensor has some extra handling logic needed. The code has enough 
differences too that I don't think combining it makes a lot of sense.

Let me know if I'm missing something obvious or if you disagree.
> 
>>
>> ...
>>
>>> +/* sysfs psensor entry */
>>> +static ssize_t psensor_state_show(struct device *dev,
>>> +                                       struct device_attribute *attr,
>>> +                                       char *buf) {
>>
>>> +       return snprintf(buf, PAGE_SIZE, "%d\n", psensor_state);
>>
>> We know that %d takes much less than PAGE_SIZE, use sprintf().
>>
>>> +}
>>
>>> +
>>
>> No blank line here.
>>
> Ack
> 
>>> +static DEVICE_ATTR_RO(psensor_state);
>>
>> ...
>>
>>> +static struct attribute *psensor_attributes[] = {
>>> +       &dev_attr_psensor_state.attr,
>>
>>> +       NULL,
>>
>> No comma for terminator line(s).
>>
> 
> Ack
> 
>>> +};
>>
>> ...
>>
>>> +       /* If support isn't available (ENODEV) then don't return an error
>>> +        * but just don't create the sysfs group
>>> +        */
>>
>> /*
>> * Consider to use a proper multi-line comment style.
>> * Like here. (It's applicable to the entire patch)  */
>>
>> ...
>>
>>> +       err = sysfs_create_group(&tpacpi_pdev->dev.kobj,
>> &psensor_attr_group);
>>> +       return err;
>>
>> return sysfs...
> Ack
> 
I'll push a patch soon with these other adjustments.

Thanks for the review
Mark

Re: [ibm-acpi-devel] [External] Re: [PATCH v5] platform/x86: thinkpad_acpi: lap or desk mode interface

[Mark P. <mar...@le...>]

On 8/3/2020 10:46 AM, Elvis Stansvik wrote:

> 
> Thanks Mark. I know the X1C6 firmware did not get the new thermal
> mode implementation, but thought that I would still be able to query
> its lap mode.
> 
> The reason I wanted query the lap mode is actually because I'm
> trying to help Matthew Garrett out in adding support for the adaptive
> DPTF performance policy on the thermald side, instead of on the
> firmware side like Lenovo decided to do for more recent models. See
> that effort here https://github.com/mjg59/thermal_daemon/issues/7).
> We were surprised to find that all of the OEM conditions in the APCT
> table remained unchanged when the laptop was taken between lap and
> desk. So I started suspecting that perhaps the X1C6 and its firmware
> simply does not have the ability to detect lap vs desk. That's why I
> wanted to use this patch to query the lap mode, as a debugging aid.
> 
> If X1C6 does not have the ability to detect lap mode, then that
> would obviously explain why the firmware does not change any APCT
> condition when put on/off lap, and any effort to get adaptive
> performance policy working on the thermald side is probably doomed
> :(
> 
Just as a follow-up for this mailing list - I responded to the X1C6 
issues on the thermal_daemon thread mentioned above - I will track those 
there.

Anything related to the thinkpad_acpi driver and improvements needed to 
my previous patch will come here. I have a question in to the firmware 
team to understand the limitations of when the lapmode variable should 
be made available or not. Looks like I have some work to do there....

Thanks!
Mark

Re: [ibm-acpi-devel] [External] Re: [PATCH v5] platform/x86: thinkpad_acpi: lap or desk mode interface

[Elvis S. <elv...@gm...>]

Den mån 3 aug. 2020 kl 16:27 skrev Mark Pearson <mar...@le...>:
>
> Hi Elvis,
>
> On 8/3/2020 9:20 AM, Elvis Stansvik wrote:
> >
> > Den sön 2 aug. 2020 kl 23:10 skrev Elvis Stansvik
> > <elv...@gm...>:
> >>
> >> (Apologies in advance for missing In-Reply-To header in this
> >> reply. I subscribed to the list just now.)
> >>
> >> Den fre 3 juli 2020 kl 01:24 skrev Mark Pearson
> >> <mar...@le...>:
> >>> Newer Lenovo Thinkpad platforms have support to identify whether
> >>> the system is on-lap or not using an ACPI DYTC event from the
> >>> firmware.
> >>>
> >>> This patch provides the ability to retrieve the current mode via
> >>> sysfs entrypoints and will be used by userspace for thermal mode
> >>> and WWAN functionality
> >>
> >> I tried the patch out on my X1C6 (20KH007BMX), running Lenovo
> >> firmware version 0.1.49.
> >>
> >> The sysfs path /sys/devices/platform/thinkpad_acpi/dytc_lapmode is
> >> showing up, but it's reporting 0 regardless if the laptop is in my
> >> lap or on a desk.
> >>
> >> I tested this by having the laptop in my lap for five minutes, and
> >> then on a desk for five minutes. The value reported through
> >> /sys/devices/platform/thinkpad_acpi/dytc_lapmode was always 0.
> >>
> Thanks for the update.
> The X1C6 doesn't have support for the new thermal mode implementation so
> I'm wondering if the lapmode being available could be related to the
> version of DYTC is available. I'll check with the firmware team and get
> back to you on that - they hadn't listed it to me as a limitation
> previously and unfortunately I don't have an X1C6 so didn't test on that
> platform.
>
> As a note - the lapmode is primarily for the thermal modes (it is also
> being used for WWAN power control but that is not supported for Linux on
> the X1C6 either for other reasons). I'm afraid the interface is of
> limited use to you anyway....

Thanks Mark. I know the X1C6 firmware did not get the new thermal mode
implementation, but thought that I would still be able to query its
lap mode.

The reason I wanted query the lap mode is actually because I'm trying
to help Matthew Garrett out in adding support for the adaptive DPTF
performance policy on the thermald side, instead of on the firmware
side like Lenovo decided to do for more recent models. See that effort
here https://github.com/mjg59/thermal_daemon/issues/7). We were
surprised to find that all of the OEM conditions in the APCT table
remained unchanged when the laptop was taken between lap and desk. So
I started suspecting that perhaps the X1C6 and its firmware simply
does not have the ability to detect lap vs desk. That's why I wanted
to use this patch to query the lap mode, as a debugging aid.

If X1C6 does not have the ability to detect lap mode, then that would
obviously explain why the firmware does not change any APCT condition
when put on/off lap, and any effort to get adaptive performance policy
working on the thermald side is probably doomed :(

Elvis

>
> Mark

Re: [ibm-acpi-devel] [External] Re: [PATCH v5] platform/x86: thinkpad_acpi: lap or desk mode interface

[Mark P. <mar...@le...>]

Hi Elvis,

On 8/3/2020 9:20 AM, Elvis Stansvik wrote:
> 
> Den sön 2 aug. 2020 kl 23:10 skrev Elvis Stansvik 
> <elv...@gm...>:
>> 
>> (Apologies in advance for missing In-Reply-To header in this
>> reply. I subscribed to the list just now.)
>> 
>> Den fre 3 juli 2020 kl 01:24 skrev Mark Pearson 
>> <mar...@le...>:
>>> Newer Lenovo Thinkpad platforms have support to identify whether 
>>> the system is on-lap or not using an ACPI DYTC event from the 
>>> firmware.
>>> 
>>> This patch provides the ability to retrieve the current mode via 
>>> sysfs entrypoints and will be used by userspace for thermal mode 
>>> and WWAN functionality
>> 
>> I tried the patch out on my X1C6 (20KH007BMX), running Lenovo 
>> firmware version 0.1.49.
>> 
>> The sysfs path /sys/devices/platform/thinkpad_acpi/dytc_lapmode is 
>> showing up, but it's reporting 0 regardless if the laptop is in my 
>> lap or on a desk.
>> 
>> I tested this by having the laptop in my lap for five minutes, and 
>> then on a desk for five minutes. The value reported through 
>> /sys/devices/platform/thinkpad_acpi/dytc_lapmode was always 0.
>> 
Thanks for the update.
The X1C6 doesn't have support for the new thermal mode implementation so 
I'm wondering if the lapmode being available could be related to the 
version of DYTC is available. I'll check with the firmware team and get 
back to you on that - they hadn't listed it to me as a limitation 
previously and unfortunately I don't have an X1C6 so didn't test on that 
platform.

As a note - the lapmode is primarily for the thermal modes (it is also 
being used for WWAN power control but that is not supported for Linux on 
the X1C6 either for other reasons). I'm afraid the interface is of 
limited use to you anyway....

Mark

Re: [ibm-acpi-devel] [PATCH v5] platform/x86: thinkpad_acpi: lap or desk mode interface

[Elvis S. <elv...@gm...>]

(I forgot a bunch of recipients in my original reply. Now added.
Apologies to those of you who now get it twice)

See my reply below.

Cheers,
Elvis

Den sön 2 aug. 2020 kl 23:10 skrev Elvis Stansvik <elv...@gm...>:
>
> (Apologies in advance for missing In-Reply-To header in this reply. I subscribed to the list just now.)
>
> Den fre 3 juli 2020 kl 01:24 skrev Mark Pearson <mar...@le...>:
> > Newer Lenovo Thinkpad platforms have support to identify whether the
> > system is on-lap or not using an ACPI DYTC event from the firmware.
> >
> > This patch provides the ability to retrieve the current mode via sysfs
> > entrypoints and will be used by userspace for thermal mode and WWAN
> > functionality
>
> I tried the patch out on my X1C6 (20KH007BMX), running Lenovo firmware version 0.1.49.
>
> The sysfs path /sys/devices/platform/thinkpad_acpi/dytc_lapmode is showing up, but it's reporting 0 regardless if the laptop is in my lap or on a desk.
>
> I tested this by having the laptop in my lap for five minutes, and then on a desk for five minutes. The value reported through /sys/devices/platform/thinkpad_acpi/dytc_lapmode was always 0.
>
> Regards,
> Elvis
>
> >
> > Co-developed-by: Nitin Joshi <njoshi1@...>
> > Signed-off-by: Nitin Joshi <njoshi1@...>
> > Reviewed-by: Sugumaran <slacshiminar@...>
> > Reviewed-by: Bastien Nocera <bnocera@...>
> > Signed-off-by: Mark Pearson <markpearson@...>
> > ---
> > Changes in v5:
> >  - Updated with review changes from Andy Shevchenko
> >  - Added ABI information to thinkpad-acpi.rst
> >  - improved error handling and parameter passing as recommended
> >  - code cleanup as recommended
> >  - added review tag from bnocera
> > Changes in v4:
> >  - Correct hotkey event comment as we're handling event
> >  - Remove unnecessary check in dytc_lapmode_refresh
> > Changes in v3:
> > - Fixed inaccurate comments
> > - Used BIT macro to check lapmode bit setting as recommended and update
> >   define name
> > - Check for new_state == dytc_lapmode in dytc_lapmode_refresh
> > Changes in v2:
> > - cleaned up initialisation sequence to be cleaner and avoid spamming
> >   platforms that don't have DYTC with warning message. Tested on P52
> > - Adding platform-driver-x86 mailing list for review as requested
> >
> >  .../admin-guide/laptops/thinkpad-acpi.rst     |  15 +++
> >  drivers/platform/x86/thinkpad_acpi.c          | 111 +++++++++++++++++-
> >  2 files changed, 124 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > index 822907dcc845..99066aa8d97b 100644
> > --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> > @@ -50,6 +50,7 @@ detailed description):
> >   - WAN enable and disable
> >   - UWB enable and disable
> >   - LCD Shadow (PrivacyGuard) enable and disable
> > + - Lap mode sensor
> >
> >  A compatibility table by model and feature is maintained on the web
> >  site, http://ibm-acpi.sf.net/. I appreciate any success or failure
> > @@ -1432,6 +1433,20 @@ The first command ensures the best viewing angle and the latter one turns
> >  on the feature, restricting the viewing angles.
> >
> >
> > +DYTC Lapmode sensor
> > +------------------
> > +
> > +sysfs: dytc_lapmode
> > +
> > +Newer thinkpads and mobile workstations have the ability to determine if
> > +the device is in deskmode or lapmode. This feature is used by user space
> > +to decide if WWAN transmission can be increased to maximum power and is
> > +also useful for understanding the different thermal modes available as
> > +they differ between desk and lap mode.
> > +
> > +The property is read-only. If the platform doesn't have support the sysfs
> > +class is not created.
> > +
> >  EXPERIMENTAL: UWB
> >  -----------------
> >
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> > index ff7f0a4f2475..037eb77414f9 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -4022,8 +4022,8 @@ static bool hotkey_notify_6xxx(const u32 hkey,
> >   return true;
> >   case TP_HKEY_EV_THM_CSM_COMPLETED:
> >   pr_debug("EC reports: Thermal Control Command set completed (DYTC)\n");
> > - /* recommended action: do nothing, we don't have
> > - * Lenovo ATM information */
> > + /* Thermal event - pass on to event handler */
> > + tpacpi_driver_event(hkey);
> >   return true;
> >   case TP_HKEY_EV_THM_TRANSFM_CHANGED:
> >   pr_debug("EC reports: Thermal Transformation changed (GMTS)\n");
> > @@ -9795,6 +9795,105 @@ static struct ibm_struct lcdshadow_driver_data = {
> >   .write = lcdshadow_write,
> >  };
> >
> > +/*************************************************************************
> > + * DYTC subdriver, for the Lenovo lapmode feature
> > + */
> > +
> > +#define DYTC_CMD_GET          2 /* To get current IC function and mode */
> > +#define DYTC_GET_LAPMODE_BIT 17 /* Set when in lapmode */
> > +
> > +static bool dytc_lapmode;
> > +
> > +static void dytc_lapmode_notify_change(void)
> > +{
> > + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_lapmode");
> > +}
> > +
> > +static int dytc_command(int command, int *output)
> > +{
> > + acpi_handle dytc_handle;
> > +
> > + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) {
> > + /* Platform doesn't support DYTC */
> > + return -ENODEV;
> > + }
> > + if (!acpi_evalf(dytc_handle, output, NULL, "dd", command))
> > + return -EIO;
> > + return 0;
> > +}
> > +
> > +static int dytc_lapmode_get(bool *state)
> > +{
> > + int output, err;
> > +
> > + err = dytc_command(DYTC_CMD_GET, &output);
> > + if (err)
> > + return err;
> > + *state = output & BIT(DYTC_GET_LAPMODE_BIT) ? true : false;
> > + return 0;
> > +}
> > +
> > +static void dytc_lapmode_refresh(void)
> > +{
> > + bool new_state;
> > + int err;
> > +
> > + err = dytc_lapmode_get(&new_state);
> > + if (err || (new_state == dytc_lapmode))
> > + return;
> > +
> > + dytc_lapmode = new_state;
> > + dytc_lapmode_notify_change();
> > +}
> > +
> > +/* sysfs lapmode entry */
> > +static ssize_t dytc_lapmode_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + return snprintf(buf, PAGE_SIZE, "%d\n", dytc_lapmode);
> > +}
> > +
> > +static DEVICE_ATTR_RO(dytc_lapmode);
> > +
> > +static struct attribute *dytc_attributes[] = {
> > + &dev_attr_dytc_lapmode.attr,
> > + NULL,
> > +};
> > +
> > +static const struct attribute_group dytc_attr_group = {
> > + .attrs = dytc_attributes,
> > +};
> > +
> > +static int tpacpi_dytc_init(struct ibm_init_struct *iibm)
> > +{
> > + int err;
> > +
> > + err = dytc_lapmode_get(&dytc_lapmode);
> > + /* If support isn't available (ENODEV) then don't return an error
> > + * but just don't create the sysfs group
> > + */
> > + if (err == -ENODEV)
> > + return 0;
> > + /* For all other errors we can flag the failure */
> > + if (err)
> > + return err;
> > +
> > + /* Platform supports this feature - create the group */
> > + err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group);
> > + return err;
> > +}
> > +
> > +static void dytc_exit(void)
> > +{
> > + sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group);
> > +}
> > +
> > +static struct ibm_struct dytc_driver_data = {
> > + .name = "dytc",
> > + .exit = dytc_exit,
> > +};
> > +
> >  /****************************************************************************
> >   ****************************************************************************
> >   *
> > @@ -9842,6 +9941,10 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
> >
> >   mutex_unlock(&kbdlight_mutex);
> >   }
> > +
> > + if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
> > + dytc_lapmode_refresh();
> > +
> >  }
> >
> >  static void hotkey_driver_event(const unsigned int scancode)
> > @@ -10280,6 +10383,10 @@ static struct ibm_init_struct ibms_init[] __initdata = {
> >   .init = tpacpi_lcdshadow_init,
> >   .data = &lcdshadow_driver_data,
> >   },
> > + {
> > + .init = tpacpi_dytc_init,
> > + .data = &dytc_driver_data,
> > + },
> >  };
> >
> >  static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
> > --
> > 2.26.2

Re: [ibm-acpi-devel] [PATCH v5] platform/x86: thinkpad_acpi: lap or desk mode interface

[Elvis S. <elv...@gm...>]

(Apologies in advance for missing In-Reply-To header in this reply. I
subscribed to the list just now.)

Den fre 3 juli 2020 kl 01:24 skrev Mark Pearson <mar...@le...>:
> Newer Lenovo Thinkpad platforms have support to identify whether the
> system is on-lap or not using an ACPI DYTC event from the firmware.
>
> This patch provides the ability to retrieve the current mode via sysfs
> entrypoints and will be used by userspace for thermal mode and WWAN
> functionality

I tried the patch out on my X1C6 (20KH007BMX), running Lenovo firmware
version 0.1.49.

The sysfs path /sys/devices/platform/thinkpad_acpi/dytc_lapmode is showing
up, but it's reporting 0 regardless if the laptop is in my lap or on a desk.

I tested this by having the laptop in my lap for five minutes, and then on
a desk for five minutes. The value reported through
/sys/devices/platform/thinkpad_acpi/dytc_lapmode was always 0.

Regards,
Elvis

>
> Co-developed-by: Nitin Joshi <njoshi1@...>
> Signed-off-by: Nitin Joshi <njoshi1@...>
> Reviewed-by: Sugumaran <slacshiminar@...>
> Reviewed-by: Bastien Nocera <bnocera@...>
> Signed-off-by: Mark Pearson <markpearson@...>
> ---
> Changes in v5:
>  - Updated with review changes from Andy Shevchenko
>  - Added ABI information to thinkpad-acpi.rst
>  - improved error handling and parameter passing as recommended
>  - code cleanup as recommended
>  - added review tag from bnocera
> Changes in v4:
>  - Correct hotkey event comment as we're handling event
>  - Remove unnecessary check in dytc_lapmode_refresh
> Changes in v3:
> - Fixed inaccurate comments
> - Used BIT macro to check lapmode bit setting as recommended and update
>   define name
> - Check for new_state == dytc_lapmode in dytc_lapmode_refresh
> Changes in v2:
> - cleaned up initialisation sequence to be cleaner and avoid spamming
>   platforms that don't have DYTC with warning message. Tested on P52
> - Adding platform-driver-x86 mailing list for review as requested
>
>  .../admin-guide/laptops/thinkpad-acpi.rst     |  15 +++
>  drivers/platform/x86/thinkpad_acpi.c          | 111 +++++++++++++++++-
>  2 files changed, 124 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> index 822907dcc845..99066aa8d97b 100644
> --- a/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> +++ b/Documentation/admin-guide/laptops/thinkpad-acpi.rst
> @@ -50,6 +50,7 @@ detailed description):
>   - WAN enable and disable
>   - UWB enable and disable
>   - LCD Shadow (PrivacyGuard) enable and disable
> + - Lap mode sensor
>
>  A compatibility table by model and feature is maintained on the web
>  site, http://ibm-acpi.sf.net/. I appreciate any success or failure
> @@ -1432,6 +1433,20 @@ The first command ensures the best viewing angle
and the latter one turns
>  on the feature, restricting the viewing angles.
>
>
> +DYTC Lapmode sensor
> +------------------
> +
> +sysfs: dytc_lapmode
> +
> +Newer thinkpads and mobile workstations have the ability to determine if
> +the device is in deskmode or lapmode. This feature is used by user space
> +to decide if WWAN transmission can be increased to maximum power and is
> +also useful for understanding the different thermal modes available as
> +they differ between desk and lap mode.
> +
> +The property is read-only. If the platform doesn't have support the sysfs
> +class is not created.
> +
>  EXPERIMENTAL: UWB
>  -----------------
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
b/drivers/platform/x86/thinkpad_acpi.c
> index ff7f0a4f2475..037eb77414f9 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -4022,8 +4022,8 @@ static bool hotkey_notify_6xxx(const u32 hkey,
>   return true;
>   case TP_HKEY_EV_THM_CSM_COMPLETED:
>   pr_debug("EC reports: Thermal Control Command set completed (DYTC)\n");
> - /* recommended action: do nothing, we don't have
> - * Lenovo ATM information */
> + /* Thermal event - pass on to event handler */
> + tpacpi_driver_event(hkey);
>   return true;
>   case TP_HKEY_EV_THM_TRANSFM_CHANGED:
>   pr_debug("EC reports: Thermal Transformation changed (GMTS)\n");
> @@ -9795,6 +9795,105 @@ static struct ibm_struct lcdshadow_driver_data = {
>   .write = lcdshadow_write,
>  };
>
>
+/*************************************************************************
> + * DYTC subdriver, for the Lenovo lapmode feature
> + */
> +
> +#define DYTC_CMD_GET          2 /* To get current IC function and mode */
> +#define DYTC_GET_LAPMODE_BIT 17 /* Set when in lapmode */
> +
> +static bool dytc_lapmode;
> +
> +static void dytc_lapmode_notify_change(void)
> +{
> + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_lapmode");
> +}
> +
> +static int dytc_command(int command, int *output)
> +{
> + acpi_handle dytc_handle;
> +
> + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) {
> + /* Platform doesn't support DYTC */
> + return -ENODEV;
> + }
> + if (!acpi_evalf(dytc_handle, output, NULL, "dd", command))
> + return -EIO;
> + return 0;
> +}
> +
> +static int dytc_lapmode_get(bool *state)
> +{
> + int output, err;
> +
> + err = dytc_command(DYTC_CMD_GET, &output);
> + if (err)
> + return err;
> + *state = output & BIT(DYTC_GET_LAPMODE_BIT) ? true : false;
> + return 0;
> +}
> +
> +static void dytc_lapmode_refresh(void)
> +{
> + bool new_state;
> + int err;
> +
> + err = dytc_lapmode_get(&new_state);
> + if (err || (new_state == dytc_lapmode))
> + return;
> +
> + dytc_lapmode = new_state;
> + dytc_lapmode_notify_change();
> +}
> +
> +/* sysfs lapmode entry */
> +static ssize_t dytc_lapmode_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return snprintf(buf, PAGE_SIZE, "%d\n", dytc_lapmode);
> +}
> +
> +static DEVICE_ATTR_RO(dytc_lapmode);
> +
> +static struct attribute *dytc_attributes[] = {
> + &dev_attr_dytc_lapmode.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group dytc_attr_group = {
> + .attrs = dytc_attributes,
> +};
> +
> +static int tpacpi_dytc_init(struct ibm_init_struct *iibm)
> +{
> + int err;
> +
> + err = dytc_lapmode_get(&dytc_lapmode);
> + /* If support isn't available (ENODEV) then don't return an error
> + * but just don't create the sysfs group
> + */
> + if (err == -ENODEV)
> + return 0;
> + /* For all other errors we can flag the failure */
> + if (err)
> + return err;
> +
> + /* Platform supports this feature - create the group */
> + err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group);
> + return err;
> +}
> +
> +static void dytc_exit(void)
> +{
> + sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group);
> +}
> +
> +static struct ibm_struct dytc_driver_data = {
> + .name = "dytc",
> + .exit = dytc_exit,
> +};
> +
>
 /****************************************************************************
>
****************************************************************************
>   *
> @@ -9842,6 +9941,10 @@ static void tpacpi_driver_event(const unsigned int
hkey_event)
>
>   mutex_unlock(&kbdlight_mutex);
>   }
> +
> + if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
> + dytc_lapmode_refresh();
> +
>  }
>
>  static void hotkey_driver_event(const unsigned int scancode)
> @@ -10280,6 +10383,10 @@ static struct ibm_init_struct ibms_init[]
__initdata = {
>   .init = tpacpi_lcdshadow_init,
>   .data = &lcdshadow_driver_data,
>   },
> + {
> + .init = tpacpi_dytc_init,
> + .data = &dytc_driver_data,
> + },
>  };
>
>  static int __init set_ibm_param(const char *val, const struct
kernel_param *kp)
> --
> 2.26.2